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

[8.16] Backport rowHeightOption: auto EuiDataGrid fix #211601

Closed

Conversation

weronikaolejniczak
Copy link
Contributor

@weronikaolejniczak weronikaolejniczak commented Feb 18, 2025

Summary

Fix for elastic/eui#8245

Backporting af189da to 8.16 Kibana version. It was introduced on elastic/eui#8251.

The npm backport release: https://www.npmjs.com/package/@elastic/eui/v/97.0.0-backport.3

Checklist

The issue being backported, elastic/eui#8245, is intermittent. You cannot consistently reproduce it on OSX, apparently it's reproducible on Windows. I'd appreciate a manual test from the reviewers 🙏🏻

The steps to reproduce in 8.16 are:

  1. Go to Security > Alerts.
  2. In the data grid, set the view from "Grid view" to "Event rendered view".
  3. Choose the settings icon, in the picker choose "Auto fit" in Row height.

The data grid's height changes to 0. It's reproducible on first load.

@weronikaolejniczak weronikaolejniczak changed the title [8.16] Backport EuiDataGrid fix [8.16] Backport rowHeightOption: auto EuiDataGrid fix Feb 18, 2025
@weronikaolejniczak weronikaolejniczak requested review from kqualters-elastic, a team and logeekal and removed request for kibanamachine and a team February 19, 2025 13:19
Copy link
Contributor

@kqualters-elastic kqualters-elastic left a comment

Choose a reason for hiding this comment

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

we might have another issue with the auto row height, blocking this until we talk through it

Copy link
Contributor

@logeekal logeekal left a comment

Choose a reason for hiding this comment

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

Testing it in following browser. Looks good from my side but I will wait to see what @kqualters-elastic has encountered

  • Firefox
  • Chrome
  • Edge ( Chromium Based )
  • Brave ( Chromium Based )
  • Safari

@weronikaolejniczak
Copy link
Contributor Author

@kqualters-elastic I will update the PR with the suggestion from @jbudz just in case we proceed with the release.

Copy link
Contributor

@logeekal logeekal left a comment

Choose a reason for hiding this comment

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

I think I know what Kevin might have encountered. But let's wait for more details. As I mentioned in 8.17 backport, I am seeing the same error in this backport as well.

The error surfaces when the height of the complete table and row are both auto.
In security Solution, whenever the number of items in Alert table is below 20, we set the table height as auto which results in this wierd error.

Screen.Recording.2025-02-20.at.11.43.14.mov

@elasticmachine
Copy link
Contributor

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

  • [job] [logs] Jest Tests #7 / useExpandedCard Hook when the card is expanded manually when scroll is enabled should scroll

Metrics [docs]

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
kbnUiSharedDeps-npmDll 5.8MB 5.8MB +12.0B

@weronikaolejniczak
Copy link
Contributor Author

Closing the backport PRs due to the above-mentioned issues. Going back to the drawing board with this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants