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

android: targetSdkVersion to 33, this time handling new notifs permission #5761

Merged
merged 1 commit into from
Sep 13, 2023

Conversation

chrisbobbe
Copy link
Contributor

@chrisbobbe chrisbobbe commented Sep 11, 2023

Tested on an emulated Android Pixel XL running Android 13, and on the office Android device (Samsung Galaxy S9 running Android 9).

Screen recording of the "rationale" modal coming soon.

(edit: just added the Fixes: #5453 line to the PR description, which I forgot, oops)

Fixes: #5753
Fixes: #5453

@chrisbobbe
Copy link
Contributor Author

Here's a screen recording of the "rationale" modal. To make it appear, I went to the system settings for Zulip and disabled notifications, and then I restarted the app. When /register finished, the rationale modal and then the permissions request appeared:

I tapped "Allow". Then I repeated the sequence: disable notifications in system settings and restart the app. The rationale and permissions prompt appeared again, and I tapped "Allow" again. I repeated the sequence two more times with the same result. I guess it makes sense that it always gave me the rationale and the prompt: if someone grants the permission and then immediately goes and revokes it, it seems like they might want an explanation of how the permission will be used, to help them decide.

If I tapped "Don't allow" instead of "Allow", the rationale and prompt did not appear on the next startup. Which is good. (In that case it would be more clearly spammy if it did.)

@gnprice
Copy link
Member

gnprice commented Sep 13, 2023

Thanks for taking care of this! This implementation looks good to me.

I also tested on my Android 13 phone, in order to see specifically the experience on upgrading an existing install (that had already had notification permission). The result was:

  • Immediately on starting the app, I got a notification prompt. (Not our rationale text, just the system dialog box.)
  • I said "allow".
  • I tested notifications, and they worked.

Merging.

…sion

We bumped this to 33 in 6f44474 / v27.210, but then reverted it in
2e196e4 because of zulip#5753, "Use the new POST_NOTIFICATIONS runtime
permission on Android".

Bump it again, this time with a fix for that issue: we declare the
new permission and add a runtime function call to request it in
context.

For users, this means:

- If on iOS: No change.

- If below Android 13: No change. A permission request follows the
  creation of a notification channel, so effectively it happens on
  app startup:
    https://developer.android.com/develop/ui/views/notifications/notification-permission#new-apps

- For a new app install on Android 13, the permission will now be
  requested in context instead of at startup:
    https://developer.android.com/develop/ui/views/notifications/notification-permission#new-apps

- For existing app installations when Android is upgraded to 13 or
  later, the permission should automatically be granted without a
  prompt as long as notifications hadn't been explicitly disabled
  before the upgrade. This should still work across a "backup and
  restore":
    https://developer.android.com/develop/ui/views/notifications/notification-permission#existing-apps
  (However: we are tracking zulip#5484 as an unrelated cause of
  notifications breaking across a "backup and restore".)

If RN and Android decide it's helpful, a "rationale" is shown to the
user just before the new in-context permission request. That's a
modal showing a message we provide (see added code), with an OK
button to close the modal and proceed to Android's plain Yes/No
dialog for the permission request.

Fixes: zulip#5753
@gnprice gnprice force-pushed the pr-target-sdk-version-33-with-notifs branch from a44f580 to 4fe472c Compare September 13, 2023 03:18
@gnprice gnprice merged commit 4fe472c into zulip:main Sep 13, 2023
@chrisbobbe
Copy link
Contributor Author

chrisbobbe commented Sep 13, 2023

Immediately on starting the app, I got a notification prompt. (Not our rationale text, just the system dialog box.)

Interesting! That's not actually what I would expect. I'd expect that notifications would "just work" in your case, without the prompt showing up, because you should have been "pre-granted" as described in the doc:

To minimize disruptions associated with the notification permission, the system automatically pre-grants the permission to all eligible apps when the user upgrades their device to Android 13 or higher. In other words, these apps can continue to send notifications to users, and users don't see a runtime permission prompt.

—Er, actually, reading that again, it's about upgrading to Android 13, which isn't what you did… 🤷 Anyway, not much harm in having to tap "Allow" even if you've already done it ages ago.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants