[DataGrid] Fix scroll jump with dynamic row height #16763
Conversation
cherniavskii
added
bug 🐛
|
Deploy preview: https://deploy-preview-16763--material-ui-x.netlify.app/ |
|
I still see a small movement when you stop scrolling and then hover over another row Watch row 5 Screen.Recording.2025-02-28.at.15.14.01.mov |
|
@arminmeh Seems like an unrelated issue – I can reproduce it without dynamic row height: Screen.Recording.2025-02-28.at.17.25.02.mov(rows 17-20 move a bit). |
Found the reason After the scrolling and the timeout, render context changes (adds buffer) and the rows are re-rendered |
| }); | ||
|
|
||
| await page.mouse.wheel(0, 150); | ||
| await page.waitForTimeout(500); |
There was a problem hiding this comment.
Can we use waitForSelector here?
In the description of waitForTimeout it says
Note that page.waitForTimeout() should only be used for debugging. Tests using the timer in production are going to be flaky. Use signals such as network events, selectors becoming visible and others instead.
There was a problem hiding this comment.
Without a timeout, the assertion will happen before the potential scroll jump, and the test will pass without the fix applied.
I can't see waitForSelector as a replacement here, but I'm open for suggestions.
There was a problem hiding this comment.
I didn't test it out, but what I had in mind is to wait for the last row to become attached or first row detached because of the virtualization and assert then
could that work?
There was a problem hiding this comment.
the last row to become attached or first row detached
Both happen before the scroll jump, so the test will pass without the fix applied.
Perhaps it's possible to make an assertion that will wait for the first row to appear again after scroll with a timeout, and throw if it does appear. But it's not that different from the current solution.
WDYT?
There was a problem hiding this comment.
yes, that would also just spend time waiting for nothing to happen 🙂
in that case, I am fine leave it as is for now
|
Cherry-pick PRs will be created targeting branches: v7.x |
Fixes #14726