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.

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

Trigger loses focus after copy event #805

Closed
patrickhlauke opened this issue Apr 14, 2022 · 3 comments · Fixed by #807
Closed

Trigger loses focus after copy event #805

patrickhlauke opened this issue Apr 14, 2022 · 3 comments · Fixed by #807
Labels

Comments

@patrickhlauke
Copy link
Contributor

🐛 Bug Report

Setting keyboard focus on one of the "Copy" buttons and activating it, focus is then lost/reset to the start of the page. While browsers try to error-correct for this (meaning that a subsequent Tab moves to the next focusable element), the issue becomes evident when using AT (e.g. Chrome/JAWS, activate the button, then use reading keys to carry on reading...focus was moved back right to the start of the page).

Note that this current behaviour is a regression from what was fixed originally in #419

This is a repost of #695 which was closed without reason.

Have you read the Contributing Guidelines on issues?

Yes

Expected Behaviour

Ideally, focus should remain on the "Copy" button, but I'm assuming this was not done in order to "clear" the visual style of the button. Perhaps this needs to dynamically set a class immediately after a copy operation was carried out, which suppresses/changes the styling (maybe suppressing the tooltip, OR adding some indicator like a tick that shows it was indeed copied), and clearing that class again as soon as the button loses focus.

Actual Behaviour

Focus is lost/reset after the "Copy" button is activated. While browsers do try to error-correct for when focus is lost, this error correction doesn't work in some situations - such as when JAWS screen reader is running. For this reason, focus should never simply be cleared/blur()red, but handled explicitly.

See this video, using Chrome and JAWS screen reader: Tab to the button - note that document.activeElement (as seen in the developer tools console) is now the button. Activate the button with Enter. document.activeElement is body, and because focus is lost/reset, JAWS goes right back to the start of the document. Using cursor keys (used by JAWS to proceed reading), you see that JAWS starts reading again right from the very start of the page.

clipboardjs-focus-lost.mp4

To Reproduce

  1. Set keyboard focus to a "Copy" button (Tab to it). document.activeElement is the button
  2. Activate the button (Enter)
  3. Note that the document.activeElement is now the body

Browsers Affected

All

Operational System

All

@obetomuniz
Copy link
Collaborator

Thanks a lot to bring such a detailed issue report @patrickhlauke. I'll review your PR in the next few days to check its impacts. Thanks a lot.

@obetomuniz
Copy link
Collaborator

obetomuniz commented Apr 29, 2022

In order to update you @patrickhlauke, after further investigation to understand the whole issue, I found out when .blur was re-inserted in the library, which guided me to this PR https://github.com/zenorocha/clipboard.js/pull/588/files#diff-96bf7f8d4171b2326c863a486fab28ba5199dc98e801a99df92b6999d02f90fd. It seems that document.activeElement.blur was added to workaround tests without considering UX (A11y) impacts, so I will be checking it in order to see if removing it broken tests anywhere before proceed in order to fix them as well.

Overall, the current state of your PR sounds great to me, but will be also checking the best approach to include the tests you removed once they are important to ensure that the trigger is being re-focused after the clipboard actions.

@patrickhlauke
Copy link
Contributor Author

great stuff, thank you for keeping me in the loop @obetomuniz (and sorry I couldn't be more use with the tests. agree it would be good to reintroduce a test, but my JS-fu when it comes to using test harnesses is weak ;) )

obetomuniz pushed a commit that referenced this issue May 4, 2022
* Remove the `blur()` following a clipboard action

It's pointless to set `focus()` on the trigger first, if in the next step you're just going to `blur()` the active element anyway.

* Tweak test to not expect active element to be body

Since it's now not `blur()`ing anymore

* Fix test

see #807 (comment)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants