Skip to content
This repository has been archived by the owner on Feb 8, 2020. It is now read-only.

feat: make it easier to navigate to a specific route in navigator #114

Merged
merged 1 commit into from
Oct 18, 2019

Conversation

satya164
Copy link
Member

@satya164 satya164 commented Sep 25, 2019

We now use the params of the route to determine initialRouteName and initialParams of a navigator.

So you can do something like this:

navigation.push('Auth', { screen: 'Login', params: { token 'xxx' } })

This will navigate to the Login screen inside the Auth navigator.

Closes #90

@satya164 satya164 requested a review from osdnk September 25, 2019 22:11
@codecov-io
Copy link

codecov-io commented Sep 25, 2019

Codecov Report

Merging #114 into master will increase coverage by 0.28%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #114      +/-   ##
==========================================
+ Coverage   93.47%   93.76%   +0.28%     
==========================================
  Files          36       33       -3     
  Lines         736      722      -14     
  Branches      189      194       +5     
==========================================
- Hits          688      677      -11     
+ Misses         40       37       -3     
  Partials        8        8
Impacted Files Coverage Δ
...ges/core/src/__tests__/__fixtures__/MockRouter.tsx 88.57% <100%> (+0.69%) ⬆️
packages/core/src/useNavigationBuilder.tsx 100% <100%> (ø) ⬆️
packages/core/src/EnsureSingleNavigator.tsx 91.66% <0%> (-1.2%) ⬇️
packages/core/src/NavigationContainer.tsx 88.4% <0%> (-0.64%) ⬇️
packages/core/src/useDevTools.tsx 57.14% <0%> (ø) ⬆️
packages/core/src/NavigationBuilderContext.tsx 50% <0%> (ø) ⬆️
packages/core/src/CommonActions.tsx 100% <0%> (ø) ⬆️
packages/core/src/BaseRouter.tsx 89.47% <0%> (ø) ⬆️
packages/core/src/useNavigationHelpers.tsx 100% <0%> (ø) ⬆️
... and 7 more

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 615b523...c383387. Read the comment docs.

@satya164 satya164 force-pushed the @satya164/navigate-to-navigator branch 5 times, most recently from 9a0cbd0 to e8eceb2 Compare September 25, 2019 23:16
@manticarodrigo
Copy link

manticarodrigo commented Sep 26, 2019

@satya164 I have a root stack navigator with a dev sitemap screen, an auth stack, and tab stack with stacks for each of its screens.

These changes fix the problem I was having navigating from a screen inside a stack from my last tab to a screen in the first tab. I can make a navigate('DashboardTab', { name: 'FilterScreen' }) call successfully. This renders the FilterScreen's back button correctly and goes back to the initial route DashboardScreen.

However, this only works because the DashboardTab has already been navigated to when pressing the TabStack from the SitemapScreen.

When I make a navigate('ConversationTab', { name: 'ConversationScreen' }) call, it now navigates to the correct screen, but no back button. The child screen becomes the initial route for that tab unless I explicitly navigate to the tab on my simulator before hitting the button on a screen from the last tab to navigate to that tab's child screen (and getting the back button).

I can reproduce the behavior using the root sitemap screen. I can navigate to child screens from my root-level sitemap screen to all the screens in my app, but it's only aware of the actual initial routes (rendering back button) of my stacks if I navigate to the tab or root screen within it beforehand.

Setting the initialState prop on NavigationNativeContainer for all these routes also does not fix the issue.

@satya164
Copy link
Member Author

@manticarodrigo it's expected behaviour. if you have never navigated to a screen (the initialRoute in this case), we shouldn't put it in navgation history. current version of React Navigation does it and there is an issue open about it since it's not desirable behaviour: https://github.com/react-navigation/react-navigation/issues/6137

We now use the `params` of the route to determine `initialRouteName` and `initialParams` of a navigator.

So you can do something like this:

```js
navigation.push('Auth', { name: 'Login', params: { token 'xxx' } })
```

This will navigate to the `Login` screen inside the `Auth` navigator.

Closes #90
@satya164 satya164 force-pushed the @satya164/navigate-to-navigator branch from e8eceb2 to c383387 Compare September 26, 2019 13:14
Copy link
Member

@osdnk osdnk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This API looks awkward for me.
How about

navigation.push('Auth', { screen: { screen: 'Login', params: { token 'xxx' } } })

@manticarodrigo
Copy link

@satya164 Is there preferred way of "deep linking" to un-initialized routes? In other words, how would I initialize my navigation history without having to navigate to it manually such that I can travel to any screen and retain the initialRoute of my choosing?

@satya164
Copy link
Member Author

@manticarodrigo you can use resetRoot on the ref of the NavigationNativeContainer component to change the navigation state as you want. though we're currently missing a way to get the current rehydrated state. but we're planning to add it. we currently have some limited deep linking support (it doesn't preserve navigation state on subsequent links, but it'll change): https://reactnavigation.org/docs/en/next/deep-linking.html

@manticarodrigo
Copy link

@satya164 using resetRoot allows me to initialize state and define routes with their proper structure, specifically, setting a stack's initialRoute. However, both resetRoot and the initialState prop for NavigationNativeContainer are not respecting the index property state properties.

So this seems to be a way to "bootstrap" my route structure which then allows me navigate correctly with your new changes, however, it automatically renders the last route in the routes array for each level of the nested state.

@satya164
Copy link
Member Author

both resetRoot and the initialState prop for NavigationNativeContainer are not respecting the index
it automatically renders the last route in the routes array for each level of the nested state.

They respect the index property. However, in a stack navigator, the routes array cannot contain more items after the index. The routes array contains the navigation history, not the routes you have defined.

If you're providing extra routes, the router will try to fix the invalid state and as a result uses the last item in the routes array. You shouldn't provide extra routes in the routes array. They will be automatically created when new screens are pushed.

@manticarodrigo
Copy link

manticarodrigo commented Sep 26, 2019

@satya164 that makes sense. In the ticket you linked to for 3.11.1, someone suggested a partial fix by doing the following:

navigation.navigate({
  routeName: 'ChannelSettings',
  action: NavigationActions.navigate({
    routeName: 'NotificationSettings',
    params: { channelId }
  })
});

Is there an equivalent method in v5? Would this allow me to step into routes and allow them to initialize before the next action?

Update:
I can do subsequent navigate calls by wrapping them in a setTimeout to step into each stack before calling the next navigation action. Not sure if this is the correct way to do it or not.

@manticarodrigo
Copy link

@satya164 Any suggestions on how to do deeper navigations into uninstantiated routes? Using setTimeout seems hacky.

@satya164
Copy link
Member Author

satya164 commented Sep 30, 2019

@manticarodrigo that's what this PR is aiming to solve.

also like I mentioned, you can also do reset to your new state.

@manticarodrigo
Copy link

@satya164 Ok but you previously said that there should not be an awareness of stack structure. This solves the navigation part, but it won't know my initial route unless I navigate to that stack beforehand.

Say for example, I have stack that pushes screens one after the next, following a specific order (i.e. registration flow). If I wanted to jump to a specific step, these changes solve that, however, the router has no idea which step of the stack this is, instead it makes that screen the initial route for the stack.

@manticarodrigo it's expected behaviour. if you have never navigated to a screen (the initialRoute in this case), we shouldn't put it in navgation history. current version of React Navigation does it and there is an issue open about it since it's not desirable behaviour: react-navigation/react-navigation#6137

@satya164
Copy link
Member Author

satya164 commented Sep 30, 2019

@manticarodrigo you are already able to do that with reset. like I mentioned, the problem you had with reset is because you pass more routes than you want to render. if you want to render 2 routes, pass 2 routes and it'll work. you don't even need to pass an index.

resetRoot({
  routes: [
    { name: 'ChannelSettings' },
    { name: 'NotificationSettings', params: { channelId } },
  ],
});

we'll also make it possible to customize the router in future which will allow you to achieve any behaviour you want.

@manticarodrigo
Copy link

@satya164 Ok I was just able to do this now. Thank you!

@satya164
Copy link
Member Author

satya164 commented Oct 1, 2019

Please open a separate bug report with repro code. This is a pull request meant for pull request review.

@satya164 satya164 merged commit a543f1b into master Oct 18, 2019
@satya164 satya164 deleted the @satya164/navigate-to-navigator branch October 18, 2019 13:16
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Being able to specify a stacks route from another stack
4 participants