Skip to content
This repository has been archived by the owner on Sep 4, 2020. It is now read-only.

check whether Push Notification Permissions have been enabled for this app #305

Closed
wants to merge 4 commits into from

Conversation

zwacky
Copy link
Contributor

@zwacky zwacky commented Nov 4, 2015

as soon as you run var push = window.PushNotification.init({ ... the app will ask the user for push notification permission. with this you have more flexibility.

example usage:

window.PushNotification.hasPermission((data) => {
                if (data.isEnabled) {
                    var push = window.PushNotification.init({
                        android: {
                            senderID: 'abc'
                        }
                    });
                }

it will always return a json in format:

{
    isEnabled: true|false
}

@macdonst
Copy link
Member

macdonst commented Nov 4, 2015

@zwacky did a quick look and it seems fine. Can you add some explanation in the README file?

@zwacky
Copy link
Contributor Author

zwacky commented Nov 9, 2015

yes i will do it tomorrow. sorry wasn't available last days.

@zwacky
Copy link
Contributor Author

zwacky commented Nov 10, 2015

@macdonst hope it's alright

@fredgalvao
Copy link
Collaborator

Making a note here so that we remember to take a new look at this with the new permission stuff from android-5.0, things might be easier with it.

@stinju
Copy link

stinju commented Nov 12, 2015

@macdonst in reference to #326, this was the patch that I put in, that seemed to disrupt the notification handling, when used perhaps inappropriately. I traced it down to the fact that in this patch, pushContext is set in the hasPermission logic. I am using hasPermission as a way to detect permissions at any moment, so instead of using it as @zwacky described above, I was invoking it even after a window.PushNotification.init call. When used in this way, init's pushContext is overridden by hasPermission, and subsequent notification events went through the context (callback) provided by hasPermission instead of init.

Given this, with regards to this patch, it might be wise to either 1) handle the hasPermission callback without setting pushContext, 2) provide an error if hasPermission is called after init, or 3) provide a note in the docs. I have no expertise here, so I defer to your judgment, but I just wanted to give you a heads up.

@zwacky
Copy link
Contributor Author

zwacky commented Nov 12, 2015

@stinju i see what you mean. i definitely would expect it to be used after init.

so instead of using the sendEvent(JSONObject _json) in android, it should directly call callbackContext.sendPluginResult(pluginResult)...

gotta test things first before this will be merged.

@macdonst
Copy link
Member

@zwacky yeah, use the new callbackContext for that method call. The sendEvent method is a convenience method for all of the remote notifications that come in.

@zwacky
Copy link
Contributor Author

zwacky commented Nov 18, 2015

also found out, the the android minSDK = 19 (android 4.4) since it uses android.app.AppOpsManager.

@QuentinFchx
Copy link

What is blocking this PR to be merged ?

@zwacky
Copy link
Contributor Author

zwacky commented Nov 27, 2015

this should fix the last issue @stinju mentioned.

@serratus
Copy link

@zwacky In your example you call window.PushNotification.hasPermission before calling window.PushNotification.init. What happens when the user opens the app for the first time? Does hasPermission also ask users if they want to receive notifications or not? I noticed, upon first load of the app, it does not have permission to send notifications, and therefore the init would never run?! Can you please clarify on that?

@zwacky
Copy link
Contributor Author

zwacky commented Nov 29, 2015

it's just an additional function, it that has nothing to do with init().

it's just to prevent the user from having a push notification permission request popup at startup, that most probably will be declined.

you can then call init() at a very specific time, so more users would accept the request.

with hasPermission() you can init() upon every startup then to keep the tokens up to date.

@serratus
Copy link

Thanks for the feedback. But my question was more focused on the fact that init() asks the user for permission if it happens for the first time. How do I know whether the user accepts the permission, or denies it? Is there any callback? Because I've noticed that the registration event is even fired when permission is denied. Do I have to poll the status afterwards with hasPermission()? Or am I missing something else here?

@zwacky
Copy link
Contributor Author

zwacky commented Nov 29, 2015

yep, polling the permission. that's how i track it though.

@serratus
Copy link

Good to know, thanks for the clarification.

@macdonst
Copy link
Member

macdonst commented Dec 1, 2015

@zwacky Do you have any time to resolve the merge conflicts on this PR?

@zwacky
Copy link
Contributor Author

zwacky commented Dec 1, 2015

maybe i'm blind... where can i see the conflicts?

@macdonst
Copy link
Member

macdonst commented Dec 1, 2015

@zwacky Oh right, you don't see the Merge box because you don't have committer access. Leave it with me.

@macdonst
Copy link
Member

@zwacky I have a problem with the example:

window.PushNotification.hasPermission((data) => {
                if (data.isEnabled) {
                    var push = window.PushNotification.init({
                        android: {
                            senderID: 'abc'
                        }
                    });
                }
});

If I'm starting up a brand new app on iOS then data.isEnabled is always false. That's to be expected as the push permission dialog doesn't get show until init is called but in this case init will never get called. Thoughts?

@macdonst
Copy link
Member

@zwacky actually it is the same thing on Android as well. The permission check works great but we may want to change the example docs.

@macdonst
Copy link
Member

Merged into v1.5.x branch

@zwacky
Copy link
Contributor Author

zwacky commented Dec 14, 2015

yeah you're right. should have a hint, that you need to check for that init somewhere else, manually.

will update the docs.

@macdonst
Copy link
Member

@zwacky if you update the docs please do so in the v1.5.x branch which is the current dev branch.

@zwacky
Copy link
Contributor Author

zwacky commented Dec 15, 2015

i did: #425

@macdonst
Copy link
Member

I'm closing this PR as it has been merged into v1.5.x and will become part of master soon.

@macdonst macdonst closed this Dec 15, 2015
@JeremyColton
Copy link

So, call init() first and then poll for a response using hasPermission()

But, hasPermission() returns false the first time the app is open since push has never been granted, so polling using this function will return false while X seconds later, the user may allow push notifications and then hasPermission() will return true.

But, it seems that the .on("registration") event ALWAYS fires even when the push is not allowed. The difference being that the provided data.registrationId is empty if the user didn't allow push and populated if push was allowed.

@zwacky
Copy link
Contributor Author

zwacky commented Feb 3, 2016

@JeremyColton there isn't a nice and sophisticated way than trying, because we don't have hooks for the native permission request modal.

i ask the user, if he'd like to stay up to date. then if he does, i init() (the native permission request modal shows up) and i start the polling 10x for 2s.

@jaigtz88
Copy link

@JeremyColton , so do you have any solution to solve that issue?

@lock
Copy link

lock bot commented Jun 3, 2018

This thread has been automatically locked.

@lock lock bot locked as resolved and limited conversation to collaborators Jun 3, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Development

Successfully merging this pull request may close these issues.

9 participants