Skip to content

Commit

Permalink
[TreeView] Do not try to focus a collapsed node when re-focusing the …
Browse files Browse the repository at this point in the history
…TreeView (#10422)
  • Loading branch information
flaviendelangle authored Sep 25, 2023
1 parent a982158 commit 598c6fe
Show file tree
Hide file tree
Showing 3 changed files with 112 additions and 5 deletions.
93 changes: 93 additions & 0 deletions packages/x-tree-view/src/TreeView/TreeView.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -442,6 +442,99 @@ describe('<TreeView />', () => {
});
});

describe('useTreeViewFocus', () => {
it('should focus the selected item when the tree is focused', () => {
const onNodeFocus = spy();

const { getByRole } = render(
<TreeView selected={'2'} onNodeFocus={onNodeFocus}>
<TreeItem nodeId="1" label="1" />
<TreeItem nodeId="2" label="2" />
</TreeView>,
);

act(() => {
getByRole('tree').focus();
});

expect(onNodeFocus.lastCall.lastArg).to.equal('2');
});

it('should focus the selected item when the tree is focused (multi select)', () => {
const onNodeFocus = spy();

const { getByRole } = render(
<TreeView multiSelect selected={['2']} onNodeFocus={onNodeFocus}>
<TreeItem nodeId="1" label="1" />
<TreeItem nodeId="2" label="2" />
</TreeView>,
);

act(() => {
getByRole('tree').focus();
});

expect(onNodeFocus.lastCall.lastArg).to.equal('2');
});

it('should focus the first visible selected item when the tree is focused (multi select)', () => {
const onNodeFocus = spy();

const { getByRole } = render(
<TreeView multiSelect selected={['1.1', '2']} onNodeFocus={onNodeFocus}>
<TreeItem nodeId="1" label="1">
<TreeItem nodeId="1.1" label="1.1" />
</TreeItem>
<TreeItem nodeId="2" label="2" />
</TreeView>,
);

act(() => {
getByRole('tree').focus();
});

expect(onNodeFocus.lastCall.lastArg).to.equal('2');
});

it('should focus the first item if the selected item is not visible', () => {
const onNodeFocus = spy();

const { getByRole } = render(
<TreeView selected="1.1" onNodeFocus={onNodeFocus}>
<TreeItem nodeId="1" label="1">
<TreeItem nodeId="1.1" label="1.1" />
</TreeItem>
<TreeItem nodeId="2" label="2" />
</TreeView>,
);

act(() => {
getByRole('tree').focus();
});

expect(onNodeFocus.lastCall.lastArg).to.equal('1');
});

it('should focus the first item if no selected item is visible (multi select)', () => {
const onNodeFocus = spy();

const { getByRole } = render(
<TreeView multiSelect selected={['1.1']} onNodeFocus={onNodeFocus}>
<TreeItem nodeId="1" label="1">
<TreeItem nodeId="1.1" label="1.1" />
</TreeItem>
<TreeItem nodeId="2" label="2" />
</TreeView>,
);

act(() => {
getByRole('tree').focus();
});

expect(onNodeFocus.lastCall.lastArg).to.equal('1');
});
});

describe('Accessibility', () => {
it('(TreeView) should have the role `tree`', () => {
const { getByRole } = render(<TreeView />);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,10 +58,23 @@ export const useTreeViewFocus: TreeViewPlugin<UseTreeViewFocusSignature> = ({

// if the event bubbled (which is React specific) we don't want to steal focus
if (event.target === event.currentTarget) {
const firstSelected = Array.isArray(models.selected.value)
? models.selected.value[0]
: models.selected.value;
instance.focusNode(event, firstSelected || instance.getNavigableChildrenIds(null)[0]);
const isNodeVisible = (nodeId: string) => {
const node = instance.getNode(nodeId);
return node && (node.parentId == null || instance.isNodeExpanded(node.parentId));
};

let nodeToFocusId: string | null | undefined;
if (Array.isArray(models.selected.value)) {
nodeToFocusId = models.selected.value.find(isNodeVisible);
} else if (models.selected.value != null && isNodeVisible(models.selected.value)) {
nodeToFocusId = models.selected.value;
}

if (nodeToFocusId == null) {
nodeToFocusId = instance.getNavigableChildrenIds(null)[0];
}

instance.focusNode(event, nodeToFocusId);
}
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import * as React from 'react';
import { TreeViewPluginSignature } from '../../models';
import type { UseTreeViewNodesSignature } from '../useTreeViewNodes';
import type { UseTreeViewSelectionSignature } from '../useTreeViewSelection';
import { UseTreeViewExpansionSignature } from '../useTreeViewExpansion';

export interface UseTreeViewFocusInstance {
isNodeFocused: (nodeId: string) => boolean;
Expand Down Expand Up @@ -31,5 +32,5 @@ export type UseTreeViewFocusSignature = TreeViewPluginSignature<
{},
UseTreeViewFocusState,
never,
[UseTreeViewNodesSignature, UseTreeViewSelectionSignature<any>]
[UseTreeViewNodesSignature, UseTreeViewSelectionSignature<any>, UseTreeViewExpansionSignature]
>;

0 comments on commit 598c6fe

Please sign in to comment.