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

feat(ios): add forceRegister option #337

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

Conversation

JGreenlee
Copy link

@JGreenlee JGreenlee commented Dec 19, 2024

Description

ios.forceRegister

boolean

false

Optional. If `true` the app will register for remote notifications, even if the user has denied notification permissions. On iOS, notification permissions only control *user-facing* notifications – background pushes can still be received.

On iOS, notification permissions only control user-facing notifications. Silent/invisible notifications can be received and handled in the background even when the user denies notification permissions.

When forceRegister is true, registerForRemoteNotifications will be called on init even if requestAuthorizationWithOptions comes back with granted=FALSE.

Related Issue

#335

Motivation and Context

On iOS, notification permissions only control user-facing notifications. Silent/invisible notifications may be received and handled in the background even when the user denies notification permissions.

However this plugin does provide a way to register for remote notifications if the user has denied UserNotifications.

How Has This Been Tested?

In my app, I observe that without forceRegister, if the user denies notification permissions prior to PushNotification.init(), neither push.on('registration', ...) nor push.on('error', ...) are invoked.

When forceRegister = true, and the same scenario, push.on('registration', ...) is called and an APNS token is established.

When forceRegister = false or not defined, the behavior is the same as before.

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

On iOS, notification permissions only control *user-facing* notifications. Silent/invisible notifications can be received and handled in the background even when the user denies notification permissions.

When forceRegister is true, registerForRemoteNotifications will be called on init even if requestAuthorizationWithOptions comes back with granted=FALSE.

| `ios.forceRegister`  | `boolean` | `false` | Optional. If `true` the app will register for remote notifications, even if the user has denied notification permissions. On iOS, notification permissions only control *user-facing* notifications – background pushes can still be received. |
@erisu
Copy link

erisu commented Dec 20, 2024

What if the granted conditional wrappers were removed instead of introducing a new flag?

I'm trying to determine if there are viable use cases where an app developer might prefer not to support background notifications by default.

In the current setup, only users who accept disruptive permissions (e.g., sounds, badges, alerts) can receive normal and background notifications. However, since background notifications can be used to update the app in the background and does not require permissions, why restrict this functionality based on the denial of disruptive permissions?

@JGreenlee
Copy link
Author

I'm trying to determine if there are viable use cases where an app developer might prefer not to support background notifications by default.

I can't think of any cases but I thought it best not to introduce any breaking changes, at least not without a discussion first.

Perhaps the strongest argument for keeping this behind a flag is that it represents a departure from Android. How important is it for this plugin to behave the same on both platforms by default?

@erisu
Copy link

erisu commented Dec 20, 2024

How important is it for this plugin to behave the same on both platforms by default?

Ideally, both platforms should behave the same by default.

I think keeping this behind a flag is acceptable for now.

I believe Android can support the same process of registering and receiving background notifications, even if the user denies notification permissions. However, it might require some investigation and adjustments to make it work properly, as the current initialization process seems to return early if permissions are denied. Eventually, we can look into expanding support for Android as well.

Once both platforms support the same functionality, we can reassess whether the flag is still needed in a future major release.

As for the flag’s description, how would the following update sound?

Optional. If set to true, the app will continue to register an APNs token for remote notifications. Notification permissions only control disruptive user-facing notifications, such as alerts, sounds, badges, and critical notifications. Even if the user denies these permissions, background push notifications can still be received.

Additionally, I’m considering adding this statement at the end:

If the user disables "Background App Refresh," silent push notifications will be blocked.

@JGreenlee
Copy link
Author

As for the flag’s description, how would the following update sound?

Optional. If set to true, the app will continue to register an APNs token for remote notifications. Notification permissions only control disruptive user-facing notifications, such as alerts, sounds, badges, and critical notifications. Even if the user denies these permissions, background push notifications can still be received.

Additionally, I’m considering adding this statement at the end:

If the user disables "Background App Refresh," silent push notifications will be blocked.

Sure, feel free to wordsmith that however you'd like

docs/API.md Outdated Show resolved Hide resolved
@erisu erisu changed the title add ios.forceRegister option feat(ios): add forceRegister option Dec 20, 2024
@erisu erisu added enhancement New feature or request ios Related to the iOS platform labels Dec 20, 2024
@erisu erisu added this to the 5.1.0 milestone Dec 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request ios Related to the iOS platform
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants