fix(react-positioning): don't treat static overflow:hidden ancestors as a hide-middleware clipping boundary - #36605
Open
PaulGMardling wants to merge 9 commits into
Conversation
…as a hide-middleware clipping boundary The Tooltip trigger-scroll-out-of-view fix (microsoft#36386, fixing microsoft#32882) added `hide` middleware with no boundary override, which defaults to floating-ui's `clippingAncestors`. That walks up to ANY overflow ancestor, including non-scrolling `overflow: hidden` containers that merely clip statically (e.g. a tightly-fitted flex toolbar), not just genuine scroll containers. This meant a Tooltip trigger placed inside such a container could have its tooltip permanently hidden, even at rest with no scrolling involved, since `escaped`/`referenceHidden` would compute true on the very first render. Fix: introduce a `hide` middleware wrapper (mirroring the existing `shift`/`flip` wrappers) that only uses `clippingAncestors` as the boundary when there is a genuinely scrollable ancestor (`hasScrollParent`, which intentionally excludes `overflow: hidden`). Otherwise it scopes detection to the viewport only, so static clipping containers no longer suppress the tooltip. Fixes microsoft#36604 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
PaulGMardling
requested review from
a team and
Victor Genaev (mainframev)
as code owners
August 21, 2026 09:08
Contributor
Author
|
cc charles izzi (@charles-izzi) — this should fix the regression you found in #36604. Thanks again for the clean repro! |
PaulGMardling
marked this pull request as draft
August 21, 2026 09:11
📊 Bundle size report
Unchanged fixtures
|
…cument floating-ui dependency Adds a unit test for the `hide` middleware wrapper introduced for microsoft#36604, asserting it maps `hasScrollableElement` to the correct `boundary` option passed to `@floating-ui/dom`'s `hide` middleware. The fix relies on an implicit, lightly-documented floating-ui behavior (passing `boundary: []` skips intermediate DOM clipping ancestors and falls back to the viewport). That deeper real-browser geometry contract is already covered by the Cypress tests in Tooltip.cy.tsx (both the pre-existing scroll regression test for microsoft#32882 and the new static overflow:hidden test for microsoft#36604); jsdom's layout emulation isn't accurate enough to reliably pin it in a unit test here, so this test and the accompanying comments make that dependency and its test coverage explicit for future maintainers. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Pull request demo site: URL |
…le-export files The CSF/babel story loader does not support multiple exports from a single non-index story file, causing test-ssr to fail with "Multiple exports from a single file are not supported". Split StaticOverflowHidden into its own file, matching the convention used by all other stories in this folder. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
PaulGMardling
marked this pull request as ready for review
August 21, 2026 13:31
Adds a public hideBoundary positioning option (react-positioning) so consumers can control the boundary used for referenceHidden/escaped detection. Tooltip now defaults hideBoundary to 'scrollParent', fixing a regression where tooltips inside a tightly-fitted, non-scrolling overflow:hidden container were incorrectly hidden, while preserving the existing scroll-based hide behavior (microsoft#32882). Also fixes hideBoundary not being forwarded through usePositioningConfigFn, which silently dropped the option before it ever reached the hide middleware, and simplifies boundary resolution to reuse getBoundary's existing 'scrollParent' handling instead of a duplicated nested-ternary implementation. Fixes microsoft#36604 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
Author
Fixed Screen.Recording.2026-08-24.at.12.40.51.movScreen.Recording.2026-08-24.at.12.42.15.mov |
…oll parent Adds a Cypress regression case verifying a non-scrolling overflow:hidden wrapper nested inside a real scroll parent doesn't interfere with either the microsoft#36604 static-container fix or the microsoft#32882 scroll-hide behavior, closing a gap where only the two top-level scenarios were covered individually. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Previous Behavior
Following #36386 (which fixed #32882 — tooltip appearing outside its overflow container when the trigger scrolled out of view), a Tooltip trigger placed inside a tightly-fitted, non-scrolling
overflow: hiddencontainer would never show its tooltip at all — even at rest, with no scrolling involved.Repro:
Root Cause
In
react-positioning'susePositioningOptions.ts, the #32882 fix added aboundaryto thehidemiddleware based onhasScrollableElementdirectly in shared code used by everyusePositioningconsumer (Combobox, Menu, Popover, TagPicker, DatePicker, Tooltip, ...):hasScrollableElement(viahasScrollParent) only treatsauto/scroll/overlayancestors as real scroll containers, so it correctly distinguishes "real scroll parent" from "static clipping ancestor." However, once a scroll parent did exist anywhere up the tree, the boundary fell back to floating-ui's'clippingAncestors', which walks up from the trigger through any ancestor withoverflow: hidden/scroll/auto— including tightly-fitted, non-scrollingoverflow: hiddenwrappers that happen to sit between the trigger and that scroll parent.In the repro above, the wrapping
<div style={{ overflow: 'hidden', display: 'flex' }}>hugs the button tightly with no extra space above it. Since Tooltip defaults toposition="above", the computed tooltip position extends past that div's clip box on the very first render — no scrolling required.escapedbecomestrueimmediately and stays that way, so the tooltip never shows.Any trigger inside a tightly-fitted
overflow: hiddencontainer — flex toolbars, truncated table/list cells, card layouts, etc. — silently lost its tooltip permanently, regardless of scroll position.New Behavior
Per review feedback, this fix intentionally avoids changing shared
hidemiddleware behavior for allusePositioningconsumers (too large a blast radius for Combobox/Menu/Popover/TagPicker/DatePicker). Instead:hideBoundary?: PositioningBoundary | nulloption toreact-positioning(PositioningOptions/PositioningProps), letting the boundary used forreferenceHidden/escapeddetection be configured independently ofoverflowBoundary/flipBoundary.hideBoundaryis resolved via the existinggetBoundary()helper, which already has first-class'scrollParent'handling: it walks up to the nearest genuine scroll parent (falling back to the document element if none exists), rather than floating-ui's default'clippingAncestors'(which clips against any overflow ancestor, scrolling or not).hideBoundaryto'scrollParent', opting in to this corrected behavior. It's the only consumer affected — every otherusePositioningconsumer keeps its original, unmodified behavior.hideBoundaryexplicitly (e.g. Tooltip's ownpositioningprop passthrough).usePositioningConfigFnforwards a fixed allow-list of option fields into the positioning config pipeline, andhideBoundarywas initially missing from that list — silently dropping the option before it ever reached thehidemiddleware, regardless of what was set. This is now included.hidemiddleware wrapper (middleware/hide.ts+ its test) that an earlier iteration introduced; the fix now calls floating-ui'shidemiddleware directly with a single, reusedhideBoundaryOptionsobject, avoiding a duplicated nested-ternary boundary computation.StaticOverflowHiddenStorybook story as a visual example, alongside the existingOverflowHiddenscroll-regression story.Related Issue(s)
Verification
react-positioning:build/react-tooltip:build— passreact-positioning:test(199 tests) /react-tooltip:test(23 tests) — all passTooltip.cy.tsx(3 specs, run uncached) — all pass:overflow:hiddenregression test ([Bug]: Tooltip permanently hidden inside tightly-fitted overflow:hidden containers (regression in 9.10.4+) #36604)overflow:hiddennested inside a real scroll parent, verifying the two behaviors compose correctlyreact-positioningchange file updated tominor(new publichideBoundaryAPI surface);react-tooltipchange file remainspatch