Skip to content

Conversation

dleroux
Copy link
Contributor

@dleroux dleroux commented Jul 23, 2019

WHY are these changes introduced?

Fixes #1215

WHAT is this pull request doing?

In order to announce the Toast using the aria-live attribut, NVDA require the element with the aria-live prop to be on screen when the content changes.

Our Toasts are wrapped in a Toast manager. This pr moves the aria-live attribute from the Toast to the Toast Manager

How to 🎩

Using NVDA (on windows) navigate to:

http://localhost:6006/?path=/story/all-components-toast--basic-toast

and ensure that the Toast messages are being announced.

Copy-paste this code in playground/Playground.tsx:
import * as React from 'react';
import {Page} from '@shopify/polaris';

interface State {}

export default class Playground extends React.Component<never, State> {
  render() {
    return (
      <Page title="Playground">
        {/* Add the code you want to test here */}
      </Page>
    );
  }
}

🎩 checklist

@BPScott BPScott temporarily deployed to polaris-react-pr-1873 July 23, 2019 15:08 Inactive
@dleroux
Copy link
Contributor Author

dleroux commented Jul 23, 2019

@dpersing I wasn't able to get the sound working on NVDA but the tools that write out the content seemed to be working as expected. Can you 🎩 and make sure this does fix our issue?

@dleroux dleroux requested review from dpersing and yourpalsonja July 23, 2019 15:12
@Shopify Shopify deleted a comment from devonpmack Jul 23, 2019
@BPScott BPScott temporarily deployed to polaris-react-pr-1873 July 23, 2019 15:18 Inactive
@BPScott BPScott temporarily deployed to polaris-react-pr-1873 July 23, 2019 15:19 Inactive
@dleroux dleroux changed the title [WIP][ToastManager] Move aria-live to containing div for a11y using NVDA [ToastManager] Move aria-live to containing div for a11y using NVDA Jul 23, 2019
Copy link
Contributor

@yourpalsonja yourpalsonja left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code looks good to me!
10:10

@dleroux dleroux merged commit f49dba2 into master Jul 26, 2019
@alex-page alex-page temporarily deployed to production July 31, 2019 14:28 Inactive
@tmlayton tmlayton temporarily deployed to patch August 12, 2019 20:21 Inactive
@dleroux dleroux deleted the toast-aria-live branch August 14, 2019 12:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[a11y] Toast not conveyed consistently by screen readers

5 participants