-
Notifications
You must be signed in to change notification settings - Fork 861
[EuiDataGrid] Reduce layout shifts when auto-sizing overscan rows #6028
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
Conversation
06ac5bb to
016cf98
Compare
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_6028/ |
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_6028/ |
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_6028/ |
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_6028/ |
|
ℹ️ This is being split up into smaller PRs: |
|
I think what's remaining in this PR without the imperative ref and row height tests work is probably ok for a single PR! But if you find another natural way to conceptually split the remaining work, I'm all for more atomic PRs! |
|
I think I might submit the functionality with a simplified example first and maybe create a more complex example later. |
🗒️ Summary
This adds a mechanism to the
EuiDataGridto compensate for vertical layout shifts when rows above the currently visible ones change their height. This often happens when the row heights are"auto"-measured and new rows are rendered in the overscan area. By specifying ascrollAnchorRow: 'start', the grid will try to scroll to keep the vertical position of the topmost visible row stable in the viewport. This behavior is opt-in.This also exposes the
scrollTo()andscrollToItemmethods on the imperative API of the grid component.closes #6024
🎨 Previews
Scrolling without scroll anchoring - observe how the indices jump as the new rows get measured
eui-data-grid-without-scroll-anchoring-2022-07-06-20-00.mp4
Scrolling with scroll anchoring - observe how the indices stay consistent as the new rows get measured
eui-data-grid-with-scroll-anchoring-2022-07-06-19-59.mp4
🕵️♀️ Review notes
RowHeightUtilsreceive references to various dependencies. It now receives them asMutableRefObjects in its constructor without the need for several imperativeset*methods.RowHeightUtilswere not working as intended, because they used some jest features incorrectly. I tried to correct for the following problems before adding additional test cases for the newcompensateForLayoutShiftmethod:jest.useFakeTimerswas called in several places directly indescribeblocks, which means they're not bound to the lifecycle of the test cases. As a consequence some tests failed unpredictably depending on the execution order of thedescribeblocks. Some test cases also were not callingjest.runAllTimers, which caused severalsetTimeoutsnot to be executed and lead to several false positive test results.jest.spyOnwas also called indescribeblocks directly, but their implementation was never restored. That means that some spied-upon function implementations would be replaced by no-ops when the mocks were reset, which in turn lead to false positives and negatives.📋 Checklist
Updated the Figma library counterpart