Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[TreeView] Set focus on the focused Tree Item instead of the Tree View #12226

Merged
merged 25 commits into from
Mar 19, 2024
Merged
Show file tree
Hide file tree
Changes from 22 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -372,6 +372,40 @@ you can use the new `onItemSelectionToggle` prop which is called whenever an ite

:::

### Focus the Tree Item instead of the Tree View
flaviendelangle marked this conversation as resolved.
Show resolved Hide resolved

The focus is now applied to the Tree Item root element instead of the Tree View root element.

This change will allow new features that require the focus to be on the Tree Item,
like the drag and drop reordering of items.
It also solves several issues with focus management,
like the inability to scroll to the focused item when a lot of items are rendered.

This will mostly impact how you write tests to interact with the Tree View:

For example, if you were writing a test with `react-testing-library`, here is what the changes could look like:

```diff
it('test example on first item', () => {
- const { getByRole } = render(
+ const { getAllByRole } = render(
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the test we tend to use getByTestId, but we could probably use getAllByRole and remove all the test ids.

<SimpleTreeView>
<TreeItem nodeId="one" />
<TreeItem nodeId="two" />
</SimpleTreeView>
);

- const tree = getByRole('tree');
+ const firstTreeItem = getAllByRole('treeitem')[0];
act(() => {
- tree.focus();
+ firstTreeItem.focus();
});
- fireEvent.keyDown(tree, { key: 'ArrowDown' });
+ fireEvent.keyDown(firstTreeItem, { key: 'ArrowDown' });
flaviendelangle marked this conversation as resolved.
Show resolved Hide resolved
})
```

### ✅ Use `useTreeItemState` instead of `useTreeItem`

The `useTreeItem` hook has been renamed `useTreeItemState`.
Expand Down
Loading
Loading