a11y(toolbar): Add toolbar accessibility demo page #6145
Conversation
googlebot
added
the
cla: yes
| @@ -0,0 +1,28 @@ | |||
| <div class="demo-toolbar"> | |||
| <section> | |||
| <h2>Display</h2> | |||
|
|
||
| <section> | ||
| <h2>Multiple Lines Toolbar</h2> | ||
| <md-toolbar>Line 1</md-toolbar> |
There was a problem hiding this comment.
Use real world example?
| <md-toolbar> | ||
| <span>My App</span> | ||
| <span class="example-spacer"></span> | ||
| <md-icon class="example-icon">favorite</md-icon> |
There was a problem hiding this comment.
Add aria-label for icon
There was a problem hiding this comment.
md-icons actually are aria-hidden by default. They should only be made aria-hidden="false" if they're communicating some information. See the accessibility section of the icon docs.
In this case, having an icon sitting by itself in the toolbar isn't very realistic to a real app; it would either be an indicator for something or an action.
tinayuangao
added
Accessibility
| <div class="demo-toolbar"> | ||
| <section> | ||
| <h2>Basic Toolbar with Text (e.g. Only display app’s name)</h2> | ||
| <md-toolbar>My App</md-toolbar> |
There was a problem hiding this comment.
A toolbar by itself isn't really representative of how the component will be used. It typically acts as a header for some content. In this capacity, it should either use role="heading" and aria-level OR it should contain a header element (h1 -> h6).
The examples should represent a real use-case as much as possible.
amcdnl
changed the title
src/demo-app/a11y/a11y.ts
Outdated
| @@ -26,6 +26,7 @@ export class AccessibilityDemo { | |||
| {name: 'Grid list', route: 'grid-list'}, | |||
| {name: 'Input', route: 'input'}, | |||
| {name: 'Radio buttons', route: 'radio'}, | |||
| {name: 'Slider', route: 'slider'}, | |||
| {name: 'Toolbar', route: 'toolbar'}, | |||
There was a problem hiding this comment.
nit: put 'Toolbar' after 'Slider'
| <md-toolbar> | ||
| <span>My App</span> | ||
| <span class="example-spacer"></span> | ||
| <md-icon aria-label="favorite">favorite</md-icon> |
There was a problem hiding this comment.
Is aria-label working on md-icon? Consider adding a span with cdk-visually-hidden or make it a button/link?
https://material.angular.io/components/icon/overview
| <h2>Basic Toolbar with Text (e.g. Only display app’s name)</h2> | ||
| <md-toolbar role="heading"> | ||
| <h1>My App</h1> | ||
| </md-toolbar> |
There was a problem hiding this comment.
I wasn't very clear in the last comments: each toolbar should have some actual content under it as well (that the toolbar describes; can just be a <p>)
jelbourn
added
pr: lgtm
action: merge
kara
added
pr: needs rebase
and removed
action: merge
|
@amcdnl Can you rebase this? |
kara
added
action: merge
kara
changed the title
|
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. |
Created accessibility demo page demonstrating: