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

[Logs Explorer] Update DataGrid default preferences #165718

Merged

Conversation

tonyghiani
Copy link
Contributor

📓 Summary

Closes #165482
Closes #165489

This PR apply new default preferences to the DataGrid for the Log Explorer:

  • Display and resize additional columns for service.name (240px) and host.name (320px) fields. The column's width is taken by the average length of those specific fields.
  • Display rows with single-line height by default.
data_grid

@tonyghiani tonyghiani added Team:Infra Monitoring UI - DEPRECATED DEPRECATED - Label for the Infra Monitoring UI team. Use Team:obs-ux-infra_services release_note:skip Skip the PR/issue when compiling release notes labels Sep 5, 2023
@tonyghiani tonyghiani requested review from a team as code owners September 5, 2023 16:15
@elasticmachine
Copy link
Contributor

Pinging @elastic/infra-monitoring-ui (Team:Infra Monitoring UI)

@apmmachine
Copy link
Contributor

🤖 GitHub comments

Expand to view the GitHub comments

Just comment with:

  • /oblt-deploy : Deploy a Kibana instance using the Observability test environments.
  • /oblt-deploy-serverless : Deploy a serverless Kibana instance using the Observability test environments.
  • run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

Copy link
Contributor

@davismcphee davismcphee left a comment

Choose a reason for hiding this comment

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

Code-only review, ROWS_HEIGHT_OPTIONS changes LGTM 👍

Copy link
Member

@weltenwort weltenwort left a comment

Choose a reason for hiding this comment

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

I like the approach to define a few standard column sizes. 👍 But I wonder if it's really justified to use two different defaults for service.name and host.name. Intuitively their different widths looked "wrong" to me. 🤷 Can we really make any assumptions about their usual length?

@tonyghiani
Copy link
Contributor Author

[Discover] Set data table row height to auto-fit by default #164218

I agree it looks kind of weird to have different sizes, but reserving a bigger space for both columns also seemed to leave less space for the message column.
I'll merge this to unblock #164218, but let's talk with @formgeist @isaclfreire to refine these sizes.

@formgeist
Copy link
Contributor

@tonyghiani Doesn't the data grid provide decent auto sizing of the columns which we can utilize? Do we need to reserve a fixed width for the values?

@tonyghiani
Copy link
Contributor Author

@tonyghiani Doesn't the data grid provide decent auto sizing of the columns which we can utilize? Do we need to reserve a fixed width for the values?

When adding new columns, if no configuration is provided for the specific Discover DataGrid component, the space is divided evenly for all the added columns.
Currently, I provided a configuration only for these new fields, so that the message column can get the rest of it.

@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
logExplorer 336 429 +93

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id before after diff
@kbn/unified-data-table 39 42 +3

Async chunks

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

id before after diff
discover 556.4KB 556.5KB +105.0B
logExplorer 220.9KB 221.1KB +262.0B
total +367.0B
Unknown metric groups

API count

id before after diff
@kbn/unified-data-table 88 92 +4

History

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

@tonyghiani tonyghiani merged commit d768657 into elastic:main Sep 12, 2023
@tonyghiani tonyghiani deleted the 165482-165489-update-log-explorer-defaults branch September 12, 2023 08:58
@kibanamachine kibanamachine added v8.11.0 backport:skip This commit does not require backporting labels Sep 12, 2023
@achyutjhunjhunwala achyutjhunjhunwala changed the title [Log Explorer] Update DataGrid default preferences [Logs Explorer] Update DataGrid default preferences Jan 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting release_note:skip Skip the PR/issue when compiling release notes Team:Infra Monitoring UI - DEPRECATED DEPRECATED - Label for the Infra Monitoring UI team. Use Team:obs-ux-infra_services v8.11.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Explorer] Use single line row height by default [Explorer] Add service and host name as default columns
8 participants