invertase / notifee

⚛️ A feature rich notifications library for React Native.
https://notifee.app
Apache License 2.0
1.86k stars 226 forks source link

App does not open on notification click (should "pressAction" be a default without needing to specify it?) #291

Closed alexanderdavide closed 1 year ago

alexanderdavide commented 2 years ago

Apparent bug

It seems like the app does not open when clicking the notification body.

Environment

react-native: 0.66.4 @notifee/react-native: 4.0.1, device/api: Pixel_5_API_31

Reproduction steps

  1. npx react-native init repro
  2. npm i @notifee/react-native
  3. add android:exported="true" to AnroidManifest.xml
  4. update build.gradle with compileSdkVersion = 31 targetSdkVersion = 31
  5. add provided code of minimal example
  6. build app
  7. click button to display notification
  8. minimize app and click notification
  9. in my case, the notification disappears after a short moment but the app does not open

minimal example:

/**
 * Sample React Native App
 * https://github.com/facebook/react-native
 *
 * @format
 * @flow strict-local
 */

import React from 'react';
import {View, Button} from 'react-native';

import notifee from '@notifee/react-native';

const App = () => {
  async function onDisplayNotification() {
    const channelId = await notifee.createChannel({
      id: 'default',
      name: 'Default Channel',
    });
    await notifee.displayNotification({
      title: 'title',
      body: 'body',
      android: {
        channelId,
        pressAction: {
          id: 'default',
        },
      },
    });
  }

  return (
    <View>
      <Button
        title="Display Notification"
        onPress={() => onDisplayNotification()}
      />
    </View>
  );
};

export default App;
mikehardy commented 2 years ago

I want to type "Oh the answer is this..." but unfortunately I'll need to run it to see. The only reason I'm typing this otherwise useless comment is to say that your bug report is fantastic, if we can't reproduce with these steps at least we'll know we're working off the same basis and we should be able to track it down. Thanks

effektsvk commented 2 years ago

Hey, I just bumped into this as well. I'm using trigger notifications, though. I've noticed that it doesn't matter if the app is killed or minimized. My device is API 30.

alexanderdavide commented 2 years ago

@mikehardy Thanks. I always try to give the best reports I can to make your lives a little easier.

@effektsvk Actually, in my real application where I experience this problem I'm using trigger notifications too. For this report, I opted for a simpler example.

helenaford commented 2 years ago

Thanks all for bringing this to our attention. Can confirm this is happening on Android 12 due to its updates. Will mark this as a priority.

helenaford commented 2 years ago

Noticed it's the same as https://github.com/invertase/notifee/issues/250

PanamaDonne commented 2 years ago

Also seeing it with same environment as reported. But when the app is terminated and receives a notification it seems to work.

effektsvk commented 2 years ago

I found out that I might be encountering a different issue, notifications don't open app even on older versions (as I mentioned in previous comment, I have API 30 which is Android 11). Today I tested all older versions down to Android 9, none of them worked.

But when I tried example project from the repo, it worked. So I'm assuming there's something wrong with my setup. I'll let you know if I manage to fix it or figure out what the issue is.

PanamaDonne commented 2 years ago

Thanks for the info @effektsvk. I will also dig deeper into my setup and I´ll let you know if I find a solution.

alexanderdavide commented 2 years ago

250 describes the workaround of using targetSdkVersion 30. Apparently, there are features in 31 that break the opening mechanism, so consider this solution as a workaround; citing @mikehardy on #250:

I think this needs to stay open, there is a workaround of using targetSdkVersion 30, but we will eventually (by Nov 1, 2022) need to support targetSdkVersion 31

effektsvk commented 2 years ago

@alexanderdavide Yes, I've checked that thread, I was already on targetSdkVersion = 30. Here's my ext

ext {
    buildToolsVersion = "30.0.2"
    minSdkVersion = 21
    compileSdkVersion = 31
    targetSdkVersion = 30
    ndkVersion = "21.4.7075529"
    supportLibVersion = "28.0.0"
    googlePlayServicesVersion = "16.0.0"
}
PanamaDonne commented 2 years ago

@effektsvk this setting worked for me now :)

        googlePlayServicesLocationVersion = "17.0.0"
        buildToolsVersion = "30.0.2"
        minSdkVersion = 21
        compileSdkVersion = 31
        targetSdkVersion = 30
        androidXCore = "1.0.2"
        ndkVersion = "21.4.7075529"
        appCompatVersion = "1.1.0"
alexanderdavide commented 2 years ago

@effektsvk Sorry, I thought your device is on API 30 but you may still have targetSdkVersion on 31. So, you had the issues while being on targetSdkVersion 30? Then there must be another problem. This is my buildscript.ext with the working workaround:

ext {
      buildToolsVersion = "30.0.2"
      minSdkVersion = 23
      compileSdkVersion = 31
      targetSdkVersion = 30
      ndkVersion = "21.4.7075529"
}
effektsvk commented 2 years ago

@alexanderdavide No problem :) Yes, I tested devices with APIs 28, 29, 30 (Android 9, 10 and 11), none of them worked with my RN project, currently I'm troubleshooting it on 28.

It works on the testing app that is here in the repo in tests_react_native folder. I can try to bump the minSdkVersion, but I'm not sure if that will help. Currently I'm comparing files between the test project and mine.

effektsvk commented 2 years ago

Okay, I'm getting somewhere, when I hardcoded pressAction with activityName to com.packagename.MainActivity it worked. But my package name has suffix .development (we're using product flavors) and Notifee probably uses that and that's why it didn't work. It was issue with my setup after all 😅 I just don't understand why adb logcat didn't spit even a single line about it...

mikehardy commented 2 years ago

Sound's like we have a target API 31 issue (we actually have a couple...), and perhaps a need for more logging and perhaps a need for better handling of gradle build suffixes if possible. I use suffixes for different release streams as well I need to see how I'm handling that.

alexanderdavide commented 2 years ago

@effektsvk Can you please elaborate your steps hardcoding pressAction? Where did you set com.packagename.MainActivity?

effektsvk commented 2 years ago

@alexanderdavide I added an action to actions array in android property when creating trigger notification, here's sample code and here's more info:

await notifee.createTriggerNotification({
  id: '123',
  title: 'title',
  body: 'body',
  android: {
    actions: [
      {
        title: 'test',
        pressAction: {
          id: 'default',
          launchActivity: 'com.packagename.MainActivity', // <-- here you can add the package name of your app
        },
      },
    ],
    channelId: '123',
  },
}, trigger)
alexanderdavide commented 2 years ago

@effektsvk Thanks. I've already thought you mean launchActivity and tried it but can't get it working in my setup. I've set launchActivity to android.defaultConfig.applicationId of android/app/build.gradle and appended .MainActivity to it which is the android:name. I've added it to pressAction on the AndroidNotification as well as on the AndroidAction.

effektsvk commented 2 years ago

@alexanderdavide Yeah, then I guess your only issue is the Android 12 bug.

Also, another side note, I checked my activity name with this app, the package name included .development but the activity didn't.

alexanderdavide commented 2 years ago

@effektsvk Where do you append your application environment like .development to the package name?

effektsvk commented 2 years ago

@alexanderdavide In android/app/build.gradle we have product flavors defined like this, and then build the app with react-native run-android --appIdSuffix development --variant deploymentDevelopmentDebug:

android {
    // ...
    flavorDimensions "deployment"
    productFlavors {
        deploymentLocal {
            dimension "deployment"
            applicationIdSuffix ".local"
        }

        deploymentDevelopment {
            dimension "deployment"
            applicationIdSuffix ".development"
        }

        deploymentTesting {
            dimension "deployment"
            applicationIdSuffix ".testing"
        }

        deploymentProduction {
            dimension "deployment"
        }
    }
    // ...
}
mikehardy commented 2 years ago

This works for me with flavors / applicationIdSuffice:


          await notifee.displayNotification({
            title: messageOptions.title,
            body: messageOptions.body,
            android: {
              channelId: BackgroundTaskService.CONNECTION_REQUEST_NOTIFICATION_CHANNEL_ID,
              smallIcon: 'drawable/minilogo_bw',
              pressAction: {
                id: 'default',
                launchActivity: 'default',
                launchActivityFlags: [AndroidLaunchActivityFlag.SINGLE_TOP],
              },
            },
          });
effektsvk commented 2 years ago

@mikehardy Holy crap, it works for me too!!! 😅 Thank you so much! Is that intended behavior, or is some kind of fix necessary?

mikehardy commented 2 years ago

Intended behavior, basically "default" is what almost everyone wants every time. But the module allows people to override it just in case that's necessary (consider a brown-field scenario where the react-native part of an app is not the launcher activity...)

effektsvk commented 2 years ago

But shouldn't that then be default value in case pressAction is missing?

mikehardy commented 2 years ago

It should maybe? :-) - unsure sorry. I suppose that itself is an issue, re-opening so this is queued for investigation but please don't wait on me/us for investigation, if you have a minute to scan through the source files and see if just assuming "default" in the case it is not specified works and is more or less backwards compatible, that would be a big help

effektsvk commented 2 years ago

Okay, I might be able to take a look at it today, if I make some sense of it, I'll let you know. (I'm a bit scared because I don't have a lot of experience with android environment and packages, but I guess it's still just code, nothing to worry about, right? 😅😅)

Again, thank you very much for your help! ❤️

helenaford commented 2 years ago

@mikehardy we do already default to this here 🤔 really strange. I was seeing the trampoline error when I was testing 😱

helenaford commented 2 years ago

@effektsvk did you also add launchActivityFlags: [AndroidLaunchActivityFlag.SINGLE_TOP]? Wondering if this is the difference rather than the launchActivity defaulting to 'default' (as we already do this)?

effektsvk commented 2 years ago

@helenaford I did both, with and without it. Both worked fine. Ended up using it without it.

edit: I'm looking at that code you linked, it only sets the default value when pressAction.id is default, I was not adding pressAction at all, maybe that's the issue.

helenaford commented 2 years ago

@effektsvk ah yeah, thanks. That's good to know. I think that is the issue 🤔 It probably makes sense to default to id: default if pressAction isn't defined.

alexanderdavide commented 2 years ago

@effektsvk @mikehardy @helenaford To clarify: Should this work on API 31 too? I've just tried this

pressAction: {
  id: 'default',
  launchActivity: 'default',
  launchActivityFlags: [AndroidLaunchActivityFlag.SINGLE_TOP],
}

on the setup I've opened the issue with but opening the app upon notification click remains dysfunctioning, hence I'm still working on 30 as #250 proposes which works with just pressAction.id defined.

effektsvk commented 2 years ago

@alexanderdavide I didn't test it on API 31, so I'm not sure, but I think the workaround from #250 is still necessary. I was already on that SDK version mentioned in the workaround.

helenaford commented 2 years ago

Just updating here on status, the latest release v4.1.0 contains a fix for apps targeting API 31 and devices running Android 12.

github-actions[bot] commented 1 year ago

Hello 👋, to help manage issues we automatically close stale issues.

This issue has been automatically marked as stale because it has not had activity for quite some time.Has this issue been fixed, or does it still require attention?

This issue will be closed in 15 days if no further activity occurs.

Thank you for your contributions.

effektsvk commented 1 year ago

Hey @helenaford, should this be closed?

mikehardy commented 1 year ago

It appears to have gone stale, so possibly? Lamentably, there is not an infinite pool of free labor on the internet. If this is really important, it may require community involvement in the form of research / experimentation and a PR if none exist at the moment

effektsvk commented 1 year ago

At least from my point of view, the workaround (adding pressAction with id: "default") is enough.

I think the main question was whether it is desired to add the default id automatically, in case no pressAction is passed. If not, then I believe the issue is solved.

mikehardy commented 1 year ago

Reasonable enough - I'm not familiar with this specific issue, reopening for more well-reasoned consideration from @helenaford

helenaford commented 1 year ago

this is definitely possible, it will be a breaking change for developers who haven't set a pressAction intentionally. My main hesitation to do this is for the scenario you don't want a pressAction, might be harder to specify that. But in terms of implementation, this would be very simple to do. It's just on the js level to add pressAction: { id: 'default' } as a default value if not set.

alexander0205 commented 1 year ago
 android: {
                    pressAction: {
                      id: 'default',
                      launchActivity: 'com.package.MainActivity', // <-- here you can add the package name of your app
                    },
                    channelId,
                  }

The solution for me, thanks team

github-actions[bot] commented 1 year ago

Hello 👋, to help manage issues we automatically close stale issues.

This issue has been automatically marked as stale because it has not had activity for quite some time.Has this issue been fixed, or does it still require attention?

This issue will be closed in 15 days if no further activity occurs.

Thank you for your contributions.

jvgeee commented 1 year ago
 android: {
                    pressAction: {
                      id: 'default',
                      launchActivity: 'com.package.MainActivity', // <-- here you can add the package name of your app
                    },
                    channelId,
                  }

The solution for me, thanks team

Just want to add a bit of SEO magic for anyone like me who's trying to search for this in 2023; if you're using Notifee and having an issue where on Android, tapping on a push notification doesn't open the app - or interacting with a push notification doesn't open the app - this is the code that fixes it.

I personally think this should be a default (who wouldn't want an app to launch by default when tapping a push notification???) but here we are.

quocluongha commented 1 year ago

In my case, setting id: 'default' works. But when I changed into id: 'somerandomtext', it stopped working.

android: {
      pressAction: {
        id: 'default', // This will open the app on press
        //id: 'somerandomtext' <-- This won't
      },
    },
timmyjose commented 6 months ago

In my case, setting id: 'default' works. But when I changed into id: 'somerandomtext', it stopped working.

android: {
      pressAction: {
        id: 'default', // This will open the app on press
        //id: 'somerandomtext' <-- This won't
      },
    },

Can confirm that this works. Thank you!

vksgautam1 commented 1 month ago

In my case, setting id: 'default' works. But when I changed into id: 'somerandomtext', it stopped working.

android: {
      pressAction: {
        id: 'default', // This will open the app on press
        //id: 'somerandomtext' <-- This won't
      },
    },

worked for me