-
Notifications
You must be signed in to change notification settings - Fork 2.1k
fix: add arialabel for pagination and select elements #20043
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
Changes from all commits
7e63b4e
7134e80
1231c9a
e246d45
0a4025b
f225b72
bc0ecc7
3cafb07
ae0e1b2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -211,6 +211,12 @@ class CDSSelect extends FormMixin(LitElement) { | |
| @property({ type: Boolean, reflect: true }) | ||
| disabled = false; | ||
|
|
||
| /** | ||
| * Specify text for the accessibility label of the 'select' element | ||
| */ | ||
| @property({ attribute: 'aria-label' }) | ||
| ariaLabel = 'Select option'; | ||
|
|
||
| /** | ||
| * The helper text. | ||
| */ | ||
|
|
@@ -395,6 +401,7 @@ class CDSSelect extends FormMixin(LitElement) { | |
|
|
||
| render() { | ||
| const { | ||
| ariaLabel, | ||
| disabled, | ||
| helperText, | ||
| hideLabel, | ||
|
|
@@ -450,6 +457,7 @@ class CDSSelect extends FormMixin(LitElement) { | |
| ?disabled="${disabled}" | ||
| aria-readonly="${String(Boolean(readonly))}" | ||
| aria-invalid="${String(Boolean(invalid))}" | ||
| aria-label=${ifDefined(ariaLabel)} | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think the way to fix this isn't through an aria-label, but refactoring the In the react version, Select always has a label. |
||
| aria-describedby="${ifDefined(!invalid ? undefined : 'invalid-text')}" | ||
| @input="${handleInput}"> | ||
| ${!placeholder || value | ||
|
|
||
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.
This should be undefined by default I think, rather than an empty string. This is why it's being included in the snapshot when I don't think it should be.