From 3012faec1b185935bd30c11711d88e92bcc00af0 Mon Sep 17 00:00:00 2001 From: Cole Bemis Date: Tue, 18 Oct 2022 09:25:48 -0700 Subject: [PATCH] TreeView: Use roving tabIndex for focus management (#2443) * Swap active descendant implementation for roving tabindex (wip) Co-authored-by: Josh Black * Remove active descendant from loading focus logic * Remove active descendant code and fix build errors * Add typeahead * Update level indicator line logic * useActiveDescendant -> useRovingTabIndex * Remove unnecessary styles * Document ref props * Forward ref to LinkItem * Create wise-keys-drum.md * Remove active descendant from tests * Update toggle on enter Co-authored-by: Josh Black --- .changeset/wise-keys-drum.md | 5 + docs/content/TreeView.mdx | 6 +- src/TreeView/TreeView.test.tsx | 353 +++++-------- src/TreeView/TreeView.tsx | 490 +++++++++--------- ...tiveDescendant.ts => useRovingTabIndex.ts} | 65 +-- src/TreeView/useTypeahead.ts | 8 +- 6 files changed, 394 insertions(+), 533 deletions(-) create mode 100644 .changeset/wise-keys-drum.md rename src/TreeView/{useActiveDescendant.ts => useRovingTabIndex.ts} (68%) diff --git a/.changeset/wise-keys-drum.md b/.changeset/wise-keys-drum.md new file mode 100644 index 00000000000..5fbd61414ab --- /dev/null +++ b/.changeset/wise-keys-drum.md @@ -0,0 +1,5 @@ +--- +"@primer/react": patch +--- + +TreeView: Use roving tabindex instead of `aria-activedescendant` for improved VoiceOver support in Safari diff --git a/docs/content/TreeView.mdx b/docs/content/TreeView.mdx index baaa63588ac..9d4f116ee1d 100644 --- a/docs/content/TreeView.mdx +++ b/docs/content/TreeView.mdx @@ -223,6 +223,7 @@ See [Storybook](https://primer.style/react/storybook?path=/story/components-tree name="onSelect" type="(event: React.MouseEvent | React.KeyboardEvent) => void" /> + {/* */} @@ -267,13 +268,10 @@ See [Storybook](https://primer.style/react/storybook?path=/story/components-tree name="onSelect" type="(event: React.MouseEvent | React.KeyboardEvent) => void" /> + {/* */} -### TreeView.LoadingItem - -{/* */} - ### TreeView.SubTree diff --git a/src/TreeView/TreeView.test.tsx b/src/TreeView/TreeView.test.tsx index be0d4859645..0fcbfd2640f 100644 --- a/src/TreeView/TreeView.test.tsx +++ b/src/TreeView/TreeView.test.tsx @@ -59,36 +59,6 @@ describe('Markup', () => { expect(subtree).toBeNull() }) - it('initializes aria-activedescendant to the current item by default', () => { - const {queryByRole} = renderWithTheme( - - Item 1 - Item 2 - Item 3 - - ) - - const root = queryByRole('tree') - const currentItem = queryByRole('treeitem', {name: 'Item 2'}) - - expect(root).toHaveAttribute('aria-activedescendant', currentItem?.id) - }) - - it('initializes aria-activedescendant to the first item if there is no current item', () => { - const {queryByRole} = renderWithTheme( - - Item 1 - Item 2 - Item 3 - - ) - - const root = queryByRole('tree') - const firstItem = queryByRole('treeitem', {name: 'Item 1'}) - - expect(root).toHaveAttribute('aria-activedescendant', firstItem?.id) - }) - it('uses aria-current', () => { const {getByRole} = renderWithTheme( @@ -322,7 +292,7 @@ describe('Markup', () => { describe('Keyboard interactions', () => { describe('ArrowDown', () => { - it('moves aria-activedescendant to the next visible treeitem', () => { + it('moves focus to the next visible treeitem', () => { const {getByRole} = renderWithTheme( @@ -342,46 +312,42 @@ describe('Keyboard interactions', () => { ) - const root = getByRole('tree') const item1 = getByRole('treeitem', {name: 'Item 1'}) const item11 = getByRole('treeitem', {name: 'Item 1.1'}) const item2 = getByRole('treeitem', {name: 'Item 2'}) const item3 = getByRole('treeitem', {name: 'Item 3'}) - // aria-activedescendant should be set to the first visible treeitem by default - expect(root).toHaveAttribute('aria-activedescendant', item1.id) - - // Focus tree - root.focus() + // Focus first item + item1.focus() // Press ↓ fireEvent.keyDown(document.activeElement || document.body, {key: 'ArrowDown'}) - // aria-activedescendant should now be set to item 1.1 - expect(root).toHaveAttribute('aria-activedescendant', item11.id) + // item 1.1 should be focused + expect(item11).toHaveFocus() // Press ↓ fireEvent.keyDown(document.activeElement || document.body, {key: 'ArrowDown'}) - // aria-activedescendant should now be set to item 2 - expect(root).toHaveAttribute('aria-activedescendant', item2.id) + // item 2 should be focused + expect(item2).toHaveFocus() // Press ↓ fireEvent.keyDown(document.activeElement || document.body, {key: 'ArrowDown'}) - // aria-activedescendant should now be set to item 3 (skips item 2.1 and item 2.2 because they are hidden) - expect(root).toHaveAttribute('aria-activedescendant', item3.id) + // item 3 should have focus (skips item 2.1 and item 2.2 because they are hidden) + expect(item3).toHaveFocus() // Press ↓ fireEvent.keyDown(document.activeElement || document.body, {key: 'ArrowDown'}) - // aria-activedescendant should not change (item 3 is the last visible treeitem) - expect(root).toHaveAttribute('aria-activedescendant', item3.id) + // focus should not change (item 3 is the last visible treeitem) + expect(item3).toHaveFocus() }) }) describe('ArrowUp', () => { - it('moves aria-activedescendant to the previous visible treeitem', () => { + it('moves focus to the previous visible treeitem', () => { const {getByRole} = renderWithTheme( @@ -401,17 +367,13 @@ describe('Keyboard interactions', () => { ) - const root = getByRole('tree') const item1 = getByRole('treeitem', {name: 'Item 1'}) const item11 = getByRole('treeitem', {name: 'Item 1.1'}) const item2 = getByRole('treeitem', {name: 'Item 2'}) const item3 = getByRole('treeitem', {name: 'Item 3'}) - // aria-activedescendant should be set to the first visible treeitem by default - expect(root).toHaveAttribute('aria-activedescendant', item1.id) - - // Focus tree - root.focus() + // Focus first item + item1.focus() // Press ↓ 4 times to move aria-activedescendant to item 3 fireEvent.keyDown(document.activeElement || document.body, {key: 'ArrowDown'}) @@ -419,32 +381,32 @@ describe('Keyboard interactions', () => { fireEvent.keyDown(document.activeElement || document.body, {key: 'ArrowDown'}) fireEvent.keyDown(document.activeElement || document.body, {key: 'ArrowDown'}) - // aria-activedescendant should now be set to item 3 - expect(root).toHaveAttribute('aria-activedescendant', item3.id) + // item 3 should be focused + expect(item3).toHaveFocus() // Press ↑ fireEvent.keyDown(document.activeElement || document.body, {key: 'ArrowUp'}) - // aria-activedescendant should now be set to item 2 (skips item 2.1 and item 2.2 because they are hidden) - expect(root).toHaveAttribute('aria-activedescendant', item2.id) + // Item 2 should have focus (skips item 2.1 and item 2.2 because they are hidden) + expect(item2).toHaveFocus() // Press ↑ fireEvent.keyDown(document.activeElement || document.body, {key: 'ArrowUp'}) - // aria-activedescendant should now be set to item 1.1 - expect(root).toHaveAttribute('aria-activedescendant', item11.id) + // Item 1.1 should be focused + expect(item11).toHaveFocus() // Press ↑ fireEvent.keyDown(document.activeElement || document.body, {key: 'ArrowUp'}) - // aria-activedescendant should now be set to item 1 - expect(root).toHaveAttribute('aria-activedescendant', item1.id) + // Item 1 should be focused + expect(item1).toHaveFocus() // Press ↑ fireEvent.keyDown(document.activeElement || document.body, {key: 'ArrowUp'}) - // aria-activedescendant should not change (item 1 is the first visible treeitem) - expect(root).toHaveAttribute('aria-activedescendant', item1.id) + // Focus should not change (item 1 is the first visible treeitem) + expect(item1).toHaveFocus() }) }) @@ -461,21 +423,17 @@ describe('Keyboard interactions', () => { ) - const root = getByRole('tree') const parentItem = getByRole('treeitem', {name: 'Parent'}) let subtree = queryByRole('group') - // aria-activedescendant should be set to the first visible treeitem by default - expect(root).toHaveAttribute('aria-activedescendant', parentItem.id) - // aria-expanded should be true expect(parentItem).toHaveAttribute('aria-expanded', 'true') // Subtree should be visible expect(subtree).toBeVisible() - // Focus tree - root.focus() + // Focus first item + parentItem.focus() // Press ← fireEvent.keyDown(document.activeElement || document.body, {key: 'ArrowLeft'}) @@ -483,8 +441,8 @@ describe('Keyboard interactions', () => { // aria-expanded should now be false expect(parentItem).toHaveAttribute('aria-expanded', 'false') - // aria-activedescendant should still be set to the parent treeitem - expect(root).toHaveAttribute('aria-activedescendant', parentItem.id) + // Parent item should still be focused + expect(parentItem).toHaveFocus() subtree = queryByRole('group') @@ -504,17 +462,13 @@ describe('Keyboard interactions', () => { ) - const root = getByRole('tree') const parentItem = getByRole('treeitem', {name: 'Parent'}) - // aria-activedescendant should be set to the first visible treeitem by default - expect(root).toHaveAttribute('aria-activedescendant', parentItem.id) - // aria-expanded should be false by default expect(parentItem).toHaveAttribute('aria-expanded', 'false') - // Focus tree - root.focus() + // Focus first item + parentItem.focus() // Press ← fireEvent.keyDown(document.activeElement || document.body, {key: 'ArrowLeft'}) @@ -522,8 +476,8 @@ describe('Keyboard interactions', () => { // aria-expanded should still be false expect(parentItem).toHaveAttribute('aria-expanded', 'false') - // aria-activedescendant should still be set to the parent treeitem - expect(root).toHaveAttribute('aria-activedescendant', parentItem.id) + // Focus should not change + expect(parentItem).toHaveFocus() }) it('does nothing on a root-level end item', () => { @@ -533,23 +487,19 @@ describe('Keyboard interactions', () => { ) - const root = getByRole('tree') const item = getByRole('treeitem', {name: 'Item'}) - // aria-activedescendant should be set to the first visible treeitem by default - expect(root).toHaveAttribute('aria-activedescendant', item.id) - - // Focus tree - root.focus() + // Focus first item + item.focus() // Press ← fireEvent.keyDown(document.activeElement || document.body, {key: 'ArrowLeft'}) - // aria-activedescendant should still be set to the item - expect(root).toHaveAttribute('aria-activedescendant', item.id) + // Focus should not change + expect(item).toHaveFocus() }) - it('moves aria-activedescendant to parent of end item', () => { + it('moves focus to parent of end item', () => { const {getByRole} = renderWithTheme( @@ -562,28 +512,27 @@ describe('Keyboard interactions', () => { ) - const root = getByRole('tree') const parentItem = getByRole('treeitem', {name: 'Parent'}) const child2 = getByRole('treeitem', {name: 'Child 2'}) - // Focus tree - root.focus() + // Focus fist item + parentItem.focus() - // Press ↓ 2 times to move aria-activedescendant to child 2 + // Press ↓ 2 times to move focus to child 2 fireEvent.keyDown(document.activeElement || document.body, {key: 'ArrowDown'}) fireEvent.keyDown(document.activeElement || document.body, {key: 'ArrowDown'}) - // aria-activedescendant should now be set to child 2 - expect(root).toHaveAttribute('aria-activedescendant', child2.id) + // Child 2 should be focused + expect(child2).toHaveFocus() // Press ← fireEvent.keyDown(document.activeElement || document.body, {key: 'ArrowLeft'}) - // aria-activedescendant should now be set to parent - expect(root).toHaveAttribute('aria-activedescendant', parentItem.id) + // Parent item should be focused + expect(parentItem).toHaveFocus() }) - it('moves aria-activedescendant to parent of collapsed item', () => { + it('moves focus to parent of collapsed item', () => { const {getByRole} = renderWithTheme( @@ -601,25 +550,24 @@ describe('Keyboard interactions', () => { ) - const root = getByRole('tree') const parentItem = getByRole('treeitem', {name: 'Parent'}) const nestedParentItem = getByRole('treeitem', {name: 'Nested parent'}) - // Focus tree - root.focus() + // Focus first item + parentItem.focus() - // Press ↓ 2 times to move aria-activedescendant to nested parent + // Press ↓ 2 times to move focus to nested parent fireEvent.keyDown(document.activeElement || document.body, {key: 'ArrowDown'}) fireEvent.keyDown(document.activeElement || document.body, {key: 'ArrowDown'}) - // aria-activedescendant should now be set to nested parent - expect(root).toHaveAttribute('aria-activedescendant', nestedParentItem.id) + // Nested parent item should be focused + expect(nestedParentItem).toHaveFocus() // Press ← fireEvent.keyDown(document.activeElement || document.body, {key: 'ArrowLeft'}) - // aria-activedescendant should now be set to parent - expect(root).toHaveAttribute('aria-activedescendant', parentItem.id) + // Parent item should be focused + expect(parentItem).toHaveFocus() }) }) @@ -636,17 +584,13 @@ describe('Keyboard interactions', () => { ) - const root = getByRole('tree') const parentItem = getByRole('treeitem', {name: 'Parent'}) - // aria-activedescendant should be set to the first visible treeitem by default - expect(root).toHaveAttribute('aria-activedescendant', parentItem.id) - // aria-expanded should be false by default expect(parentItem).toHaveAttribute('aria-expanded', 'false') - // Focus tree - root.focus() + // Focus first item + parentItem.focus() // Press → fireEvent.keyDown(document.activeElement || document.body, {key: 'ArrowRight'}) @@ -654,8 +598,8 @@ describe('Keyboard interactions', () => { // aria-expanded should now be true expect(parentItem).toHaveAttribute('aria-expanded', 'true') - // aria-activedescendant should still be set to the parent treeitem - expect(root).toHaveAttribute('aria-activedescendant', parentItem.id) + // Parent item should still be focused + expect(parentItem).toHaveFocus() const subtree = getByRole('group') @@ -663,7 +607,7 @@ describe('Keyboard interactions', () => { expect(subtree).toBeVisible() }) - it('moves aria-activedescendant to first child of an expanded item', () => { + it('moves focus to first child of an expanded item', () => { const {getByRole} = renderWithTheme( @@ -675,25 +619,21 @@ describe('Keyboard interactions', () => { ) - const root = getByRole('tree') const parentItem = getByRole('treeitem', {name: 'Parent'}) - // aria-activedescendant should be set to the first visible treeitem by default - expect(root).toHaveAttribute('aria-activedescendant', parentItem.id) - // aria-expanded should be true expect(parentItem).toHaveAttribute('aria-expanded', 'true') - // Focus tree - root.focus() + // Focus first item + parentItem.focus() // Press → fireEvent.keyDown(document.activeElement || document.body, {key: 'ArrowRight'}) const childItem = getByRole('treeitem', {name: 'Child'}) - // aria-activedescendant should now be set to the first child treeitem - expect(root).toHaveAttribute('aria-activedescendant', childItem.id) + // Child item should be focused + expect(childItem).toHaveFocus() // aria-expanded should still be true expect(parentItem).toHaveAttribute('aria-expanded', 'true') @@ -712,28 +652,28 @@ describe('Keyboard interactions', () => { ) - const root = getByRole('tree') + const parentItem = getByRole('treeitem', {name: 'Parent'}) const child1 = getByRole('treeitem', {name: 'Child 1'}) - // Focus tree - root.focus() + // Focus first item + parentItem.focus() - // Press ↓ to move aria-activedescendant to child 1 + // Press ↓ to move focus to child 1 fireEvent.keyDown(document.activeElement || document.body, {key: 'ArrowDown'}) - // aria-activedescendant should now be set to child 1 - expect(root).toHaveAttribute('aria-activedescendant', child1.id) + // Child 1 should be focused + expect(child1).toHaveFocus() // Press → fireEvent.keyDown(document.activeElement || document.body, {key: 'ArrowRight'}) - // aria-activedescendant should still be set to child 1 - expect(root).toHaveAttribute('aria-activedescendant', child1.id) + // Focus should not change + expect(child1).toHaveFocus() }) }) describe('Home', () => { - it('moves aria-activedescendant to first visible item', () => { + it('moves focus to first visible item', () => { const {getByRole} = renderWithTheme( @@ -757,29 +697,28 @@ describe('Keyboard interactions', () => { ) - const root = getByRole('tree') const parent1 = getByRole('treeitem', {name: 'Parent 1'}) const parent3 = getByRole('treeitem', {name: 'Parent 2'}) - // Focus tree - root.focus() + // Focus first item + parent1.focus() - // Press ↓ 2 times to move aria-activedescendant to parent 3 + // Press ↓ 2 times to move focus to parent 3 fireEvent.keyDown(document.activeElement || document.body, {key: 'ArrowDown'}) - // aria-activedescendant should now be set to parent 3 - expect(root).toHaveAttribute('aria-activedescendant', parent3.id) + // Parent 3 should be focused + expect(parent3).toHaveFocus() // Press Home fireEvent.keyDown(document.activeElement || document.body, {key: 'Home'}) - // aria-activedescendant should now be set to parent 1 - expect(root).toHaveAttribute('aria-activedescendant', parent1.id) + // Parent 1 should be focused + expect(parent1).toHaveFocus() }) }) describe('End', () => { - it('moves aria-activedescendant to last visible item', () => { + it('moves focus to last visible item', () => { const {getByRole} = renderWithTheme( @@ -803,21 +742,17 @@ describe('Keyboard interactions', () => { ) - const root = getByRole('tree') const parent1 = getByRole('treeitem', {name: 'Parent 1'}) const parent3 = getByRole('treeitem', {name: 'Parent 3'}) - // Focus tree - root.focus() - - // aria-activedescendant should be set to parent 1 - expect(root).toHaveAttribute('aria-activedescendant', parent1.id) + // Focus first item + parent1.focus() // Press End fireEvent.keyDown(document.activeElement || document.body, {key: 'End'}) - // aria-activedescendant should now be set to parent 3 - expect(root).toHaveAttribute('aria-activedescendant', parent3.id) + // Parent 3 should be focused + expect(parent3).toHaveFocus() // Press → to expand parent 3 fireEvent.keyDown(document.activeElement || document.body, {key: 'ArrowRight'}) @@ -827,8 +762,8 @@ describe('Keyboard interactions', () => { const child3 = getByRole('treeitem', {name: 'Child 3'}) - // aria-activedescendant should now be set to child 3 - expect(root).toHaveAttribute('aria-activedescendant', child3.id) + // Child 3 should be focused + expect(child3).toHaveFocus() }) }) @@ -841,10 +776,10 @@ describe('Keyboard interactions', () => { ) - const root = getByRole('tree') + const item = getByRole('treeitem') - // Focus tree - root.focus() + // Focus first item + item.focus() // Press Enter fireEvent.keyDown(document.activeElement || document.body, {key: 'Enter'}) @@ -866,11 +801,10 @@ describe('Keyboard interactions', () => { ) - const root = getByRole('tree') const parent = getByRole('treeitem', {name: 'Parent'}) - // Focus tree - root.focus() + // Focus first item + parent.focus() // aria-expanded should be false expect(parent).toHaveAttribute('aria-expanded', 'false') @@ -911,10 +845,10 @@ describe('Keyboard interactions', () => { ) - const root = getByRole('tree') + const item = getByRole('treeitem') - // Focus tree - root.focus() + // Focus first item + item.focus() // Press Enter fireEvent.keyDown(document.activeElement || document.body, {key: 'Enter'}) @@ -928,7 +862,7 @@ describe('Keyboard interactions', () => { }) describe('Typeahead', () => { - it('moves aria-activedescendant to the next item that matches the typed character', () => { + it('moves focus to the next item that matches the typed character', () => { const {getByRole} = renderWithTheme( @@ -943,23 +877,19 @@ describe('Keyboard interactions', () => { ) - const root = getByRole('tree') const apple = getByRole('treeitem', {name: 'Apple'}) const cherry = getByRole('treeitem', {name: 'Cherry'}) - // Focus tree - root.focus() - - // aria-activedescendant should be set to apple - expect(root).toHaveAttribute('aria-activedescendant', apple.id) + // Focus first item + apple.focus() // Press C fireEvent.keyDown(document.activeElement || document.body, {key: 'c'}) - // aria-activedescendant should now be set to cherry - expect(root).toHaveAttribute('aria-activedescendant', cherry.id) + // Cherry should be focused + expect(cherry).toHaveFocus() - // Notice that the aria-activedescendant is not set to cantalope because + // Notice that the focus is not set to cantalope because // it is a child of apple and apple is collapsed. }) @@ -973,20 +903,19 @@ describe('Keyboard interactions', () => { ) - const root = getByRole('tree') const apple = getByRole('treeitem', {name: 'Apple'}) - // Focus tree - root.focus() + // Focus first item + apple.focus() - // aria-activedescendant should be set to apple - expect(root).toHaveAttribute('aria-activedescendant', apple.id) + // Apple should be focused + expect(apple).toHaveFocus() // Press Z fireEvent.keyDown(document.activeElement || document.body, {key: 'z'}) - // aria-activedescendant should still be set to apple - expect(root).toHaveAttribute('aria-activedescendant', apple.id) + // Apple should still be focused + expect(apple).toHaveFocus() }) it('supports multiple typed characters', () => { @@ -1000,23 +929,19 @@ describe('Keyboard interactions', () => { ) - const root = getByRole('tree') const apple = getByRole('treeitem', {name: 'Apple'}) const cantalope = getByRole('treeitem', {name: 'Cantalope 1'}) - // Focus tree - root.focus() - - // aria-activedescendant should be set to apple - expect(root).toHaveAttribute('aria-activedescendant', apple.id) + // Focus first item + apple.focus() // Press C + A + N fireEvent.keyDown(document.activeElement || document.body, {key: 'c'}) fireEvent.keyDown(document.activeElement || document.body, {key: 'a'}) fireEvent.keyDown(document.activeElement || document.body, {key: 'n'}) - // aria-activedescendant should now be set to cantalope - expect(root).toHaveAttribute('aria-activedescendant', cantalope.id) + // Cantalope should be focused + expect(cantalope).toHaveFocus() }) it('prioritizes items following the current aria-activedescendant', () => { @@ -1028,21 +953,24 @@ describe('Keyboard interactions', () => { ) - const root = getByRole('tree') + const cucumber = getByRole('treeitem', {name: 'Cucumber'}) const cherry = getByRole('treeitem', {name: 'Cherry'}) const cantalope = getByRole('treeitem', {name: 'Cantalope'}) - // Focus tree - root.focus() + // Focus first item + cucumber.focus() + + // Press ↓ to move focus to cherry + fireEvent.keyDown(document.activeElement || document.body, {key: 'ArrowDown'}) - // aria-activedescendant should be set to cherry - expect(root).toHaveAttribute('aria-activedescendant', cherry.id) + // Cherry should be focused + expect(cherry).toHaveFocus() // Press C fireEvent.keyDown(document.activeElement || document.body, {key: 'c'}) - // aria-activedescendant should now be set to cantalope - expect(root).toHaveAttribute('aria-activedescendant', cantalope.id) + // Cantalope should be focused + expect(cantalope).toHaveFocus() }) it('wraps around to the beginning if no items match after the current aria-activedescendant', () => { @@ -1055,21 +983,24 @@ describe('Keyboard interactions', () => { ) - const root = getByRole('tree') const cantalope = getByRole('treeitem', {name: 'Cantalope'}) const cucumber = getByRole('treeitem', {name: 'Cucumber'}) - // Focus tree - root.focus() + // Focus first item + cucumber.focus() - // aria-activedescendant should be set to cantalope - expect(root).toHaveAttribute('aria-activedescendant', cantalope.id) + // Press ↓ 2 times to move focus to cantalope + fireEvent.keyDown(document.activeElement || document.body, {key: 'ArrowDown'}) + fireEvent.keyDown(document.activeElement || document.body, {key: 'ArrowDown'}) + + // Cantalope should be focused + expect(cantalope).toHaveFocus() // Press C fireEvent.keyDown(document.activeElement || document.body, {key: 'c'}) - // aria-activedescendant should now be set to cucumber - expect(root).toHaveAttribute('aria-activedescendant', cucumber.id) + // Cucumber should be focused + expect(cucumber).toHaveFocus() }) }) }) @@ -1092,7 +1023,6 @@ describe('Controlled state', () => { const {getByRole} = renderWithTheme() - const root = getByRole('tree') const parent = getByRole('treeitem', {name: 'Parent'}) const child = getByRole('treeitem', {name: 'Child'}) @@ -1100,11 +1030,8 @@ describe('Controlled state', () => { expect(parent).toHaveAttribute('aria-expanded', 'true') expect(child).toBeVisible() - // aria-activedescendant should be set to parent - expect(root).toHaveAttribute('aria-activedescendant', parent.id) - - // Focus tree - root.focus() + // Focus first item + parent.focus() // Press ← to collapse the parent fireEvent.keyDown(document.activeElement || document.body, {key: 'ArrowLeft'}) @@ -1150,7 +1077,7 @@ describe('Asyncronous loading', () => { expect(liveRegion).toHaveTextContent('Parent content loaded') }) - it('moves active descendant from loading item to first child', async () => { + it('moves focus from loading item to first child', async () => { function TestTree() { const [state, setState] = React.useState('loading') @@ -1174,28 +1101,24 @@ describe('Asyncronous loading', () => { const {getByRole, findByRole} = renderWithTheme() - const root = getByRole('tree') const parentItem = getByRole('treeitem', {name: 'Parent'}) const loadingItem = await findByRole('treeitem', {name: 'Loading...'}) - // Focus tree - root.focus() - - // aria-activedescendant should be set to parent item - expect(root).toHaveAttribute('aria-activedescendant', parentItem.id) + // Focus first item + parentItem.focus() - // Press ↓ to move aria-activedescendant to loading item + // Press ↓ to move focus to loading item fireEvent.keyDown(document.activeElement || document.body, {key: 'ArrowDown'}) - // aria-activedescendant should now be set to loading item - expect(root).toHaveAttribute('aria-activedescendant', loadingItem.id) + // Loading item should be focused + expect(loadingItem).toHaveFocus() // Wait for async loading to complete const firstChild = await findByRole('treeitem', {name: 'Child 1'}) setTimeout(() => { - // aria-activedescendant should now be set to first child - expect(root).toHaveAttribute('aria-activedescendant', firstChild.id) + // First child should be focused + expect(firstChild).toHaveFocus() }) }) }) diff --git a/src/TreeView/TreeView.tsx b/src/TreeView/TreeView.tsx index b76a49c4d5f..a144e80be12 100644 --- a/src/TreeView/TreeView.tsx +++ b/src/TreeView/TreeView.tsx @@ -18,7 +18,7 @@ import {Theme} from '../ThemeProvider' import createSlots from '../utils/create-slots' import VisuallyHidden from '../_VisuallyHidden' import {getAccessibleName} from './shared' -import {getFirstChildElement, useActiveDescendant} from './useActiveDescendant' +import {getFirstChildElement, useRovingTabIndex} from './useRovingTabIndex' import {useTypeahead} from './useTypeahead' // ---------------------------------------------------------------------------- @@ -26,12 +26,8 @@ import {useTypeahead} from './useTypeahead' const RootContext = React.createContext<{ announceUpdate: (message: string) => void - activeDescendant: string - setActiveDescendant: React.Dispatch> }>({ - announceUpdate: () => {}, - activeDescendant: '', - setActiveDescendant: () => {} + announceUpdate: () => {} }) const ItemContext = React.createContext<{ @@ -65,11 +61,15 @@ const Root: React.FC = ({'aria-label': ariaLabel, 'aria-labelledb const containerRef = React.useRef(null) const [ariaLiveMessage, setAriaLiveMessage] = React.useState('') - const [activeDescendant, setActiveDescendant] = useActiveDescendant({containerRef}) + useRovingTabIndex({containerRef}) useTypeahead({ containerRef, - onFocusChange: element => setActiveDescendant(element.id) + onFocusChange: element => { + if (element instanceof HTMLElement) { + element.focus() + } + } }) const announceUpdate = React.useCallback((message: string) => { @@ -77,25 +77,20 @@ const Root: React.FC = ({'aria-label': ariaLabel, 'aria-labelledb }, []) return ( - + <> {ariaLiveMessage} {children} @@ -116,244 +111,229 @@ export type TreeViewItemProps = { defaultExpanded?: boolean expanded?: boolean onExpandedChange?: (expanded: boolean) => void - onSelect?: (event: React.MouseEvent | KeyboardEvent) => void + onSelect?: (event: React.MouseEvent | React.KeyboardEvent) => void } const {Slots, Slot} = createSlots(['LeadingVisual', 'TrailingVisual']) -const Item: React.FC = ({ - current: isCurrentItem = false, - defaultExpanded = false, - expanded, - onExpandedChange, - onSelect, - children -}) => { - const {setActiveDescendant} = React.useContext(RootContext) - const itemId = useSSRSafeId() - const labelId = useSSRSafeId() - const leadingVisualId = useSSRSafeId() - const trailingVisualId = useSSRSafeId() - const itemRef = React.useRef(null) - const [isExpanded, setIsExpanded] = useControllableState({ - name: itemId, - defaultValue: defaultExpanded, - value: expanded, - onChange: onExpandedChange - }) - const {level, expandParents} = React.useContext(ItemContext) - const {hasSubTree, subTree, childrenWithoutSubTree} = useSubTree(children) - - // Expand or collapse the subtree - const toggle = React.useCallback( - (event?: React.MouseEvent) => { - setIsExpanded(!isExpanded) - event?.stopPropagation() - }, - // setIsExpanded is stable - // eslint-disable-next-line react-hooks/exhaustive-deps - [isExpanded] - ) - - // Expand all parents of this item including itself - const expandParentsAndSelf = React.useCallback( - () => { - expandParents() - setIsExpanded(true) - }, - // setIsExpanded is stable - // eslint-disable-next-line react-hooks/exhaustive-deps - [expandParents] - ) - - // If this item is the current item, expand it and all its parents - React.useLayoutEffect(() => { - if (isCurrentItem) { - expandParentsAndSelf() - } - }, [isCurrentItem, expandParentsAndSelf]) - - React.useEffect(() => { - const element = itemRef.current - - function handleKeyDown(event: KeyboardEvent) { - // WARNING: Removing this line will cause an infinite loop! - // The root element receives all keyboard events and forwards them - // to the active descendant. If we don't stop propagation here, - // the event will bubble back up to the root element and be forwarded - // back to the active descendant infinitely. - event.stopPropagation() - - switch (event.key) { - case 'Enter': - if (onSelect) { - onSelect(event) - } else { - toggle() - } - break +const Item = React.forwardRef( + ({current: isCurrentItem = false, defaultExpanded = false, expanded, onExpandedChange, onSelect, children}, ref) => { + const itemId = useSSRSafeId() + const labelId = useSSRSafeId() + const leadingVisualId = useSSRSafeId() + const trailingVisualId = useSSRSafeId() + const [isExpanded, setIsExpanded] = useControllableState({ + name: itemId, + defaultValue: defaultExpanded, + value: expanded, + onChange: onExpandedChange + }) + const {level, expandParents} = React.useContext(ItemContext) + const {hasSubTree, subTree, childrenWithoutSubTree} = useSubTree(children) + + // Expand or collapse the subtree + const toggle = React.useCallback( + (event?: React.MouseEvent | React.KeyboardEvent) => { + setIsExpanded(!isExpanded) + event?.stopPropagation() + }, + // setIsExpanded is stable + // eslint-disable-next-line react-hooks/exhaustive-deps + [isExpanded] + ) - case 'ArrowRight': - event.preventDefault() - setIsExpanded(true) - break + // Expand all parents of this item including itself + const expandParentsAndSelf = React.useCallback( + () => { + expandParents() + setIsExpanded(true) + }, + // setIsExpanded is stable + // eslint-disable-next-line react-hooks/exhaustive-deps + [expandParents] + ) - case 'ArrowLeft': - event.preventDefault() - setIsExpanded(false) - break + // If this item is the current item, expand it and all its parents + React.useLayoutEffect(() => { + if (isCurrentItem) { + expandParentsAndSelf() } - } - - element?.addEventListener('keydown', handleKeyDown) - return () => element?.removeEventListener('keydown', handleKeyDown) - }, [toggle, onSelect, setIsExpanded]) + }, [isCurrentItem, expandParentsAndSelf]) - return ( - -
  • - { - setActiveDescendant(itemId) + const handleKeyDown = React.useCallback( + (event: React.KeyboardEvent) => { + switch (event.key) { + case 'Enter': if (onSelect) { onSelect(event) } else { toggle(event) } - }} - sx={{ - '--toggle-width': '1rem', // 16px - position: 'relative', - display: 'grid', - gridTemplateColumns: `calc(${level - 1} * (var(--toggle-width) / 2)) var(--toggle-width) 1fr`, - gridTemplateAreas: `"spacer toggle content"`, - width: '100%', - height: '2rem', // 32px - fontSize: 1, - color: 'fg.default', - borderRadius: 2, - cursor: 'pointer', - '&:hover': { - backgroundColor: 'actionListItem.default.hoverBg', - '@media (forced-colors: active)': { - outline: '2px solid transparent', - outlineOffset: -2 - } - }, - '@media (pointer: coarse)': { - '--toggle-width': '1.5rem', // 24px - height: '2.75rem' // 44px - }, - // WARNING: styled-components v5.2 introduced a bug that changed - // how it expands `&` in CSS selectors. The following selectors - // are unnecessarily specific to work around that styled-components bug. - // Reference issue: https://github.com/styled-components/styled-components/issues/3265 - [`[role=tree][aria-activedescendant="${itemId}"]:focus-visible #${itemId} > &:is(div)`]: { - boxShadow: (theme: Theme) => `inset 0 0 0 2px ${theme.colors.accent.emphasis}`, - '@media (forced-colors: active)': { - outline: '2px solid SelectedItem', - outlineOffset: -2 + break + + case 'ArrowRight': + event.preventDefault() + event.stopPropagation() + setIsExpanded(true) + break + + case 'ArrowLeft': + event.preventDefault() + event.stopPropagation() + setIsExpanded(false) + break + } + }, + [onSelect, setIsExpanded, toggle] + ) + + return ( + +
  • } + tabIndex={0} + id={itemId} + role="treeitem" + aria-labelledby={labelId} + aria-describedby={`${leadingVisualId} ${trailingVisualId}`} + aria-level={level} + aria-expanded={hasSubTree ? isExpanded : undefined} + aria-current={isCurrentItem ? 'true' : undefined} + style={{outline: 'none'}} + onKeyDown={handleKeyDown} + > + { + if (onSelect) { + onSelect(event) + } else { + toggle(event) } - }, - '[role=treeitem][aria-current=true] > &:is(div)': { - bg: 'actionListItem.default.selectedBg', - '&::after': { - position: 'absolute', - top: 'calc(50% - 12px)', - left: -2, - width: '4px', - height: '24px', - content: '""', - bg: 'accent.fg', - borderRadius: 2 + }} + sx={{ + '--toggle-width': '1rem', // 16px + position: 'relative', + display: 'grid', + gridTemplateColumns: `calc(${level - 1} * (var(--toggle-width) / 2)) var(--toggle-width) 1fr`, + gridTemplateAreas: `"spacer toggle content"`, + width: '100%', + height: '2rem', // 32px + fontSize: 1, + color: 'fg.default', + borderRadius: 2, + cursor: 'pointer', + '&:hover': { + backgroundColor: 'actionListItem.default.hoverBg', + '@media (forced-colors: active)': { + outline: '2px solid transparent', + outlineOffset: -2 + } + }, + '@media (pointer: coarse)': { + '--toggle-width': '1.5rem', // 24px + height: '2.75rem' // 44px + }, + // WARNING: styled-components v5.2 introduced a bug that changed + // how it expands `&` in CSS selectors. The following selectors + // are unnecessarily specific to work around that styled-components bug. + // Reference issue: https://github.com/styled-components/styled-components/issues/3265 + [`#${itemId}:focus-visible > &:is(div)`]: { + boxShadow: (theme: Theme) => `inset 0 0 0 2px ${theme.colors.accent.emphasis}`, + '@media (forced-colors: active)': { + outline: '2px solid SelectedItem', + outlineOffset: -2 + } + }, + '[role=treeitem][aria-current=true] > &:is(div)': { + bg: 'actionListItem.default.selectedBg', + '&::after': { + position: 'absolute', + top: 'calc(50% - 12px)', + left: -2, + width: '4px', + height: '24px', + content: '""', + bg: 'accent.fg', + borderRadius: 2 + } } - } - }} - > - - - - {hasSubTree ? ( + }} + > + + + + {hasSubTree ? ( + { + if (onSelect) { + toggle(event) + } + }} + sx={{ + gridArea: 'toggle', + display: 'flex', + alignItems: 'center', + justifyContent: 'center', + height: '100%', + color: 'fg.muted', + borderTopLeftRadius: level === 1 ? 2 : 0, + borderBottomLeftRadius: level === 1 ? 2 : 0, + '&:hover': { + backgroundColor: onSelect ? 'actionListItem.default.hoverBg' : null + } + }} + > + {isExpanded ? : } + + ) : null} { - if (onSelect) { - setActiveDescendant(itemId) - toggle(event) - } - }} + id={labelId} sx={{ - gridArea: 'toggle', + gridArea: 'content', display: 'flex', alignItems: 'center', - justifyContent: 'center', height: '100%', - color: 'fg.muted', - borderTopLeftRadius: level === 1 ? 2 : 0, - borderBottomLeftRadius: level === 1 ? 2 : 0, - '&:hover': { - backgroundColor: onSelect ? 'actionListItem.default.hoverBg' : null - } + px: 2, + gap: 2 }} > - {isExpanded ? : } + + {slots => ( + <> + {slots.LeadingVisual} + + {childrenWithoutSubTree} + + {slots.TrailingVisual} + + )} + - ) : null} - - - {slots => ( - <> - {slots.LeadingVisual} - - {childrenWithoutSubTree} - - {slots.TrailingVisual} - - )} - - - {subTree} -
    • -     - ) -} + {subTree} + + + ) + } +) /** Lines to indicate the depth of an item in a TreeView */ const LevelIndicatorLines: React.FC<{level: number}> = ({level}) => { @@ -377,7 +357,7 @@ const LevelIndicatorLines: React.FC<{level: number}> = ({level}) => { // sure the component remains simple when not in use. '@media (hover: hover)': { borderColor: 'transparent', - '[role=tree]:hover &, [role=tree]:focus &': { + '[role=tree]:hover &, [role=tree]:focus-within &': { borderColor: 'border.subtle' } } @@ -398,9 +378,10 @@ export type TreeViewLinkItemProps = TreeViewItemProps & { } // TODO: Use an element to enable native browser behavior like opening links in a new tab -const LinkItem: React.FC = ({href, onSelect, ...props}) => { +const LinkItem = React.forwardRef(({href, onSelect, ...props}, ref) => { return ( { window.open(href, '_self') onSelect?.(event) @@ -408,7 +389,7 @@ const LinkItem: React.FC = ({href, onSelect, ...props}) = {...props} /> ) -} +}) LinkItem.displayName = 'TreeView.LinkItem' @@ -423,16 +404,12 @@ export type TreeViewSubTreeProps = { } const SubTree: React.FC = ({state, children}) => { - const {activeDescendant, setActiveDescendant, announceUpdate} = React.useContext(RootContext) + const {announceUpdate} = React.useContext(RootContext) const {itemId, isExpanded} = React.useContext(ItemContext) const [isLoadingItemVisible, setIsLoadingItemVisible] = React.useState(false) const {safeSetTimeout, safeClearTimeout} = useSafeTimeout() const timeoutId = React.useRef(0) - const activeDescendantRef = React.useRef(activeDescendant) - - React.useEffect(() => { - activeDescendantRef.current = activeDescendant - }, [activeDescendant]) + const loadingItemRef = React.useRef(null) // Announce when content has loaded React.useEffect(() => { @@ -448,33 +425,38 @@ const SubTree: React.FC = ({state, children}) => { // Show loading indicator after a short delay React.useEffect(() => { + // If we're in the loading state, but not showing the loading indicator yet, + // start a timer to show the loading indicator after a short delay. if (state === 'loading' && !isLoadingItemVisible) { timeoutId.current = safeSetTimeout(() => { setIsLoadingItemVisible(true) }, 300) } + // If we're not in the loading state, but we're still showing a loading indicator, + // hide the loading indicator and move focus if necessary if (state !== 'loading' && isLoadingItemVisible) { + const isLoadingItemFocused = document.activeElement === loadingItemRef.current + safeClearTimeout(timeoutId.current) setIsLoadingItemVisible(false) - // If the active descendant was removed from the DOM after hiding the - // LoadingItem, we know the LoadingItem was the active descendant - // and we need to update the active descendant to the first loaded item - // or the parent item (if the subtree is empty). - setTimeout(() => { - const activeElement = document.getElementById(activeDescendantRef.current) - - if (!activeElement) { + if (isLoadingItemFocused) { + setTimeout(() => { const parentElement = document.getElementById(itemId) if (!parentElement) return const firstChild = getFirstChildElement(parentElement) - setActiveDescendant(firstChild ? firstChild.id : parentElement.id) - } - }) + + if (firstChild) { + firstChild.focus() + } else { + parentElement.focus() + } + }) + } } - }, [state, safeSetTimeout, safeClearTimeout, setActiveDescendant, isLoadingItemVisible, itemId]) + }, [state, safeSetTimeout, safeClearTimeout, isLoadingItemVisible, itemId]) return ( = ({state, children}) => { margin: 0 }} > - {isLoadingItemVisible ? : children} + {isLoadingItemVisible ? : children} ) } SubTree.displayName = 'TreeView.SubTree' -const LoadingItem = () => { +const LoadingItem = React.forwardRef((props, ref) => { return ( - + Loading... ) -} +}) function useSubTree(children: React.ReactNode) { return React.useMemo(() => { diff --git a/src/TreeView/useActiveDescendant.ts b/src/TreeView/useRovingTabIndex.ts similarity index 68% rename from src/TreeView/useActiveDescendant.ts rename to src/TreeView/useRovingTabIndex.ts index 15211ce8c27..bc3bc3cc048 100644 --- a/src/TreeView/useActiveDescendant.ts +++ b/src/TreeView/useRovingTabIndex.ts @@ -1,62 +1,17 @@ import React from 'react' +import {FocusKeys, useFocusZone} from '../hooks/useFocusZone' -type ActiveDescendantOptions = { - containerRef: React.RefObject -} +export function useRovingTabIndex({containerRef}: {containerRef: React.RefObject}) { + // TODO: Initialize focus to the aria-current item if it exists + useFocusZone({ + containerRef, + bindKeys: FocusKeys.ArrowVertical | FocusKeys.ArrowHorizontal | FocusKeys.HomeAndEnd, + getNextFocusable: (direction, from, event) => { + if (!(from instanceof HTMLElement)) return -export function useActiveDescendant({ - containerRef -}: ActiveDescendantOptions): [string, React.Dispatch>] { - const [activeDescendant, setActiveDescendant] = React.useState('') - - // Initialize value of active descendant - React.useEffect(() => { - if (containerRef.current && !activeDescendant) { - const currentItem = containerRef.current.querySelector('[role="treeitem"][aria-current="true"]') - const firstItem = containerRef.current.querySelector('[role="treeitem"]') - - // If current item exists, use it as the initial value for active descendant - if (currentItem) { - setActiveDescendant(currentItem.id) - } - // Otherwise, initialize the active descendant to the first item in the tree - else if (firstItem) { - setActiveDescendant(firstItem.id) - } + return getNextFocusableElement(from, event) ?? from } - }, [containerRef, activeDescendant]) - - const handleKeyDown = React.useCallback( - (event: KeyboardEvent) => { - const activeElement = document.getElementById(activeDescendant) - - if (!activeElement) return - - const nextElement = getNextFocusableElement(activeElement, event) - - if (nextElement) { - // Move active descendant if necessary - setActiveDescendant(nextElement.id) - event.preventDefault() - } else { - // If the active descendant didn't change, - // forward the event to the active descendant - activeElement.dispatchEvent(new KeyboardEvent(event.type, event)) - } - }, - [activeDescendant] - ) - - React.useEffect(() => { - const container = containerRef.current - - if (!container) return - - container.addEventListener('keydown', handleKeyDown) - return () => container.removeEventListener('keydown', handleKeyDown) - }, [containerRef, handleKeyDown]) - - return [activeDescendant, setActiveDescendant] + }) } // DOM utilities used for focus management diff --git a/src/TreeView/useTypeahead.ts b/src/TreeView/useTypeahead.ts index 262744aeb17..02886db2631 100644 --- a/src/TreeView/useTypeahead.ts +++ b/src/TreeView/useTypeahead.ts @@ -59,13 +59,11 @@ export function useTypeahead({containerRef, onFocusChange}: TypeaheadOptions) { // Filter out collapsed items .filter(element => !element.parentElement?.closest('[role=treeitem][aria-expanded=false]')) - // Get the index of active descendant - const activeDescendantIndex = elements.findIndex( - element => element.id === containerRef.current?.getAttribute('aria-activedescendant') - ) + // Get the index of active element + const activeIndex = elements.findIndex(element => element === document.activeElement) // Wrap the array elements such that the active descendant is at the beginning - let sortedElements = wrapArray(elements, activeDescendantIndex) + let sortedElements = wrapArray(elements, activeIndex) // Remove the active descendant from the beginning of the array // when the user initiates a new search