-
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] Supports multi rows headers for the table visualization #127447
Conversation
Pinging @elastic/kibana-vis-editors @elastic/kibana-vis-editors-external (Team:VisEditors) |
@MichaelMarcialis as you will see in the description, I added another set of settings for the datatable that concern the header rows. The |
FWIW I think it's an important case to have multi-line header without having multi line cells. There are quite some cases with long column names but small numeric values in the cells. |
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.
It seems like specific line count is not respected if there is no header with that height which is different than the data row height behavior:
I see how this would be hard to fix, I don't think it's a blocker though, just something to point out.
LGTM otherwise, but I agree we can improve the look of the UI
x-pack/plugins/lens/public/datatable_visualization/components/columns.tsx
Outdated
Show resolved
Hide resolved
@flash1293 yes I agree on having separate settings. I just don't like the UI and I wonder if we can improve it a bit. To be more clear where each 'lines per row' refer to. About your other comment on the header height not respected if there is no header with that height, I was thinking that it has the same behavior with the legend lines. Maybe 20 lines max for the header is a big number though. We could change it in 5 max, does it sound better? |
Hey, @stratoula! Good question. My first instinct would be to make the following changes:
Rough idea: Let me know if that makes sense. Thanks! |
I love it! |
@elasticmachine merge upstream |
To wrap up:
|
@elasticmachine merge upstream |
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.
x-pack/plugins/lens/public/datatable_visualization/components/toolbar.tsx
Outdated
Show resolved
Hide resolved
@elasticmachine merge upstream |
💚 Build SucceededMetrics [docs]Module Count
Public APIs missing comments
Async chunks
Page load bundle
History
To update your PR or re-run it, just comment with: |
Summary
Closes #122965
It adds an extra setting for enabling multi rows for the table header.
The user can select among:
It defaults to the single setting.
Checklist
Delete any items that are not applicable to this PR.