这是indexloc提供的服务,不要输入任何密码
Skip to content
This repository was archived by the owner on Jan 13, 2023. It is now read-only.

Conversation

@harrisrobin
Copy link
Contributor

@harrisrobin harrisrobin commented Feb 7, 2020

This PR upgrade react-navigation to v5 while maintaining all the functionality such as navigation state persistence & back button handler.

✅ upgrade to react-navigation v5
✅ migrate existing back button handler and navigation persistence to the recommended way of doing it in V5.
✅ use react-native-safearea-context instead of SafeAreaView

Demo

intro-demo

This project was generated using Bowser with my changes to include React Navigation V5:
https://github.com/harrisrobin/ignite-bowser-react-navigation-v5

@nirre7
Copy link
Contributor

nirre7 commented Feb 7, 2020

Just curious, if I have an ignite based app with react-navigation 4.x and upgrade the app to use react-navigation v5 will this be seamless for production apps or will they crash or "loose" the route state?

@harrisrobin
Copy link
Contributor Author

harrisrobin commented Feb 7, 2020

Just curious, if I have an ignite based app with react-navigation 4.x and upgrade the app to use react-navigation v5 will this be seamless for production apps or will they crash or "loose" the route state?

I don't think it will be "seamless" because we are now storing navigation state separately and not part of the MobX store. You would probably want to reset the user's storage or version your stores so that you can easily migrate them just to be sure.

@chakrihacker
Copy link

@harrisrobin that's a well-crafted PR, any reason why mobx is not used to store navigation state, is it because react-navigation already provides a way to do that?

@harrisrobin
Copy link
Contributor Author

harrisrobin commented Feb 9, 2020

@harrisrobin that's a well-crafted PR, any reason why mobx is not used to store navigation state, is it because react-navigation already provides a way to do that?

It's not possible to do it anymore in v5 and is not recommended by the react-navigation team.

@fortydegrees
Copy link

This woulod be great to be integrated. Has anyone been able to get this to work? I've struggled, though left an 'issue' on the repo.

@harrisrobin
Copy link
Contributor Author

This woulod be great to be integrated. Has anyone been able to get this to work? I've struggled, though left an 'issue' on the repo.

@fortydegrees you can always generate a project using this branch to test it.

  1. git clone git@github.com:harrisrobin/ignite-bowser.git

  2. ignite new <YOUR_BOILERPLATE_NAME> -b /full/path/to/cloned-boiler-plate

@fortydegrees
Copy link

Can confirm that this update works seamlessly!

@jamonholmgren
Copy link
Member

@harrisrobin Can you run yarn ci:test on your local machine and see if you can replicate the issues that CI is reporting?

Copy link
Member

@robinheinze robinheinze left a comment

Choose a reason for hiding this comment

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

@harrisrobin This is really great, thorough PR. Really nice work 🎉 I just mentioned one super tiny change, since I think it might break model generators, but otherwise it looks awesome.

@robinheinze
Copy link
Member

@harrisrobin Actually, you might also want to double check the screen and navigator generators. The screen one inserts the new screen into the root navigator so I think this would introduce some changes there, and the generated navigator will look different as well.

@bhaveshdaswani93
Copy link

any updates when this PR is going to be merged

@harrisrobin
Copy link
Contributor Author

@osamaaamer95 @robinheinze done in c58583a

@osamaaamer95
Copy link

@harrisrobin what about paddingBottom on this line? I think once the app has been wrapped in <SafeAreaProvider /> you can simply import <SafeAreaView /> from react-native-safe-area-context and use that as the wrapper here: const Wrapper = props.unsafe ? View : SafeAreaView

I am using this in a project (couldn't wait for this PR to be merged) and it works as expected.

@harrisrobin
Copy link
Contributor Author

harrisrobin commented Mar 12, 2020

@harrisrobin what about paddingBottom on this line?

I am using this in a project (couldn't wait for this PR to be merged) and it works as expected.

I personally don't handle paddingBottom in screen, just the notch. What's your use-case for that?

I think once the app has been wrapped in <SafeAreaProvider /> you can simply import <SafeAreaView /> from react-native-safe-area-context and use that as the wrapper here: const Wrapper = props.unsafe ? View : SafeAreaView

I prefer to use the hook instead. The ternary with View/SafeAreaView causes typescript to complain when passing certain props.

@osamaaamer95
Copy link

The use-case is on Android where content goes under the transparent/translucent navigation bar if no padding is applied.

@viralS-tuanlv
Copy link

i saw this pr not enough to ready merge

@jamonholmgren jamonholmgren merged commit f68ec23 into infinitered:master Mar 29, 2020
infinitered-circleci pushed a commit that referenced this pull request Mar 29, 2020
# [5.0.0](v4.14.3...v5.0.0) (2020-03-29)

* ✨Upgrade React Navigation to V5 ✨ (#311 by @harrisrobin) ([f68ec23](f68ec23)), closes [#311](#311)

### BREAKING CHANGES

* * React Navigation upgraded to version 5, a backwards incompatible change
* Navigation state is no longer handled in MobX-State-Tree
* Removed `NavigationStore`
* Navigation state is now handled in React hooks (also persisted to async storage)
* Back button handler is a hook now rather than a component
* Added `react-native-safe-area-context` wrapping the primary navigator
* Removed `ignite g navigator` generator
* Screen generator no longer tries to add the new screen to a navigator
@infinitered-circleci
Copy link
Collaborator

🎉 This PR is included in version 5.0.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@anasjaghoub
Copy link

@harrisrobin @robinheinze
Thanks for your great efforts and this amazing boilerplate that really helps getting started with react native.

I've noticed that in this ticket you've removed NavigationStore. I would love if you can help me by providing an example for navigating with params between screens. I was following this tutorial https://reactnavigation.org/docs/MST-integration/#overview
However I've found you've removed the NavigationStore, so my question is it for a reason? what's the alternative way? and if you can update the demo application to provide an example for navigating with params like models / snapshots.

@jamonholmgren
Copy link
Member

@anasjaghoub Thanks for your comment -- we're working on some additional examples and documentation as well as a blog post!

@anasjaghoub
Copy link

that's great @jamonholmgren can't wait for it! Best wishes.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.