Skip to content
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

Use props to scroll #105

Closed
heyimalex opened this issue Dec 12, 2018 · 3 comments
Closed

Use props to scroll #105

heyimalex opened this issue Dec 12, 2018 · 3 comments

Comments

@heyimalex
Copy link

This is more of a question than an issue, but I figure you've thought about this more than me!

In my experience prop changes often coincide with forced scrolling. Think of a select dropdown; every time the selection moves we re-render to update the "selected" item and then scroll it into view. With the current api this leads to two calls to render where one would suffice, the first triggered by the parent's setState, the second from the scrollToItem call in the setState callback or cDU.

It is imperceptible in my experience. And react-window has been an absolute dream to work with. But...

Have you thought about adding scrolling via props? So I know you wrote the article specifically against using derived state, and I understand why mirroring doesn't work. But instead of having a prop that's literally scrollOffset, imagine something like scrollEffect that's actually an object that describes a scroll. The parent component internally updates its scrollEffect to { index: nextCursor, alignment: 'auto'} whenever something like a cursor change happens, and in the list's getDerivedStateFromProps you update the internal scrollOffset when the scrollEffect changes in an equality sense. It's a way to kind of bridge the declarative and imperative, for the parent to control scrolling when they want and not when they don't.

I've been using a wrapper component that uses this declarative scrollEffect api and at least for select-like components it's generally what I want. There are some other cool effects, like time traveling updates scroll position as you'd expect. Anyway, just a thought!

@bvaughn
Copy link
Owner

bvaughn commented Dec 13, 2018

It is imperceptible in my experience.

That's because React synchronously processes the cascading update (the one triggered from componentDidMount / componentDidUpdate) before yielding to the browser to paint, so you'll never see the in-between state.

And react-window has been an absolute dream to work with.

Thanks 😄

Have you thought about adding scrolling via props?

Yes. I added such an API to react-virtualized and regretted it for reasons I mentioned in the derived state article you also mentioned.

What you're describing would avoid some of the downsides listed in the anti-patterns section, but it would add some complexity to the list and grid components that I'm not sure is worth it.

@heyimalex
Copy link
Author

I'm with you; it's an unconventional api, and performance isn't really effected by duplicate renders in my case. Honestly I was trying to avoid flashes on cascading update, but since that isn't actually an issue I can just keep this api in a wrapping component. Thanks for all your work! :)

@bvaughn
Copy link
Owner

bvaughn commented Dec 14, 2018

You're welcome! Thanks for the discussion. 😊

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

No branches or pull requests

2 participants