-
Notifications
You must be signed in to change notification settings - Fork 156
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
Add draft for fixing (aria-)label bug on OcSelect #10823
Add draft for fixing (aria-)label bug on OcSelect #10823
Conversation
Thanks for opening this pull request! The maintainers of this repository would appreciate it if you would create a changelog item based on your changes. |
While I usually like is and has prefixes I don't think they align with the props already present... I'm not sure it's more elegant, but another approach might be to be able to pass in a label or an aria label... lf no aria label is passed, it uses the regular label. If only the aria label is passed, no label is shown. No idea if that is a good or a bad thing but it allows to pass in two different values... and it avoids a boolean prop which is not super bad but always smells a bit to me for things like this |
Good idea, let me get started on it! |
7675f7e
to
b692c4d
Compare
I've decided to switch the used |
b692c4d
to
34426ce
Compare
I think this has been superseded by #11396. |
True, thanks for the housekeeping! |
Description
From my understanding, the
OcSelect
component currently always gets a hardcoded "Search for option" aria-label, which overwrites the optional label that can be passed as a prop.I've fixed that by only setting the "Search for option" aria-label if no label is passed via prop.
Another consideration is the visibility of the custom label, for which I added another prop - now, a custom label can be passed but also hidden in the GUI, while still available for screenreader users.
The next step for finalizing this PR (granted that my approach is approved) would be checking all occurances of the
OcSelect
and decide on whether they need a (hidden) custom label.Related Issue
Motivation and Context
Accessibility improvements