Skip to content
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.

Sign up for GitHub

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

Jump to bottom

fix(Copy): add aria-live to announce text updates #5386

Merged
merged 4 commits into from
Feb 20, 2020
Merged

fix(Copy): add aria-live to announce text updates #5386

merged 4 commits into from
Feb 20, 2020

Conversation

emyarod
Copy link
Member

@emyarod emyarod commented Feb 18, 2020 •
edited
Loading

Closes #5385
Closes #2523

This PR updates WAI-ARIA state so that changes in the <CopyButton> are read by screen readers

Changelog

New

  • aria-live region

Changed

  • change aria-label based on animation state

Testing / Reviewing

Interact with the copy button using a screen reader and observe if button label changes are immediately read

@emyarod emyarod requested a review from a team as a code owner February 18, 2020 17:38
@ghost ghost requested a review from asudoh February 18, 2020 17:38
@emyarod
Copy link
Member Author

emyarod commented Feb 18, 2020

this may address #2523 as well cc @tw15egan

@netlify
Copy link

netlify bot commented Feb 18, 2020 •
edited
Loading

Deploy preview for carbon-elements ready!

Built with commit e4fa42b

https://deploy-preview-5386--carbon-elements.netlify.com

@netlify
Copy link

netlify bot commented Feb 18, 2020 •
edited
Loading

Deploy preview for carbon-components-react ready!

Built with commit e4fa42b

https://deploy-preview-5386--carbon-components-react.netlify.com

@abbeyhrt
Copy link
Contributor

This looks great! I'm not sure why but for me it's announcing 'Copied' twice:
Copy

@emyarod
Copy link
Member Author

emyarod commented Feb 18, 2020

for me inside storybook it does not repeat the text in the voiceover dialog, it just reads it twice

image

but this seems to be only in storybook, I can't reproduce it when viewing the component outside of storybook or even in the storybook canvas on a new tab

asudoh
asudoh approved these changes Feb 19, 2020
View reviewed changes
Copy link
Contributor

@asudoh asudoh left a comment

Choose a reason for hiding this comment

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

LGTM 👍 - Thanks @emyarod!

tw15egan
tw15egan approved these changes Feb 19, 2020
View reviewed changes
Copy link
Collaborator

@tw15egan tw15egan left a comment

Choose a reason for hiding this comment

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

Fix looks great! Confirmed on MacOS voiceover util, getting 1 "Copied!" when the button is clicked. No longer seeing the issue where CodeSnippet is announced as an editable text field as well so this should take care of #2523

Screen Shot 2020-02-19 at 11 00 53 AM

@asudoh
Copy link
Contributor

asudoh commented Feb 20, 2020

Got a green light from @emyarod to move this to "ready to merge" state - Thanks @emyarod!

@asudoh asudoh merged commit 75acb17 into carbon-design-system:master Feb 20, 2020
@emyarod emyarod deleted the 5385-copy-button-announce-state-change-screenreader branch February 24, 2020 20:38
@joshblack joshblack mentioned this pull request Feb 25, 2020
Closed
@kuri-sun kuri-sun mentioned this pull request Dec 23, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@abbeyhrt abbeyhrt abbeyhrt approved these changes

@tw15egan tw15egan tw15egan approved these changes

@asudoh asudoh asudoh approved these changes

Assignees
No one assigned
Labels
status: ready to merge 🎉
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

CopyButton does not announce state change with screen readers Code snippet screen reader issues in JAWS 2020
5 participants
@emyarod @abbeyhrt @asudoh @tw15egan @joshblack