-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
feat(snackbar): Accessibility: Announce label text to screen readers #4090
Changes from 3 commits
b4b19b7
6f34f18
90a52ff
c924dd5
b92a170
593cfaf
276ac38
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 |
---|---|---|
@@ -0,0 +1,88 @@ | ||
/** | ||
* @license | ||
* Copyright 2018 Google Inc. | ||
* | ||
* Permission is hereby granted, free of charge, to any person obtaining a copy | ||
* of this software and associated documentation files (the "Software"), to deal | ||
* in the Software without restriction, including without limitation the rights | ||
* to use, copy, modify, merge, publish, distribute, sublicense, and/or sell | ||
* copies of the Software, and to permit persons to whom the Software is | ||
* furnished to do so, subject to the following conditions: | ||
* | ||
* The above copyright notice and this permission notice shall be included in | ||
* all copies or substantial portions of the Software. | ||
* | ||
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR | ||
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, | ||
* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE | ||
* AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER | ||
* LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, | ||
* OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN | ||
* THE SOFTWARE. | ||
*/ | ||
|
||
import {numbers, strings} from './constants'; | ||
|
||
const {ARIA_LIVE_DELAY_MS} = numbers; | ||
const {ARIA_LIVE_LABEL_TEXT_ATTR} = strings; | ||
|
||
/** | ||
* @param {!HTMLElement} ariaEl | ||
* @param {!HTMLElement=} labelEl | ||
*/ | ||
export function announce(ariaEl, labelEl = ariaEl) { | ||
const priority = ariaEl.getAttribute('aria-live'); | ||
const labelText = labelEl.textContent.trim(); // Ignore ` ` (see below) | ||
if (!labelText) { | ||
return; | ||
} | ||
|
||
// Temporarily disable `aria-live` to prevent JAWS+Firefox from announcing the message twice. | ||
ariaEl.setAttribute('aria-live', 'off'); | ||
|
||
// Temporarily clear `textContent` to force a DOM mutation event that will be detected by screen readers. | ||
// `aria-live` elements are only announced when the element's `textContent` *changes*, so snackbars | ||
// sent to the browser in the initial HTML response won't be read unless we clear the element's `textContent` first. | ||
// Similarly, displaying the same snackbar message twice in a row doesn't trigger a DOM mutation event, | ||
// so screen readers won't announce the second message unless we first clear `textContent`. | ||
// | ||
// We have to clear the label text two different ways to make it work in all browsers and screen readers: | ||
// | ||
// 1. `textContent = ''` is required for IE11 + JAWS | ||
// 2. `innerHTML = ' '` is required for Chrome + JAWS and NVDA | ||
// | ||
// All other browser/screen reader combinations support both methods. | ||
// | ||
// The wrapper `<span>` visually hides the space character so that it doesn't cause jank when added/removed. | ||
// N.B.: Setting `position: absolute`, `opacity: 0`, or `height: 0` prevents Chrome from detecting the DOM change. | ||
// | ||
// This technique has been tested in: | ||
// | ||
// * JAWS 18.0 & 2019: | ||
// - Chrome 70 | ||
// - Firefox 60 (ESR) | ||
// - IE 11 | ||
// * NVDA 2017 & 2018: | ||
// - Chrome 70 | ||
// - Firefox 60 (ESR) | ||
// - IE 11 | ||
labelEl.textContent = ''; | ||
labelEl.innerHTML = '<span style="display: inline-block; width: 0; height: 1px;"> </span>'; | ||
|
||
// Prevent visual jank by temporarily displaying the label text in the ::before pseudo-element. | ||
// CSS generated content is normally announced by screen readers | ||
// (except in IE 11; see https://tink.uk/accessibility-support-for-css-generated-content/); | ||
// however, `aria-live` is turned off, so this DOM update will be ignored by screen readers. | ||
labelEl.setAttribute(ARIA_LIVE_LABEL_TEXT_ATTR, labelText); | ||
|
||
setTimeout(() => { | ||
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'm a little wary that we're not storing / returning the handle on this timeout, but maybe I'm being paranoid. Is there a chance of the snackbar being dismissed before this timeout even elapses and causing weird behavior? Or will it simply not announce because the snackbar will become display: none? 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. If it's The timeout is responsible for resetting the I made sure to test for non-empty From my testing, Windows screen readers queue the message and read it fully as long as the |
||
// Allow screen readers to announce changes to the DOM again. | ||
ariaEl.setAttribute('aria-live', priority); | ||
|
||
// Remove the message from the ::before pseudo-element. | ||
labelEl.removeAttribute(ARIA_LIVE_LABEL_TEXT_ATTR); | ||
|
||
// Restore the original label text, which will be announced by screen readers. | ||
labelEl.textContent = labelText; | ||
}, ARIA_LIVE_DELAY_MS); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -47,12 +47,11 @@ | |
<button class="test-snackbar-open-button test-font--redact-all" data-test-snackbar-id="test-snackbar--with-action" autofocus>Open snackbar with action</button> | ||
|
||
<div class="mdc-snackbar mdc-snackbar--open" | ||
id="test-snackbar--with-action" | ||
role="status" | ||
aria-live="polite" | ||
aria-hidden="true"> | ||
id="test-snackbar--with-action"> | ||
<div class="mdc-snackbar__surface"> | ||
<div class="mdc-snackbar__label">Can't send photo. Retry in 5 seconds.</div> | ||
<div class="mdc-snackbar__label" | ||
role="status" | ||
aria-live="polite">Can't send photo. Retry in 5 seconds.</div> | ||
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. If we're announcing it programmatically do we even need to mention it here? Seems like we're turning it off explicitly to avoid repetition.
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. Great question! Yes, I think we want to keep it here in the markup so that clients can easily change it to I experimented with having I also tried doing something like I've arrived at the present solution after many hours of painful experimentation. It seems to work reliably in 6 of the most commonly-used browser/screen reader combinations: JAWS and NVDA in Chrome, IE 11 💩, and Firefox. It doesn't seem to work with Safari + VoiceOver on macOS 10.13 High Sierra, unfortunately. 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. In my testing Safari+VO seemed to work with assertive rather than polite, but I can re-test when we get to the readme. 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. Thanks for explaining! The announce adapter method seem to be coupled with what util.announce() method does which has its own business logic. It has to follow certain sequence of DOM operations to implement it. For example, some wrappers have their own announcer utilities which makes it hard for them to reuse it or do they've to reimplement this util function? 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 expect internal teams to use I added unit tests for the 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. As discussed offline, as long as if the screen reader doesn't end up start pronouncing it twice (or jank) because of |
||
<div class="mdc-snackbar__actions"> | ||
<button type="button" class="mdc-button mdc-snackbar__action-button">Retry</button> | ||
<button class="mdc-icon-button mdc-snackbar__action-icon material-icons" title="Dismiss">close</button> | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -48,12 +48,11 @@ | |
|
||
<div class="mdc-snackbar mdc-snackbar--wide mdc-snackbar--open" | ||
id="test-snackbar--wide" | ||
role="status" | ||
aria-live="polite" | ||
aria-hidden="true" | ||
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. You've been moving the role/aria-live attributes to the label but you didn't do that here? (I'm actually curious why we want these attributes only on the label - does that mean the action/dismiss buttons won't be announced? How will a user relying on screen readers know that they exist?) 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. Thanks, fixed! You are correct, the action/dismiss buttons won't be announced. According to WCAG 2.0 — SC 2.2.3 — No Timing, UI controls should not be timing-dependent. We don't want to announce a button that won't be there by the time the user navigates to it. The snackbar a11y spec also says that apps should provide a redundant means of achieving the same action elsewhere in the app. So for AT users, this is simply a passive notification. |
||
data-test-snackbar-timeout-ms="10000"> | ||
<div class="mdc-snackbar__surface"> | ||
<div class="mdc-snackbar__label"> | ||
<div class="mdc-snackbar__label" | ||
role="status" | ||
aria-live="polite"> | ||
Connection timed out. Showing the latest locally saved version of this document. | ||
Edits made while offline will not be visible to other users until network connectivity is restored. | ||
</div> | ||
|
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.
What's the purpose of labelEl being a separate parameter? We don't seem to be using it that way.
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.
Every time the DOM or CSS changes (e.g., adding
display: none
), it breaks in at least one AT/browser combination. I've gone back and forth between using separate elements or the same element, depending on the moods of the Screen Reader Gods.For example, if we decided to announce the action button to screen reader users (which I advise against—see my comment above), we would need to move the ARIA attributes back to the root element and add
aria-atomic="true"
. Then they would be two separate elements again.I'm mainly trying to make future changes easier.