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

Remove new lines when copy/pasting in Discover #177952

Closed
Tracked by #182611
juste-bob opened this issue Feb 28, 2024 · 24 comments · Fixed by elastic/eui#8019
Closed
Tracked by #182611

Remove new lines when copy/pasting in Discover #177952

juste-bob opened this issue Feb 28, 2024 · 24 comments · Fixed by elastic/eui#8019
Labels
blocked bug Fixes for quality problems that affect the customer experience EUI impact:low Addressing this issue will have a low level of impact on the quality/strength of our product. Team:DataDiscovery Discover App Team (Document Explorer, Saved Search, Surrounding documents, Data, DataViews) upstream

Comments

@juste-bob
Copy link

Hello,

I coming back on this subject since the last topic was automatically closed. There is still issues when copy/pasting the Discover's output.

In the following example, there is 3 columns (@timestamp, host, report_desc). When copying the first 3 lines (see screenshot attached) I have the following output (ELK is 8.12.2):

copy_paste_elk_8 12 2

2016-10-05 07:48:23.000
- @timestamp, column 3, row 1



samples
[evtx/powershell/600] provider 'WSMan' new state is 'Started', ps_host_name: 'ConsoleHost', cmd:''', cmd_path:'', script_name:'', host_application:''


2016-10-05 07:48:23.000
samples
[evtx/powershell/600] provider 'Function' new state is 'Started', ps_host_name: 'ConsoleHost', cmd:''', cmd_path:'', script_name:'', host_application:''


2016-10-05 07:48:23.000
samples
[evtx/powershell/600] provider 'Certificate' new state is 'Started', ps_host_name: 'ConsoleHost', cmd:''', cmd_path:'', script_name:'', host_application:''

While the expected output is:

2016-10-05 07:48:23.000 samples [evtx/powershell/600] provider 'WSMan' new state is 'Started', ps_host_name: 'ConsoleHost', cmd:''', cmd_path:'', script_name:'', host_application:''
2016-10-05 07:48:23.000 samples [evtx/powershell/600] provider 'Function' new state is 'Started', ps_host_name: 'ConsoleHost', cmd:''', cmd_path:'', script_name:'', host_application:''
2016-10-05 07:48:23.000 samples [evtx/powershell/600] provider 'Certificate' new state is 'Started', ps_host_name: 'ConsoleHost', cmd:''', cmd_path:'', script_name:'', host_application:''

We are currently working around this issue by turning on the legacy's doc_table: doc_table:legacy.

Could it be possible to patch it ?

Thanks in advance.

@juste-bob juste-bob added the bug Fixes for quality problems that affect the customer experience label Feb 28, 2024
@kertal
Copy link
Member

kertal commented Mar 4, 2024

In Discover you can select and export rows, in JSON format. if you would have a way to select rows and export them as text, row by row, would this cover your use case?

@cee-chen
Copy link
Member

cee-chen commented Mar 4, 2024

Moving this from the EUI repo to Kibana, as at a glance I don't think this is an issue with EUI specifically as opposed to Discover

@cee-chen cee-chen transferred this issue from elastic/eui Mar 4, 2024
@cee-chen cee-chen added the Team:DataDiscovery Discover App Team (Document Explorer, Saved Search, Surrounding documents, Data, DataViews) label Mar 4, 2024
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-data-discovery (Team:DataDiscovery)

@davismcphee
Copy link
Contributor

@cee-chen I think this related to EuiDataGrid since I can reproduce the same behaviour in the EUI docs. But I also think it might just be the default browser copy behaviour, and not necessarily something EUI is doing.

When copying the first three rows of an example data grid like this:
data_grid_copy

I get this as the output:

Controls
Effertz, Lauretta V
Logan.Ebert@hotmail.com
West Mariamfurt, South Sudan
61401469
Thu Nov 09 2023 17:49:28 GMT-0400 (Atlantic Standard Time)
509.00
1-627-853-9626 x68380
5.9.3

- Press the Enter key to interact with this cell's contents.
Roob, Marlin Jr.
Nola_Rau43@hotmail.com
Evelynbury, Italy
72490165
Mon Mar 27 2023 09:30:01 GMT-0300 (Atlantic Daylight Time)
166.00
(701) 306-3240 x2462
6.3.8

- Press the Enter key to interact with this cell's contents.
Padberg, Rowena MD
Nadia_Schimmel20@hotmail.com
Andersonworth, Saint Kitts and Nevis
90531331
Sun Apr 23 2023 15:17:13 GMT-0300 (Atlantic Daylight Time)
94.00
496-847-9738 x805
7.9.0

@kertal
Copy link
Member

kertal commented Mar 5, 2024

@cee-chen I think this related to EuiDataGrid since I can reproduce the same behaviour in the EUI docs. But I also think it might just be the default browser copy behaviour, and not necessarily something EUI is doing.

I think it's related how EuiDataGrid is handling screen reader content, because I've been testing with a simple div based table, which keeps the rows in lines in the result

@juste-bob
Copy link
Author

In Discover you can select and export rows, in JSON format. if you would have a way to select rows and export them as text, row by row, would this cover your use case?

Hi @kertal, I don't want to perform export, in any format, beacause I need to do this copy/pasting very often. The export function is not usable in my case.

@juste-bob
Copy link
Author

I've made a bit of experimentation with different HTML tags and browsers.

Example 1

The table use <div> tags with display: inline-block style for columns:

<html>
    <head>
        <style>
            .row {
                display: block;
            }

            .column {
                display: inline-block;
            }
        </style>
    </head>
    <body>
        <h1>Table with <i>inline-block div</i></h1>
        <div class="row">
            <div class="column">first</div>
            <div class="column">second</div>
            <div class="column">third</div>
        </div>
        <div class="row">
            <div class="column">first</div>
            <div class="column">second</div>
            <div class="column">third</div>
        </div>
        <div class="row">
            <div class="column">first</div>
            <div class="column">second</div>
            <div class="column">third</div>
        </div>
    </body>
</html>

Copying the lines with Chromium (v122) gives:

first second third
first second third
first second third

This is the expected output with a space between each columns.

With Firefox (v123):

first
second
third
first
second
third
first
second
third

This is unexpected and firefox insert a new lines between each and every cells.

Example 2

The next example use the table, td and rd tags instead:

<html>
    <head>
    </head>
    <body>
        <h1>Table with <i>table</i></h1>
        <table>
            <tr>
                <td>first</td>
                <td>second</td>
                <td>third</td>
            </tr>
            <tr>
                <td>first</td>
                <td>second</td>
                <td>third</td>
            </tr>
            <tr>
                <td>first</td>
                <td>second</td>
                <td>third</td>
            </tr>
        </table>
    </body>
</html>

Result with Chromium (v122):

first	second	third
first	second	third
first	second	third

Kinda expected result but this time, columns are separated with tabs instead of space.

And with Firefox (v123):

first 	second 	third
first 	second 	third
first 	second 	third

This is the same result as Chromium.

Conclusion

Using the table, tr and td tags yields better and more "expected" results when copying a table.

Note that Discover "data table" used to use table in legacy mode. However, the new mode (using EUI I guess) is mostly built using <div>.

Strictly speacking, I think using <table> make more sense semantically to actually display... a table. Furthermore, it let the browser have a better handling of user action (like copying). As such, we get the same results in both Firefox and Chromium. So it might worth considering using them instead of div?

Note that there is also other elements like <span> and <p> added into a cell values which even if they are "hidden" (screen reader), can mess up the copy behavior.

Let me know what you think about it.

Regards.

@kertal
Copy link
Member

kertal commented Mar 8, 2024

thx for the feedback, I don't think we will go back to Table near term. Coming back to my question, it's not about export, or better .. it's about export to clipboard. so you would select the document of interest, and then export them to clipboard .. currently just JSON is supported, that's why I was asking, if we would have an Text option additional to JSON, if this would help your use case,

Image

@juste-bob
Copy link
Author

Thank you for your point. Indeed, JSON, CSV, etc. All these formats are the wrong ones and are not the point. I'm only looking to copy what's displayed on the screen, nothing more.

The problem with your proposal is that :

  1. you have to select the line, which makes it difficult to select what you want. Imagine with 5, 15, 300, 500 or more lines (and the fact that they don't have to be contiguous).
  2. I don't understand the need to add an action button to the user for a simple copy/paste. It adds yet another step and detracts from the user experience. However, it could be an option in the short term.

If we take 1000 lines. The exercise of going through the whole thing, selecting each line of interest by hand, then going back to the top to export to the clipboard is time-consuming.

Do you plan to return to Table at a later date?

@kertal
Copy link
Member

kertal commented Mar 14, 2024

Linking elastic/eui#6561

@kertal kertal self-assigned this Mar 14, 2024
@kertal
Copy link
Member

kertal commented Mar 18, 2024

@cee-chen Isn't one part of the described problem similar to what's described in this issue:
elastic/eui#6804
And the screen reader text will no longer be part of the copy paste text when this is merged?
elastic/eui#6806

@juste-bob
Copy link
Author

juste-bob commented Mar 27, 2024

Hi,

Is there any news / updates or conclusions on this issue?

@davismcphee
Copy link
Contributor

Unfortunately I don't believe there's anything we can do to address this on the Kibana side until the upstream issue in EUI is addressed. FWIW I think the best solution to this issue is probably this one since it would allow us to customize the copy behaviour when selecting multiple data grid cells: elastic/eui#6561.

@cee-chen
Copy link
Member

@cee-chen Isn't one part of the described problem similar to what's described in this issue:
elastic/eui#6804
And the screen reader text will no longer be part of the copy paste text when this is merged?
elastic/eui#6806

Newlines aren't related to screenreader text. It's likely an artifact of JSX rendering, and there's not a whole lot we can do there without running output HTML through a minifier (which then treads into dangerously set inner HTML territory).

I think Davis's suggestion of prioritizing elastic/eui#6561 instead makes total sense. I'm not sure it's something EUI has time roadmap-wise for though. Would the Discover team be open to contributing the feature to EUI?

@davismcphee
Copy link
Contributor

@cee-chen We'll have to discuss how this fits into our roadmap too, but we'll definitely keep this issue open to track on our end, and it might be something we can contribute if we can find some time for it.

@kertal kertal added impact:low Addressing this issue will have a low level of impact on the quality/strength of our product. EUI labels Apr 2, 2024
@elasticmachine
Copy link
Contributor

Pinging @elastic/eui-team (EUI)

@kertal kertal removed their assignment Apr 2, 2024
@kertal
Copy link
Member

kertal commented Apr 2, 2024

Related issue: #179731

@nickofthyme
Copy link
Contributor

One minor detail to add...The cell content across columns should be joined with the horizontal tab (U+0009) character not the space (U+0020) character. This enables pasting the content in tabular form such as pasting into Google sheets, Excel, Numbers, etc.

So something like this...

2016-10-05 7:48:23	samples	[evtx/powershell/600] provider 'WSMan' new state is 'Started', ps_host_name: 'ConsoleHost', cmd:''', cmd_path:'', script_name:'', host_application:''
2016-10-05 7:48:23	samples	[evtx/powershell/600] provider 'Function' new state is 'Started', ps_host_name: 'ConsoleHost', cmd:''', cmd_path:'', script_name:'', host_application:''
2016-10-05 7:48:23	samples	[evtx/powershell/600] provider 'Certificate' new state is 'Started', ps_host_name: 'ConsoleHost', cmd:''', cmd_path:'', script_name:'', host_application:''

Note: the spacing after the time looks larger than the one after samples but it's the same character, it just behaves strange after time in this example.

jughosta added a commit 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>
@davismcphee
Copy link
Contributor

@kertal Now that we have the "Copy as text" functionality that copies selected rows in the expected TSV format, do you think we should close this issue or keep it open for elastic/eui#6561?

@juste-bob
Copy link
Author

juste-bob commented Sep 2, 2024

@kertal Now that we have the "Copy as text" functionality that copies selected rows in the expected TSV format, do you think we should close this issue or keep it open for elastic/eui#6561?

Hello,

Thanks for the update on this issue, I appreciate that you keep working on it.

However, as mentioned previously in the thread above, the proposed solution is really inconvenient. Would you really click on these "small select buttons" 1000 times, click the selected button and then Copy selection as text, or:

  1. Click at the start
  2. Scroll to the desired end line
  3. Click at the end with Shift key held
  4. CTRL+C

That is only 4 actions vs 1000 + 2 actions. Not mentioning this could be done tens or thousands of time each day as an analyst.

Again, this is something that any tabular software supports natively (including the browser itself) and it used to work in Kibana as well. So to me, it really sounds like a regression bug. Is there any reason why this bug (feature?) isn't solved yet?

Regards.

@kertal
Copy link
Member

kertal commented Sep 2, 2024

However, as mentioned previously in the thread above, the proposed solution is really inconvenient. Would you really click on these "small select buttons" 1000 times, click the selected button and then Copy selection as text, or:

  1. Click at the start
  2. Scroll to the desired end line
  3. Click at the end with Shift key held
  4. CTRL+C

That is only 4 actions vs 1000 + 2 actions. Not mentioning this could be done tens or thousands of time each day as an analyst.

Again, this is something that any tabular software supports natively (including the browser itself) and it used to work in Kibana as well. So to me, it really sounds like a regression bug. Is there any reason why this bug (feature?) isn't solved yet?

You certainly don't need to click those select buttons 1000 times, we support selecting all

#184241

So it's still not 100% what you're are asking for, but we improved what's possible from Kibana side.

What you are asking for is blocked by a bug in Chromium, and the issue in EUI was closed since there seems to be no intention to resolve it:
elastic/eui#6804 (comment)
https://issues.chromium.org/issues/40692280

So I think we did what we could from Kibana side, I upvoted and subscribed to the Chromium bug, but I don't think we will iterate on this from Kibana side, so I'd say @davismcphee it's a not planned ... or an iceboxed feature, so it is still tracked when our requirements could provide a solution for this

@juste-bob
Copy link
Author

juste-bob commented Sep 3, 2024

Thanks @kertal for the quick response.

You certainly don't need to click those select buttons 1000 times, we support selecting all

#184241

So it's still not 100% what you're are asking for, but we improved what's possible from Kibana side.

The example shown in the previous post was a bit too simplistic, so I will rephrase it. Suppose there is 300 lines in discover and you only want to select the 100 lines in the middle of it. Or even worse, fifty lines here, two there, another thirty here, etc. How one would do to extract those without selecting them one by one? The "select all" feature doesn't seem to be of any help here.

Also, @cee-chen, could you explain why this issue has been moved from the EUI repository into Discover? This is clearly an issue in the EUI part, while Discover is only a consequence of it. As demonstrated by @davismcphee, the bug is reproducible in any EuiDataGrid component (including the sample). For instance, the issue is present in data tables that we use in homemade Dashboard. Furthermore, in those case there is neither "Copy selection as text" nor select button to export and we are stuck with manual selection. What are the possible solution in those cases?

Right now we are stuck with the "legacy mode" and cannot use any new features introduced since EUI. Column resizing or cell wrapping comes to my mind. In addition, we are a bit worry that this mode might be dropped in the future and cannot use Kibana anymore.

What you are asking for is blocked by a bug in Chromium, and the issue in EUI was closed since there seems to be no intention to resolve it: elastic/eui#6804 (comment) https://issues.chromium.org/issues/40692280

I think we are mixing two issues here. One is the "screen reader text" and the other one (this issue) are the new lines. From my understanding, what is blocked by a "bug in Chromium" is a possible workaround using a CSS trick with user-select: none (which questionably happens to be the first response in Stackoverflow). Correct me if I'm wrong, but why the screen reader should be rendered in the first place if we don't use any actually? Rendering it in all cases and then preventing it from being selected afterward just feels wrong to me. The purpose of user-select: none is to prevent a user from selecting something, not to accidently avoid selecting something that user cannot see anyway to prevent a copy in the clipboard! I would love to hear what's the rational behind it. I am not an front-end developer but there should be another/proper way to do this.

Back the new line bug, I really think this is a CSS/style issue. It is possible to have it working in both Firefox and Chromium as shown here. So this is an implementation bug. How does the other Webapp does that Kibana don't to make it possible to copy a table in a proper way? Another example here on Github:

a b
oifiufds fdsfdsfds
fdss dfsfsduihfds

Selecting them CTRL+C/CTRL+V produces this result in Notepad:

a 	b
oifiufds 	fdsfdsfds
fdss 	dfsfsduihfds

So, I really think this is not related to any Browsers bugs.

Looking forward to your returns.

@kertal
Copy link
Member

kertal commented Sep 10, 2024

@juste-bob I think your usecase, aiming to select 100 lines in the middle of 300 should be covered by implementing a shift-select solution? #192366

@cee-chen
Copy link
Member

cee-chen commented Sep 11, 2024

Back the new line bug, I really think this is a CSS/style issue. [...] So, I really think this is not related to any Browsers bugs.

It absolutely is a browser rendering issue. Your example works because the table content is simple strings/inline display content. Try this example instead with block display content:

a b

Test 1

Test 2

Test 3

Test 4

They paste with newlines and not on the same lines. Inspect the DOM; there's nothing going on here except vanilla table/td/h2-5 elements.

What's happening is that browsers determine newline behavior in clipboard based on the display property of elements. Inline displays do not cause newlines, block type displays do. Furthermore, display: flex (a very common CSS display method used for vertical alignment) adds a newline per child, which additionall varies per browser (Chrome in particular handles this buggily while Firefox does not):

So essentially: any EUI table or datagrid with any kind of complex rendered component (for example, EuiBadge or EuiHealth) will generate newlines (you can test this for yourself in our table docs examples which do use table/tr/td). There are workarounds we can look into for this (which will almost certainly require hooking into onCopy and manually regexing/cleaning up strings), but it's not going to be easy to accomplish or make flexible/bulletproof enough, and ultimately consumers can still render completely custom flex/block content into their cells which which could add newlines to the clipboard.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocked bug Fixes for quality problems that affect the customer experience EUI impact:low Addressing this issue will have a low level of impact on the quality/strength of our product. Team:DataDiscovery Discover App Team (Document Explorer, Saved Search, Surrounding documents, Data, DataViews) upstream
Projects
None yet
6 participants