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

Make <Select> debounce time configurable #27513

Open
1 task done
jakub-bao opened this issue Jul 29, 2021 · 6 comments
Open
1 task done

Make <Select> debounce time configurable #27513

jakub-bao opened this issue Jul 29, 2021 · 6 comments
Labels
component: select This is the name of the generic UI component, not the React module! new feature New feature or request

Comments

@jakub-bao
Copy link

jakub-bao commented Jul 29, 2021

  • I have searched the issues of this repository and believe that this is not a duplicate.

Issue ❗

If you open a Select and press a letter "A" on your keyboard, the first option starting with "A" will become focused.
If you press "A" again you'll focus second option starting with "A"

Unfortunately, there is a 500ms hard-coded debounce time. So you gotta wait half a second between each key-press.

Why is that a problem ❓

In our project we use Select components for menus which have as many as 30 items.
In this case it's really important to be able to quickly jump through the menu using keyboard.
Unfortunately, with the 0.5s expected wait between each key-press it becomes too annoying to use.

Solution 👍

I fished through the source and found that the issue is on this line:

https://github.com/mui-org/material-ui/blob/4ae71c6ab0478300fe7265f68c7195a3c6731533/packages/material-ui/src/MenuList/MenuList.js#L172

The Menu component has a hardcoded 500ms debounce time between keypresses. It would be really nice if this would be configurable.

@jakub-bao jakub-bao added the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Jul 29, 2021
@eps1lon
Copy link
Member

eps1lon commented Jul 30, 2021

Thanks for the feedback.

This makes sense to me. Though I'd like to use the opportunity and check again if the time is appropriate. .5s sounds really long. The native <select /> feels more responsive. Regardless, we should document that behavior anyway and explain why we choose the 500ms default.

/cc @ryancogswell

@eps1lon eps1lon added component: select This is the name of the generic UI component, not the React module! new feature New feature or request and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Jul 30, 2021
@ryancogswell
Copy link
Contributor

Characterizing the 500ms as a "debounce" is not accurate. It is a reset of the character buffer. Let's say you want to go directly to "Angel". You can do this by typing "Ang" as long as you do each key press in less 500ms since the previous. If you type "A" repeatedly with less than 500ms in between then it might find "Aardvark" if that was one of your options. If you make the buffer reset time shorter then it will be more difficult to match the beginning of words via multiple characters. For instance, with too short of a reset time, typing "A" then "n" might jump to the first "n" word rather than words starting with "An". Assuming your options are sorted, you can use the keyboard to go quickly through the "A" words by typing "A" and then switching to the down arrow key.

This is at least how it was working when I first completed the feature. It does however appear that the multi-character matching was broken at some point. In the latest v4 it isn't working correctly, but it works correctly in 4.3.3: https://codesandbox.io/s/select-keyboard-nav-check-uq101?file=/demo.js. I'll zero in on when it was broken.

@ryancogswell
Copy link
Contributor

The multi-character matching was working in 4.9.7 and broken in 4.9.8. I'll take a look at what changes happened then.

@ryancogswell
Copy link
Contributor

In #19967 the boolean return values from moveFocus were removed which I'm pretty sure is what broke the behavior. I think my original tests would have caught the bug, but @eps1lon simplified them significantly for easier maintenance when converting the tests to new approaches.

@jakub-bao
Copy link
Author

@ryancogswell I see. Thanks for the explanation! I've misread the code. But thanks for backtracking the issue.

@ryancogswell
Copy link
Contributor

Also, prior to the introduction of the bug in 4.9.8, repeatedly pressing the same character within the 500ms timer did cycle between the options starting with that character if there weren't any entries starting with the full sequence (e.g. "aardvark" would get precedence when typing "aa" over going to the next "a" word, but if there were no "aa" words, then that would go to the second "a" word).

ryancogswell added a commit to ryancogswell/material-ui that referenced this issue Jul 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: select This is the name of the generic UI component, not the React module! new feature New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants