-
Notifications
You must be signed in to change notification settings - Fork 1.1k
fix: do not use animated scroll on initial render #1009
base: main
Are you sure you want to change the base?
fix: do not use animated scroll on initial render #1009
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead, I'd expect that on the initial render the screen already looks good (and the current tab is already positioned to the center), and only after that, for example when a route is changed, the view scrolls to the new current tab.
This PR makes it work in such a way.
How can it work such a way? To have the correct scroll position on initial render, it would need to set something like contentOffset
which is currently iOS only, there's a commit on master adding Android support but not in any release version.
And for auto width tabs, first we need to measure the tabs to know how much to scroll. Since measurements are not sync, it's impossible to ensure correct value on initial render.
When contentOffset
support gets released for Android, it'll be possible to have correct position on initial render for non-auto-width tabs. But I don't think it's possible for initial render to already look as expected currently. If it did, then the animation won't make a difference anyway since it was already at the right place.
Removing animation would render the tab bar in wrong position initially, and then jump to correct position after calculations which is worse than animating to the correct position.
In our case, we set width of each tab manually, and so it just works. (Because then both the screen width and tabs width are known prior to the component render.)
I guess it depends on your taste what's "worse"... We prefer having an instant render on a proper position than a needless animation that makes the user focus their attention on the tabs bar, when it's not really needed. Nevertheless. If I made it configurable via a prop, would you merge this PR then? |
It's not an instant render though, it's incorrect position initially and then jarring shifting to correct position, as opposed to a smooth animation.
I think a prop is unnecessary, when we can handle it properly without needing an animation, we should handle it automatically: basically if width is already known (like you have), and there's support for So we should be able to set a |
As per facebook/react-native@ed29ba1 this should now land in RN 0.64 which is in RC and will be released soon. Just a heads up, hopefully you can resume work on this PR as the functionality is sorely needed. |
Is it the reason that I can set any other tabs active on my initial render? Even after setting the index state to 1. @jtomaszewski Here is the issue that I am facing btw - #1168 |
ed348ad
to
703f148
Compare
Currently the component scrolls to the current tab with an animation, whenever:
Which is good, but it causes the component to trigger a scroll animation on its' initial render, which might be undesired especially when you have multiple tabs, enabled scroll, and begin the view with not-the-first tab as the initial screen. The screen appears, the tab is highlighted as current... and the scroll animation starts.
Instead, I'd expect that on the initial render the screen already looks good (and the current tab is already positioned to the center), and only after that, for example when a route is changed, the view scrolls to the new current tab.
This PR makes it work in such a way.