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] Add support for copying the focused cell's value with keyboard shortcut only #6561

Open
Tracked by #182611
davismcphee opened this issue Jan 27, 2023 · 15 comments

Comments

@davismcphee
Copy link
Contributor

We're working on improving copy/paste support for the Discover data grid (elastic/kibana#149525), and we think there's an opportunity to improve general support for it in the EuiDataGrid.

Rather than the user having to manually highlight the entirety of the cell value to copy it, or expanding the cell and highlighting the popover value, both of which can be difficult (especially when there are cell actions or spaces in the text), it would be convenient to offer built-in support for copying the focused cell's value. I believe this should be possible by hooking into the browser's copy event: https://developer.mozilla.org/en-US/docs/Web/API/Element/copy_event.

Two notes about this request:

  • It should probably avoid hijacking the copy event if there's actual text highlighted (i.e. the user is trying to copy a partial cell value).
  • It would be preferable if EuiDataGrid also offered support for customizing this behaviour via an onCellCopy callback or similar.

Here's a 10-minute hack job I threw together to demonstrate the concept that can be pasted into the EuiDataGrid docs page or anywhere else containing an EuiDataGrid:

document.addEventListener('copy', (e) => {
  if (document.activeElement.classList.contains('euiDataGridRowCell') && !document.getSelection().toString()) {
    const text = document.activeElement.querySelector('[data-datagrid-cellcontent]').innerText;
    event.clipboardData.setData('text/plain', text);
    event.preventDefault();
    console.log(text);
  }
});
@cee-chen
Copy link
Contributor

Hi @davismcphee! We discussed this in our EUI sync today and being able to Ctrl/Cmd+C to copy a cell's value is definitely a cool concept we'd like to support and add to the keyboard shortcuts menu. I strongly agree with your proposal of an onCellCopy API vs us trying to detect cell content via scraping the DOM (although we can certainly attempt to fall back to innerText) - the consumer knows their own data much better than EUI does and is in a better position to customize that for copy purposes.

As a heads up however, it's currently a medium priority for us, unless you have a use case for multiple teams/products that would bump it to a high. This means we don't currently have an ETA for this - but if you're interested in open source contributing to EuiDataGrid, we'd definitely take a PR on this!

@davismcphee
Copy link
Contributor Author

@cee-chen Thanks for the update, and I'm glad to hear the EUI team agrees that it would be a useful addition! As of right now we don't have an urgent need for the functionality since we at least have a workaround implemented in Discover using a Copy value action, so not giving it a high a priority is totally fine for us. But I'll keep this in mind next time I have some time to work on an EUI contribution and may take it on myself if it's still open 🙂.

@jughosta

This comment was marked as off-topic.

@cee-chen
Copy link
Contributor

cee-chen commented May 23, 2023

@jughosta it's not clear to me from those threads whether or not those users are complaining about the ability to use a keyboard shortcut to copy the text, or if said data grid implementations are simply missing a clickable copy cell action entirely.

This issue is for the former (cmd/ctrl+C to copy) - the latter is up to your team to implement.

@jughosta

This comment was marked as off-topic.

@cee-chen

This comment was marked as off-topic.

@cee-chen cee-chen changed the title [EuiDataGrid] Add support for copying the focused cell's value [EuiDataGrid] Add support for copying the focused cell's value with keyboard shortcut only May 23, 2023
Copy link

👋 Hi there - this issue hasn't had any activity in 6 months. If the EUI team has not explicitly expressed that this is something on our roadmap, it's unlikely that we'll pick this issue up. We would sincerely appreciate a PR/community contribution if this is something that matters to you! If not, and there is no further activity on this issue for another 6 months (i.e. it's stale for over a year), the issue will be auto-closed.

@davismcphee
Copy link
Contributor Author

Just bumping this one to prevent staleness since it's still a valuable feature to have 🙂

@cee-chen
Copy link
Contributor

Totally agreed - any chance you'd be interested in contributing the feature to EUI? 😄

@davismcphee
Copy link
Contributor Author

I'm definitely interested! Just not sure yet if I'll be able to find time to work on it near term 😅

@cee-chen
Copy link
Contributor

cee-chen commented Apr 2, 2024

All good! I know that feeling hahaa 🫠

@JasonStoltz
Copy link
Member

@cee-chen We have this labeled as a "Large" effort. Do you still think this is a large amount of work?

@cee-chen
Copy link
Contributor

cee-chen commented Jun 4, 2024

I think anything involving EuiDataGrid is a large effort 😅 But yes, since I think the API on this one will be fairly complex since it will involve adding a new prop allowing consumers to pass us copyable content and falling back to inner text if not.

@github-actions github-actions bot added the Stale label Oct 4, 2024
@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Oct 11, 2024
@cee-chen
Copy link
Contributor

@davismcphee Our stalebot went rogue and closed this unintentionally - but just curious, would you consider this feature request moot since copying cells directly now works as of #8019? Can we go ahead and leave it closed?

@davismcphee
Copy link
Contributor Author

@cee-chen Thanks for the ping. While #8019 is a great improvement, personally I think this is still a valuable feature for easily copying entire cell values, vs having to manually highlight the text to copy. It seems especially useful for cells with truncated content where it's trickier to select the entire value, and for keyboard users. It's also a common pattern in other data grids I've used, and we've encountered users in Discover with similar expectations.

@cee-chen cee-chen reopened this Oct 17, 2024
@cee-chen cee-chen removed the Stale label Oct 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants