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

[EuiScreenReaderOnly] Add preventCopy prop #6806

Closed
wants to merge 7 commits into from

Conversation

cee-chen
Copy link
Member

@cee-chen cee-chen commented May 23, 2023

Summary

closes #6804
reverts #6817 (isFocused workaround should no longer be needed)

This PR adds a preventCopy prop to EuiScreenReaderOnly, and applies said functionality to EuiDataGrid to prevent customers copying data from grabbing the SR information about row/column count.

I recommend, as always, following along by commit to separate out cleanup tasks.

QA

  • Go to http://localhost:8030/#/utilities/accessibility
  • Copy both lines in the first demo
  • Paste into any notepad and confirm the visually hidden text was copied (should have 3 lines)
  • Copy both lines in the second demo
  • Paste into any notepad and confirm the visually hidden text was not copied (should have 2 lines)

General checklist

  • Checked in Chrome, Safari, Edge, and Firefox
  • Checked for accessibility including keyboard-only and screenreader modes
  • Props have proper autodocs (using @default if default values are missing) and playground toggles
  • Added documentation
  • Added or updated jest and cypress tests
  • A changelog entry exists and is marked appropriately

- [ ] Checked in both light and dark modes
- [ ] Checked in mobile
- [ ] Checked Code Sandbox works for any docs examples
- [ ] Checked for breaking changes and labeled appropriately
- [ ] Updated the Figma library counterpart

@cee-chen cee-chen requested a review from 1Copenut May 23, 2023 23:02
@cee-chen
Copy link
Member Author

cee-chen commented May 23, 2023

lol F, this works for Firefox and Safari with richtext paste but not Chrome/Edge (only works on plain paste). 💀

https://stackoverflow.com/questions/65021694/user-selectnone-still-part-of-copied-rich-text-in-chrome

https://bugs.chromium.org/p/chromium/issues/detail?id=1083125

Looks like this might be fixed soon on Chromium's end (fingers crossed) 😬 - rather than trying to do JS shenanigans with onCopy, it might be better to wait until a little bit / keep this PR dangling until Chrome pushes a fix

@kibanamachine
Copy link

Preview documentation changes for this PR: https://eui.elastic.co/pr_6806/

Copy link
Contributor

@1Copenut 1Copenut left a comment

Choose a reason for hiding this comment

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

I reviewed the PR and tried out the recommended QA tasks for the EuiScreenReaderOnly and EuiDataGrid components. They worked as expected using the keyboard and VoiceOver on Safari.

I'm okay letting this hang out for a bit if you feel Chromium is going to ship a fix soon. I generally like to ship even partial fixes if it improves the experience for users without degrading it for others. In this case, you've improved the experience for Firefox and Safari users, and AFAIK the experience is the same for Chromium.

@cee-chen
Copy link
Member Author

I'm going to wait for Chromium I think. I guarantee if we release a prop promising to do something that doesn't work quite correctly for the largest market share of browsers, people will be flooding our GitHub issues 😅

@1Copenut
Copy link
Contributor

I'm going to wait for Chromium I think. I guarantee if we release a prop promising to do something that doesn't work quite correctly for the largest market share of browsers, people will be flooding our GitHub issues 😅

Excellent point, this ^^.

@kibanamachine
Copy link

Preview documentation changes for this PR: https://eui.elastic.co/pr_6806/

@github-actions
Copy link

👋 Hey there. This PR hasn't had any activity for 90 days. We'll automatically close it if that trend continues for another week. If you feel this issue is still valid and needs attention please let us know with a comment.

@cee-chen
Copy link
Member Author

Removing the stale PR label for now in hopes that Chromium eventually fixes the bug 🤞

@cee-chen cee-chen removed the stale-pr label Aug 29, 2023
Copy link

👋 Hey there. This PR hasn't had any activity for 90 days. We'll automatically close it if that trend continues for another week. If you feel this issue is still valid and needs attention please let us know with a comment.

@cee-chen
Copy link
Member Author

cee-chen commented Feb 5, 2024

I'm just gonna move this back to draft since Chromium doesn't appear to be resolving the bug anytime soon. 🥲

Copy link

github-actions bot commented May 6, 2024

👋 Hey there. This PR hasn't had any activity for 90 days. We'll automatically close it if that trend continues for another week. If you feel this issue is still valid and needs attention please let us know with a comment.

Copy link

❌ We're automatically closing this PR due to lack of activity. Please comment if you feel this was done in error.

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.

[EuiDataGrid] Skip screen reader text when selecting text from grid
3 participants