Skip to content

Commit 5016229

Browse files
adamsamecAdam Samec
andauthored
Fix(react-menu-grid-preview): Left arrow behavior in MenuGrid submenus (#35928)
Co-authored-by: Adam Samec <asamec@microsoft.com>
1 parent 0e111eb commit 5016229

9 files changed

Lines changed: 45 additions & 13 deletions

File tree

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
{
2+
"type": "minor",
3+
"comment": "Fix: Left arrow behavior in MenuGrid submenus",
4+
"packageName": "@fluentui/react-menu",
5+
"email": "email not defined",
6+
"dependentChangeType": "patch"
7+
}
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
{
2+
"type": "minor",
3+
"comment": "Fix: Left arrow behavior in MenuGrid submenus",
4+
"packageName": "@fluentui/react-menu-grid-preview",
5+
"email": "email not defined",
6+
"dependentChangeType": "patch"
7+
}

packages/react-components/react-menu-grid-preview/library/src/components/MenuGrid/MenuGrid.cy.tsx

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -308,4 +308,12 @@ describe('With MenuList submenus', () => {
308308
cy.get(menuSelector).should('have.length', 1);
309309
cy.focused().should('have.attr', 'role', 'menuitem');
310310
});
311+
312+
it('should not close submenu when pressing ArrowLeft inside it', () => {
313+
mount(<WithSubmenuExample />);
314+
cy.get(menuGridItemSelector).first().focus().realPress('ArrowRight').realPress('Enter');
315+
cy.get(menuSelector).should('have.length', 1);
316+
cy.focused().should('have.attr', 'role', 'menuitem').realPress('ArrowLeft');
317+
cy.get(menuSelector).should('have.length', 1);
318+
});
311319
});

packages/react-components/react-menu-grid-preview/library/src/components/MenuGrid/useMenuGrid.test.ts

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -22,11 +22,11 @@ describe('useMenuGrid_unstable', () => {
2222

2323
beforeEach(() => {
2424
rows = [
25-
{ textContent: 'Apple', focus: jest.fn(), hasAttribute: () => true, getAttribute: () => 'row' },
26-
{ textContent: 'Banana', focus: jest.fn(), hasAttribute: () => true, getAttribute: () => 'row' },
27-
{ textContent: 'Cherry', focus: jest.fn(), hasAttribute: () => true, getAttribute: () => 'row' },
28-
{ textContent: 'Apricot', focus: jest.fn(), hasAttribute: () => true, getAttribute: () => 'row' },
29-
{ textContent: 'Date', focus: jest.fn(), hasAttribute: () => true, getAttribute: () => 'row' },
25+
{ textContent: 'Apple', focus: jest.fn(), role: 'row' },
26+
{ textContent: 'Banana', focus: jest.fn(), role: 'row' },
27+
{ textContent: 'Cherry', focus: jest.fn(), role: 'row' },
28+
{ textContent: 'Apricot', focus: jest.fn(), role: 'row' },
29+
{ textContent: 'Date', focus: jest.fn(), role: 'row' },
3030
];
3131

3232
(useFocusFinders as jest.Mock).mockReturnValue({
@@ -37,7 +37,7 @@ describe('useMenuGrid_unstable', () => {
3737
const createEvent = (key: string, target?: Record<string, unknown>): React.KeyboardEvent<HTMLElement> =>
3838
({
3939
key,
40-
target: target ?? { hasAttribute: () => true, getAttribute: () => 'row' },
40+
target: target ?? { role: 'row' },
4141
} as unknown as React.KeyboardEvent<HTMLElement>);
4242

4343
it('should focus the next row matching the pressed character', () => {
@@ -97,7 +97,7 @@ describe('useMenuGrid_unstable', () => {
9797

9898
it('should not apply first-letter navigation when event target is not a row', () => {
9999
const current = rows[0]; // Apple
100-
const nonRowTarget = { hasAttribute: () => true, getAttribute: () => 'gridcell' };
100+
const nonRowTarget = { role: 'gridcell' };
101101

102102
const { result } = renderHook(() => useMenuGrid_unstable({}, React.createRef()));
103103
(result.current.root.ref as React.RefCallback<HTMLElement>)?.(document.createElement('div'));

packages/react-components/react-menu-grid-preview/library/src/components/MenuGrid/useMenuGrid.ts

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -56,14 +56,11 @@ export const useMenuGrid_unstable = (props: MenuGridProps, ref: React.Ref<HTMLDi
5656
const target = e.target as HTMLElement;
5757

5858
// Only apply first-letter navigation when event target is a grid row, otherwise it may conflict with other components inside the grid row
59-
if (!target.hasAttribute('role') || target.getAttribute('role') !== 'row') {
59+
if (target.role !== 'row') {
6060
return;
6161
}
6262

63-
const rows = findAllFocusable(
64-
innerRef.current,
65-
(el: HTMLElement) => el.hasAttribute('role') && el.getAttribute('role') === 'row',
66-
);
63+
const rows = findAllFocusable(innerRef.current, (el: HTMLElement) => el.role === 'row');
6764

6865
let startIndex = rows.indexOf(itemEl) + 1;
6966
if (startIndex === rows.length) {

packages/react-components/react-menu-grid-preview/library/src/components/MenuGrid/useMenuGridContextValues.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ const menuList = {
88
hasIcons: false,
99
hasCheckmarks: false,
1010
shouldOpenOnArrowRight: false,
11+
shouldCloseOnArrowLeft: false,
1112
};
1213

1314
export function useMenuGridContextValues_unstable(state: MenuGridState): MenuGridContextValues {

packages/react-components/react-menu/library/etc/react-menu.api.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -231,6 +231,7 @@ export type MenuListContextValue = Pick<MenuListProps, 'checkedValues' | 'hasIco
231231
selectRadio?: SelectableHandler;
232232
onCheckedValueChange?: (e: MenuCheckedValueChangeEvent, data: MenuCheckedValueChangeData) => void;
233233
shouldOpenOnArrowRight?: boolean;
234+
shouldCloseOnArrowLeft?: boolean;
234235
};
235236

236237
// @public (undocumented)

packages/react-components/react-menu/library/src/components/MenuPopover/useMenuPopover.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import { getIntrinsicElementProps, useEventCallback, useMergedRefs, slot, useTim
88
import * as React from 'react';
99

1010
import { useMenuContext_unstable } from '../../contexts/menuContext';
11+
import { useMenuListContext_unstable } from '../../contexts/menuListContext';
1112
import { dispatchMenuEnterEvent, useIsSubmenu } from '../../utils/index';
1213
import { MenuPopoverProps, MenuPopoverState } from './MenuPopover.types';
1314

@@ -31,6 +32,8 @@ export const useMenuPopover_unstable = (props: MenuPopoverProps, ref: React.Ref<
3132
const triggerRef = useMenuContext_unstable(context => context.triggerRef);
3233

3334
const isSubmenu = useIsSubmenu();
35+
const shouldCloseOnArrowLeft = useMenuListContext_unstable(ctx => ctx.shouldCloseOnArrowLeft ?? true);
36+
3437
const canDispatchCustomEventRef = React.useRef(true);
3538
const restoreFocusSourceAttributes = useRestoreFocusSource();
3639
const [setThrottleTimeout, clearThrottleTimeout] = useTimeout();
@@ -93,7 +96,7 @@ export const useMenuPopover_unstable = (props: MenuPopoverProps, ref: React.Ref<
9396
});
9497
rootProps.onKeyDown = useEventCallback((event: React.KeyboardEvent<HTMLDivElement>) => {
9598
const key = event.key;
96-
if (key === Escape || (isSubmenu && key === CloseArrowKey)) {
99+
if (key === Escape || (isSubmenu && shouldCloseOnArrowLeft && key === CloseArrowKey)) {
97100
if (open && popoverRef.current?.contains(event.target as HTMLElement) && !event.isDefaultPrevented()) {
98101
setOpen(event, { open: false, keyboard: true, type: 'menuPopoverKeyDown', event });
99102
// stop propagation to avoid conflicting with other elements that listen for `Escape`

packages/react-components/react-menu/library/src/contexts/menuListContext.tsx

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,14 @@ export type MenuListContextValue = Pick<MenuListProps, 'checkedValues' | 'hasIco
4444
* @default true
4545
*/
4646
shouldOpenOnArrowRight?: boolean;
47+
/**
48+
* Whether child menus (submenus) should close when the user presses the ArrowLeft key.
49+
* Set to `false` when the list context is provided by a grid-like container (e.g. MenuGrid) where
50+
* ArrowLeft is reserved for column navigation.
51+
*
52+
* @default true
53+
*/
54+
shouldCloseOnArrowLeft?: boolean;
4755
};
4856

4957
export const MenuListProvider = MenuListContext.Provider;

0 commit comments

Comments
 (0)