[RAC] Fix scrolling on Obs alerts table #109139
Conversation
Kerry350
added
bug
There was a problem hiding this comment.
LGTM. Security team does it differently though. Shall we be consistent between the solutions? cc @katrin-freihofner
| @@ -347,7 +347,7 @@ export function AlertsTableTGrid(props: AlertsTableTGridProps) { | |||
| end: rangeTo, | |||
| filters: [], | |||
| indexNames: [indexName], | |||
| itemsPerPage: 10, | |||
| itemsPerPage: 50, | |||
There was a problem hiding this comment.
It doesn't make much sense to me that we hard code itemsPerpage here but we also have this value inside the store. We could remove this prop and use itemsPerPageStore here:
There was a problem hiding this comment.
I've added 38be6e5 to address this, could you let me know if this looks okay? It meant adjusting the store defaults so that itemsPerPageStore produced 50. And maybe the TGridStandaloneComponent should've kept itemsPerPage as an optional prop, but this goes for the "expose when needed" route.
|
@Kerry350 Some types are failing for this PR |
💚 Build SucceededMetrics [docs]Async chunks
History
To update your PR or re-run it, just comment with: cc @Kerry350 |
|
I’m going to merge this as-is as it’s a 7.15 UX blocker. If #109139 (comment) isn’t suitable we can follow up. |
* Fix scrolling on obs alerts table and default to 50 items per page
* Fix scrolling on obs alerts table and default to 50 items per page
* Fix scrolling on obs alerts table and default to 50 items per page Co-authored-by: Kerry Gallagher <[email protected]>
* Fix scrolling on obs alerts table and default to 50 items per page Co-authored-by: Kerry Gallagher <[email protected]>
Summary
Fixes #108188.
Before:
After:
ℹ️ Notes
We do lose the fixed nature of the table header here. I have spoken to @katrin-freihofner about this and we decided to opt for this solution first. We could opt for a solution that goes for a "take up all of the remaining vertical space" approach (rather than a fixed 490px), but that would still have the double scrollbars problem. It would also be a more complex solution. We should see if this approach creates problems first.