Skip to content
This repository was archived by the owner on Jan 14, 2025. It is now read-only.

Make local notifications on android notify the application too #574

Conversation

ErikMikkelson
Copy link

@ErikMikkelson ErikMikkelson commented Oct 14, 2017

RNPushNotificationPublisher receives the local notifications, but it doesn't route them to the application if it's in the foreground like the ListenerService does. This makes it work the same for both, and thus duplicates the behavior that we see on iOS.

Also, I missed it when I first set up the code, but it appeared that userInteraction was overridden to false in the code, but I removed that so we can know if we were clicked from the notification center.

Hey, for people trying to handle background notifications on Android, the intent filter trick listed in 122 worked for me.

I can now receive local notifications in the foreground and background, warm and cold start on iOS and Android and onNotification is called.

@ital0
Copy link

ital0 commented Oct 26, 2017

Please @zo0r accept this PR :)

@nonameolsson
Copy link

@zo0r Have you had time to try it? Looks good!

@ErikMikkelson
Copy link
Author

My team has been using this fork for the last three weeks and so far no problems.

@nonameolsson
Copy link

Could it be merged? It's really important to get this one working.

@amazingmarvin
Copy link

I have tested local notifications when app is in the foreground with this PR and it works just like with PushNotificationIOS.

Thanks @ErikMikkelson!

@supercamilo
Copy link

Merge plz

gtriggiano added a commit to gtriggiano/react-native-push-notification that referenced this pull request Feb 14, 2018
@royipressburger
Copy link

Will be happy to see that as an official solution to the problem, and not using other libraries like react-native-fcm

calcazar pushed a commit to calcazar/react-native-push-notification-CE that referenced this pull request Apr 30, 2018
@calcazar
Copy link

Added this PR to the CE clone

tuohis pushed a commit to tuohis/react-native-push-notification that referenced this pull request May 14, 2018
@buddhamangler
Copy link

This pull request saved my butt. Please merge

@andrzejbe
Copy link

a PR with plenty of positive comments - that has no conflicts, adds long-awaited functionality (which makes local notifications finally usable) and works like a charm.. Please merge!

@bgerhards
Copy link

bgerhards commented Dec 7, 2018

This seemed to fix the issue, but now it's old enough that Android 8 notification requirements are missing. This requirement regarding the mandatory notification channel has been resolved in enhancements already added to the package. Update your repo @ErikMikkelson and keep me up to date. I hope @zo0r @npomfret approves this PR, it solved my recent issues with foreground notifications.

@ErikMikkelson
Copy link
Author

It's 2020 and the changes listed above still aren't in. @zo0r @npomfret @bgerhards I just pulled from master and compared old changes with new files and they appear the same. I'm not sure what changes are necessary for Android 8? Most of my changes were around getting RNPushNotificationPublisher to work the same as RNPushNotificationListenerService while getting the application context a different way.

@Dallas62 Dallas62 changed the base branch from master to dev May 4, 2020 19:19
@Dallas62 Dallas62 self-assigned this May 4, 2020
@Dallas62
Copy link
Collaborator

This PR has been implemented on dev.

@Dallas62 Dallas62 closed this May 31, 2020
@Dallas62
Copy link
Collaborator

Dallas62 commented Jun 30, 2020

Hi,

I merged this PR, but it seems Apple has deprecated this feature:
https://developer.apple.com/documentation/uikit/uiapplicationdelegate/1622930-application?changes=latest_minor&language=objc

And alternative isn't handling background notification when there is no user actions...
https://developer.apple.com/documentation/usernotifications/unusernotificationcenterdelegate

I'm not sure allowing this on Android is a good idea anymore, since there is no way to do this on iOS.

What do you think ?

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

Successfully merging this pull request may close these issues.