-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Improve control over Menu
and Listbox
options while searching
#2471
Merged
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
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
thecrypticace
requested changes
May 3, 2023
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. Just one small change re: the regex and a question about eliminating the hidden elements.
This makes it a bit slower but also more correct. We can use a cache on another level to ensure that we are not creating useless work.
This will add a cache and only if the `innerText` changes, only then will we calculate the new text value.
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.
This PR improves the control over the "text value" of a
Menu.Item
orListbox.Option
.When a
Menu
or aListbox
is in an open state, then you can just start typing to search for a specific item. It uses a simple algorithm where it looks at the beginning of the option and if it matches your search or not. If you are on an option and you re-type the same thing, then it will jump to the next option that matches.Concretely, let's look at this setup:
If you open the Listbox, then type:
united
then it will jump to theUnited States of America
bel
then it will jump to theBelgium
bel
(again) then it will jump toBelarus
That said, there is a bit of a problem. If you include an emoji in the beginning (for example a country flag) then typing
bel
forBelgium
won't work because you need to type the emoji first.To solve this, we will strip out emojis from the text value representation of the option.
This example will behave the same as the example without the flags.
If you open the Listbox, then type:
united
then it will jump to theUnited States of America
bel
then it will jump to theBelgium
bel
(again) then it will jump toBelarus
But there is an additional problem. It could be that the contents of the option contains "image"-like text, such as ASCII art. Or for a more realistic example, an icon font that uses some special characters to display a certain "image" using a font.
Example using ASCII art:
This now means that you won't be able to "delete", because these strange characters are in front of the "delete" word. They are also not emojis so they won't be stripped by default.
To solve this, we will also make sure to drop any text content that is in an element with one of these characteristics:
hidden
attribute (e.g.:<div hidden>Hidden for everyone</div>
)aria-hidden
attribute (e.g.:<div aria-hidden="true">Hidden for assistive technology</div>
)role="img"
attribute (e.g.:<div role="img">image-like text</div>
)This has to crawl the DOM and can be expensive, however in some scenarios you know exactly what should happen, in those cases you can also drop all the "visual" artifacts and use an
aria-label="delete"
attribute to describe what this means exactly.Note: Assistive technology will also use
aria-label
andaria-labelledby
when it is present on the element instead of the contents of the element.Now you have complete control over the items:
To be complete with the
aria-label
attribute, this PR also tries to resolve the contents of the element referenced viaaria-labelledby
if you use that attribute instead.Fixes: #2454