Skip to content

[Android] Fix native handlers attaching to a nested button instead of the detector's child - #4464

Open
m-bert wants to merge 1 commit into
mainfrom
@mbert/remove-detector-button-search
Open

[Android] Fix native handlers attaching to a nested button instead of the detector's child#4464
m-bert wants to merge 1 commit into
mainfrom
@mbert/remove-detector-button-search

Conversation

@m-bert

@m-bert m-bert commented Aug 21, 2026

Copy link
Copy Markdown
Collaborator

Description

tryFindGestureHandlerButton was added in #3634 to find the button inside the wrapper View of the display: contents sandwich. #4044 replaced that structure with a single-view button, so the correct target is the detector's direct child again - but the search was left in and still fired whenever the child's first child happened to be a bare ButtonViewGroup (e.g. Pressable or Touchable as the first child of a button or of a view under a native-gesture detector), attaching the handler to that inner button instead.

Before this change, the outer button in the test screen did not react to presses anywhere except over the inner pressable, and pressing the inner pressable fired the outer handler's callbacks alongside the inner ones (with a doubled pressIn on the inner pressable).

This PR removes the search so native handlers always attach to the detector's child, with the existing exception of RefreshControl unwrapping.

Test plan

Compared builds from this branch and its base commit on the Android emulator using the test screen below:

Test screen
import React, { useCallback, useState } from 'react';
import { StyleSheet, Text, View } from 'react-native';
import {
  GestureDetector,
  Pressable,
  RawButton,
  RefreshControl,
  ScrollView,
  useNativeGesture,
} from 'react-native-gesture-handler';

export default function EmptyExample() {
  const [log, setLog] = useState<string[]>([]);

  const append = useCallback((entry: string) => {
    setLog((prev) => [entry, ...prev].slice(0, 10));
  }, []);

  const [refreshing, setRefreshing] = useState(false);
  const onRefresh = () => {
    append('3. refresh triggered');
    setRefreshing(true);
    setTimeout(() => setRefreshing(false), 1500);
  };

  const wrappedViewGesture = useNativeGesture({
    disableReanimated: true,
    onBegin: () => append('2. native gesture on View: begin'),
    onActivate: () => append('2. native gesture on View: activate'),
    onFinalize: () => append('2. native gesture on View: finalize'),
  });

  return (
    <View style={styles.container}>
      <View style={styles.logPane}>
        {log.length === 0 ? (
          <Text style={styles.logEntry}>-- log --</Text>
        ) : (
          log.map((entry, i) => (
            <Text key={`${i}-${entry}`} style={styles.logEntry}>
              {entry}
            </Text>
          ))
        )}
      </View>
      <ScrollView
        style={styles.scroll}
        refreshControl={
          <RefreshControl refreshing={refreshing} onRefresh={onRefresh} />
        }>
        <Text style={styles.title}>1. Button with a bare button inside</Text>
        <RawButton
          style={styles.outerButton}
          rippleColor="#88a"
          onBegin={() => append('1. outer button: begin')}
          onActivate={() => append('1. outer button: activate')}
          onFinalize={() => append('1. outer button: finalize')}>
          <Pressable
            style={styles.innerButton}
            onPressIn={() => append('1. inner pressable: pressIn')}
            onPress={() => append('1. inner pressable: press')}>
            <Text>Inner Pressable (first child)</Text>
          </Pressable>
          <Text>Outer button area</Text>
        </RawButton>

        <Text style={styles.title}>2. Native gesture on a plain View</Text>
        <GestureDetector gesture={wrappedViewGesture}>
          <View style={styles.wrappedView}>
            <Pressable
              style={styles.innerButton}
              onPress={() => append('2. inner pressable: press')}>
              <Text>Inner Pressable (first child)</Text>
            </Pressable>
            <Text>Plain View area</Text>
          </View>
        </GestureDetector>

        <Text style={styles.title}>3. Pull to refresh</Text>
        {Array.from({ length: 15 }, (_, i) => (
          <View key={i} style={styles.row}>
            <Text>Row {i + 1}</Text>
          </View>
        ))}
      </ScrollView>
    </View>
  );
}

const styles = StyleSheet.create({
  container: { flex: 1 },
  logPane: { minHeight: 170, padding: 8, backgroundColor: '#222' },
  logEntry: { color: '#eee', fontSize: 12, fontVariant: ['tabular-nums'] },
  scroll: { flex: 1 },
  title: { fontSize: 16, fontWeight: 'bold', marginHorizontal: 12, marginTop: 16, marginBottom: 8 },
  outerButton: { backgroundColor: '#ccd5ff', marginHorizontal: 12, padding: 24, borderRadius: 8 },
  wrappedView: { backgroundColor: '#cdf5cd', marginHorizontal: 12, padding: 24, borderRadius: 8 },
  innerButton: {
    backgroundColor: '#f5c6c6',
    padding: 12,
    borderRadius: 6,
    marginBottom: 8,
    alignSelf: 'flex-start',
  },
  row: {
    height: 44,
    justifyContent: 'center',
    paddingHorizontal: 12,
    borderBottomWidth: StyleSheet.hairlineWidth,
    borderBottomColor: '#ccc',
  },
});

Copilot AI lite review requested due to automatic review settings August 21, 2026 13:34
@coderabbitai

coderabbitai Bot commented Aug 21, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: d985b468-1d4b-45de-a7ce-9d28c0cc60c9

📥 Commits

Reviewing files that changed from the base of the PR and between 8a4d05d and 2c94b50.

📒 Files selected for processing (1)
  • packages/react-native-gesture-handler/android/src/main/java/com/swmansion/gesturehandler/react/RNGestureHandlerDetectorView.kt
💤 Files with no reviewable changes (1)
  • packages/react-native-gesture-handler/android/src/main/java/com/swmansion/gesturehandler/react/RNGestureHandlerDetectorView.kt

Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review.


📝 Walkthrough

Summary by CodeRabbit

  • Bug Fixes
    • Improved native gesture-handler attachment for nested views.
    • Gesture handlers now consistently attach to the child view itself, avoiding incorrect targeting of nested button views.
    • Preserved specialized handling for swipe-to-refresh components.

Walkthrough

The Android gesture detector now attaches native handlers directly to each non-ReactSwipeRefreshLayout child view. It removes nested button lookup logic and its unused imports.

Changes

Native handler attachment

Layer / File(s) Summary
Use direct child IDs
packages/react-native-gesture-handler/android/src/main/java/com/swmansion/gesturehandler/react/RNGestureHandlerDetectorView.kt
Native handlers now use the child view's own ID. The nested button lookup extension and unused imports were removed.

Merge Risk: ⚪ Minimal · up to 2c94b

This localized Android change corrects which view receives native gesture handling; no actionable merge-blocking risk remains after normal checks and review.

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly describes the main Android change: preventing native handlers from attaching to a nested button instead of the detector's direct child.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Removes Android-only logic that attempted to “dive into” the detector child view hierarchy to find an inner ButtonViewGroup, ensuring native gesture handlers attach to the GestureDetector’s direct child again (while preserving the existing RefreshControl unwrapping behavior).

Changes:

  • Removed the ViewGroup/ButtonViewGroup search path so the native handler target is always the detector’s direct child.
  • Deleted the now-unused tryFindGestureHandlerButton helper and related imports.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@m-bert
m-bert requested a review from j-piasecki August 21, 2026 13:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants