Skip to content

Conversation

emarchak
Copy link
Contributor

(Try 2 for CodeCov)

WHY are these changes introduced?

We're using the Banner component multiple times on a page to communicate information in a form. When switching context (e.g. changing the form data but keeping the form), we need to update the banners.

When doing so, the aria-live state of the banners is causing them to be read out one after another. I'd like the option of turning aria-live off until we need it.

Feel free to slack @emarchak for more context on this.

WHAT is this pull request doing?

Adding a new prop to the banner component, stopAnnouncements that allows devs to set aria-live to off.

How to 🎩

  1. Set up the playground with the following code. You'll get a warning banner that has a primary action that updates text.
  2. Boot up your screen reader, and click the Update banner button.
    • You should hear the new content being read out.
  3. Uncomment // stopAnnouncements and allow the component to reload.
  4. Boot up your screen reader, and click the Update banner button.
    • You should not hear the new content being read out.

🖥 Local development instructions
🗒 General tophatting guidelines
📄 Changelog guidelines

Copy-paste this code in playground/Playground.tsx:
import React from 'react';
import {Banner} from '../src';

interface State {
  bannerTitle: string;
  bannerContent: string;
}

export default class Playground extends React.Component<State> {
  constructor(props) {
    super(props);
    this.state = {
      bannerTitle: 'This is this original banner title.',
      bannerContent: 'This is the original banner content.',
    };
  }

  render() {
    const {bannerTitle, bannerContent} = this.state;
    return (
      <Banner
        title={bannerTitle}
        // stopAnnouncements
        status="warning"
        action={{
          content: 'Update banner',
          onAction: () => {
            this.setState({
              bannerTitle: 'This is a new banner title',
              bannerContent: 'This is some new banner content',
            });
          },
        }}
      >
        {bannerContent}
      </Banner>
    );
  }
}

🎩 checklist

@BPScott BPScott temporarily deployed to polaris-react-pr-1719 June 20, 2019 15:05 Inactive
@emarchak emarchak requested a review from dpersing June 20, 2019 15:19
@emarchak emarchak force-pushed the banner-stop-aria-live branch from 7920ff0 to 392ff48 Compare June 20, 2019 15:25
@BPScott BPScott temporarily deployed to polaris-react-pr-1719 June 20, 2019 15:25 Inactive
Copy link

@dpersing dpersing left a comment

Choose a reason for hiding this comment

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

🤞

@emarchak emarchak force-pushed the banner-stop-aria-live branch from 392ff48 to 6a6107d Compare June 20, 2019 16:40
@BPScott BPScott temporarily deployed to polaris-react-pr-1719 June 20, 2019 16:40 Inactive
@emarchak emarchak merged commit ee1dbad into master Jun 20, 2019
@emarchak emarchak deleted the banner-stop-aria-live branch June 20, 2019 17:13
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.

3 participants