Skip to content

Add live region to announce "Copied!" on ClipboardCopy component - #2843

Merged
lindseywild merged 17 commits into
mainfrom
lw/announce-on-clipboard-copy
May 16, 2024
Merged

Add live region to announce "Copied!" on ClipboardCopy component#2843
lindseywild merged 17 commits into
mainfrom
lw/announce-on-clipboard-copy

Conversation

@lindseywild

@lindseywild lindseywild commented May 15, 2024

Copy link
Copy Markdown
Contributor

What are you trying to accomplish?

Adds a live region to the ClipboardCopy component so that by default, "Copied!" is announced when a user successfully copies something.

Screenshots

Before

Screen recording of the ClipboardCopy button on Lookbook, when pressing "Enter" or "Control + Option + Space", no feedback is announced but there is a visual icon change from 2 pages stacked on top of each other to a green check mark

CleanShot.2024-05-15.at.11.43.23.mp4

After

Screen recording of the ClipboardCopy button on Lookbook, when pressing "Enter" or "Control + Option + Space", "Copied!" is announced

CleanShot.2024-05-15.at.11.42.25.mp4

Integration

No

List the issues that this change affects.

Related to https://github.com/github/accessibility-audits/issues/6469 & https://github.com/github/accessibility/issues/6096.

Risk Assessment

  • Low risk the change is small, highly observable, and easily rolled back.
  • Medium risk changes that are isolated, reduced in scope or could impact few users. The change will not impact library availability.
  • High risk changes are those that could impact customers and SLOs, low or no test coverage, low observability, or slow to rollback.

What approach did you choose and why?

I am adding a live region to the ClipboardCopy button so that, by default, screen reader feedback will be announced.

I considered adding a live region to the clipboard-copy-element web component which is used under the hood, but changing that component to include a live region would be a breaking change since we'd need to wrap the current inner text with another element so that we can have a live region as a sibling. Nesting a live region inside of a <button> element has poor support and is not recommended.

Anything you want to highlight for special attention from reviewers?

  • Is there a better way to wrap the component?
  • Is there a better way than looking for the parentNode of the target element to target the specific aria-live region for one component (thinking if there are multiple ClipboardCopy elements on the page and not wanting them to conflict)? I didn't want to change the API too much by making the wrapper I added the new <clipboard-copy> component but maybe it's ok?

Merge checklist

  • Added/updated tests
  • Added/updated documentation
  • Added/updated previews (Lookbook)
  • Tested in Chrome
  • Tested in Firefox
  • Tested in Safari
  • Tested in Edge

Take a look at the What we look for in reviews section of the contributing guidelines for more information on how we review PRs.

@changeset-bot

changeset-bot Bot commented May 15, 2024

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 59e8e0a

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@primer/view-components Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@lindseywild
lindseywild marked this pull request as ready for review May 15, 2024 17:34
@lindseywild
lindseywild requested a review from a team as a code owner May 15, 2024 17:34
@lindseywild
lindseywild requested review from JoyceZhu and camertron May 15, 2024 17:34

@camertron camertron left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks! 🎉

Comment thread app/components/primer/beta/clipboard_copy.ts
Comment thread app/components/primer/beta/clipboard_copy.ts Outdated
Comment thread test/system/beta/clipboard_copy_button_test.rb
Co-authored-by: Cameron Dutro <camertron@gmail.com>
Comment thread test/system/beta/clipboard_copy_button_test.rb
Comment thread app/components/primer/beta/clipboard_copy.ts Outdated
Co-authored-by: Cameron Dutro <camertron@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants