Skip to content

Commit

Permalink
fix(Menu): Do not rely on tabster restorer for focus restore (#32840)
Browse files Browse the repository at this point in the history
Co-authored-by: Juraj Kapsiar <jurokapsiar@gmail.com>
  • Loading branch information
ling1726 and jurokapsiar authored Sep 23, 2024
1 parent c189a92 commit 1a4a95e
Showing 6 changed files with 27 additions and 8 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "patch",
"comment": "fix(Menu): Do not rely on tabster restorer for focus restore",
"packageName": "@fluentui/react-menu",
"email": "lingfangao@hotmail.com",
"dependentChangeType": "patch"
}
Original file line number Diff line number Diff line change
@@ -4,7 +4,6 @@ exports[`Menu renders a default state 1`] = `
<div>
<button
aria-haspopup="menu"
data-tabster="{\\"restorer\\":{\\"type\\":1}}"
id="menu1"
>
Menu trigger
Original file line number Diff line number Diff line change
@@ -13,6 +13,7 @@ import {
useOnScrollOutside,
elementContains,
useTimeout,
useFirstMount,
} from '@fluentui/react-utilities';
import { useFluent_unstable as useFluent } from '@fluentui/react-shared-contexts';
import { useFocusFinders } from '@fluentui/react-tabster';
@@ -173,6 +174,8 @@ const useMenuOpenState = (
> &
Pick<MenuProps, 'open' | 'defaultOpen' | 'onOpenChange'>,
) => {
'use no memo';

const { targetDocument } = useFluent();
const parentSetOpen = useMenuContext_unstable(context => context.setOpen);
const onOpenChange: MenuProps['onOpenChange'] = useEventCallback((e, data) => state.onOpenChange?.(e, data));
@@ -265,11 +268,26 @@ const useMenuOpenState = (
firstFocusable?.focus();
}, [findFirstFocusable, state.menuPopoverRef]);

const firstMount = useFirstMount();
React.useEffect(() => {
if (open) {
focusFirst();
} else {
if (!firstMount) {
if (targetDocument?.activeElement === targetDocument?.body) {
// We know that React effects are sync so we focus the trigger here
// after any event handler (event handlers will update state and re-render).
// Since the browser only performs the default behaviour for the Tab key once
// keyboard events have fully bubbled up the window, the browser will move
// focus to the next tabbable element before/after the trigger if needed.
// If the Tab key was not pressed, focus will remain on the trigger as expected.
state.triggerRef.current?.focus();
}
}
}
}, [open, focusFirst]);
// firstMount change should not re-run this effect
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [state.triggerRef, state.isSubmenu, open, focusFirst, targetDocument, state.menuPopoverRef]);

return [open, setOpen] as const;
};
Original file line number Diff line number Diff line change
@@ -57,7 +57,6 @@ describe('MenuTrigger', () => {
Array [
<button
aria-haspopup="menu"
data-tabster="{\\"restorer\\":{\\"type\\":1}}"
id="id"
>
Trigger
@@ -88,7 +87,6 @@ describe('MenuTrigger', () => {
Array [
<button
aria-haspopup="menu"
data-tabster="{\\"restorer\\":{\\"type\\":1}}"
id="id"
>
Trigger
Original file line number Diff line number Diff line change
@@ -3,7 +3,6 @@
exports[`MenuTrigger renders a default state 1`] = `
<button
aria-haspopup="menu"
data-tabster="{\\"restorer\\":{\\"type\\":1}}"
id="id"
onClick={[Function]}
onContextMenu={[Function]}
Original file line number Diff line number Diff line change
@@ -2,7 +2,7 @@ import * as React from 'react';
import { MenuTriggerProps, MenuTriggerState } from './MenuTrigger.types';
import { useMenuContext_unstable } from '../../contexts/menuContext';
import { useIsSubmenu } from '../../utils/useIsSubmenu';
import { useFocusFinders, useRestoreFocusTarget } from '@fluentui/react-tabster';
import { useFocusFinders } from '@fluentui/react-tabster';
import { useFluent_unstable as useFluent } from '@fluentui/react-shared-contexts';
import { ArrowRight, ArrowLeft, Escape, ArrowDown } from '@fluentui/keyboard-keys';
import {
@@ -31,7 +31,6 @@ export const useMenuTrigger_unstable = (props: MenuTriggerProps): MenuTriggerSta
const triggerId = useMenuContext_unstable(context => context.triggerId);
const openOnHover = useMenuContext_unstable(context => context.openOnHover);
const openOnContext = useMenuContext_unstable(context => context.openOnContext);
const restoreFocusTargetAttribute = useRestoreFocusTarget();

const isSubmenu = useIsSubmenu();

@@ -125,7 +124,6 @@ export const useMenuTrigger_unstable = (props: MenuTriggerProps): MenuTriggerSta

const contextMenuProps = {
id: triggerId,
...restoreFocusTargetAttribute,
...child?.props,
ref: useMergedRefs(triggerRef, child?.ref),
onMouseEnter: useEventCallback(mergeCallbacks(child?.props.onMouseEnter, onMouseEnter)),

0 comments on commit 1a4a95e

Please sign in to comment.