petyosi / react-virtuoso

The most powerful virtual list component for React
https://virtuoso.dev
MIT License
5.26k stars 301 forks source link

[BUG] restoreStateFrom doesn't restore the top position of the list correctly #957

Closed matthewwo closed 1 year ago

matthewwo commented 1 year ago

Describe the bug When the list is restoring with a previously stored snapshot, it doesn't seem to be aware of the previous top position of the list relative to the document, such that it always use the current top position of the list to restore, and not the previous scroll top of the list relative to the scroller.

I'm not sure if this is a bug in the library or if there's a mistake while using the customScrollParent prop and useWindowScroll, a clarification would be much appreciated!

Thanks a lot for this awesome library once again, I can't appreciate you more!

Reproduction CodeSandbox

To Reproduce Steps to reproduce the behavior:

  1. Scroll to the top of the window
  2. Click on the Log state button
  3. The page is scrolled to the beginning of the list while I believe it should remain at the same position.
  4. Scroll down a bit the page while the red div is still visible
  5. Click on the Log state button
  6. Again the page is scrolled to the beginning of the list, while it should remain the same position.
  7. Scroll down until the red div is not visible
  8. Click on the Log state button
  9. Observe that the red div flashes during the first render cycle, while I believe it should just scroll straight to the list without seeing the flash.

Expected behavior The red element should maintain its scroll position after the first time "Log state" is pressed, and also shouldn't appear in the first render cycle while the list is restoring the scroll position.

Screenshots

https://github.com/petyosi/react-virtuoso/assets/2998111/c5abdca5-604a-4e97-af64-6429eec367e2

Desktop (please complete the following information):

petyosi commented 1 year ago

There are two things here:

matthewwo commented 1 year ago

thanks for the feedback @petyosi! About the flash, I'm not sure if it's related to react 18 though, since the reproduction in codesandbox is already on react 18 but still produces the same issue?

petyosi commented 1 year ago

What I meant with the above is that the batch rendering in React 18 (doing some async stuff) causes it.

matthewwo commented 1 year ago

thanks for the clarification @petyosi about react 18 😆. however I could still reproduce the flash in react 17 in this CodeSandbox fork. In this case I'm not sure if it's caused by the new batch rendering process?

petyosi commented 1 year ago

That's because the code in react-virtuoso is adjusted to account for the delays for React 18, with no forks for React 17. It's hard (and far from great practice) to do React version based code logic :(.

matthewwo commented 1 year ago

@petyosi fyi I've found a workaround solution that seems to fix the incorrect scroll position issue, however it involves accessing the list element using the data-testid which isn't safe.

https://codesandbox.io/s/determined-tamas-vd5rqn?file=%2Fsrc%2FApp.tsx

It will be much appreciated if the fix is in virtuoso! Thanks a lot in advance again!

petyosi commented 1 year ago

@prankymat it's great that you've found a workaround. It's a bug that I will get to, but I'm hard-pressed on working for my income at the moment, so this gets a very low priority.

If you're using Virtuoso for your work, I would appreciate a sponsorship up to your capabilities. It helps me justifying spending more time on it.

github-actions[bot] commented 1 year ago

:tada: This issue has been resolved in version 4.4.3 :tada:

The release is available on:

Your semantic-release bot :package::rocket: