Skip to content

Conversation

@RayJW
Copy link
Contributor

@RayJW RayJW commented Nov 20, 2025

This is my 2nd attempt at getting this done. I was able to remove autosizer and follow the new recommended way of doing things according to the react-window guide. At least in my testing on Linux, all the functionality is the same while getting rid of dependencies and finally adopting the newer API for react-window. As always, please test if these changes also work for you.

Some sizing logic had to be changed, but I made sure that it's still reactive while making things faster (from my limited comparison of render times). I left some comments for a few things I stumbled on while developing this, but if it's preferred, I can remove them before merging.

@RayJW RayJW requested a review from CyberTimon as a code owner November 20, 2025 12:21
@CyberTimon
Copy link
Owner

Thank you - it works correctly on my system.

Regarding the code quality: I noticed that several fallback paths were included to infer which new APIs react-window v2 might use. Now that the working APIs are known, those conditional branches can be removed. Please also remove the code comments.

Thanks again.

@RayJW
Copy link
Contributor Author

RayJW commented Nov 20, 2025

Now that the working APIs are known, those conditional branches can be removed.

Yes, sorry, that actually reminds me that there is still a regression when navigating back the scroll position isn't being restored. I'll have to fix that before being ready.

Shouldn't finish code like this late at night, sorry ^^'

@RayJW RayJW marked this pull request as draft November 20, 2025 18:02
@CyberTimon
Copy link
Owner

No problem. Thanks for the PR :)

@CyberTimon
Copy link
Owner

Hi @RayJW

While implementing #466, I also migrated to react-window v2. Everything is running fine now, except for two things:

  • In react-window v2, the scroll height is accessed like this: onScroll={(event) => setLibraryScrollTop(event.currentTarget.scrollTop)}. Without a debounce this causes noticeable lag.
  • I burned way too many hours today trying to restore the scroll position after leaving the editor. I have no idea why, but it genuinely feels impossible in v2 ._.

I’m sorry that I ended up doing most of the work and couldn’t merge yours. Re-basing after the #466 changes would have been a nightmare.

If you’re up for it, I would really appreciate help with restoring the scroll offset/position after leaving the editor.

Thank you!

@RayJW
Copy link
Contributor Author

RayJW commented Nov 24, 2025

  • I burned way too many hours today trying to restore the scroll position after leaving the editor. I have no idea why, but it genuinely feels impossible in v2 ._.

I feel you, this has been most of the work for me as well.

I’m sorry that I ended up doing most of the work and couldn’t merge yours.

Dw about it, I'm happy this can be moved along any way :)

If you’re up for it, I would really appreciate help with restoring the scroll offset/position after leaving the editor.

From what I gathered in the docs, the best possible way would be storing the number of the first row in view and then restoring that with the scrollToRow property and behaviour: "instant". But I couldn't quite make that work yet in an elegant way.

@CyberTimon
Copy link
Owner

Reverted back in the meantime as it really looks like a direct scrollTo is not possible with React-Window V2. Maybe we can take a look at this again in the future, or if you want, @RayJW, feel free to keep trying 💯

Commit: b1d5aee

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants