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.

Sign up for GitHub

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

Jump to bottom

[Lens] make the datatable content selectable #106982

Merged
merged 1 commit into from
Jul 30, 2021
Merged

[Lens] make the datatable content selectable #106982

merged 1 commit into from
Jul 30, 2021

Conversation

mbondyra
Copy link
Contributor

Summary

Fixes #106436

@mbondyra mbondyra requested a review from a team as a code owner July 28, 2021 11:09
@mbondyra mbondyra added Feature:Lens release_note:skip Skip the PR/issue when compiling release notes Team:Visualizations Visualization editors, elastic-charts and infrastructure v7.15.0 v8.0.0 labels Jul 28, 2021
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-app (Team:KibanaApp)

@mbondyra mbondyra requested review from dej611 and stratoula July 28, 2021 11:10
dej611
dej611 approved these changes Jul 28, 2021
View reviewed changes
Copy link
Contributor

@dej611 dej611 left a comment

Choose a reason for hiding this comment

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

The fix works. Tested on Safari.

Minor nitpick would be to enable it only for metric and table maybe, or more in general disable for charts with legends (as they are svg elements):

Screenshot 2021-07-28 at 14 12 01

Screenshot 2021-07-28 at 14 11 13

It's ok for metrics I guess:

Screenshot 2021-07-28 at 14 12 12

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

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
lens 1.6MB 1.6MB +168.0B

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

@mbondyra
Copy link
Contributor Author

Hmmm, now I tested it and I am not sure why we would like to disable selection on legend? It works pretty ok. Maybe I am missing something?

Jul-29-2021.08-06-55.mp4

stratoula
stratoula approved these changes Jul 29, 2021
View reviewed changes
Copy link
Contributor

@stratoula stratoula left a comment

Choose a reason for hiding this comment

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

@mbondyra I just tested it and I agree. It looks fine to me :) LGTM!

@dej611
Copy link
Contributor

dej611 commented Jul 29, 2021

Hmmm, now I tested it and I am not sure why we would like to disable selection on legend? It works pretty ok. Maybe I am missing something?

Jul-29-2021.08-06-55.mp4

Just an idea, no strong opinion on that

@mbondyra
Copy link
Contributor Author

I am gonna merge this one because it's not really about styles, but functionality, so I think it's fine to do it without kibana-design team input.

@mbondyra mbondyra merged commit 6ef6574 into elastic:master Jul 30, 2021
@mbondyra mbondyra deleted the lens/select-table-content branch July 30, 2021 09:39
@mbondyra mbondyra mentioned this pull request Jul 30, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@stratoula stratoula stratoula approved these changes

@dej611 dej611 dej611 approved these changes

Assignees
No one assigned
Labels
Feature:Lens release_note:skip Skip the PR/issue when compiling release notes Team:Visualizations Visualization editors, elastic-charts and infrastructure v7.15.0 v8.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Lens] make data table selectable
5 participants
@mbondyra @elasticmachine @kibanamachine @dej611 @stratoula