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

[EuiDataGrid] Skip screen reader text when selecting text from grid #6804

Closed
Tracked by #182611
jughosta opened this issue May 23, 2023 · 8 comments
Closed
Tracked by #182611

[EuiDataGrid] Skip screen reader text when selecting text from grid #6804

jughosta opened this issue May 23, 2023 · 8 comments
Assignees

Comments

@jughosta
Copy link
Contributor

jughosta commented May 23, 2023

As a user when selecting text in the grid, I would expect to get only the selection to my clipboard. Instead, screen reader text is being added too.

Screenshot 2023-05-23 at 19 49 40
  • Expected result: only the originaly selected text.

  • Actual:

May 23, 2023 @ 19:49:14.276
- timestamp, column 3, row 1



200
- response, column 4, row 1
https://cdn.elastic-elastic-elastic.org/styles/app.css
- url, column 5, row 1
125.112.150.127
- clientip, column 6, row 1
win xp
- machine.os, column 7, row 1
[success, security]
- tags, column 8, row 1

- openDetails, column 1, row 2

- select, column 2, row 2
May 23, 2023 @ 19:48:36.092
- timestamp, column 3, row 2
200
- response, column 4, row 2
https://artifacts.elastic.co/downloads/beats/metricbeat/metricbeat-6.3.2-i686.rpm
- url, column 5, row 2
35.169.122.161
- clientip, column 6, row 2
ios
- machine.os, column 7, row 2
[success, info]
- tags, column 8, row 2

- openDetails, column 1, row 3

- select, column 2, row 3
May 23, 2023 @ 19:45:42.944
- timestamp, column 3, row 3
200

Initially discussed in #6561 (comment)

Feedback from the community:
https://discuss.elastic.co/t/copy-paste-from-kibana-without-row-and-column-information/320048/
https://discuss.elastic.co/t/copy-paste-from-kibana-without-row-and-column-information-2/334026

@cee-chen
Copy link
Contributor

cee-chen commented May 23, 2023

This will need thorough cross-browser testing, but I believe we should be able to simply use user-select: none CSS on the below EuiScreenReaderOnly text to prevent it from showing up in copy/paste:

<EuiScreenReaderOnly>
<p>
{'- '}
<EuiI18n
token="euiDataGridCell.position"
default="{columnId}, column {col}, row {row}"
values={{
columnId: column?.displayAsText || rest.columnId,
col: colIndex + 1,
row: ariaRowIndex,
}}
/>
</p>
</EuiScreenReaderOnly>

In fact, this feels so generally useful (as opposed to a one-off fix), that I think we should make this a new prop, e.g. <EuiScreenReaderOnly preventCopy={true} />

@cee-chen
Copy link
Contributor

user-select: none works perfectly except in Chromium, which has a bizarre bug around rich text pasting still having the hidden text, but plain text pasting not having it (https://bugs.chromium.org/p/chromium/issues/detail?id=1083125). We'll likely wait to deploy a fix until Chromium has a resolution - it looks like they're really close to one.

@cee-chen
Copy link
Contributor

@jughosta Until the linked Chromium bug is fixed, I'm going to add a workaround for now that makes it so only the current open/focused cell has copy-able SR context. So there will still be extra text for 1 cell, but that's much easier to edit out / significantly better than every single cell 😅 I won't close this bug out until #6806 lands however.

BTW, in your opinion, is this bug severe enough to warrant a backport to the next Kibana 8.8 release? Or is the next 8.9 fine?

@jughosta
Copy link
Contributor Author

Great! Thanks, @cee-chen ! Not only Discover will benefit from it but also other Kibana pages which don't have CSV export.

I would say yes. Let's confirm with @davismcphee.

@davismcphee
Copy link
Contributor

@cee-chen @jughosta IMO it depends -- if @cee-chen considers the fix low risk and easy to backport to 8.8, I'd say go for it. If not, the issue has existed on our end for a while at this point, so it wouldn't be a huge deal to ship it in 8.9. Also, thanks for the quick fix!

@cee-chen
Copy link
Contributor

cee-chen commented May 30, 2023

It's a fairly easy-to-backport fix for 8.8. I can do a quick release and PR for Kibana sometime in the next week here. Thanks y'all!

cee-chen added a commit to elastic/kibana that referenced this issue May 31, 2023
This is a backport EUI upgrade to Kibana v8.8.1 containing an
EuiDataGrid bugfix requested by the Discover team:
elastic/eui#6804 (comment)

## [`77.1.4`](https://github.com/elastic/eui/tree/v77.1.4)

- Updated `EuiDataGrid` to only render screen reader text announcing
cell position if the cell is currently focused. This should improve the
ability to copy and paste multiple cells without SR text.
([#6817](elastic/eui#6817))
@cee-chen
Copy link
Contributor

The workaround is merged into Kibana and should be in the next v8.8.1. Whenever Chromium is finally updated to enable a full fix, I'll backport that as well and close out this issue then.

@cee-chen
Copy link
Contributor

I'm going to mark this issue as closed for now because it looks like Chromium has no intention of fixing the bug anytime soon, and the workaround suffices in the meanwhile.

jughosta added a commit to elastic/kibana that referenced this issue Aug 12, 2024
- Closes #179731
- Related to #177952 
- Related to elastic/eui#6804

## Summary

This PR adds "Copy selection as text" action.

<img width="926" alt="Screenshot 2024-07-31 at 18 09 21"
src="https://github.com/user-attachments/assets/a4fc7456-7cd9-4493-a4dd-45151f845566">


### Checklist

- [x] Any text added follows [EUI's writing
guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses
sentence case text and includes [i18n
support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)
- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios
- [x] This was checked for [cross-browser
compatibility](https://www.elastic.co/support/matrix#matrix_browsers)

---------

Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
Co-authored-by: Davis McPhee <davismcphee@hotmail.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 a pull request may close this issue.

3 participants