-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
fix(timezoneSelect): a11y aria-label #6960
base: develop
Are you sure you want to change the base?
fix(timezoneSelect): a11y aria-label #6960
Conversation
Generate changelog in
|
@@ -162,6 +162,7 @@ export class TimezoneSelect extends AbstractPureComponent<TimezoneSelectProps, T | |||
|
|||
return ( | |||
<Select<TimezoneWithNames> | |||
aria-label="timezone" |
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.
On what element in Select does this aria-label
get applied?
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.
On what element in Select does this aria-label get applied?
could you provide a bit more context as to the accessibility failure you're experiencing and how adding this label addresses it?
Here is the failure:
Every type of input needs an aria-label
or aria-labelledby
, including a select
(which in this case is the combobox
).
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.
It doesn't look like providing the aria-label
prop to Select does anything. Inside of <Select>
, it will be passed via restProps
to <QueryList>
, where it is then discarded:
blueprint/packages/select/src/components/select/select.tsx
Lines 137 to 142 in 3f4a894
public render() { | |
// omit props specific to this component, spread the rest. | |
const { filterable, inputProps, menuProps, popoverProps, ...restProps } = this.props; | |
return ( | |
<QueryList<T> |
const { className, items, renderer, itemListRenderer = this.renderItemList, menuProps } = this.props; |
Did you mean to pass this aria-label
via popoverTargetProps
?
<Select<TimezoneWithNames>
popoverTargetProps={{
"aria-label": "timezone",
}}
...
/>
Regardless, it feels a bit redundant to have to add an additional label when the Select's input already has a placeholder value of "Select timezone...". Is there any way we can leverage the existing placeholder to improve accessibility?
@bvandercar-vt could you provide a bit more context as to the accessibility failure you're experiencing and how adding this label addresses it? |
Changes proposed in this pull request:
Add aria-label to timezone select to fix a11y aXe failure.