diff --git a/.changeset/menuitem-hover-focus-ring.md b/.changeset/menuitem-hover-focus-ring.md new file mode 100644 index 0000000000..a9483cf1fe --- /dev/null +++ b/.changeset/menuitem-hover-focus-ring.md @@ -0,0 +1,5 @@ +--- +'@fluentui-react-native/menu': patch +--- + +Fix `MenuItem` showing a focus ring when focus moves to it from mouse hover on win32. The hover-initiated focus is now tracked with a ref instead of stale pressable state. diff --git a/packages/components/Menu/src/MenuItem/useMenuItem.test.win32.tsx b/packages/components/Menu/src/MenuItem/useMenuItem.test.win32.tsx new file mode 100644 index 0000000000..71bf14d3be --- /dev/null +++ b/packages/components/Menu/src/MenuItem/useMenuItem.test.win32.tsx @@ -0,0 +1,64 @@ +import * as React from 'react'; +import { act } from 'react'; +import * as renderer from 'react-test-renderer'; + +import type { MenuItemInfo } from './MenuItem.types'; +import { useMenuItem } from './useMenuItem'; + +describe('useMenuItem focus ring (win32)', () => { + let info: MenuItemInfo; + const Probe = () => { + info = useMenuItem({ componentRef: React.useRef({ focus: jest.fn(), blur: jest.fn() } as any) }); + return null; + }; + + beforeEach(() => { + act(() => { + renderer.create(); + }); + }); + + it('keeps the focus ring hidden when hover moves focus before React re-renders', () => { + // Simulate the native focus event firing with the handlers from the same render as onHoverIn. + const { onHoverIn, onFocus } = info.props; + act(() => { + onHoverIn({} as any); + onFocus({} as any); + }); + expect(info.props.enableFocusRing).toBe(false); + }); + + it('shows the focus ring for a keyboard focus after a hover focus', () => { + act(() => { + info.props.onHoverIn({} as any); + }); + act(() => { + info.props.onFocus({} as any); + }); + expect(info.props.enableFocusRing).toBe(false); + + act(() => { + info.props.onBlur({} as any); + }); + act(() => { + info.props.onHoverOut({} as any); + }); + act(() => { + info.props.onFocus({} as any); + }); + expect(info.props.enableFocusRing).toBe(true); + }); + + it('shows the focus ring for a keyboard focus after a hover that never focused the item', () => { + // Hover in and out before the hover focus lands, so no focus event comes from the hover. + const { onHoverIn, onHoverOut } = info.props; + act(() => { + onHoverIn({} as any); + onHoverOut({} as any); + }); + act(() => { + info.props.onFocus({} as any); + }); + expect(info.props.enableFocusRing).toBe(true); + }); +}); diff --git a/packages/components/Menu/src/MenuItem/useMenuItem.ts b/packages/components/Menu/src/MenuItem/useMenuItem.ts index 7364567e47..990353852c 100644 --- a/packages/components/Menu/src/MenuItem/useMenuItem.ts +++ b/packages/components/Menu/src/MenuItem/useMenuItem.ts @@ -68,23 +68,38 @@ export const useMenuItem = (props: MenuItemProps): MenuItemInfo => { useHoverFocusEffect(pressable.state.hovered, componentRef); const [enableFocusRing, setEnableFocusRing] = React.useState(!pressable.state.hovered); + // Tracked in a ref rather than state: the focus caused by hovering can fire before React re-renders, so state read in + // onFocus would still be the pre-hover value. + const isHoverFocus = React.useRef(false); const onHoverIn = React.useCallback( (e) => { pressable.props.onHoverIn(e); // when it's a hover focus, set enableFocusRing explicitly to false if (!pressable.state.focused) { + isHoverFocus.current = true; setEnableFocusRing(false); } }, [pressable], ); + const onHoverOut = React.useCallback( + (e) => { + pressable.props.onHoverOut(e); + // the hover ended, so any focus the hover asked for may never arrive; don't let it eat the next real focus + isHoverFocus.current = false; + }, + [pressable], + ); + const onFocus = React.useCallback( (e) => { pressable.props.onFocus(e); // when it's not a hover focus, set enableFocusRing explicitly to true - if (!pressable.state.focused) { + if (isHoverFocus.current) { + isHoverFocus.current = false; + } else { setEnableFocusRing(true); } }, @@ -101,6 +116,10 @@ export const useMenuItem = (props: MenuItemProps): MenuItemInfo => { macos: pressable.props.onHoverIn, default: onHoverIn, // win32 }), + onHoverOut: Platform.select({ + macos: pressable.props.onHoverOut, + default: onHoverOut, // win32 + }), onFocus: Platform.select({ macos: pressable.props.onFocus, default: onFocus, // win32