Skip to content

Commit

Permalink
fix(tree): focus navigation
Browse files Browse the repository at this point in the history 
- preserve tabindex for specified elements
  TreeNodeBase change tabindex of all interactable elements, but this
  can change elements which are under focus management,
  eg. Menu, nested lists, etc.
- stop stealing focus after tree structure update
- Reduce useItemSelect warnings
- Fail gracefully, print error instead of throw
  • Loading branch information
@maxinteger
maxinteger committed Sep 19, 2024
1 parent 0f29d7c commit 57f034f
Show file tree
Hide file tree
Showing 17 changed files with 338 additions and 59 deletions.
40 changes: 40 additions & 0 deletions src/components/SpaceTreeNode/SpaceTreeNode.unit.test.tsx.snap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ exports[`<SpaceTreeNode /> snapshot checks divider dot position in compact mode
>
<div
className="md-tree-wrapper"
onBlur={[Function]}
onFocus={[Function]}
onKeyDown={[Function]}
role="tree"
>
Expand Down Expand Up @@ -163,6 +165,8 @@ exports[`<SpaceTreeNode /> snapshot should match snapshot 1`] = `
>
<div
className="md-tree-wrapper"
onBlur={[Function]}
onFocus={[Function]}
onKeyDown={[Function]}
role="tree"
>
Expand Down Expand Up @@ -270,6 +274,8 @@ exports[`<SpaceTreeNode /> snapshot should match snapshot with action 1`] = `
>
<div
className="md-tree-wrapper"
onBlur={[Function]}
onFocus={[Function]}
onKeyDown={[Function]}
role="tree"
>
Expand Down Expand Up @@ -391,6 +397,8 @@ exports[`<SpaceTreeNode /> snapshot should match snapshot with className 1`] = `
>
<div
className="md-tree-wrapper"
onBlur={[Function]}
onFocus={[Function]}
onKeyDown={[Function]}
role="tree"
>
Expand Down Expand Up @@ -499,6 +507,8 @@ exports[`<SpaceTreeNode /> snapshot should match snapshot with firstLine 1`] = `
>
<div
className="md-tree-wrapper"
onBlur={[Function]}
onFocus={[Function]}
onKeyDown={[Function]}
role="tree"
>
Expand Down Expand Up @@ -610,6 +620,8 @@ exports[`<SpaceTreeNode /> snapshot should match snapshot with id 1`] = `
>
<div
className="md-tree-wrapper"
onBlur={[Function]}
onFocus={[Function]}
onKeyDown={[Function]}
role="tree"
>
Expand Down Expand Up @@ -719,6 +731,8 @@ exports[`<SpaceTreeNode /> snapshot should match snapshot with isAlert 1`] = `
>
<div
className="md-tree-wrapper"
onBlur={[Function]}
onFocus={[Function]}
onKeyDown={[Function]}
role="tree"
>
Expand Down Expand Up @@ -859,6 +873,8 @@ exports[`<SpaceTreeNode /> snapshot should match snapshot with isAlertMuted 1`]
>
<div
className="md-tree-wrapper"
onBlur={[Function]}
onFocus={[Function]}
onKeyDown={[Function]}
role="tree"
>
Expand Down Expand Up @@ -999,6 +1015,8 @@ exports[`<SpaceTreeNode /> snapshot should match snapshot with isDisabled 1`] =
>
<div
className="md-tree-wrapper"
onBlur={[Function]}
onFocus={[Function]}
onKeyDown={[Function]}
role="tree"
>
Expand Down Expand Up @@ -1107,6 +1125,8 @@ exports[`<SpaceTreeNode /> snapshot should match snapshot with isEnterRoom 1`] =
>
<div
className="md-tree-wrapper"
onBlur={[Function]}
onFocus={[Function]}
onKeyDown={[Function]}
role="tree"
>
Expand Down Expand Up @@ -1247,6 +1267,8 @@ exports[`<SpaceTreeNode /> snapshot should match snapshot with isError 1`] = `
>
<div
className="md-tree-wrapper"
onBlur={[Function]}
onFocus={[Function]}
onKeyDown={[Function]}
role="tree"
>
Expand Down Expand Up @@ -1387,6 +1409,8 @@ exports[`<SpaceTreeNode /> snapshot should match snapshot with isMention 1`] = `
>
<div
className="md-tree-wrapper"
onBlur={[Function]}
onFocus={[Function]}
onKeyDown={[Function]}
role="tree"
>
Expand Down Expand Up @@ -1527,6 +1551,8 @@ exports[`<SpaceTreeNode /> snapshot should match snapshot with isNewActivity 1`]
>
<div
className="md-tree-wrapper"
onBlur={[Function]}
onFocus={[Function]}
onKeyDown={[Function]}
role="tree"
>
Expand Down Expand Up @@ -1636,6 +1662,8 @@ exports[`<SpaceTreeNode /> snapshot should match snapshot with isSelected 1`] =
>
<div
className="md-tree-wrapper"
onBlur={[Function]}
onFocus={[Function]}
onKeyDown={[Function]}
role="tree"
>
Expand Down Expand Up @@ -1745,6 +1773,8 @@ exports[`<SpaceTreeNode /> snapshot should match snapshot with isSelected and dr
>
<div
className="md-tree-wrapper"
onBlur={[Function]}
onFocus={[Function]}
onKeyDown={[Function]}
role="tree"
>
Expand Down Expand Up @@ -1887,6 +1917,8 @@ exports[`<SpaceTreeNode /> snapshot should match snapshot with isUnread 1`] = `
>
<div
className="md-tree-wrapper"
onBlur={[Function]}
onFocus={[Function]}
onKeyDown={[Function]}
role="tree"
>
Expand Down Expand Up @@ -2027,6 +2059,8 @@ exports[`<SpaceTreeNode /> snapshot should match snapshot with multiple string s
>
<div
className="md-tree-wrapper"
onBlur={[Function]}
onFocus={[Function]}
onKeyDown={[Function]}
role="tree"
>
Expand Down Expand Up @@ -2203,6 +2237,8 @@ exports[`<SpaceTreeNode /> snapshot should match snapshot with secondLine 1`] =
>
<div
className="md-tree-wrapper"
onBlur={[Function]}
onFocus={[Function]}
onKeyDown={[Function]}
role="tree"
>
Expand Down Expand Up @@ -2341,6 +2377,8 @@ exports[`<SpaceTreeNode /> snapshot should match snapshot with style 1`] = `
>
<div
className="md-tree-wrapper"
onBlur={[Function]}
onFocus={[Function]}
onKeyDown={[Function]}
role="tree"
>
Expand Down Expand Up @@ -2463,6 +2501,8 @@ exports[`<SpaceTreeNode /> snapshot should match snapshot with teamColor 1`] = `
>
<div
className="md-tree-wrapper"
onBlur={[Function]}
onFocus={[Function]}
onKeyDown={[Function]}
role="tree"
>
Expand Down
2 changes: 1 addition & 1 deletion src/components/Tree/Tree.stories.docs.mdx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ make it possible to use it with Virtualized trees.

Tree nodes must be wrapped in a `<TreeNodeBase />` component to ensure proper keyboard navigation and focus management.

Any node inside the `<Tree />` can access to the tree context using the `useTreeContext` hook.
Any node inside the `<Tree />` can access to the tree context using the `useTreeContext` hook.#

## Nested VS Flat tree

Expand Down
2 changes: 2 additions & 0 deletions src/components/Tree/Tree.stories.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import { TreeNodeRecord, TreeRefObject } from './Tree.types';
import ButtonPill from '../ButtonPill';
import Flex from '../Flex';
import { v4 as uuidV4 } from 'uuid';
import { PRESERVE_TABINDEX_CLASSNAME } from '../../utils/navigation';

// prettier-ignore
const exampleTree =
Expand Down Expand Up @@ -89,6 +90,7 @@ const ExampleTreeNode = ({ node }: ExampleTreeNodeProps) => {
</div>
{nodeDetails.isLeaf && (
<MenuTrigger
className={PRESERVE_TABINDEX_CLASSNAME}
triggerComponent={
<ButtonCircle size={20} ghost aria-label="More menu">
<Icon name="more" weight="bold" autoScale={100} />
Expand Down
81 changes: 79 additions & 2 deletions src/components/Tree/Tree.test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,8 @@ import userEvent from '@testing-library/user-event';
import Tree, { TREE_CONSTANTS, TreeProps } from './index';
import { createTreeNode as tNode } from './test.utils';
import TreeNodeBase from '../TreeNodeBase';
import { convertNestedTree2MappedTree, mapTree } from './Tree.utils';
import { convertNestedTree2MappedTree, getTreeRootId, mapTree } from './Tree.utils';
import { act } from 'react-test-renderer';

const getSampleTree = () => {
// prettier-ignore
Expand Down Expand Up @@ -385,6 +386,39 @@ describe('<Tree />', () => {
onToggleNode.mockReset();
}
});

it('should exclude interactable elements with preserved-tabindex form navigation ', async () => {
expect.assertions(3);
const user = userEvent.setup();

const { getByTestId, getByText } = render(
<Tree treeStructure={{ id: 'root', children: [] }} excludeTreeRoot={false}>
<TreeNodeBase nodeId="root" data-testid="root">
{() => (
<>
<button tabIndex={-1}>button 1</button>
<button tabIndex={-1} className="md-nav-preserve-tabindex">
button 2
</button>
<div className="md-nav-preserve-tabindex">
<button tabIndex={-1}>button 3</button>
</div>
</>
)}
</TreeNodeBase>
</Tree>
);

await user.tab();

expect(getByTestId('root')).toHaveFocus();
await user.tab();
// active root node change the tabindex for button 1...
expect(getByText('button 1')).toHaveFocus();
// ...but not for button 2 and 3
await user.tab();
expect(document.body).toHaveFocus();
});
});

describe('virtual tree connector', () => {
Expand Down Expand Up @@ -747,7 +781,24 @@ describe('<Tree />', () => {
expect(getByTestId('1.1')).toHaveFocus();
});

it('should select the next visible chhild of the root when excludeTreeRoot true', async () => {
it('should select the next visible child of the root when excludeTreeRoot true', async () => {
const { rerender, getByTestId } = render(getTreeComponent(getSampleTree(), true));

await userEvent.tab();
await userEvent.keyboard('{ArrowRight}');
await userEvent.keyboard('{ArrowDown}');
expect(getByTestId('1.1')).toHaveFocus();

const nextTree = getSampleTree();
nextTree.children = nextTree.children.filter(({ id }) => id !== '1');

rerender(getTreeComponent(nextTree, true));

// Focus should move to node '2'
expect(getByTestId('2')).toHaveFocus();
});

it('should autofocus after update when the focused element was inside of the tree', async () => {
const { rerender, getByTestId } = render(getTreeComponent(getSampleTree(), true));

await userEvent.tab();
Expand All @@ -763,6 +814,32 @@ describe('<Tree />', () => {
// Focus should move to node '2'
expect(getByTestId('2')).toHaveFocus();
});

it('should not autofocus after update when the focused element was outside of the tree', async () => {
const renderTreeWithButton = (tree) => (
<div>
{getTreeComponent(tree, true)}
<button>button</button>
</div>
);
const { rerender, getByTestId, getByText } = render(renderTreeWithButton(getSampleTree()));

await userEvent.tab();
await userEvent.keyboard('{ArrowRight}');
await userEvent.keyboard('{ArrowDown}');
expect(getByTestId('1.1')).toHaveFocus();

// move focus to the button
getByText('button').focus();

const nextTree = getSampleTree();
nextTree.children = nextTree.children.filter(({ id }) => id !== '1');

rerender(renderTreeWithButton(nextTree));

// Focus should remain on the button
expect(getByText('button')).toHaveFocus();
});
});

describe('selection', () => {
Expand Down
12 changes: 12 additions & 0 deletions src/components/Tree/Tree.test.tsx.snap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ exports[`<Tree /> snapshot should match snapshot 1`] = `
>
<div
className="md-tree-wrapper"
onBlur={[Function]}
onFocus={[Function]}
onKeyDown={[Function]}
role="tree"
/>
Expand Down Expand Up @@ -53,6 +55,8 @@ exports[`<Tree /> snapshot should match snapshot with className 1`] = `
>
<div
className="example-class md-tree-wrapper"
onBlur={[Function]}
onFocus={[Function]}
onKeyDown={[Function]}
role="tree"
/>
Expand Down Expand Up @@ -84,6 +88,8 @@ exports[`<Tree /> snapshot should match snapshot with id 1`] = `
<div
className="md-tree-wrapper"
id="example-id"
onBlur={[Function]}
onFocus={[Function]}
onKeyDown={[Function]}
role="tree"
/>
Expand Down Expand Up @@ -118,6 +124,8 @@ exports[`<Tree /> snapshot should match snapshot with style 1`] = `
>
<div
className="md-tree-wrapper"
onBlur={[Function]}
onFocus={[Function]}
onKeyDown={[Function]}
role="tree"
style={
Expand Down Expand Up @@ -157,6 +165,8 @@ exports[`<Tree /> snapshot should match snapshot with style 2`] = `
>
<div
className="md-tree-wrapper"
onBlur={[Function]}
onFocus={[Function]}
onKeyDown={[Function]}
role="tree"
style={
Expand Down Expand Up @@ -196,6 +206,8 @@ exports[`<Tree /> snapshot should match snapshot with style 3`] = `
>
<div
className="md-tree-wrapper"
onBlur={[Function]}
onFocus={[Function]}
onKeyDown={[Function]}
role="tree"
style={
Expand Down
Loading

0 comments on commit 57f034f

Please sign in to comment.