Skip to content

Commit b4ea4fe

Browse files
authored
fix: tree focus management (adobe#9531)
* fix: tree focus management * one possible fix * fix issue in treeview directly * add table test because behavior should be different * fix lint
1 parent ff9c58b commit b4ea4fe

3 files changed

Lines changed: 120 additions & 3 deletions

File tree

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -200,6 +200,12 @@ export function useSelectableItem(options: SelectableItemOptions): SelectableIte
200200
};
201201
}
202202

203+
useEffect(() => {
204+
if (isDisabled && manager.focusedKey === key) {
205+
manager.setFocusedKey(null);
206+
}
207+
}, [manager, isDisabled, key]);
208+
203209
// With checkbox selection, onAction (i.e. navigation) becomes primary, and occurs on a single click of the row.
204210
// Clicking the checkbox enters selection mode, after which clicking anywhere on any row toggles selection for that row.
205211
// With highlight selection, onAction is secondary, and occurs on double click. Single click selects the row.

packages/@react-spectrum/s2/test/TableView.test.tsx

Lines changed: 42 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,17 +24,21 @@ import {
2424
TableView,
2525
Text
2626
} from '../src';
27+
import {DisabledBehavior} from '@react-types/shared';
2728
import Filter from '../s2wf-icons/S2_Icon_Filter_20_N.svg';
28-
import React from 'react';
29-
import {User} from '@react-aria/test-utils';
29+
import {pointerMap, User} from '@react-aria/test-utils';
30+
import React, {useState} from 'react';
31+
import userEvent from '@testing-library/user-event';
3032

3133
// @ts-ignore
3234
window.getComputedStyle = (el) => el.style;
3335

3436
describe('TableView', () => {
3537
let offsetWidth, offsetHeight;
38+
let user;
3639
let testUtilUser = new User({advanceTimer: jest.advanceTimersByTime});
3740
beforeAll(function () {
41+
user = userEvent.setup({delay: null, pointerMap});
3842
offsetWidth = jest.spyOn(window.HTMLElement.prototype, 'clientWidth', 'get').mockImplementation(() => 400);
3943
offsetHeight = jest.spyOn(window.HTMLElement.prototype, 'clientHeight', 'get').mockImplementation(() => 200);
4044
jest.useFakeTimers();
@@ -108,4 +112,40 @@ describe('TableView', () => {
108112
await tableTester.triggerColumnHeaderAction({column: 1, action: 0, interactionType: 'keyboard'});
109113
expect(onAction).toHaveBeenCalledTimes(1);
110114
});
115+
116+
it('if the previously focused cell\'s row is disabled, the focus should still be restored to the cell when the disabled behavior is changed and the user navigates to the collection', async () => {
117+
function Example() {
118+
let [disabledBehavior, setDisabledBehavior] = useState<DisabledBehavior>('selection');
119+
return (
120+
<>
121+
<button>Before</button>
122+
<TableView aria-label="Dynamic table" disabledBehavior={disabledBehavior} disabledKeys={['2']}>
123+
<TableHeader columns={columns}>
124+
{(column) => <Column {...column}>{column.name}</Column>}
125+
</TableHeader>
126+
<TableBody>
127+
<Row id="1"><Cell>Foo 1</Cell><Cell>Bar 1</Cell><Cell>Baz 1</Cell><Cell>Yah 1</Cell></Row>
128+
<Row id="2"><Cell>Foo 2</Cell><Cell>Bar 2</Cell><Cell>Baz 2</Cell><Cell>Yah 2</Cell></Row>
129+
<Row id="3"><Cell>Foo 3</Cell><Cell>Bar 3</Cell><Cell>Baz 3</Cell><Cell>Yah 3</Cell></Row>
130+
</TableBody>
131+
</TableView>
132+
<button onClick={() => setDisabledBehavior('all')}>After</button>
133+
</>
134+
);
135+
}
136+
137+
let {getAllByRole, getByRole} = render(<Example />);
138+
await user.click(document.body);
139+
140+
let cells = getAllByRole('gridcell');
141+
let afterButton = getByRole('button', {name: 'After'});
142+
await user.click(cells[3]); // Bar 2
143+
expect(document.activeElement).toBe(cells[3]);
144+
145+
await user.click(afterButton);
146+
await user.tab({shift: true});
147+
await user.tab({shift: true});
148+
await user.tab();
149+
expect(document.activeElement).toBe(cells[3]);
150+
});
111151
});

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

Lines changed: 72 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,9 +9,10 @@ import {
99
TreeViewItemContent
1010
} from '../src';
1111
import {AriaTreeTests} from '../../../react-aria-components/test/AriaTree.test-util';
12+
import {DisabledBehavior} from '@react-types/shared';
1213
import FileTxt from '../s2wf-icons/S2_Icon_FileText_20_N.svg';
1314
import Folder from '../s2wf-icons/S2_Icon_Folder_20_N.svg';
14-
import React from 'react';
15+
import React, {useState} from 'react';
1516
import userEvent from '@testing-library/user-event';
1617

1718
AriaTreeTests({
@@ -493,4 +494,74 @@ describe('TreeView', () => {
493494
let menu = queryByRole('menu');
494495
expect(menu).not.toBeInTheDocument();
495496
});
497+
498+
it('if the previously focused item is disabled, the focus should move to the first item when coming back to the collection', async () => {
499+
function Example() {
500+
let [disabledBehavior, setDisabledBehavior] = useState<DisabledBehavior>('selection');
501+
return (
502+
<>
503+
<button>Before</button>
504+
<TreeView aria-label="Test tree" disabledBehavior={disabledBehavior} disabledKeys={['school']}>
505+
<TreeViewItem id="projects" textValue="Projects">
506+
<TreeViewItemContent>
507+
<Text>Projects</Text>o
508+
<Folder />
509+
<ActionMenu>
510+
<MenuItem id="edit">
511+
<Text>Edit</Text>
512+
</MenuItem>
513+
<MenuItem id="delete">
514+
<Text>Delete</Text>
515+
</MenuItem>
516+
</ActionMenu>
517+
</TreeViewItemContent>
518+
</TreeViewItem>
519+
<TreeViewItem id="school" textValue="School">
520+
<TreeViewItemContent>
521+
<Text>School</Text>o
522+
<Folder />
523+
<ActionMenu>
524+
<MenuItem id="edit">
525+
<Text>Edit</Text>
526+
</MenuItem>
527+
<MenuItem id="delete">
528+
<Text>Delete</Text>
529+
</MenuItem>
530+
</ActionMenu>
531+
</TreeViewItemContent>
532+
<TreeViewItem id="homework-1" textValue="Homework-1" isDisabled>
533+
<TreeViewItemContent>
534+
<Text>Homework-1</Text>
535+
<Folder />
536+
<ActionMenu>
537+
<MenuItem id="edit">
538+
<Text>Edit</Text>
539+
</MenuItem>
540+
<MenuItem id="delete">
541+
<Text>Delete</Text>
542+
</MenuItem>
543+
</ActionMenu>
544+
</TreeViewItemContent>
545+
</TreeViewItem>
546+
</TreeViewItem>
547+
</TreeView>
548+
<button onClick={() => setDisabledBehavior('all')}>After</button>
549+
</>
550+
);
551+
}
552+
553+
let {getAllByRole, getByRole} = render(<Example />);
554+
await user.click(document.body);
555+
556+
let rows = getAllByRole('row');
557+
let afterButton = getByRole('button', {name: 'After'});
558+
await user.click(rows[1]);
559+
expect(document.activeElement).toBe(rows[1]);
560+
561+
await user.click(afterButton);
562+
await user.tab({shift: true});
563+
await user.tab({shift: true});
564+
await user.tab();
565+
expect(document.activeElement).toBe(rows[0]);
566+
});
496567
});

0 commit comments

Comments
 (0)