Use MaterialTopTabNavigator for main navigation #2831
Conversation
|
Thanks! As you see in my comment earlier today over here: #2702 (comment) , I am interested in exactly this switch :) For this PR, I'm looking at the tip commit since that's the one that's new relative to #2702 . That commit does a bunch of other things too, in addition to switching to MaterialBottomTabNavigator (btw the commit message and PR title seem to have the name flipped); those changes generally look like good improvements (like clarifying the naming around When the other refactors and improvements are separated out, does the change that actually switches to MaterialBottomTabNavigator end up being very big? Perhaps it'd be simplest to skip moving MainTabs over to MaterialTopTabNavigator (one of the commits in #2702 ), and move it straight to MaterialBottomTabNavigator. |
|
The The last commit that implements the Another thing I will need to figure-out - on transition to another tab, there is some color used to 'blend' to a new tab. When our background is white, it looks like a proper blending transition. When we are on the night theme, the same color results in a 'flashing' transition and not blending. |
|
Some of the changes in the last commit (and the purpose of this PR) are both an improvement in the structure of the app and also needed for this Bottom-Material component to work (it sets a background and doesn't work with our philosophy of 'transparent background simplifies stuff') I'll split this into several commits too. From coding perspective, I think they will be simple but will be good that you test the behavior by running the branch to verify if it feels good in use. |
|
Heads up @borisyankov, we just merged some commits that conflict with the changes your made in this pull request! You can review this repository's recent commits to see where the conflicts occur. Please rebase your feature branch against the |
Upgrade to React Navigation 2 and do the minimal changes that make our code compatible. Navigating behavior has changed, and pushing a route with the same key does not result in duplicated entry in the history. Fixes zulip#2682 The code to wire RN navigator to Redux have changed. The newer approach is simpler, just using `reduxifyNavigator` helper does most of the work.
Update definitions by running `flow-typed update`. This lets Flow know about the new constructor functions.
`NavigationActions.pop` was removed in RN v2 and now `back` behaves
the same as `pop` before. We used `pop` to skip several routes if
they have the same key ('chat' in our case) but this matches
the new behavior of 'back'.
The `StackNavigator` constructor function has been deprcated. Using the `createStackNavigator` that expects the same inputs.
`TabNavigator` constructor is deprecated in favor of the function 'createMaterialTopTabNavigator'. Our config no longer needs some parameters passed, so this is simplified while keeping the very same behavior.
Replace `TabNavigator` constructor with `createMaterialTopTabNavigator` creator function which is the one that matched the old behavior. Note: While `createBottomTabNavigator` sounds like what we might need, it is a simpler navigator compared to what we were using and actually `createMaterialTopTabNavigator` is the correct one.
The type `NavigationNavigateAction` we use is too specific and not the correct one. We want the `NavigationAction` type instead. Also, to make it simpler and consistent, we rename our own type `NavigateAction` to `NavigationAction`.
borisyankov
force-pushed
the
rn2-enhanced
branch
from
3ec9582
to
c1bcba2
Compare
|
The further work discussed above never happened; the PR still says the opposite of what it does, among everything else. With the way we actually did the react-navigation v2 upgrade (in #3573), we never switched to the inappropriate |
This is based on the React Navigation 2 PR.
The last commit uses the new MaterialTopTabNavigator which is recommended to best follow Material design guidelines (it is also more configurable and nicer)