Skip to content
This repository has been archived by the owner on May 24, 2024. It is now read-only.

[terra-alert] Update notification banner for screen reader experience #3908

Merged
merged 9 commits into from
Sep 22, 2023

Conversation

trandrew1023
Copy link
Contributor

@trandrew1023 trandrew1023 commented Sep 13, 2023

Summary

What was changed:

  • Added visually hidden default title (notification type) for screen readers to read alongside custom titles when notification type is present.
  • Tabbing on the dismiss button will read out the corresponding notification type and custom title if provided

Why it was changed:

  • Expectation is the screen reader reads the notification type alongside custom titles (which the title ideally should be the notification type)

Testing

This change was tested using:

  • WDIO
  • Jest
  • Visual testing (please attach a screenshot or recording)
  • Other (please describe below)
  • No tests are needed

To test, use Custom Prop Alert example under the Tests tab and add the title prop to the notification
Tested with Jaws and Edge/Chrome (also VO on Mac by Steven)
https://github.com/cerner/terra-core/assets/37750902/530dd406-a193-41d0-bdcb-4e1ddaf6535a

Reviews

In addition to engineering reviews, this PR needs:

  • UX review
  • Accessibility review
  • Functional review

Additional Details

This PR resolves:

UXPLATFORM-9604


Thank you for contributing to Terra.
@cerner/terra

@trandrew1023
Copy link
Contributor Author

@eawww Test comment

@trandrew1023 trandrew1023 marked this pull request as ready for review September 19, 2023 23:36
@trandrew1023 trandrew1023 requested a review from a team as a code owner September 19, 2023 23:36
@trandrew1023 trandrew1023 changed the title Update notification banner for screen reader experience [terra-alert] Update notification banner for screen reader experience Sep 20, 2023
@trandrew1023 trandrew1023 self-assigned this Sep 20, 2023
@mjpalazzo mjpalazzo added ⭐ Accessibility Reviewed Accessibility has been reviewed and approved. and removed Accessibility Review Required labels Sep 21, 2023
@trandrew1023 trandrew1023 added ⭐ UX Reviewed UX Reviewed and approved. and removed UX Review Required labels Sep 21, 2023
let mockSpyUuid;
beforeAll(() => {
mockSpyUuid = jest.spyOn(uuidv4, 'v4').mockReturnValue('00000000-0000-0000-0000-000000000000');
mockSpyUuid = jest.spyOn(uuidv4, 'v4').mockReturnValue(mockUuid);
Copy link
Contributor

Choose a reason for hiding this comment

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

We should hardcode this otherwise, it may show up as undefined when testing in other components.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will undo these!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated here 4678cc7

@github-actions github-actions bot temporarily deployed to preview-pr-3908 September 21, 2023 22:18 Destroyed
@sdadn sdadn merged commit e39847c into main Sep 22, 2023
22 checks passed
@sdadn sdadn deleted the alert-at-updates branch September 22, 2023 14:31
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
📦 terra-alert ⭐ Accessibility Reviewed Accessibility has been reviewed and approved. ⭐ UX Reviewed UX Reviewed and approved.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants