fix(react-menu): let setTarget win for context menus - #36486
Draft
AKnassa wants to merge 1 commit into
Draft
Conversation
For openOnContext menus, useMenu stored the right-click mouse position in state and passed it as the positioning target option. usePositioning re-syncs that option into its override ref in a layout effect at commit time, so a user's imperative positioningRef.setTarget() (e.g. anchoring a Shift+F10 context menu to the focused row) was always clobbered. Set the context target imperatively instead, through an internal positioning ref merged with the user's, and stop passing the state-driven target option (the state stays in sync because MenuState.contextTarget is public). An explicit positioning.target from props still wins. onOpenChange now fires after the internal context target is set, so a setTarget call inside it takes precedence - the ordering endorsed in the issue discussion. Covered by new jsdom tests (imperative target wins; mouse anchoring preserved; trigger restored on close) and Cypress geometry tests that reproduced the bug in a real browser before the fix. Full react-menu (496) and react-positioning (199) suites green. The same latent pattern exists in react-popover and the headless Menu copy - follow-up candidates. Fixes microsoft#31727
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.
Previous Behavior
When a Menu opens as a context menu (
openOnContext), it always anchors to the mouse point — even when an app explicitly asks for a different anchor withpositioningRef.setTarget(). For example, anchoring a Shift+F10 context menu to the focused row: the user's choice was silently overwritten, because the menu re-applied its own stored mouse position after the app's call.New Behavior
The app's
setTarget()call wins, the way the API promises. The menu now sets its mouse-point anchor imperatively (instead of through a state-driven option that kept re-applying itself), andonOpenChangefires after the menu's own anchor is set — so anything the app does in that callback takes precedence. Plain right-click context menus behave exactly as before, and an explicitpositioning.targetprop still beats everything. No public API changes.This is the fix direction a maintainer endorsed in the issue discussion.
What changed
useMenusets the context-menu anchor through an internal positioning ref (merged with the user's ref); the stored state stays in sync because it's public API.onOpenChangenow fires after the internal anchor is set.Full react-menu suite (496 tests), react-positioning suite (199), and the Menu Cypress spec (69) are all green. The same latent pattern exists in Popover and the headless Menu copy — noted as follow-ups, not touched here.
Related Issue(s)