[datagrid] Fix broken scroll #19178
[datagrid] Fix broken scroll #19178
Conversation
romgrk
added
scope: data grid
|
Deploy preview: https://deploy-preview-19178--material-ui-x.netlify.app/ Bundle size report
|
|
Some tests failed unfortunately |
| expect(vi.getTimerCount()).to.equal(3); | ||
| expect(vi.getTimerCount()).to.equal(2); | ||
|
|
||
| await act(async () => { | ||
| await vi.advanceTimersByTimeAsync(100); | ||
| }); | ||
| expect(vi.getTimerCount()).to.equal(1); | ||
| expect(vi.getTimerCount()).to.equal(0); |
There was a problem hiding this comment.
There was a problem hiding this comment.
I must say I don't understand how useStoreEffect affected this, it doesn't use any timer directly.
|
The tests are passing, except one argos screenshot but it's the flaky density selector test, so I think this is ready. |
|
Nevermind, still seeing some weird behavior that doesn't happen on my playground page -_- |
|
Now it seems to be working properly everywhere, ready for review. |
| const refSetter = (name: keyof typeof refs) => (node: HTMLDivElement | null) => { | ||
| if (node && refs[name].current !== node) { | ||
| refs[name].current = node; | ||
| setRefTick((tick) => tick + 1); |
There was a problem hiding this comment.
OK, so we need to enforce rerender for ref nodes to be propagated after they're set, and this is why some tests were failing?
There was a problem hiding this comment.
That's specifically for the observeRootNode effect that needs to be reactive on refs.container.current change, but there might be more cases where it's needed. I might want to refactor this away in the future, along with the API refactor.
Follow-up of #19156 (comment)
Remove the state update inside a store effect.