Skip to content

fix(menu): don't show focus ring when MenuItem is focused by hover - #4368

Open
udit (UditDewan) wants to merge 1 commit into
microsoft:mainfrom
UditDewan:fix/menuitem-stale-focus-ring
Open

udit (UditDewan) wants to merge 1 commit into
microsoft:mainfrom
UditDewan:fix/menuitem-stale-focus-ring

Conversation

@UditDewan

Copy link
Copy Markdown

Platforms Impacted

  • iOS
  • macOS
  • win32 (Office)
  • windows
  • android

Description of changes

Fixes #4083.

When the mouse hovers a MenuItem, onHoverIn hides the focus ring and useHoverFocusEffect then calls .focus() on the item. onFocus tried to detect "this focus came from hover" by checking !pressable.state.focused, but that value is the pre-hover render's state, so the check always passed and onFocus re-enabled the focus ring, overwriting the false that onHoverIn had just set. The result is a focus ring on hovered menu items.

packages/components/Menu/src/MenuItem/useMenuItem.ts:

  • Track hover-initiated focus with a ref (isHoverFocus) set in onHoverIn. Refs are read synchronously, so they aren't affected by React not having re-rendered yet.
  • onFocus consumes the flag: if focus came from hover the ring stays hidden, otherwise (keyboard/programmatic focus) the ring is enabled as before.

macOS is unaffected (it already uses the plain pressable handlers and never shows the ring).

Verification

  • Added src/MenuItem/useMenuItem.test.win32.tsx covering:
    • hover-in followed immediately by focus with handlers from the same render (the stale-state case from the issue) keeps enableFocusRing false
    • a keyboard focus after a hover/blur cycle still shows the focus ring
  • Both tests fail on main and pass with this change.
  • yarn build, yarn lint, yarn test for @fluentui-react-native/menu; yarn format:check.
Before After
Hovering a menu item on win32 moves focus to it and shows the keyboard focus ring Hovering moves focus without the focus ring; keyboard navigation still shows it

Pull request checklist

This PR has considered (when applicable):

  • Automated Tests
  • Documentation and examples
  • Keyboard Accessibility
  • Voiceover
  • Internationalization and Right-to-left Layouts

On hover, onHoverIn hides the focus ring and useHoverFocusEffect then
focuses the item. onFocus decided whether the focus came from hover by
reading pressable.state.focused, which is stale at that point, so it
always re-enabled the focus ring. Track hover-initiated focus with a
ref instead so onFocus sees it synchronously.

Fixes microsoft#4083
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.

useMenuItem relies on stale React state

1 participant