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

NativeStackNavigation modals result in notifications beneath the modal #95

Open
z1haze opened this issue Dec 6, 2023 · 10 comments
Open

Comments

@z1haze
Copy link

z1haze commented Dec 6, 2023

Hi! 👋

Firstly, thanks for your work on this project! 🙂

Today I used patch-package to patch [email protected] for the project I'm working on.

When using a NativeStackNavigator on iOS, notifications are forced beneath modal screens. This is due to how iOS treats modals, and that is to always force them to the top. We can use the FullWindowOverlay component provided by react-native-screens to conditionally wrap the notification for iOS only, which results in notifications being positioned on top of modals where they belong, and no change on android.

Here is the diff that solved my problem:

diff --git a/node_modules/react-native-notifier/.DS_Store b/node_modules/react-native-notifier/.DS_Store
new file mode 100644
index 0000000..9104acb
Binary files /dev/null and b/node_modules/react-native-notifier/.DS_Store differ
diff --git a/node_modules/react-native-notifier/src/Notifier.tsx b/node_modules/react-native-notifier/src/Notifier.tsx
index 92bfaf2..750fb7a 100644
--- a/node_modules/react-native-notifier/src/Notifier.tsx
+++ b/node_modules/react-native-notifier/src/Notifier.tsx
@@ -12,6 +12,8 @@ import {
   PanGestureHandlerStateChangeEvent,
 } from 'react-native-gesture-handler';
 
+import { FullWindowOverlay } from 'react-native-screens';
+
 import styles from './Notifier.styles';
 import { Notification as NotificationComponent } from './components';
 import {
@@ -260,6 +262,12 @@ export class NotifierRoot extends React.PureComponent<ShowNotificationParams, St
     this.hiddenComponentValue = -Math.max(heightWithMargin, DEFAULT_COMPONENT_HEIGHT);
   }

   render() {
     const {
       title,
@@ -275,7 +283,7 @@ export class NotifierRoot extends React.PureComponent<ShowNotificationParams, St
     const additionalContainerStyle =
       typeof containerStyle === 'function' ? containerStyle(this.translateY) : containerStyle;
 
-    return (
+    const notificationContent = (
       <PanGestureHandler
         enabled={swipeEnabled}
         onGestureEvent={this.onGestureEvent}
@@ -306,5 +314,11 @@ export class NotifierRoot extends React.PureComponent<ShowNotificationParams, St
         </Animated.View>
       </PanGestureHandler>
     );
+
+    return  Platform.OS === 'ios' ? (
+      <FullWindowOverlay>{notificationContent}</FullWindowOverlay>
+    ) : (
+      notificationContent
+    )
   }
 }

This issue body was partially generated by patch-package.

@z1haze
Copy link
Author

z1haze commented Dec 6, 2023

fixes #94
relates to #71

Copy link

stale bot commented Mar 17, 2024

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Mar 17, 2024
@vanenshi
Copy link

vanenshi commented Apr 4, 2024

@z1haze Thanks a bunch
you don't mind if I create a PR using this fix?

@stale stale bot removed the stale label Apr 4, 2024
@bryanltobing
Copy link

Platform.OS === 'ios' ? (
<FullWindowOverlay>{notificationContent}</FullWindowOverlay>

wonder if there is a downside of doing this?

@vanenshi
Copy link

Platform.OS === 'ios' ? (
<FullWindowOverlay>{notificationContent}</FullWindowOverlay>

wonder if there is a downside of doing this?

There was a downside, the GestureHandler stopped working
So, i had to add GestureHandlerRootView inside the overlay

      <FullWindowOverlay>
        <GestureHandlerRootView >
             ...
        </ GestureHandlerRootView >      
      </FullWindowOverlay>

@bryanltobing
Copy link

bryanltobing commented Jun 2, 2024

Platform.OS === 'ios' ? (
<FullWindowOverlay>{notificationContent}</FullWindowOverlay>

this solution is stopped working in expo 51, the screen is just freezing everytime i'm wrapping my app with NotifierWrapper

@seniv
Copy link
Owner

seniv commented Jul 21, 2024

Hello 👋
Based on @z1haze’s patch and other comments, I found out how the problem can be solved. Thanks a lot!
I've released a new version that introduces the useRNScreensOverlay prop that enables FullWindowOverlay.
It's disabled by default just to be safe that it will not break anything. To enable it - simply pass useRNScreensOverlay to NotifierWrapper or NotifierRoot:

<GestureHandlerRootView>
  <NotifierWrapper useRNScreensOverlay>
    // ...app code
  </NotifierWrapper>
</GestureHandlerRootView>

It was tested on the latest Expo (51) with Expo Go and on the Bare React Native app without Expo.
@bryanltobing can you try it on your end? That will be very helpful

simulator_screenshot_7B8CD562-DEC4-4427-A550-C9490E2EE405

@tastydev
Copy link

tastydev commented Oct 2, 2024

Hello @seniv, i have a question regarding to theese changes here:

I need to implement notifications which render behind the a rn-navigation transparentModals but only for a specific stack. Is this still possible with the changes that have been made here, cause with version 2.0.0 all my notifications are rendered on top of theese modals even with useRNScreensOverlay set to false?

I.E I would like to have a Notifier Provider around my app which renders Notifications which need to be handled as mentioned in this issue (Always on top of everything)
But i would also like to have them rendered beneath react-native-navigation modals/transparentModals in specific stacks. Maybe with a new NotifierProvider around theese specific stacks?
Or is it possible to have a different behaviour per showNotification call?

Thanks in advance

@seniv
Copy link
Owner

seniv commented Oct 6, 2024

@tastydev Hello, that's an interesting case. First of all, I want to clarify a few questions:

  1. are you use https://github.com/wix/react-native-navigation? if so, which version?
  2. do you have https://github.com/software-mansion/react-native-screens installed?
  3. In version 1.*, the behavior was different than with 2.*?

If you don't have react-native-screens installed, then usage of useRNScreensOverlay will not make any difference, as it uses component from react-native-screens under the hood.

If you want to render a second instance of NotifierWrapper or NotifierRoot inside your screen/navigator, you can render it with omitGlobalMethodsHookup={true} prop and control it with ref, so you will have one global instance that can be controlled with Notifier.showNotification, and the second one that can be opened with ref.current.showNotification

Also, it would be helpful if you can share a a code where I can reproduce the issue

@tastydev
Copy link

tastydev commented Oct 7, 2024

Hey @seniv,

thanks for your response i am currently using the following Package versions:

"react-native-screens": "3.31.1",
"react-native": "0.74.5",
"@react-navigation/bottom-tabs": "6.0.9",
"@react-navigation/devtools": "6.0.7",
"@react-navigation/elements": "1.2.1",
"@react-navigation/material-top-tabs": "6.0.6",
"@react-navigation/native": "6.0.6",
"@react-navigation/stack": "6.0.11",

As mentioning above useRNScreensOverlay doesn't seem to have an effect using transparentModal for screens.

To workaround my issue i added a Zustand store to conditionally render the notification if a modal is present where i wan't to give the modal screen more "priority".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants