[Discover] Show "Copy value" button for any grid cell

[jughosta]

Closes #108857

Summary

Before we had "Copy to clipboard" only for cell popovers with JSON content. This PR adds it also for any other cell values.

Checklist

• Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
• This renders correctly on smaller devices using a responsive layout. (You can test this in your browser)
• This was checked for cross-browser compatibility

[elasticmachine]

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

[davismcphee]

Code review, and pulled & tested locally. Works as expected, and something I've wanted in Discover since I started at Elastic!

Two notes that don't need to be addressed in this PR, but I think are worth considering:

• While this is great to have as is, personally I would find it more useful as an in-cell action for quicker access (maybe too many buttons though), and/or implementing a Ctrl/Cmd + C shortcut to copy the focused cell value.
• Keyword values are copied exactly as shown in the table, but strings, numeric values, and possibly other types become quoted (e.g. 1,902.22 is copied as "1,902.22"). This seems to be expected behaviour based on convertValueToString, but as a user I found it unexpected and a bit annoying.

cc @kertal and @ryankeairns

[kertal]

Code review, and pulled & tested locally. Works as expected, and something I've wanted in Discover since I started at Elastic!

Two notes that don't need to be addressed in this PR, but I think are worth considering:

• While this is great to have as is, personally I would find it more useful as an in-cell action for quicker access (maybe too many buttons though), and/or implementing a Ctrl/Cmd + C shortcut to copy the focused cell value.

I guess in-cell action would mean 3 tiny icons in a row, I think this was possible a while ago, and then it was limited for the "To many buttons" reason. The Ctrl/Cmd + C option sound like a good idea, but should be enabled on EUI level, so this sounds like a good EUI enhancement issue 👍

• Keyword values are copied exactly as shown in the table, but strings, numeric values, and possibly other types become quoted (e.g. 1,902.22 is copied as "1,902.22"). This seems to be expected behaviour based on convertValueToString, but as a user I found it unexpected and a bit annoying.

Ok, I think we're using the output of the field formatters here for this case, and yes, you know when I'd copy paste 1,902.22 into German Excel, this number would high likely turn into 1,9 ... So thinking of copying the raw value for numeric fields sound like a good enhancement issue on our side

Kudos @jughosta to implement this so quickly!

[kertal]

LGTM, just approving since the @elastic/kibana-data-discovery team was still assigned to review although @davismcphee approved, also tested and it works as expected. One note, it's not also possible to copy the whole document, and since there are no filter buttons in this column this can be done directly without expanding:

[jughosta]

@davismcphee @kertal Before merging the PR I can look into skipping quotes when copied via the cell popover but keep it when a whole column is copied.

[jughosta]

@davismcphee @kertal Pushed a commit which prevents quotes when copying from a cell popover.

This made me thinking maybe we should have 2 options: "Copy" and "Copy value" (original, non-formatted value which can be useful for number, dates, etc).

[davismcphee]

Pulled and tested again, and now it's copying as expected! 🎉 Thanks for this, it's great functionality to have.

I guess in-cell action would mean 3 tiny icons in a row, I think this was possible a while ago, and then it was limited for the "To many buttons" reason. The Ctrl/Cmd + C option sound like a good idea, but should be enabled on EUI level, so this sounds like a good EUI enhancement issue 👍

@kertal Good call, I created an EUI issue to request it here: elastic/eui#6561.

This made me thinking maybe we should have 2 options: "Copy" and "Copy value" (original, non-formatted value which can be useful for number, dates, etc).

@jughosta This is also a reasonable possibility IMO. I think as far as this PR goes, it makes sense to have the initial implementation copy exactly what's seen in the grid (as it now does), since I think that's what most users will be expecting, but it may be worth noting this to have a discussion and potentially opening another issue for the other use case. Power users especially might find the 'copy unformatted value' functionality useful.

[jughosta]

@davismcphee Sounds good! Would you suggest to keep "Copy value" label or change to something like "Copy" in preparation for extending the functionality in the future?

[kibana-ci]

💚 Build Succeeded

• Buildkite Build
• Commit: 52917c1

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
discover	375.6KB	375.7KB	+41.0B

History

• 💚 Build #102980 succeeded 87a7bd4
• 💚 Build #102758 succeeded 2eb0b96
• 💚 Build #102710 succeeded 6f2b449

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

cc @jughosta

[davismcphee]

@davismcphee Sounds good! Would you suggest to keep "Copy value" label or change to something like "Copy" in preparation for extending the functionality in the future?

@jughosta Personally I prefer copy value for the functionality we have now, and something like copy raw value or copy unformatted value for copying the underlying unformatted value, but others may feel differently about this.