Skip to content

Commit 7f41c64

Browse files
authored
Menu escape should not clear selection (#4285)
* Menu escape should not clear selection
1 parent a8903d3 commit 7f41c64

File tree

2 files changed

+40
-3
lines changed

2 files changed

+40
-3
lines changed

packages/@react-aria/menu/src/useMenu.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,13 @@ export function useMenu<T>(props: AriaMenuOptions<T>, state: TreeState<T>, ref:
7474
return {
7575
menuProps: mergeProps(domProps, {
7676
role: 'menu',
77-
...listProps
77+
...listProps,
78+
onKeyDown: (e) => {
79+
// don't clear the menu selected keys if the user is presses escape since escape closes the menu
80+
if (e.key !== 'Escape') {
81+
listProps.onKeyDown(e);
82+
}
83+
}
7884
})
7985
};
8086
}

packages/@react-spectrum/menu/test/MenuTrigger.test.js

Lines changed: 33 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -370,7 +370,6 @@ describe('MenuTrigger', function () {
370370
act(() => {jest.runAllTimers();}); // FocusScope raf
371371
}
372372

373-
// Can't figure out why this isn't working for the v2 component
374373
it.each`
375374
Name | Component | props
376375
${'MenuTrigger'} | ${MenuTrigger} | ${{onOpenChange}}
@@ -389,7 +388,39 @@ describe('MenuTrigger', function () {
389388
expect(document.activeElement).toBe(button);
390389
});
391390

392-
// Can't figure out why this isn't working for the v2 component
391+
it.each`
392+
Name | Component | props
393+
${'MenuTrigger'} | ${MenuTrigger} | ${{onOpenChange}}
394+
`('$Name does not clear selection with escape', function ({Component, props}) {
395+
let onSelectionChange = jest.fn();
396+
tree = renderComponent(Component, props, {selectionMode: 'multiple', defaultSelectedKeys: ['Foo'], onSelectionChange});
397+
let button = tree.getByRole('button');
398+
triggerPress(button);
399+
act(() => {jest.runAllTimers();});
400+
expect(onSelectionChange).not.toHaveBeenCalled();
401+
402+
let menu = tree.getByRole('menu');
403+
expect(menu).toBeTruthy();
404+
expect(within(menu).getAllByRole('menuitemcheckbox')[0]).toHaveAttribute('aria-checked', 'true');
405+
fireEvent.keyDown(menu, {key: 'Escape', code: 27, charCode: 27});
406+
act(() => {jest.runAllTimers();}); // FocusScope useLayoutEffect cleanup
407+
act(() => {jest.runAllTimers();}); // FocusScope raf
408+
expect(menu).not.toBeInTheDocument();
409+
expect(document.activeElement).toBe(button);
410+
expect(onSelectionChange).not.toHaveBeenCalled();
411+
412+
// reopen and make sure we still have the selection
413+
triggerPress(button);
414+
act(() => {jest.runAllTimers();});
415+
expect(onSelectionChange).not.toHaveBeenCalled();
416+
417+
menu = tree.getByRole('menu');
418+
expect(within(menu).getAllByRole('menuitemcheckbox')[0]).toHaveAttribute('aria-checked', 'true');
419+
expect(menu).toBeTruthy();
420+
fireEvent.keyDown(menu, {key: 'Escape', code: 27, charCode: 27});
421+
expect(onSelectionChange).not.toHaveBeenCalled();
422+
});
423+
393424
it.each`
394425
Name | Component | props
395426
${'MenuTrigger'} | ${MenuTrigger} | ${{onOpenChange}}

0 commit comments

Comments
 (0)