-
Notifications
You must be signed in to change notification settings - Fork 194
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 accessibility of <selectedoption> example #1079
Open
josepharhar
wants to merge
1
commit into
openui:main
Choose a base branch
from
josepharhar:selectedoptionax
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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.
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.
only problem with this is being a generic element, spans are name prohibited. that's not to say it shouldn't work - because this is unfortunately something authors do so browsers have tried to mitigate for that in the accName computation. but it's still not really what authors should be doing, and validators should be noting this as an error/warning.
if that was
span role=img aria-label=heart
that'd work without issue. or if the aria-label was on the option, that'd generally be the way to do this.or, the opposite approach is taken and the examples are made as:
if we only want to use the name from the emoji and disregard the visible text repeating part of the name.
or or,
or or or (because developers hate IDs),
the above two examples use the emoji to get the name, and then use the visible text as the additional description - which is effectively what i'd hope the
<description>
element would eventually allow for.this being a lot easier for the dev than having to know anything about the ARIA bits i mentioned above.
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.
FWIW, another way we could approach this without getting into any of the weeds above, is to just simply state that "using both the emoji and the text together can make for some awkward/redundant names for these options. In the future we hope to improve the UX by making a new feature for developers that doesn't require in depth understanding of ARIA"
my main goal with this and the other comments about the split button markup is not to put a screeching halt to any of this work, but to acknowledge there could be better ways / more things needed to markup the ideal experience for these features - and if the code examples can't be updated to accommodate, then we should at least acknowledge there's more work for the implantation or web authors to do that may be beyond the scope of this initial work.
(sorry, it's my default to be in the weeds and try to figure this stuff out)
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.
Thanks for the feedback and ideas!
I think that a more typical case will definitely be
<img>
s rather than emojis, so maybe we should change this example to use<img>
s and/or show an example which works for both images and emojis.Now that I'm thinking about it more, maybe we should have custom behavior for the way that
<selectedoption>
is announced. What if we made it pull the accessible name/label/text from the selected<option>
and announce that? And authors won't have to use any aria for this kind of thing.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.
showing an img example instead would definitely be easier.
i'm not sure i entirely follow your custom behavior idea. it'd still require some use of aria, or using the value attribute in a way it hasn't been used before?
so like, instead of using aria-hidden in the above, if someone waned their option to just be named 'heart', they'd have to do
or potentially?
i wanna make sure i'm understanding this right before i comment further