[Autocomplete] Fix hidden items during keyboard navigation #19305
Conversation
This file contains hidden or 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
While scrolling up with the keyboard, position the top part of the option so that the both the group label and the row are fully visible. No changes were made for the scroll down behaviour, which already handled skipping the group label. Doesn't interfere with the current behaviour of autoselects without groups. The 1.3 ratio was selected through visual inspection, and is the value that minimizes misaligments and/or cropping of the topmost option.
Details of bundle changes.Comparing: f1080b6...0671abb
|
eps1lon
reviewed
Jan 20, 2020
Member
eps1lon
left a comment
eps1lon
left a comment
There was a problem hiding this comment.
Could you slow down the GIF? I can't see any difference.
Would it be possible to send keystrokes to the element before taking a visual regression snapshot, for example?
I don't know of any and wouldn't spend too much time on it. At some point I want to improve how we do visual regression tests. So far they are good enough.
oliviertassinari
added
scope: autocomplete
Contributor
Author
Sure!
Issue on the first one:
Issues on the second one:
|
oliviertassinari
approved these changes
Jan 20, 2020
oliviertassinari
changed the title
Member
|
@aisamu Thank you! |
Contributor
Author
|
@oliviertassinari oh, don't mention it! With all the hand holding and a suggestion that took it almost all the way, you're the one that should be thanked 😄 |
This was referenced Mar 20, 2020
This was referenced Apr 6, 2020
This was referenced Apr 16, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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.
This PR implements @oliviertassinari's suggestion for #19217 with a couple tweaks
The differences were mainly to preserve the existing behaviour on lists without groups and on the downward scrolling of lists with groups.
While scrolling up with the keyboard, position the top part of the
option so that the both the group label and the row are fully visible.
No changes were made for the scroll down behaviour, which already
handled skipping the group label.
Doesn't interfere with the current behaviour of autoselects without
groups.
The 1.3 ratio was selected through visual inspection, and is the value
that minimizes misaligments and/or cropping of the topmost option.
Fixes #19217
Some guidance is definitely needed on how to test this behaviour given its dynamic nature. Would it be possible to send keystrokes to the element before taking a visual regression snapshot, for example?