-
Notifications
You must be signed in to change notification settings - Fork 8.1k
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
[Lens] Datatable: Do not apply truncation in value popover #132005
Conversation
Pinging @elastic/kibana-vis-editors @elastic/kibana-vis-editors-external (Team:VisEditors) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, @flash1293. Leaving one question below for your review.
@@ -2,7 +2,8 @@ | |||
height: 100%; | |||
} | |||
|
|||
.lnsTableCell { | |||
// only apply truncation inside of a table cell (not within popover) | |||
[data-test-subj='dataGridRowCell'] .lnsTableCell { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I feel like we shouldn't be using the data-test-subj
attribute as a selector for styles. Would it be possible to apply a new class instead?
Thanks for the review @MichaelMarcialis , fixed |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for making that change, @flash1293.
Forgive me, but I have one follow-up question. I was under the impression that EuiDataGrid
handled the truncation of data grid cell contents natively, and (to my knowledge) does not carry forward that truncation to the cell popovers. If that is true, why are we running with our own truncation implementation here rather than using the native data grid truncation?
@elasticmachine merge upstream |
@MichaelMarcialis AFAIK this is happening because we are breaking out of the default handling for the cell contents because a bunch of custom things are happening for the Lens table (alignment, coloring, ...): kibana/x-pack/plugins/lens/public/datatable_visualization/components/cell_value.tsx Line 83 in 3730dd0
|
@elasticmachine merge upstream |
💚 Build SucceededMetrics [docs]Async chunks
History
To update your PR or re-run it, just comment with: |
Fixes #131937
This PR makes sure the truncation mixin is only applied to the actual cell in the data grid, not the popover. This makes sure a value that's too large for the cell can be viewed in its entirety in the popover:
Before
![Screenshot 2022-05-11 at 10 44 34](https://user-images.githubusercontent.com/1508364/167808817-594d3aba-5ae2-404b-8a94-5d01c97f3def.png)
After
![Screenshot 2022-05-11 at 10 44 17](https://user-images.githubusercontent.com/1508364/167808822-8ac8fa11-659d-4c79-9762-911cd2d93a8f.png)