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

Add back support for lazy tabs and use removeClippedSubviews #3538

Merged
merged 5 commits into from Feb 17, 2018

Conversation

@brentvatne
Copy link
Member

brentvatne commented Feb 17, 2018

Motivation

Support for the lazy prop was removed from react-native-tab-view. This has been a significant pain point for users (#2488). This PR implements it within react-navigation itself. Additionally, it handles applying removeClippedSubviews and re-positioning the children of the tab such that they will be clipped when the tab isn't active. This helps with memory usage, and is particularly helpful on Android.

Test plan

removeClippedSubviews is a bit flakey on iOS, but I found that it worked reliably if it was only applied when the screen is not visible. In the event that users run into problems with this, they can toggle it off with removeClippedSubviews: false in the tab navigator config. It should work well on Android.

@codecov-io

This comment has been minimized.

Copy link

codecov-io commented Feb 17, 2018

Codecov Report

Merging #3538 into master will decrease coverage by 0.03%.
The diff coverage is 68%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3538      +/-   ##
==========================================
- Coverage   70.51%   70.47%   -0.04%     
==========================================
  Files          52       53       +1     
  Lines        1665     1690      +25     
==========================================
+ Hits         1174     1191      +17     
- Misses        491      499       +8
Impacted Files Coverage Δ
src/navigators/TabNavigator.js 100% <ø> (ø) ⬆️
src/views/TabView/TabView.js 85.36% <ø> (ø) ⬆️
src/views/ResourceSavingSceneView.js 68% <68%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7f4706e...3173782. Read the comment docs.

@brentvatne brentvatne merged commit 3600ecb into master Feb 17, 2018
1 check passed
1 check passed
ci/circleci Your tests passed on CircleCI!
Details
@satya164

This comment has been minimized.

Copy link
Member

satya164 commented Feb 17, 2018

Does it work with sliding tabs at all? The main reason it was removed because trying to do this resulted in buggy behavior which I didn't have enough time to research on and fix.

In addition, rendering a new screen mood transition affected the indicator animation and makes it laggy. When switching to a tab with animation will show uninitialized screens in between, which is another undesirable effect.

For the best user experience, users could show a placeholder (like there FB app loading placeholder) and then update the content after switching the tab, which will ensure no frames are dropped and the experience stays smooth.

@satya164

This comment has been minimized.

Copy link
Member

satya164 commented Feb 17, 2018

Also not sure how the resource saving container works with animated tabs, but will you be willing to contribute it to react-native-tab-view? I think it's a useful feature to have.

Also there were issues with removeClippedSubviews on iOS with listviews. Are they fixed now? I always had to disable it in my tabs on iOS. Otherwise the content of scroll view in the hidden tab will never render.

@TheTekton

This comment has been minimized.

Copy link
Contributor

TheTekton commented Feb 17, 2018

When using removeClippedSubviews, the FlatList, SectionList, and VirtualizedList components have a recordInteraction method that can be used in componentDidMount (see issue 1831). Works well for the initial render, just not ideal.

What kind of performance trade-offs are there with using something like this versus just setting the render to null for the tab scene?

@brentvatne brentvatne deleted the @brent/lazy-tabs-experiment branch Feb 22, 2018
leethree added a commit to leethree/flow-typed that referenced this pull request Mar 1, 2018
- `back` action doesn't require `payload`. (similar to flow-typed#1880)
- `SafeAreaView` doesn't require `children`.
-  Add `lazy` and `removeClippedSubviews` for TabNavigator. (see react-navigation/react-navigation#3538)
AndrewSouthpaw added a commit to flow-typed/flow-typed that referenced this pull request Mar 1, 2018
- `back` action doesn't require `payload`. (similar to #1880)
- `SafeAreaView` doesn't require `children`.
-  Add `lazy` and `removeClippedSubviews` for TabNavigator. (see react-navigation/react-navigation#3538)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.