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

Hide foreground notification with --variable on iOS #913

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

XavierBesnault
Copy link

Was inspired by #835

Install the plugin with cordova plugin add cordova-plugin-firebase --variable IOS_HIDE_FOREGROUND_NOTIFICATION="true" to hide notifications on iOS when foreground.

Also added a value foreground: "true" in the notification payload for both Android and iOS when the notification is received in foreground to be able to manage this specific case in the Javascript side, especially useful when the notification is hidden.

Xavier added 3 commits October 14, 2018 23:58
… foreground on iOS

Also add a foreground variable to true in the payload when the notification is received in the foreground
src/android/FirebasePluginMessagingService.java Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@soumak77 soumak77 requested a review from briantq October 14, 2018 16:52
@wildhart
Copy link

Looking at the comments on #835 it looks like using UNNotificationPresentationOptionNone doesn't work in all cases - me an others still get the notifications when the app is in the foreground.

@XavierBesnault
Copy link
Author

@wildhart
What's your iOS device version ?
What's your cordova/cordova-ios version ?
What plugins do you have ?
Did you put a breakpoint in the function in XCode ? Does it go through ?

@wildhart
Copy link

What's your iOS device version ? I'm testing on a iPad 4 mini, iOS 12.0.1
What's your cordova/cordova-ios version ? cordova-ios 4.5.4 - not sure of the cordova version - whatever comes with Meteor 1.6.1.1
What plugins do you have ?

cordova-plugin-device@2.0.2
cordova-plugin-file@6.0.1
cordova-plugin-file-opener2@https://github.com/JuanjoPP/cordova-plugin-file-opener2.git#96e394315b926e7787e30cf348d1b9dfd5a653c4
cordova-plugin-firebase@2.0.2
cordova-plugin-fullscreen@1.2.0
cordova-plugin-inappbrowser@3.0.0
cordova-plugin-mauron85-background-geolocation@3.0.0-alpha.42
cordova-plugin-meteor-webapp@1.6.0
cordova-plugin-splashscreen@5.0.2
cordova-plugin-statusbar@2.4.1
cordova-universal-links-plugin@https://github.com/aramando/cordova-universal-links-plugin.git#35b3ed7e9a0310b12f1ac92a5159b21ce50eee57
phonegap-plugin-mobile-accessibility@1.0.5

Did you put a breakpoint in the function in XCode ? Does it go through ? I can't easily do that - I'm actually compiling the iOS app on a VM. I can get the app to run on the iOS simulator but notifications don't really work on the simulator, but I do get the callback when the app comes into the foreground.

However, when I upload to TestFlight, then on my iPad Mini the notifications come through whether the app is open or not. With or without changing AppDelegate+FirebasePlugin.m line 168 to UNNotificationPresentationOptionNone

Sorry, my setup is not very typical, but other commentators on #835 report the same problem.

@sebastianludwig
Copy link

This definitely works as expected - good work 👍

@lmanata
Copy link

lmanata commented Mar 25, 2019

@soumak77 @briantq Ran into this issue and was happy to see a PR was already open.

The requested changes also seem to have been handled so I was wondering if this needs further changes or is just awaiting reviews.

Thanks!

@globules-io
Copy link

Any update on this please?

@tom-hartz
Copy link

Yo @XavierBesnault ! FYI, I opened a new PR that covers this as well. I took your idea one step further and made it configurable via API method instead of using config.xml setting.

We should only merge one of these, either yours or mine.
#1056

Take a look and let me know what you think!

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

Successfully merging this pull request may close these issues.

7 participants