Skip to content

[Web] Make getEffectiveBoundingRect recursive#4303

Merged
m-bert merged 4 commits into
mainfrom
@mbert/recursive-bounding-rect
Jul 8, 2026
Merged

[Web] Make getEffectiveBoundingRect recursive#4303
m-bert merged 4 commits into
mainfrom
@mbert/recursive-bounding-rect

Conversation

@m-bert

@m-bert m-bert commented Jul 7, 2026

Copy link
Copy Markdown
Collaborator

Description

Currently getEffectiveBoundingRect takes bounding box either from view, or children if element has display: contents;. However, in case of nested detectors this brakes, as can be seen when running the example code below.

This PR makes getEffectiveBoundingRect recursive, so it will not stop on the first layer of children.

Test plan

Tested on the following code:
import React, { useState } from 'react';
import { StyleSheet, Text, View } from 'react-native';
import {
  GestureDetector,
  useHoverGesture,
  useNativeGesture,
} from 'react-native-gesture-handler';

// Web-only repro for the getEffectiveBoundingRect fix.
//
// The OUTER hover gesture has a `hitSlop`, which makes its handler run
// checkHitSlop() → measureView() → getEffectiveBoundingRect() on the detector's
// `display:contents` host. The INNER GestureDetector nests a SECOND
// `display:contents` host between that host and the real box (the same shape as
// a hover detector wrapping a Touchable). Before the fix, the outer host's only
// child was itself `display:contents`, so its measured rect collapsed to 0×0 at
// the origin and checkHitSlop rejected every real pointer — hover never fired.
// After the fix, getEffectiveBoundingRect recurses through the inner
// `display:contents` child to the real box, so hover activates normally.
export default function EmptyExample() {
  const [hovered, setHovered] = useState(false);

  const hover = useHoverGesture({
    manualActivation: true,
    disableReanimated: true,
    hitSlop: { top: 10, bottom: 10, left: 10, right: 10 },
    onBegin: () => setHovered(true),
    onFinalize: () => setHovered(false),
  });

  // Any inner gesture works — it exists only to nest a second detector host.
  const inner = useNativeGesture({});

  return (
    <View style={styles.container}>
      <GestureDetector gesture={hover}>
        <GestureDetector gesture={inner}>
          <View style={[styles.box, hovered && styles.boxHovered]}>
            <Text style={styles.label}>
              {hovered ? 'hovered ✅' : 'hover me (web)'}
            </Text>
          </View>
        </GestureDetector>
      </GestureDetector>
    </View>
  );
}

const styles = StyleSheet.create({
  container: {
    flex: 1,
    justifyContent: 'center',
    alignItems: 'center',
  },
  box: {
    width: 220,
    height: 130,
    borderRadius: 16,
    backgroundColor: '#a78bfa',
    alignItems: 'center',
    justifyContent: 'center',
  },
  boxHovered: {
    backgroundColor: '#34d399',
  },
  label: { fontSize: 18, fontWeight: '700', color: '#1a1a2e' },
});

Copilot AI review requested due to automatic review settings July 7, 2026 13:30

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

This PR fixes a web-specific hit-testing/measurement issue for nested GestureDetector wrappers that use display: contents by making getEffectiveBoundingRect traverse through multiple layers of display: contents elements until it reaches elements with real layout boxes.

Changes:

  • Make getEffectiveBoundingRect recursive so nested display: contents detector hosts no longer collapse to a 0×0 rect.
  • Ignore zero-sized intermediate rects and fall back to the current element’s getBoundingClientRect() when no non-zero child bounds are found.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread packages/react-native-gesture-handler/src/web/utils.ts Outdated

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

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

Comment thread packages/react-native-gesture-handler/src/web/tools/GestureHandlerWebDelegate.ts Outdated
Comment on lines 146 to 149
this.gestureHandler.usesNativeOrVirtualDetector() &&
this.view.style.display === 'contents'
hasDisplayContents(this.view)
? (this.view.children[0] as HTMLElement)
: this.view;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why does this work? Now you're checking for display: contents recursively, but you're still taking the first child when it's set.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Well, this one (absoluteToLocal) is used only in Pinch and Rotation to calculate correct anchor and focal points. I have not tried degenerate example of nesting these handlers with other display: contents; view. Problems with Hover were discovered only because of our on-going Pressable refactor.

Anyway, given that it is better to use correct view rather than assuming correct hierarchy, I've changed that (bd1fa6e).

Let me know what you think

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Ideally, I think we would calculate the bounding box of all non-contents nodes, but this is fine for now.

@m-bert
m-bert requested a review from j-piasecki July 8, 2026 12:45
@m-bert
m-bert merged commit c8d66b4 into main Jul 8, 2026
2 checks passed
@m-bert
m-bert deleted the @mbert/recursive-bounding-rect branch July 8, 2026 13:27
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.

3 participants