fix: Menu popover doesn't flip when a user-set maxHeight can't fit below the trigger - #10470
Open
timges wants to merge 5 commits into
Open
fix: Menu popover doesn't flip when a user-set maxHeight can't fit below the trigger#10470timges wants to merge 5 commits into
timges wants to merge 5 commits into
Conversation
…low the trigger (adobe#10176) Folds the user-provided maxHeight into the height-axis flip-trigger comparison in calculatePositionInternal, so the decision to flip is based on the overlay's true requested size instead of a stale clamped height left over from a previous positioning pass.
Ground the new adobe#10176 flip-regression test titles in behavior instead of the issue number, and move them next to the existing sibling maxHeight test instead of after an unrelated arrow test.
Pare comments down to the non-obvious why (the offsetHeight mock and the maxHeight fold-in rationale); drop ones that just restated code or assertions already visible nearby.
beforeEach already resets it before every test, so the declaration-time value was never actually observed.
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
1 participant
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.
Closes #10176.
Menu popovers with a user-set
maxHeightweren't flipping to the other side of the trigger when they no longer fit below it.calculatePositionInternalcompared the overlay's current, already-clamped height against the available space, so once the overlay had been clamped down to fit on an earlier pass, it kept looking like it fit and never re-evaluated the flip. This folds the user-providedmaxHeightinto that height-axis comparison so the flip decision is based on the overlay's true requested size instead of the stale clamped one.✅ Pull Request Checklist:
📝 Test Instructions:
yarn jest packages/react-aria/test/overlays/useOverlayPosition.test.tsx— 16/16 passing, including a new regression test for themaxHeightflip fix and an RTL case confirming overlays that should stay put still don't flip.yarn format:check— clean.yarn oxlinton the two changed files — clean (the full repo run has pre-existing, unrelated errors inscripts/and storybook config files onmain).maxHeightlarger than the remaining space below the trigger; confirm it flips above the trigger instead of staying clamped below it, in both LTR and RTL.🧢 Your Project:
Spectrum Metrics