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] Use the order in which the items are displayed for type-ahead #12827

Merged
merged 2 commits into from Apr 23, 2024

Conversation

flaviendelangle
Copy link
Member

@flaviendelangle flaviendelangle commented Apr 18, 2024

Fixes #12826

Not sure if this is a bug fix or just a behavior change.
The current type-ahead was alphabetical, if you have the items ["A3", "A2", "A1"], you focus "A3" and press "a", the focus goes to "A1" because it's the 1st matching item alphabetically.
This PR switches for a search based on the displayed order (you focus "A2" because it is the 1st matching item below the currently focused one.

IMHO this is a lot more consistent, it's the approach taken by this TreeView for instance.

Nice side-effect, we no longer generate the list of the 1st letter of every item on each keystroke
Now that getNextNavigableItem is cheap it's a lot more efficient that way

Follow up

Add test in #12811

@flaviendelangle flaviendelangle self-assigned this Apr 18, 2024
@flaviendelangle flaviendelangle added bug 🐛 Something doesn't work component: tree view TreeView, TreeItem. This is the name of the generic UI component, not the React module! labels Apr 18, 2024
@mui-bot
Copy link

mui-bot commented Apr 18, 2024

Deploy preview: https://deploy-preview-12827--material-ui-x.netlify.app/

Generated by 🚫 dangerJS against f4261c3

@flaviendelangle flaviendelangle marked this pull request as ready for review April 18, 2024 10:00
@flaviendelangle flaviendelangle requested review from LukasTy, noraleonte and michelengelen and removed request for LukasTy April 18, 2024 10:00
@flaviendelangle flaviendelangle changed the title [TreeView] Fix type-ahead [TreeView] Use the order in which the items are displayed for type-ahead Apr 18, 2024
Copy link
Member

@michelengelen michelengelen left a comment

Choose a reason for hiding this comment

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

nit only ... LGTM

…Navigation/useTreeViewKeyboardNavigation.ts

Co-authored-by: Michel Engelen <32863416+michelengelen@users.noreply.github.com>
Signed-off-by: Flavien DELANGLE <flaviendelangle@gmail.com>
@flaviendelangle flaviendelangle merged commit 7bc98f1 into mui:master Apr 23, 2024
17 checks passed
@flaviendelangle flaviendelangle deleted the type-ahead-fix branch April 23, 2024 06:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 Something doesn't work component: tree view TreeView, TreeItem. This is the name of the generic UI component, not the React module!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[TreeView] Type-ahead do not respect the order of the items
3 participants