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

feat: expose new container for 2-state navigators #1029

Merged
merged 12 commits into from
Sep 6, 2021

Conversation

WoLewicki
Copy link
Member

@WoLewicki WoLewicki commented Jul 28, 2021

Description

PR adding RNSScreenNavigationContainer component on iOS. This component should be used in bottom-tabs and drawer navigators of react-navigation in order to resolve layout issues connected to integration between UIViewController and UINavigationController containers (see #619 and #564). stack navigator unfortunately cannot use UINavigationController since it needs the previous screens to be visible e.g. during the transition. Connected PR in react-navigation: react-navigation/react-navigation#9772

Changes

Added new component and hasTwoStates prop for ScreenContainer which controls which native component should be used.

Test code and steps to reproduce

Test619.js and Test564.js to see the layout issues gone. It needs setting hasTwoStates props on bottom-tabs navigator in order for this to work.

Checklist

  • Included code example that can be used to test this change
  • Updated TS types
  • Ensured that CI passes

src/types.tsx Outdated
/**
* A prop to be used in drawer and bottom-tabs navigators for better implementation of ScreenContainer on iOS.
*/
tabsOrDrawer?: boolean;
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think a prop name like hasTwoStates (didn't think a lot about the name) or something like that would be more generic and more appropriate

@@ -47,6 +47,8 @@ function enableScreens(shouldEnableScreens = true): void {

// const that tells if the library should use new implementation, will be undefined for older versions
const shouldUseActivityState = true;
const isNavigationContainerAvailable =
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is there a case when this won't be available when someone is using this version of react-native-screens?

Copy link
Member Author

Choose a reason for hiding this comment

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

What if someone updates to this version of library in Expo managed workflow? I know it should not be done, but 🤷‍♂️ .

Copy link
Collaborator

Choose a reason for hiding this comment

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

@WoLewicki updating to an incompatible version of the library could also cause other issues as well, and surface bugs which are already fixed in latest version, so probably not a use case we should support.

@WoLewicki WoLewicki changed the title fix: layout issues in nested stack header feat: expose new container for 2-state navigators Jul 30, 2021
@WoLewicki WoLewicki merged commit 149f924 into master Sep 6, 2021
@WoLewicki WoLewicki deleted the @wolewicki/make-new-container branch September 6, 2021 12:05
satya164 pushed a commit to react-navigation/react-navigation that referenced this pull request Feb 1, 2022
Added new prop `tabsOrDrawer` for `ScreenContainer` which makes `react-native-screens` on `iOS` use new component if it is available. See software-mansion/react-native-screens#1029 for context.
darkhorse-coder pushed a commit to darkhorse-coder/react-navigation that referenced this pull request Apr 7, 2022
Added new prop `tabsOrDrawer` for `ScreenContainer` which makes `react-native-screens` on `iOS` use new component if it is available. See software-mansion/react-native-screens#1029 for context.
nenad0212 pushed a commit to nenad0212/temp1 that referenced this pull request Sep 6, 2022
Added new prop `tabsOrDrawer` for `ScreenContainer` which makes `react-native-screens` on `iOS` use new component if it is available. See software-mansion/react-native-screens#1029 for context.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants