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

[Cloud Security] row height is too big in the Security Findings #170502

Closed
maxcold opened this issue Nov 3, 2023 · 4 comments · Fixed by #170503
Closed

[Cloud Security] row height is too big in the Security Findings #170502

maxcold opened this issue Nov 3, 2023 · 4 comments · Fixed by #170503
Assignees
@maxcold
Labels
8.11 candidate 8.12 candidate bug Fixes for quality problems that affect the customer experience csp: quick win cloud-security label: tagging issues which are relatively small in effort and lowered in priority Team:Cloud Security Cloud Security team related verified

Comments

@maxcold
Copy link
Contributor

maxcold commented Nov 3, 2023 •
edited
Loading

Kibana version:
8.11

Describe the bug:
Row height is set to 3 on the Findings data grid in Security due to the change made after BC4 #169724

Steps to reproduce:

  1. have Findings in the env
  2. navigate to Security > Findings
  3. row height is 3

Expected behavior:
Row height should be 1

Screenshots (if relevant):
Screenshot 2023-11-03 at 11 40 26

Errors in browser console (if relevant):

Provide logs and/or server output (if relevant):

Any additional context:
@maxcold maxcold added bug Fixes for quality problems that affect the customer experience Team:Cloud Security Cloud Security team related labels Nov 3, 2023
@maxcold maxcold self-assigned this Nov 3, 2023
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-cloud-security-posture (Team:Cloud Security)

@maxcold maxcold mentioned this issue Nov 3, 2023
Merged
@maxcold maxcold changed the title [Cloud Security] fix row height issue in the Security Findings [Cloud Security] row height is too big in the Security Findings Nov 3, 2023
@maxcold
Copy link
Contributor Author

maxcold commented Nov 3, 2023

As per @davismcphee comment in the email thread, the ROW_HEIGHT_OPTION is a Discover-specific advanced setting and we shouldn't rely on it. There is also an EUIDataGrid option to allow users to change the row height in the data grid itself, we probably should leverage this option. fyi @opauloh
For now, I'm just hardcoding the rowHeight value to 0 as it was the value used when the ROW_HEIGHT_OPTION option default was -1

maxcold added a commit that referenced this issue Nov 4, 2023
@maxcold @kibanamachine
[Cloud Security] set rowHeight to 0 to fix the bug of large row heigh…
5b72f77 
…t in Findings (#170503)

## Summary

fixes
- #170502

defaulting to row height 0 for the findings table. As the default was -1
before, we were defaulting to 0, but due to this change
https://github.com/elastic/kibana/pull/169724/files the default became
3, which broke our table. I guess the logic of taking the UI setting if
it's differnt from default -1 was to cater for users changing the row
height somewhere in the settings, but we need to bring to product/design
to see if we want to support it

---------

Co-authored-by: kibanamachine <[email protected]>
kibanamachine pushed a commit to kibanamachine/kibana that referenced this issue Nov 4, 2023
@maxcold
[Cloud Security] set rowHeight to 0 to fix the bug of large row heigh…
064d642 
…t in Findings (elastic#170503)

## Summary

fixes
- elastic#170502

defaulting to row height 0 for the findings table. As the default was -1
before, we were defaulting to 0, but due to this change
https://github.com/elastic/kibana/pull/169724/files the default became
3, which broke our table. I guess the logic of taking the UI setting if
it's differnt from default -1 was to cater for users changing the row
height somewhere in the settings, but we need to bring to product/design
to see if we want to support it

---------

Co-authored-by: kibanamachine <[email protected]>
(cherry picked from commit 5b72f77)
kibanamachine referenced this issue Nov 4, 2023
@kibanamachine @maxcold
[8.11] [Cloud Security] set rowHeight to 0 to fix the bug of large ro…
42cd5ad 
…w height in Findings (#170503) (#170587)

# Backport

This will backport the following commits from `main` to `8.11`:
- [[Cloud Security] set rowHeight to 0 to fix the bug of large row
height in Findings
(#170503)](#170503)

<!--- Backport version: 8.9.7 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Maxim
Kholod","email":"[email protected]"},"sourceCommit":{"committedDate":"2023-11-04T14:26:33Z","message":"[Cloud
Security] set rowHeight to 0 to fix the bug of large row height in
Findings (#170503)\n\n## Summary\r\n\r\nfixes\r\n-
https://github.com/elastic/kibana/issues/170502\r\n\r\ndefaulting to row
height 0 for the findings table. As the default was -1\r\nbefore, we
were defaulting to 0, but due to this
change\r\nhttps://github.com//pull/169724/files the
default became\r\n3, which broke our table. I guess the logic of taking
the UI setting if\r\nit's differnt from default -1 was to cater for
users changing the row\r\nheight somewhere in the settings, but we need
to bring to product/design\r\nto see if we want to support
it\r\n\r\n---------\r\n\r\nCo-authored-by: kibanamachine
<[email protected]>","sha":"5b72f77e078ecde5754161c8ba23f1ce63cfeb9f","branchLabelMapping":{"^v8.12.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","Team:Cloud
Security","backport:prev-minor","v8.12.0"],"number":170503,"url":"https://github.com/elastic/kibana/pull/170503","mergeCommit":{"message":"[Cloud
Security] set rowHeight to 0 to fix the bug of large row height in
Findings (#170503)\n\n## Summary\r\n\r\nfixes\r\n-
https://github.com/elastic/kibana/issues/170502\r\n\r\ndefaulting to row
height 0 for the findings table. As the default was -1\r\nbefore, we
were defaulting to 0, but due to this
change\r\nhttps://github.com//pull/169724/files the
default became\r\n3, which broke our table. I guess the logic of taking
the UI setting if\r\nit's differnt from default -1 was to cater for
users changing the row\r\nheight somewhere in the settings, but we need
to bring to product/design\r\nto see if we want to support
it\r\n\r\n---------\r\n\r\nCo-authored-by: kibanamachine
<[email protected]>","sha":"5b72f77e078ecde5754161c8ba23f1ce63cfeb9f"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v8.12.0","labelRegex":"^v8.12.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/170503","number":170503,"mergeCommit":{"message":"[Cloud
Security] set rowHeight to 0 to fix the bug of large row height in
Findings (#170503)\n\n## Summary\r\n\r\nfixes\r\n-
https://github.com/elastic/kibana/issues/170502\r\n\r\ndefaulting to row
height 0 for the findings table. As the default was -1\r\nbefore, we
were defaulting to 0, but due to this
change\r\nhttps://github.com//pull/169724/files the
default became\r\n3, which broke our table. I guess the logic of taking
the UI setting if\r\nit's differnt from default -1 was to cater for
users changing the row\r\nheight somewhere in the settings, but we need
to bring to product/design\r\nto see if we want to support
it\r\n\r\n---------\r\n\r\nCo-authored-by: kibanamachine
<[email protected]>","sha":"5b72f77e078ecde5754161c8ba23f1ce63cfeb9f"}}]}]
BACKPORT-->

Co-authored-by: Maxim Kholod <[email protected]>
@tehilashn
Copy link

tehilashn commented Nov 5, 2023 •
edited
Loading

@maxcold - was it backported?

Edit - I see it was - [8.11] [Cloud Security] set rowHeight to 0 to fix the bug of large ro…, thank you!

@kfirpeled kfirpeled linked a pull request Nov 5, 2023 that will close this issue
[Cloud Security] set rowHeight to 0 to fix the bug of large row height in Findings #170503
Merged
@kfirpeled kfirpeled added the csp: quick win cloud-security label: tagging issues which are relatively small in effort and lowered in priority label Nov 5, 2023
@Omolola-Akinleye
Copy link
Contributor

Row height back to normal
Image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@maxcold maxcold

Labels
8.11 candidate 8.12 candidate bug Fixes for quality problems that affect the customer experience csp: quick win cloud-security label: tagging issues which are relatively small in effort and lowered in priority Team:Cloud Security Cloud Security team related verified
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

[Cloud Security] set rowHeight to 0 to fix the bug of large row height in Findings elastic/kibana
5 participants
@maxcold @elasticmachine @Omolola-Akinleye @kfirpeled @tehilashn