Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "minor",
"comment": "Fix: Left arrow behavior in MenuGrid submenus",
"packageName": "@fluentui/react-menu",
"email": "email not defined",
"dependentChangeType": "patch"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "minor",
"comment": "Fix: Left arrow behavior in MenuGrid submenus",
"packageName": "@fluentui/react-menu-grid-preview",
"email": "email not defined",
"dependentChangeType": "patch"
}
Original file line number Diff line number Diff line change
Expand Up @@ -308,4 +308,12 @@ describe('With MenuList submenus', () => {
cy.get(menuSelector).should('have.length', 1);
cy.focused().should('have.attr', 'role', 'menuitem');
});

it('should not close submenu when pressing ArrowLeft inside it', () => {
mount(<WithSubmenuExample />);
cy.get(menuGridItemSelector).first().focus().realPress('ArrowRight').realPress('Enter');
cy.get(menuSelector).should('have.length', 1);
cy.focused().should('have.attr', 'role', 'menuitem').realPress('ArrowLeft');
cy.get(menuSelector).should('have.length', 1);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,11 @@ describe('useMenuGrid_unstable', () => {

beforeEach(() => {
rows = [
{ textContent: 'Apple', focus: jest.fn(), hasAttribute: () => true, getAttribute: () => 'row' },
{ textContent: 'Banana', focus: jest.fn(), hasAttribute: () => true, getAttribute: () => 'row' },
{ textContent: 'Cherry', focus: jest.fn(), hasAttribute: () => true, getAttribute: () => 'row' },
{ textContent: 'Apricot', focus: jest.fn(), hasAttribute: () => true, getAttribute: () => 'row' },
{ textContent: 'Date', focus: jest.fn(), hasAttribute: () => true, getAttribute: () => 'row' },
{ textContent: 'Apple', focus: jest.fn(), role: 'row' },
{ textContent: 'Banana', focus: jest.fn(), role: 'row' },
{ textContent: 'Cherry', focus: jest.fn(), role: 'row' },
{ textContent: 'Apricot', focus: jest.fn(), role: 'row' },
{ textContent: 'Date', focus: jest.fn(), role: 'row' },
];

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

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

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

const { result } = renderHook(() => useMenuGrid_unstable({}, React.createRef()));
(result.current.root.ref as React.RefCallback<HTMLElement>)?.(document.createElement('div'));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,14 +56,11 @@ export const useMenuGrid_unstable = (props: MenuGridProps, ref: React.Ref<HTMLDi
const target = e.target as HTMLElement;

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

const rows = findAllFocusable(
innerRef.current,
(el: HTMLElement) => el.hasAttribute('role') && el.getAttribute('role') === 'row',
);
const rows = findAllFocusable(innerRef.current, (el: HTMLElement) => el.role === 'row');

let startIndex = rows.indexOf(itemEl) + 1;
if (startIndex === rows.length) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ const menuList = {
hasIcons: false,
hasCheckmarks: false,
shouldOpenOnArrowRight: false,
shouldCloseOnArrowLeft: false,
};

export function useMenuGridContextValues_unstable(state: MenuGridState): MenuGridContextValues {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -231,6 +231,7 @@ export type MenuListContextValue = Pick<MenuListProps, 'checkedValues' | 'hasIco
selectRadio?: SelectableHandler;
onCheckedValueChange?: (e: MenuCheckedValueChangeEvent, data: MenuCheckedValueChangeData) => void;
shouldOpenOnArrowRight?: boolean;
shouldCloseOnArrowLeft?: boolean;
};

// @public (undocumented)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import { getIntrinsicElementProps, useEventCallback, useMergedRefs, slot, useTim
import * as React from 'react';

import { useMenuContext_unstable } from '../../contexts/menuContext';
import { useMenuListContext_unstable } from '../../contexts/menuListContext';
import { dispatchMenuEnterEvent, useIsSubmenu } from '../../utils/index';
import { MenuPopoverProps, MenuPopoverState } from './MenuPopover.types';

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

const isSubmenu = useIsSubmenu();
const shouldCloseOnArrowLeft = useMenuListContext_unstable(ctx => ctx.shouldCloseOnArrowLeft ?? true);

const canDispatchCustomEventRef = React.useRef(true);
const restoreFocusSourceAttributes = useRestoreFocusSource();
const [setThrottleTimeout, clearThrottleTimeout] = useTimeout();
Expand Down Expand Up @@ -93,7 +96,7 @@ export const useMenuPopover_unstable = (props: MenuPopoverProps, ref: React.Ref<
});
rootProps.onKeyDown = useEventCallback((event: React.KeyboardEvent<HTMLDivElement>) => {
const key = event.key;
if (key === Escape || (isSubmenu && key === CloseArrowKey)) {
if (key === Escape || (isSubmenu && shouldCloseOnArrowLeft && key === CloseArrowKey)) {
if (open && popoverRef.current?.contains(event.target as HTMLElement) && !event.isDefaultPrevented()) {
setOpen(event, { open: false, keyboard: true, type: 'menuPopoverKeyDown', event });
// stop propagation to avoid conflicting with other elements that listen for `Escape`
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,14 @@ export type MenuListContextValue = Pick<MenuListProps, 'checkedValues' | 'hasIco
* @default true
*/
shouldOpenOnArrowRight?: boolean;
/**
* Whether child menus (submenus) should close when the user presses the ArrowLeft key.
* Set to `false` when the list context is provided by a grid-like container (e.g. MenuGrid) where
* ArrowLeft is reserved for column navigation.
*
* @default true
*/
shouldCloseOnArrowLeft?: boolean;
};

export const MenuListProvider = MenuListContext.Provider;
Expand Down
Loading