[EuiDataGrid] Account for horizontal scrollbar when determining height #5478
Conversation
|
It looks like part of #5475 was left in this PR, you may want to rebase against main and force-push |
12e8c3f to
3230f05
Compare
I tried to be all fancy with branches and failed. But having the hot reload helped testing a lot! :) Resolved && force-pushed |
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_5478/ |
|
Tested on Chrome, Safari, Edge, and Firefox (finally figured out what was going on with my local Firefox instance - I have We should def test #4848 and #5131 on Kibana after release/upgrade - I tested fullscreen mode as well and can confirm the horizontal scrollbar works there (although it looks like it's also already working on prod, so maybe I'm not reading the issue correctly?) |
| @@ -50,6 +50,7 @@ import { | |||
| } from '../data_grid_types'; | |||
| import { makeRowManager } from './data_grid_row_manager'; | |||
| import { useForceRender } from '../../../services/hooks/useForceRender'; | |||
| import { useUpdateEffect } from '../../../services'; | |||
There was a problem hiding this comment.
Yay, glad this is already coming in handy! 🎉
| // https://stackoverflow.com/a/5038256 | ||
| const hasHorizontalScroll = | ||
| (outerGridRef.current?.scrollWidth ?? 0) > | ||
| (outerGridRef.current?.clientWidth ?? 0); | ||
| // https://stackoverflow.com/a/24797425 |
There was a problem hiding this comment.
Haha, I appreciate the extra context / honesty of adding stack overflow links 😆
| const outerGridRef = useRef<HTMLDivElement | null>(null); // container that becomes scrollable | ||
| const innerGridRef = useRef<HTMLDivElement | null>(null); // container sized to fit all content |
There was a problem hiding this comment.
Also appreciate this extra comment context + reorganization
I agree, that does appear to be already fixed. |
|
jenkins test this |
Co-authored-by: Constance <constancecchen@users.noreply.github.com>
Co-authored-by: Constance <constancecchen@users.noreply.github.com>
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_5478/ |
Summary
When computing the unconstrained height, now determine if a horizontal scroll is present & find its height, adding that to the datagrid contents' height. This helps address (and may close) some open issues like #4848 , but there's been a lot of movement and re-validating these issues will be necessary before closing them.
To test: in the primary data grid example, resize any of the columns so you get a horizontal scrollbar. Previously, the vertical scrollbar would be present to scroll the extra 13 pixels. Now, all the cells + horizontal scroll are rendered and no vertical scrolling is needed.
Checklist
- [ ] Check against all themes for compatibility in both light and dark modes- [ ] Props have proper autodocs and playground toggles- [ ] Added documentation- [ ] Checked Code Sandbox works for any docs examples