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

Added missing notification permission statuses, Provisional and Ephemeral #437

Merged
merged 2 commits into from
Aug 27, 2021

Conversation

matt-auckland
Copy link

@matt-auckland matt-auckland commented May 27, 2021

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Documentation changes

PR Checklist

For bug fixes / features, please check if your PR fulfills the following requirements:

  • Testing has been carried out for the changes have been added
  • Regression testing has been carried out for existing functionality
  • Docs have been added / updated

What is the purpose of this PR?

At the moment this plugin doesn't deal with two new Notification Permissions statuses that have been added to iOS in v12 ("Provisional") and v14 ("Ephemeral").

This PR updates the plugin so that it does, and it also makes the default behavior such that if you are in either of those states, it will prompt the user for notification permissions when you call requestRemoteNotificationsAuthorization

Does this PR introduce a breaking change?

  • Yes
  • No

At the moment when you call requestRemoteNotificationsAuthorization because these two permission states are not recognized, the system prompt for notifs is not shown. This PR changes that so the prompt is shown.

Happy to update the PR to make this optional or otherwise if needed.

What testing has been done on the changes in the PR?

I've tested on iOS 14.6 with the PROVISIONAL and NOT_DETERMINED auth statuses, works as expected.

@matt-auckland
Copy link
Author

Hi @dpa99c, I've run into an issue using your plugin where the Provisional status wasn't supported and I was unable to request permission for push notifications when the user was in that status.
This is my attempt to remedy the situation, based on my testing it works but, I'm not sure if my approach is correct.

@matt-auckland matt-auckland marked this pull request as ready for review June 9, 2021 21:39
@matt-auckland
Copy link
Author

I've updated the docs now. Please let me know if there's anything else you'd like to see in order to get this merged

@dpa99c dpa99c changed the base branch from master to dev August 27, 2021 13:06
@dpa99c dpa99c merged commit f1b9f0c into dpa99c:dev Aug 27, 2021
@matt-auckland matt-auckland deleted the add-missing-notif-status branch August 27, 2021 13:25
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.

2 participants