seniv / react-native-notifier

Fast and simple in-app notifications for React Native
MIT License
1.07k stars 59 forks source link

NativeStackNavigation modals result in notifications beneath the modal #95

Open z1haze opened 11 months ago

z1haze commented 11 months ago

Hi! 👋

Firstly, thanks for your work on this project! 🙂

Today I used patch-package to patch react-native-notifier@1.9.0 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 commented 11 months ago

fixes #94 relates to #71

stale[bot] commented 7 months ago

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.

vanenshi commented 7 months ago

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

bryanltobing commented 6 months ago

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

wonder if there is a downside of doing this?

vanenshi commented 6 months ago

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 commented 5 months ago

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 commented 3 months ago

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 commented 1 month ago

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 commented 4 weeks ago

@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 commented 4 weeks ago

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".