-
Notifications
You must be signed in to change notification settings - Fork 224
Lazy initialized MaterialTopTabNavigator routes #9
Lazy initialized MaterialTopTabNavigator routes #9
Conversation
src/utils/createTabNavigator.js
Outdated
@@ -24,8 +24,16 @@ export type InjectedProps = { | |||
screenProps?: any, | |||
}; | |||
|
|||
type State = { | |||
isSwiping: boolean, |
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
src/utils/createTabNavigator.js
Outdated
@@ -119,6 +135,9 @@ export default function createTabNavigator(TabView: React.ComponentType<*>) { | |||
navigation={navigation} | |||
descriptors={descriptors} | |||
screenProps={screenProps} | |||
onSwipeStart={this._handleSwipeStart} |
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
@@ -17,16 +18,35 @@ type Props = InjectedProps & { | |||
tabBarPosition?: 'top' | 'bottom', | |||
tabBarComponent?: React.ComponentType<*>, | |||
tabBarOptions?: TabBarOptions, | |||
lazyOnSwipe: boolean, |
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
@@ -17,16 +18,35 @@ type Props = InjectedProps & { | |||
tabBarPosition?: 'top' | 'bottom', | |||
tabBarComponent?: React.ComponentType<*>, | |||
tabBarOptions?: TabBarOptions, | |||
lazyOnSwipe: boolean, | |||
sceneAlwaysVisible: boolean, |
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
@@ -17,16 +18,35 @@ type Props = InjectedProps & { | |||
tabBarPosition?: 'top' | 'bottom', | |||
tabBarComponent?: React.ComponentType<*>, | |||
tabBarOptions?: TabBarOptions, | |||
lazyOnSwipe: boolean, | |||
sceneAlwaysVisible: boolean, |
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
@@ -113,18 +133,109 @@ class TabView extends React.PureComponent<Props> { | |||
|
|||
_renderPanPager = props => <TabViewPagerPan {...props} />; | |||
|
|||
_renderScene = ({ route, focused }) => { | |||
const { renderScene, animationEnabled, swipeEnabled } = this.props; | |||
componentWillMount() { |
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
} | ||
} | ||
|
||
componentWillReceiveProps(nextProps) { |
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
} | ||
} | ||
|
||
_onAction = payload => { |
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
transitioningFromIndex: | ||
(payload.lastState && payload.lastState.index) || 0, | ||
}); | ||
} else if (payload.action.type === 'Navigation/COMPLETE_TRANSITION') { |
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
bf378d3
to
def63d6
Compare
@satya164 I have done your requested changes and upgraded it to match with react-navigation@2.0.1. |
I'll check this over the weekend. |
I've done some non-scientific benchmarks in a application that have many — probably too much — tabs, and we have noticed a significant improvement with the memory. Check these videos below: createMaterialTopTabNavigator from this PR.createMaterialTopTabNavigator from react-navigation@2.0.4createTabNavigator (deprecated) from react-navigation@2.0.4 |
I'm sorry I haven't been able to test this. Will definitely do tomorrow. Also it'll be great to have this feature directly in |
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.
After trying this on device, it doesn't work very well with swipes as I suspected. For example, when you are on tab 1, and then you swipe quickly to reach tab 3, tab 3 will stay blank until the animation ends. I updated the example app on master to add an image gallery, so it should be visible after you rebase.
This is a very useful optimization otherwise, so I think landing this, but turning it off by default would be the best choice. Also should avoid extra setStates
when the feature is turned off.
Also since we added lazy
prop in bottom tabs, we should add it here. We can also add another prop optimizationsEnabled
for the clipped thingy and set it to false
by default.
const mustBeVisible = this._mustBeVisible(props); | ||
|
||
if (!loaded.includes(index) && !mustBeVisible) { | ||
return null; |
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
animationEnabled: true, | ||
}; | ||
|
||
static getDerivedStateFromProps(nextProps, prevState) { |
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
def63d6
to
dfcca27
Compare
I totally agree I should have done that back in the days. But for now, I just want to land this and maybe improve it and port it to react-native-tab-view after.
Extra setStates are avoided in case lazy is false. But if we want to go further, these extra setStates are useful to properly lazy load scenes on swipe. If we really want to avoid these extra setStates, we would have to do one of the following:
In case swipe is disabled,
Sure, but bottom tabs are already using the clipped thingy (ResourceSavingScene) without optimizationsEnabled prop. Thoughts? |
I meant extra setStates when lazy is turned off. Since we don't need them. We can also avoid setStates for the resource saving stuff. I did that in bottom navigation. But that's a different thing.
Yeah, but in bottom tabs it's impossible to notice the optimization. Here we can see the blank page when swiping, so it's important to have it customizable.
I was thinking about heuristics to enable/disable |
Introduced in the last commit. 👍 Thanks for the feedback. |
@brentvatne Any thoughts? |
let's skip heuristics for now and just make it explicit |
It should be fine like this then 😃 |
Can we turn off |
Done! |
@charpeni why i update latest version react-navigation (2.9,3) and lazy in creatMaterialTopTabNavigator not work? |
Because this PR is not included in react-navigation yet. |
@charpeni how I can use lazy in creatMaterialTopTabNavigator :( |
Same question here. I'm using "react-navigation": "^2.14.2" but found lazy not effective - createMaterialTopTabNavigator is still rendering all screens at once. |
Routes in
MaterialTopTabNavigator
are now lazy initialized like inMaterialBottomTabNavigator
.A scene visibility is computed from multiple states and props:
To handle the pan between tabs, we check if you're currently swiping between tabs and the prop
lazyOnSwipe
is true (default value) or if the tab have been already loaded, we'll check if this tab is a sibling of the focused tab. Then, we'll display the tab if it's a sibling.With the propanimationEnabled
to true, we shouldn't hide a tab before the transition is done. So we're waitingCOMPLETE_TRANSITION
action to hide it. Also, if the propsceneAlwaysVisible
is true (default value), we won't hide scenes between A and D while transitioning.If the current tab has not been loaded and must not be visible, we do not render it.
I'll update the docs accordingly to this PR.