Skip to content

Commit cce6f16

Browse files
authored
feat: Add escapeKeyBehavior to GridList/ListBox/Menu/Table/Tree (#7974)
* Add disallowClearAll to Menu/ListBox so Autocomplete in Popover can close without clearing selection * add support for diallowClearAll to grid/tree/table * make sure RSP components also surface disallowClearAll * update api naming to escapeKeyBehavior * skip 17 tests for build, investigate later * review comments
1 parent dd22a53 commit cce6f16

File tree

22 files changed

+482
-83
lines changed

22 files changed

+482
-83
lines changed

packages/@react-aria/grid/src/useGrid.ts

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,16 @@ export interface GridProps extends DOMProps, AriaLabelingProps {
5353
/** Handler that is called when a user performs an action on the row. */
5454
onRowAction?: (key: Key) => void,
5555
/** Handler that is called when a user performs an action on the cell. */
56-
onCellAction?: (key: Key) => void
56+
onCellAction?: (key: Key) => void,
57+
/**
58+
* Whether pressing the escape key should clear selection in the grid or not.
59+
*
60+
* Most experiences should not modify this option as it eliminates a keyboard user's ability to
61+
* easily clear selection. Only use if the escape key is being handled externally or should not
62+
* trigger selection clearing contextually.
63+
* @default 'clearSelection'
64+
*/
65+
escapeKeyBehavior?: 'clearSelection' | 'none'
5766
}
5867

5968
export interface GridAria {
@@ -77,7 +86,8 @@ export function useGrid<T>(props: GridProps, state: GridState<T, GridCollection<
7786
scrollRef,
7887
getRowText,
7988
onRowAction,
80-
onCellAction
89+
onCellAction,
90+
escapeKeyBehavior = 'clearSelection'
8191
} = props;
8292
let {selectionManager: manager} = state;
8393

@@ -106,7 +116,8 @@ export function useGrid<T>(props: GridProps, state: GridState<T, GridCollection<
106116
keyboardDelegate: delegate,
107117
isVirtualized,
108118
scrollRef,
109-
disallowTypeAhead
119+
disallowTypeAhead,
120+
escapeKeyBehavior
110121
});
111122

112123
let id = useId(props.id);

packages/@react-aria/gridlist/src/useGridList.ts

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,16 @@ export interface AriaGridListProps<T> extends GridListProps<T>, DOMProps, AriaLa
4848
* via the left/right arrow keys or the tab key.
4949
* @default 'arrow'
5050
*/
51-
keyboardNavigationBehavior?: 'arrow' | 'tab'
51+
keyboardNavigationBehavior?: 'arrow' | 'tab',
52+
/**
53+
* Whether pressing the escape key should clear selection in the grid list or not.
54+
*
55+
* Most experiences should not modify this option as it eliminates a keyboard user's ability to
56+
* easily clear selection. Only use if the escape key is being handled externally or should not
57+
* trigger selection clearing contextually.
58+
* @default 'clearSelection'
59+
*/
60+
escapeKeyBehavior?: 'clearSelection' | 'none'
5261
}
5362

5463
export interface AriaGridListOptions<T> extends Omit<AriaGridListProps<T>, 'children'> {
@@ -105,7 +114,8 @@ export function useGridList<T>(props: AriaGridListOptions<T>, state: ListState<T
105114
onAction,
106115
disallowTypeAhead,
107116
linkBehavior = 'action',
108-
keyboardNavigationBehavior = 'arrow'
117+
keyboardNavigationBehavior = 'arrow',
118+
escapeKeyBehavior = 'clearSelection'
109119
} = props;
110120

111121
if (!props['aria-label'] && !props['aria-labelledby']) {
@@ -124,7 +134,8 @@ export function useGridList<T>(props: AriaGridListOptions<T>, state: ListState<T
124134
shouldFocusWrap: props.shouldFocusWrap,
125135
linkBehavior,
126136
disallowTypeAhead,
127-
autoFocus: props.autoFocus
137+
autoFocus: props.autoFocus,
138+
escapeKeyBehavior
128139
});
129140

130141
let id = useId(props.id);

packages/@react-aria/selection/src/useSelectableCollection.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,11 @@ export interface AriaSelectableCollectionOptions {
5454
* @default false
5555
*/
5656
disallowSelectAll?: boolean,
57+
/**
58+
* Whether pressing the Escape should clear selection in the collection or not.
59+
* @default 'clearSelection'
60+
*/
61+
escapeKeyBehavior?: 'clearSelection' | 'none',
5762
/**
5863
* Whether selection should occur automatically on focus.
5964
* @default false
@@ -108,6 +113,7 @@ export function useSelectableCollection(options: AriaSelectableCollectionOptions
108113
shouldFocusWrap = false,
109114
disallowEmptySelection = false,
110115
disallowSelectAll = false,
116+
escapeKeyBehavior = 'clearSelection',
111117
selectOnFocus = manager.selectionBehavior === 'replace',
112118
disallowTypeAhead = false,
113119
shouldUseVirtualFocus,
@@ -279,7 +285,7 @@ export function useSelectableCollection(options: AriaSelectableCollectionOptions
279285
}
280286
break;
281287
case 'Escape':
282-
if (!disallowEmptySelection && manager.selectedKeys.size !== 0) {
288+
if (escapeKeyBehavior === 'clearSelection' && !disallowEmptySelection && manager.selectedKeys.size !== 0) {
283289
e.stopPropagation();
284290
e.preventDefault();
285291
manager.clearSelection();

packages/@react-spectrum/list/test/ListView.test.js

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -812,6 +812,24 @@ describe('ListView', function () {
812812
expect(announce).toHaveBeenCalledTimes(3);
813813
});
814814

815+
it('should prevent Esc from clearing selection if escapeKeyBehavior is "none"', async function () {
816+
let tree = renderSelectionList({onSelectionChange, selectionMode: 'multiple', escapeKeyBehavior: 'none'});
817+
818+
let rows = tree.getAllByRole('row');
819+
await user.click(within(rows[1]).getByRole('checkbox'));
820+
checkSelection(onSelectionChange, ['bar']);
821+
822+
onSelectionChange.mockClear();
823+
await user.click(within(rows[2]).getByRole('checkbox'));
824+
checkSelection(onSelectionChange, ['bar', 'baz']);
825+
826+
onSelectionChange.mockClear();
827+
await user.keyboard('{Escape}');
828+
expect(onSelectionChange).not.toHaveBeenCalled();
829+
expect(rows[1]).toHaveAttribute('aria-selected', 'true');
830+
expect(rows[2]).toHaveAttribute('aria-selected', 'true');
831+
});
832+
815833
it('should support range selection', async function () {
816834
let tree = renderSelectionList({onSelectionChange, selectionMode: 'multiple'});
817835

packages/@react-spectrum/listbox/test/ListBox.test.js

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -499,6 +499,30 @@ describe('ListBox', function () {
499499

500500
expect(onSelectionChange).toBeCalledTimes(0);
501501
});
502+
503+
it('should prevent Esc from clearing selection if escapeKeyBehavior is "none"', async function () {
504+
let user = userEvent.setup({delay: null, pointerMap});
505+
let tree = renderComponent({onSelectionChange, selectionMode: 'multiple', escapeKeyBehavior: 'none'});
506+
let listbox = tree.getByRole('listbox');
507+
508+
let options = within(listbox).getAllByRole('option');
509+
let firstItem = options[3];
510+
await user.click(firstItem);
511+
expect(firstItem).toHaveAttribute('aria-selected', 'true');
512+
513+
let secondItem = options[1];
514+
await user.click(secondItem);
515+
expect(secondItem).toHaveAttribute('aria-selected', 'true');
516+
517+
expect(onSelectionChange).toBeCalledTimes(2);
518+
expect(onSelectionChange.mock.calls[0][0].has('Blah')).toBeTruthy();
519+
expect(onSelectionChange.mock.calls[1][0].has('Bar')).toBeTruthy();
520+
521+
await user.keyboard('{Escape}');
522+
expect(onSelectionChange).toBeCalledTimes(2);
523+
expect(onSelectionChange.mock.calls[0][0].has('Blah')).toBeTruthy();
524+
expect(onSelectionChange.mock.calls[1][0].has('Bar')).toBeTruthy();
525+
});
502526
});
503527

504528
describe('supports no selection', function () {

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

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -267,6 +267,30 @@ describe('MenuTrigger', function () {
267267
expect(onOpenChange).toBeCalledTimes(1);
268268
});
269269

270+
it.each`
271+
Name | Component | props
272+
${'MenuTrigger'} | ${MenuTrigger} | ${{onOpenChange}}
273+
`('$Name should prevent Esc from clearing selection and close the menu if escapeKeyBehavior is "none"', async function ({Component, props}) {
274+
tree = renderComponent(Component, props, {selectionMode: 'multiple', escapeKeyBehavior: 'none', onSelectionChange});
275+
let menuTester = testUtilUser.createTester('Menu', {root: tree.container, interactionType: 'keyboard'});
276+
expect(onOpenChange).toBeCalledTimes(0);
277+
await menuTester.open();
278+
279+
expect(onOpenChange).toBeCalledTimes(1);
280+
expect(onSelectionChange).toBeCalledTimes(0);
281+
282+
await menuTester.selectOption({option: 'Foo', menuSelectionMode: 'multiple', keyboardActivation: 'Space'});
283+
expect(onSelectionChange).toBeCalledTimes(1);
284+
expect(onSelectionChange.mock.calls[0][0].has('Foo')).toBeTruthy();
285+
await menuTester.selectOption({option: 'Bar', menuSelectionMode: 'multiple', keyboardActivation: 'Space'});
286+
expect(onSelectionChange).toBeCalledTimes(2);
287+
expect(onSelectionChange.mock.calls[1][0].has('Bar')).toBeTruthy();
288+
289+
await menuTester.close();
290+
expect(menuTester.menu).not.toBeInTheDocument();
291+
expect(onOpenChange).toBeCalledTimes(2);
292+
});
293+
270294
it.each`
271295
Name | Component | props | menuProps
272296
${'MenuTrigger multiple'} | ${MenuTrigger} | ${{closeOnSelect: true}} | ${{selectionMode: 'multiple', onClose}}

packages/@react-spectrum/table/test/Table.test.js

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -140,9 +140,9 @@ function pointerEvent(type, opts) {
140140
}
141141

142142
export let tableTests = () => {
143-
// Temporarily disabling these tests in React 16 because they run into a memory limit and crash.
143+
// Temporarily disabling these tests in React 16/17 because they run into a memory limit and crash.
144144
// TODO: investigate.
145-
if (parseInt(React.version, 10) <= 16) {
145+
if (parseInt(React.version, 10) <= 17) {
146146
return;
147147
}
148148

@@ -2217,6 +2217,26 @@ export let tableTests = () => {
22172217
checkSelectAll(tree, 'indeterminate');
22182218
});
22192219

2220+
it('should prevent Esc from clearing selection if escapeKeyBehavior is "none"', async function () {
2221+
let onSelectionChange = jest.fn();
2222+
let tree = renderTable({onSelectionChange, selectionMode: 'multiple', escapeKeyBehavior: 'none'});
2223+
let tableTester = testUtilUser.createTester('Table', {root: tree.getByRole('grid')});
2224+
tableTester.setInteractionType('keyboard');
2225+
2226+
2227+
await tableTester.toggleRowSelection({row: 0});
2228+
expect(tableTester.selectedRows).toHaveLength(1);
2229+
expect(onSelectionChange).toHaveBeenCalledTimes(1);
2230+
2231+
await tableTester.toggleRowSelection({row: 1});
2232+
expect(tableTester.selectedRows).toHaveLength(2);
2233+
expect(onSelectionChange).toHaveBeenCalledTimes(2);
2234+
2235+
await user.keyboard('{Escape}');
2236+
expect(tableTester.selectedRows).toHaveLength(2);
2237+
expect(onSelectionChange).toHaveBeenCalledTimes(2);
2238+
});
2239+
22202240
it('should not allow selection of a disabled row via checkbox click', async function () {
22212241
let onSelectionChange = jest.fn();
22222242
let tree = renderTable({onSelectionChange, disabledKeys: ['Foo 1']});

packages/@react-spectrum/tree/test/TreeView.test.tsx

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -463,6 +463,27 @@ describe('Tree', () => {
463463
expect(treeTester.selectedRows[0]).toBe(row1);
464464
});
465465

466+
it('should prevent Esc from clearing selection if escapeKeyBehavior is "none"', async () => {
467+
let {getByRole} = render(<StaticTree treeProps={{selectionMode: 'multiple', escapeKeyBehavior: 'none'}} />);
468+
let treeTester = testUtilUser.createTester('Tree', {user, root: getByRole('treegrid')});
469+
let rows = treeTester.rows;
470+
let row1 = rows[1];
471+
await treeTester.toggleRowSelection({row: row1});
472+
expect(onSelectionChange).toHaveBeenCalledTimes(1);
473+
expect(new Set(onSelectionChange.mock.calls[0][0])).toEqual(new Set(['Projects']));
474+
expect(treeTester.selectedRows).toHaveLength(1);
475+
476+
let row2 = rows[2];
477+
await treeTester.toggleRowSelection({row: row2});
478+
expect(onSelectionChange).toHaveBeenCalledTimes(2);
479+
expect(new Set(onSelectionChange.mock.calls[1][0])).toEqual(new Set(['Projects', 'Projects-1']));
480+
expect(treeTester.selectedRows).toHaveLength(2);
481+
482+
await user.keyboard('{Escape}');
483+
expect(onSelectionChange).toHaveBeenCalledTimes(2);
484+
expect(treeTester.selectedRows).toHaveLength(2);
485+
});
486+
466487
it('should render a chevron for an expandable row marked with hasChildItems', () => {
467488
let {getAllByRole} = render(
468489
<TreeView aria-label="test tree">

packages/@react-types/listbox/src/index.d.ts

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,17 @@ export interface ListBoxProps<T> extends CollectionBase<T>, MultipleSelection, F
2020
shouldFocusWrap?: boolean
2121
}
2222

23-
interface AriaListBoxPropsBase<T> extends ListBoxProps<T>, DOMProps, AriaLabelingProps {}
23+
interface AriaListBoxPropsBase<T> extends ListBoxProps<T>, DOMProps, AriaLabelingProps {
24+
/**
25+
* Whether pressing the escape key should clear selection in the listbox or not.
26+
*
27+
* Most experiences should not modify this option as it eliminates a keyboard user's ability to
28+
* easily clear selection. Only use if the escape key is being handled externally or should not
29+
* trigger selection clearing contextually.
30+
* @default 'clearSelection'
31+
*/
32+
escapeKeyBehavior?: 'clearSelection' | 'none'
33+
}
2434
export interface AriaListBoxProps<T> extends AriaListBoxPropsBase<T> {
2535
/**
2636
* An optional visual label for the listbox.

packages/@react-types/menu/src/index.d.ts

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,17 @@ export interface MenuProps<T> extends CollectionBase<T>, MultipleSelection {
6262
onClose?: () => void
6363
}
6464

65-
export interface AriaMenuProps<T> extends MenuProps<T>, DOMProps, AriaLabelingProps {}
65+
export interface AriaMenuProps<T> extends MenuProps<T>, DOMProps, AriaLabelingProps {
66+
/**
67+
* Whether pressing the escape key should clear selection in the menu or not.
68+
*
69+
* Most experiences should not modify this option as it eliminates a keyboard user's ability to
70+
* easily clear selection. Only use if the escape key is being handled externally or should not
71+
* trigger selection clearing contextually.
72+
* @default 'clearSelection'
73+
*/
74+
escapeKeyBehavior?: 'clearSelection' | 'none'
75+
}
6676
export interface SpectrumMenuProps<T> extends AriaMenuProps<T>, StyleProps {}
6777

6878
export interface SpectrumActionMenuProps<T> extends CollectionBase<T>, Omit<SpectrumMenuTriggerProps, 'children'>, StyleProps, DOMProps, AriaLabelingProps {

0 commit comments

Comments
 (0)