ankidroid
feat(reminders): request notification permissions
Purpose / Description
Creates a BottomSheet for requesting notification permissions from the user. This BottomSheet shows up when the user first triggers a sync and when the user first creates a review reminder, as these are the two features that rely on notifications. Also refactors some of the backend Permissions logic to ensure we don't try to launch the system UI permissions request dialog when we know it will fail (i.e. when the user has conclusively denied us a permission), as that might decrease our Play Store discoverability.
For more details, read the extended discussion below or the commit messages, which I've tried to make as detailed as possible.
UI
Fixes
- Fixes https://github.com/ankidroid/Anki-Android/issues/15952
- Part of GSoC 2025: Review Reminders
Approach
- Creates a new PermissionsBottomSheet. My intention is: mandatory permissions -> use PermissionsActivity; optional permissions -> use PermissionsBottomSheet. I think the BottomSheet is less intrusive compared to the full-screen PermissionsActivity, as befits an optional permission.
- For more details on thoughts and reasoning, see the detailed discussion in this PR thread.
How Has This Been Tested?
- Physical Samsung S23, API 34.
- Medium Phone, API 35.
- A fresh install of the app does not trigger the notifications request dialog upon logging in (sometimes LeakCanary will cause the system UI dialog to show up, but that's unrelated to our codebase)
- Syncing for the first time causes the BottomSheet to show up. Clicking on the button triggers the system UI dialog or directs the user to Settings instead if anything goes wrong.
- Adding the first review reminder also causes the BottomSheet to show up, as above.
- Rejecting notification permissions twice in a row causes the BottomSheet dialog to never show up again
- Can register for notifications through the review reminders screen or the sync user flow
- PermissionsExplanationFragment still works as expected and has full functionality
- Clicking on a permission toggle when it is enabled offers the ability to revoke it from the OS settings, like before
- Audio permission requesting in PermissionsExplanationFragment also safely falls back to opening the OS settings screen after two rejections
- Viewing a permission request BottomSheet and dismissing it without pressing the toggle does not count as "denying" the permission and future BottomSheets will still trigger.
Learning (optional, can help others)
- Spent a lot of time stuck on a bug involving fragmentManagers, but managed to figure it out.
- Curse
shouldShowPermissionRequestRationale!!
Checklist
- [x] You have a descriptive commit message with a short title (first line, max 50 chars).
- [x] You have commented your code, particularly in hard-to-understand areas
- [x] You have performed a self-review of your own code
- [x] UI changes: include screenshots of all affected screens (in particular showing any new or changed strings)
- [x] UI Changes: You have tested your change using the Google Accessibility Scanner
[!IMPORTANT] Maintainers: This PR contains https://github.com/ankidroid/Anki-Android/labels/Strings changes
- Sync Translations before merging this PR and wait for the action to complete
- Review and merge the auto-generated PR in order to sync all user-submitted translations
- Sync Translations again and merge the PR so the huge automated string changes caused by merging this PR are by themselves and easy to review
![github-actions[bot] avatar](https://avatars.githubusercontent.com/in/15368?v=4 "Published by a pub.dev verified publisher"){kind=small}
- Fixed the headline of the BottomSheet, which was hardcoded for some reason. It now reads:
AnkiDroid works best with these permissions. - Rebased on top of https://github.com/ankidroid/Anki-Android/pull/18978
- Rebased. The optional permissions header has been moved to https://github.com/ankidroid/Anki-Android/pull/18978, which this PR is based on top of.
- Rebased and resolved merge conflicts.
- Now only blocked by https://github.com/ankidroid/Anki-Android/pull/19109.
Ashish has requested that I move the close button to the top right corner, which makes sense to me. I'll also look into potentially turning it into an "X" icon.
- Moved the close button to the top right corner and changed it into an "X" icon. I personally think this looks cleaner.
- Adjusted some margins.
- See updated UI in PR description!
Ready for review!
Rebased and resolved merge conflicts!
@criticalAY I noticed you made major changes to the login screens in your recently-merged PR. I've resolved my merge conflicts and re-implemented my feature by deleting some of your code. Please let me know if I've accidentally modified anything important that you've written -- I think I was pretty careful, though!
Also, @mikehardy, any thoughts on shouldShowRequestPermissionRationale over here?
Reviewing this one now and the only thought I have on the description (before I get in to code) is related to this:
In the future, I may add a help screen to help users who are confused why they are not getting notifications and who may have notifications accidentally disabled
I think a sticky warning / orange top bar where ever notifications are configured that says "AnkiDroid notifications are off. Tap here to turn them on.." (or similar) might be the easiest way to handle them. It will never annoy people that turned them off and don't want them. But for anyone interested enough to go in the reminder settings (maybe wondering why they aren't showing up...) it will be a very visible indicator of what's going on
?
On me -- will try to respond and revise this PR this weekend, but no promises as I'm entering final exam season where I'm at. Thanks!!
(First, a small sidenote: when I say that the user has "conclusively" rejected a permission, that means on API <30 that they have clicked "don't ask again" on the system UI dialog, whereas on API 30+, that means that they have clicked "deny" on the system UI dialog twice.)
Major changes to this PR! Here's an explanation of my thought process / how all the changes came to be:
Previously, when the user expressed a desire to grant notification permissions, we would eagerly launch the system UI permission request dialog. However, if the user had already conclusively rejected the permission, this would fail. To handle this, I added a callback to the system UI dialog launcher to check if the permission was actually granted. If it wasn't, then I would open the OS settings menu and ask the user to manually grant the permission. The assumption I made was that no reasonable user would click to toggle a reminder on (in the AnkiDroid UI's PermissionsBottomSheet) and then immediately press "deny" afterwards.
However, as Mike has pointed out, this has multiple flaws. First, my assumption above might be wrong: some users might legitimately click on the BottomSheet toggle, then change their minds after pressing the system UI text and click "deny". Second, as Mike says, if we add more places in the app where the BottomSheet can be launched from, we'll start to hit scenarios where the user encounters all three launch points and clicks "deny" at all three. The first two rejections will cause the rejection to be conclusive, and the third one will need to fall back to the OS settings launch. And, in general, we should not be pestering the user with three permission request BottomSheets if they've already denied it twice.
The fact that the OS settings launches is not necessarily bad: I would argue that if the user clicks on the toggle for the third occurrence of a notifications permission BottomSheet, they probably know what they're doing and have perhaps changed their minds about granting notification permissions, so we should ignore Android telling us they've "conclusively" rejected the permission and offer them the OS settings menu. The issue is performing this OS settings screen launch after first trying to launch the system UI permissions request dialog. As Mike says, launching that dialog when Android views the user as having conclusively denied the permission is likely stored in Play Console statistics and may decrease Play Store discoverability.
The obvious solution is to check if the user has conclusively denied the permission before trying to launch the dialog, rather than relying on the fallback callback after the system UI dialog attempts to open. However, this requires having some way to determine conclusive rejection, which -- as Mike as noted -- is impossible without internal Pref state management given the limited power of shouldShowRequestPermissionRationale. #ThanksAndroid
Fair enough. Well, since I've already added reminderNotifsRequestShown and syncNotifsRequestShown Prefs in this PR to check if the notification request BottomSheet has been shown, we can just check if at least one of them is set to true, as Mike suggested, right?
Not so fast. Those flags check if the BottomSheet has been launched, not if the system UI dialog has been triggered. A user might trigger the BottomSheet and then immediately dismiss it without clicking on the system UI dialog at all. That would cause Mike's suggested code to believe that permissions have been conclusively rejected, as shouldShowRequestPermissionRationale will still return false since the permission has never actually been "requested" yet (due to the system UI dialog not launching).
Hence we need a new Prefs flag to check if the system UI dialog has been launched for notification permission requesting. With this flag now in place, we are able to distinguish State 2 and State 4, which Mike identified here.
However, while implementing this change, I noticed that the audio recording permission is also very similar to the notifications permission. Via AllPermissionsExplanationFragment, the user is able to repeatedly click and deny it, also potentially leading to a decrease in Play Store discoverability. Hence, I also implemented a new Prefs flag to track if the system UI permissions request dialog has been launched for the audio recording permission and refactored AllPermissionsExplanationFragment.
Having done that, it became apparent that I had substantial duplicated code. I've decided to abstract out my work so that future developers can implement further permission requests similarly; hence the creation of isUserOpenToPermission and requestPermissionThroughDialogOrSettings. With that all in place, the permissions request launcher is now never triggered in any scenario if the user has already conclusively denied the permission; instead, the OS settings menu is immediately launched.
This also then nicely allowed me to implement Mike's suggested warning dialog that persistently shows up in the ScheduleReminders fragment if notification permissions are not granted. I've decided to implement it using a Snackbar, as that feels like the simplest out-of-the-box solution. An "enable" button on the Snackbar, when pressed, calls requestPermissionThroughDialogOrSettings.
A final wrinkle: helper methods for launching the system UI permission request dialog used to be protected within PermissionsFragment, but since ScheduleReminders now needs to use them, I've moved them to Permissions and made them public.
Okay! I think that explains all the changes here. Ready for review!
Fair enough. Well, since I've already added reminderNotifsRequestShown and syncNotifsRequestShown Prefs in this PR to check if the notification request BottomSheet has been shown, we can just check if at least one of them is set to true, as Mike suggested, right?
Not so fast. Those flags check if the BottomSheet has been launched, not if the system UI dialog has been triggered
Ah ha!
This in particular
Those flags check if the BottomSheet has been launched, not if the system UI dialog has been triggered
I did not figure that out, so yes - my assumption and initial proposal was incorrect. Based on your comment describing the changes it appears you found your way to a good state. Will go through the code now