-
Notifications
You must be signed in to change notification settings - Fork 6.8k
docs(a11y): Add a11y docs for table, expansion panel, dialog, chips, autocomplete… #6751
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
Conversation
src/lib/tabs/tabs.md
Outdated
@@ -79,3 +79,7 @@ provides a tab-like UI for navigating between routes. | |||
The tab-nav-bar is not tied to any particular router; it works with normal `<a>` elements and uses | |||
the `active` property to determine which tab is currently active. The corresponding | |||
`<router-outlet>` can be placed anywhere in the view. | |||
|
|||
### Accessibility | |||
`MdTanNav`s without text or labels should be given a meaningful label via `aria-label` or |
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.
Typo: MdTabNav
.
@@ -150,3 +150,6 @@ Here are the available global options: | |||
| Name | Type | Description | | |||
| ----------------- | -------- | ----------- | | |||
| errorStateMatcher | Function | Returns a boolean specifying if the error should be shown | | |||
|
|||
### Accessibility | |||
The `mdInput` directive works with native `<input>` to provide an accessible experience. |
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.
Should also mention that if you add a placeholder
attribute to the input or a md-placeholder
element in the form field that placeholder text will automatically be used as the label for the input. If you don't specify any placeholder you need to add aria-label
aria-labeledby
or <label for=...>
Possibly also worth mentioning that any md-error
and md-hint
are automatically added to the input's aria-describedby
@@ -92,3 +92,11 @@ Here are the available global options: | |||
- <kbd>DOWN_ARROW</kbd>: Focus next option | |||
- <kbd>UP_ARROW</kbd>: Focus previous option | |||
- <kbd>ENTER</kbd> or <kbd>SPACE</kbd>: Select focused item | |||
|
|||
### Accessibility | |||
The select component without text or label should be given a meaningful label via |
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.
FYI, the placeholder, hint, and error stuff will work the same way as input after the refactor. For now this is fine though
src/lib/chips/chips.md
Outdated
@@ -38,3 +38,7 @@ also gain focus when clicked, ensuring keyboard navigation starts at the appropr | |||
The selected color of an `<md-chip>` can be changed by using the `color` property. By default, chips | |||
use a neutral background color based on the current theme (light or dark). This can be changed to | |||
`'primary'`, `'accent'`, or `'warn'`. | |||
|
|||
### Accessibility | |||
Chips has `role="option"` while chip list has `role="listbox"`. The chip input should have a |
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.
Chips have role="option"
, while the chip list has...
src/lib/dialog/dialog.md
Outdated
@@ -126,3 +126,7 @@ that the AOT compiler knows to create the `ComponentFactory` for it. | |||
}) | |||
export class AppModule() {} | |||
``` | |||
|
|||
### Accessibility | |||
The dialog has `role="dialog". The dialog will focus on the first tabbable element when opened, |
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 mention that the dialog role can be overwritten to alertdialog
in the appropriate context.
src/lib/expansion/expansion.md
Outdated
The expansion panel header has `role="button"`. The expansion panel header has attribute | ||
`aria-controls` with the expansion panel's id as value. | ||
|
||
Users can use keyboard to activate the expansion panel header to switch between expanded state |
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.
"can use keyboard" -> "can use the keyboard".
src/lib/select/select.md
Outdated
`aria-label` or `aria-labelledby`. | ||
|
||
The select component has `role="listbox"` and options inside select has `role="option"`. | ||
Autocomplete trigger sets `aria-owns` to the autocomplete's id, and sets `aria-activedescendant` to |
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 one talks about autocomplete instead of select. Also select doesn't use aria-activedescendant
, but it moves focus between the options.
src/lib/sort/sort.md
Outdated
|
||
### Accessibility | ||
The `aria-label` for sort button can be set in `MdSortHeaderIntl`. |
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.
"the aria-label
for sort button" -> "the aria-label
for the sort button".
Comments addressed. Please take another look. Thanks! |
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.
LGTM, a couple of minor nits.
src/lib/dialog/dialog.md
Outdated
@@ -126,3 +126,10 @@ that the AOT compiler knows to create the `ComponentFactory` for it. | |||
}) | |||
export class AppModule() {} | |||
``` | |||
|
|||
### Accessibility | |||
The dialog has `role="dialog", and dialog role can be overwritten to `alertdialog` in the |
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.
Seems like you're missing a backtick after the role
.
src/lib/select/select.md
Outdated
The select component without text or label should be given a meaningful label via | ||
`aria-label` or `aria-labelledby`. | ||
|
||
The select component has `role="listbox"` and options inside select has `role="option"`. |
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.
has -> have
src/lib/expansion/expansion.md
Outdated
The expansion panel header has `role="button"`. The expansion panel header has attribute | ||
`aria-controls` with the expansion panel's id as value. | ||
|
||
Users can use the keyboard to activate the expansion panel header to switch between expanded state |
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.
Is it worth noting that as the panel headers are buttons, they are able to be navigated to via keyboard as well?
f10853a
to
b80386e
Compare
Comments addressed. Please take another look. Thanks! |
LGTM for input |
@@ -95,3 +95,10 @@ desired display value. Then bind it to the autocomplete's `displayWith` property | |||
</md-optgroup> | |||
</md-autocomplete> | |||
``` | |||
|
|||
### Accessibility | |||
The input for autocomplete without text or labels should be given a meaningful label via |
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 would also be good to mention that the autocomplete trigger is given role="combobox"
src/lib/chips/chips.md
Outdated
@@ -38,3 +38,7 @@ also gain focus when clicked, ensuring keyboard navigation starts at the appropr | |||
The selected color of an `<md-chip>` can be changed by using the `color` property. By default, chips | |||
use a neutral background color based on the current theme (light or dark). This can be changed to | |||
`'primary'`, `'accent'`, or `'warn'`. | |||
|
|||
### Accessibility | |||
Chips have role="option", while the chip list has `role="listbox"`. The chip input should have a |
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.
I'd phrase this first sentence as
A chip-list behaves as a `role="listbox"`, with each chip being a `role="option"`.
src/lib/chips/chips.md
Outdated
|
||
### Accessibility | ||
Chips have role="option", while the chip list has `role="listbox"`. The chip input should have a | ||
placeholder or be given a meaningful label via `aria-label` or `aria-labelledby`. |
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 made me wonder whether we should somehow associate the chip-list and the input (Maybe aria-controls
on the input? Not sure what that would do.).
Also I wonder whether having an input
inside of a role="listbox"
is problematic- how does the screen reader treat it now?
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.
The screen reader treats chip input and chip list as two separate components now.
src/lib/dialog/dialog.md
Outdated
|
||
### Accessibility | ||
The dialog has `role="dialog"`, and dialog role can be overwritten to `alertdialog` in the | ||
appropriate context. |
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.
By default, each dialog has `role="dialog"` on the root element. The role can be changed to
`alertdialog` via the `MdDialogConfig` when opening.
The `aria-label`, `aria-labelledby`, and `aria-describedby` attributes can all be set to the
dialog element via the `MdDialogConfig` as well. Each dialog should typically have a label
set via `aria-label` or `aria-labelledby`.
#### Focus management
By default, the first tabbable element within the dialog will receive focus upon open.
Tabbing through the elements of the dialog will keep focus inside of the dialog element,
wrapping back to the first tabbable element when reaching the end of the tab sequence.
(ariaLabel being added in #6558)
We can expand this in a later PR to mention the focus control attributes from cdkFocusTrap
once we have documentation for that
src/lib/expansion/expansion.md
Outdated
@@ -98,3 +98,10 @@ panel can be expanded at a given time: | |||
|
|||
</md-accordion> | |||
``` | |||
|
|||
### Accessibility | |||
The expansion panel header has `role="button"`. The expansion panel header has attribute |
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.
I'd add a sentence at the beginning like
The expansion-panel aims to mimic the experience of the native `<details>` and `<summary>` elements.
src/lib/expansion/expansion.md
Outdated
`aria-controls` with the expansion panel's id as value. | ||
|
||
The expansion panel headers are buttons. Users can use the keyboard to activate the expansion panel | ||
header to switch between expanded state and collapsed state. |
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.
I'd add another sentence
Because the header acts as a button, additional interactive elements
should not be put inside of the header.
src/lib/table/table.md
Outdated
Tables without text or labels should be given a meaningful label via `aria-label` or | ||
`aria-labelledby`. | ||
|
||
Table's default role is 'grid', and it can be changed through `role` attribute. |
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.
We should mention that it can be changed to treegrid
specifically, since that's the only other role that makes sense (being that it contains rows and gridcells)
src/lib/table/table.md
Outdated
|
||
### Accessibility | ||
Tables without text or labels should be given a meaningful label via `aria-label` or | ||
`aria-labelledby`. |
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.
We should mention aria-readonly
regardless of what we do with it.
(it defaults to true
if it's not set, but most people probably make readonly tables)
src/lib/table/table.md
Outdated
Tables without text or labels should be given a meaningful label via `aria-label` or | ||
`aria-labelledby`. | ||
|
||
Table's default role is 'grid', and it can be changed through `role` attribute. |
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's probably also worth mentioning that the md-table
does not manage any focus/keyboard interaction on its own; if users want to add this, it can be added in their application.
src/lib/tabs/tabs.md
Outdated
@@ -79,3 +79,7 @@ provides a tab-like UI for navigating between routes. | |||
The tab-nav-bar is not tied to any particular router; it works with normal `<a>` elements and uses | |||
the `active` property to determine which tab is currently active. The corresponding | |||
`<router-outlet>` can be placed anywhere in the view. | |||
|
|||
### Accessibility | |||
`MdTabNav`s without text or labels should be given a meaningful label via `aria-label` or |
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 is true for either tab variant, not just the nav-tabs.
For nav-tabs, the <nav>
element should probably have a label as well.
src/lib/tabs/tabs.md
Outdated
|
||
### Accessibility | ||
`MdTabNav`s without text or labels should be given a meaningful label via `aria-label` or | ||
`aria-labelledby`. |
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.
Oh, and we should mention the keyboard shortcuts for md-tab-group
src/lib/dialog/dialog.md
Outdated
The dialog has `role="dialog"`, and dialog role can be overwritten to `alertdialog` in the | ||
appropriate context. | ||
|
||
The dialog will focus on the first tabbable element when opened, |
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.
I forgot about escape, too:
#### Keyboard interaction
By default pressing the escape key will close the dialog. While this behavior can
be turned off via the `disableClose` option, users should generally avoid doing so
as it breaks the expected interaction pattern for screen-reader users.
src/lib/input/input.md
Outdated
### Accessibility | ||
The `mdInput` directive works with native `<input>` to provide an accessible experience. | ||
|
||
If a placeholder attributed is added to the input, or a `md-placeholder` element is added |
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.
Typo: attributed -> attribute
b80386e
to
371060c
Compare
Comments addressed. Please take another look. Thanks! |
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.
LGTM, one last nit; add "merge-ready" when ready
src/lib/tabs/tabs.md
Outdated
`aria-labelledby`. For `MdTabNav`, the `<nav>` element should have a label as well. | ||
|
||
|
||
### Keyboard shortcuts |
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.
Should be level 4 header ####
371060c
to
f356a72
Compare
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
…, and select