fix(navigation-menu): keep dropdown open on click after hover-open#3882
Open
BnayaZil wants to merge 2 commits into
Open
fix(navigation-menu): keep dropdown open on click after hover-open#3882BnayaZil wants to merge 2 commits into
BnayaZil wants to merge 2 commits into
Conversation
Previously, hovering a trigger opened the dropdown and a subsequent click on the same trigger closed it, because onItemSelect toggled based solely on whether the current open value matched the trigger's value — with no notion of how the menu was opened. Swallow the click when the dropdown was just opened by hover (using the existing hasPointerMoveOpenedRef signal), matching Base UI's stickIfOpen behavior. A click on an already-click-opened trigger still toggles closed; a click on a closed trigger still opens it.
🦋 Changeset detectedLatest commit: 0c18635 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Spell out the why behind the fix (the toggle had no notion of how the menu was opened) and frame the Base UI reference as relevant to folks comparing primitives in the shadcn ecosystem, rather than as a generic appeal to Base UI's behavior.
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.
Summary
Fixes a behavior bug in
NavigationMenu: a trigger that the user has hovered (which opens the dropdown) gets closed if the user then clicks it. The click should be a no-op — leaving the dropdown open — which is what Base UI'sNavigationMenuTriggerdoes via Floating UI'suseClick({ stickIfOpen, toggle }).Repro
In any
NavigationMenustory (e.g.NavigationMenu / Basic):Root cause
NavigationMenuTriggerfiresonTriggerEnterononPointerMove(which setsvalue = itemValue) and firesonItemSelectononClick(which togglesvaluebased onprevValue === itemValue). So the sequence "hover → click" first opens via hover, then the toggle on click — having no notion of how the menu was opened — closes it.Fix
Reuse the existing
hasPointerMoveOpenedRef(the per-trigger ref the component already keeps to suppress a different hover-vs-click race inonPointerMove). InonClick, if the trigger is currently open andhasPointerMoveOpenedRef.currentis true, swallow the click and clear the ref. A subsequent click — without an intervening pointer-leave + re-hover — falls through to the normal toggle path. This mirrors Base UI'sstickIfOpensemantics without touching the provider-level state machine or the public API.The change is one
ifblock in one handler inpackages/react/navigation-menu/src/navigation-menu.tsx.Verified regressions
A new Cypress spec (
cypress/e2e/NavigationMenu.cy.ts) locks the behavior. All five cases pass with the fix; the two "sticks open" cases fail without it (3 pass / 2 fail), confirming the test captures the bug:hasPointerMoveOpenedRefreset onpointerLeaveis the right reset point)Also verified:
pnpm --filter @radix-ui/react-navigation-menu lintpnpm --filter @radix-ui/react-navigation-menu typecheckpnpm --filter @radix-ui/react-navigation-menu buildpnpm test(vitest, 233 passing)