Improve UX on the select dropdown #1361
Conversation
| /** | ||
| * Removes any highlighted choice options | ||
| */ | ||
| _removeHighlightedChoices(): void { |
There was a problem hiding this comment.
Kind of hard to see in this diff but this is simply a refactor of code that I am now using in L543 as well (to reset the highlighted css classes)
|
|
||
| if (activeElement !== null && !isScrolledIntoView(activeElement, this.choiceList.element)) { | ||
| // We use the native scrollIntoView function instead of choiceList.scrollToChildElement to avoid animated scroll. | ||
| activeElement.scrollIntoView(); |
There was a problem hiding this comment.
We could make use of the container scrollIntoViewOption once it has been implemented in in all major browsers. Right now it's not supported in FF and Safari.
| } | ||
| .#{$choices-selector}__item--selectable { | ||
| &[data-select-text] { | ||
| &.is-highlighted[data-select-text] { |
There was a problem hiding this comment.
Small change but we only ever want to show the data-select-text when an option has not been selected :)
There was a problem hiding this comment.
Unrelated to this PR but the icons for the windows runner did change, just a chore to make all tests pass:
public/assets/scripts/choices.js
Outdated
| // We use the native scrollIntoView function instead of choiceList.scrollToChildElement to avoid animated scroll. | ||
| activeElement.scrollIntoView(); |
There was a problem hiding this comment.
| // We use the native scrollIntoView function instead of choiceList.scrollToChildElement to avoid animated scroll. | |
| activeElement.scrollIntoView(); | |
| this.choiceList.scrollToChildElement(activeElement, 1); |
We could also go native here, this one however adds a scroll animation and some unnecessary request animation frames 🤔 /cc @Xon
|
I plan to look at this in the following week, once this is in I think I'll cut a v11.2.0 release |
2 participants
#1339 introduced the selected option being correctly highlighted when opening the dropdown but did not account for the option also being visible when opening the dropdown, leading to bad UX (unsure if selected or not).
This PR thus introduces the following bug fixes and some other behavior changes to improve UX and make the select act more native:
When opening the select dropdown, we now have a native scroll to the first selected option.
scrollToElementfunction here to avoid any smooth scroll or animated scroll.The selected state will always be visible; the hover (highlighted) state will be additionally visible.
is-selectedCSS class. It should now match the same behaviour as other libraries (e.g., SlimSelect). I also made sure to exclude the "Press to select" from the styles on already selected options, as this hint does not make sense here.When closing the dropdown, the hovered (highlighted) choices will be reset, matching the native
<select>behaviour.Screenshots (if appropriate)
The following video should show all behavioral changes
choices.mp4
Types of changes
Checklist