Add conditional scrolling into view of focused option #2170
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Previously the focused option would be scrolled to the top of the scroll window when the menu is opened, regardless of whether it was already in view. An example of a situation where this is problematic is when a list starts with one or more disabled items. In this situation, the disabled items are scrolled out of view when the menu opens, and because they're at the top of the list it's hard to notice they're there. This commit modifies the behavior such that when the menu is opened, it will only scroll the focused option into view if it's not already fully in view.
|
If this behavior is not something we can impose on everyone, I'd be happy to look at adding a prop controlling whether we do this check or not. |
|
This looks like a good change, I'm happy to make it across the board 👍 Thanks for the PR and especially the clear instructions to reproduce! Made this really easy to test and merge. |
|
No worries! Thanks @JedWatson for the quick turnaround, will look out for other ways to contribute more in the future. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Previously the focused option would be scrolled to the top of the scroll window when the menu is opened, regardless of whether it was already in view. An example of a situation where this is problematic is when a list starts with one or more disabled items. In this situation, the disabled items are scrolled out of view when the menu opens, and because they're at the top of the list it's hard to notice they're there.
This PR modifies the behavior such that when the menu is opened, it will only scroll the focused option into view if it's not already fully in view.
The current behavior is reproducible in the live demo: