From f973c6cb967904d1f2ebcf83619bf992175cb223 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Tue, 20 Jan 2026 03:08:42 +0000 Subject: [PATCH] fix(ui): prevent permission dialogs from stacking on fresh install - Extract battery optimization check to separate method - Add sequential permission flow: notification permission completes before battery optimization dialog shows - Add toast feedback when user denies notification permission - Add toast feedback when user defers battery optimization - Add plan document for tracking this fix Fixes #114 Co-authored-by: wharris --- .../quarck/calnotify/ui/MainActivityBase.kt | 122 ++++++++++++++---- android/app/src/main/res/values/strings.xml | 4 + docs/dev_todo/permission_dialogs_stacking.md | 104 +++++++++++++++ 3 files changed, 205 insertions(+), 25 deletions(-) create mode 100644 docs/dev_todo/permission_dialogs_stacking.md diff --git a/android/app/src/main/java/com/github/quarck/calnotify/ui/MainActivityBase.kt b/android/app/src/main/java/com/github/quarck/calnotify/ui/MainActivityBase.kt index ab94b1294..7ac185381 100644 --- a/android/app/src/main/java/com/github/quarck/calnotify/ui/MainActivityBase.kt +++ b/android/app/src/main/java/com/github/quarck/calnotify/ui/MainActivityBase.kt @@ -27,6 +27,7 @@ import android.database.SQLException import android.net.Uri import android.os.Bundle import android.view.MenuItem +import android.widget.Toast import androidx.appcompat.app.AppCompatActivity import androidx.appcompat.widget.SearchView import com.github.quarck.calnotify.BuildConfig @@ -73,6 +74,9 @@ abstract class MainActivityBase : AppCompatActivity() { protected var calendarRescanEnabled = true protected var shouldRemindForEventsWithNoReminders = true protected var shouldForceRepost = false + + // Flag to track if battery optimization check should run after notification permission flow + private var pendingBatteryOptimizationCheck = false override fun onCreate(savedInstanceState: Bundle?) { super.onCreate(savedInstanceState) @@ -153,33 +157,17 @@ abstract class MainActivityBase : AppCompatActivity() { } } else { // Check notification permission (Android 13+) + // Battery optimization check is deferred until notification permission flow completes checkNotificationPermission() - - // Check for power manager optimisations - if (!settings.doNotShowBatteryOptimisationWarning && - !powerManager.isIgnoringBatteryOptimizations(BuildConfig.APPLICATION_ID)) { - AlertDialog.Builder(this) - .setTitle(getString(R.string.battery_optimisation_title)) - .setMessage(getString(R.string.battery_optimisation_details)) - .setPositiveButton(getString(R.string.you_can_do_it)) { _, _ -> - val intent = Intent() - .setAction(android.provider.Settings.ACTION_REQUEST_IGNORE_BATTERY_OPTIMIZATIONS) - .setData(Uri.parse("package:" + BuildConfig.APPLICATION_ID)) - startActivity(intent) - } - .setNeutralButton(getString(R.string.you_can_do_it_later)) { _, _ -> } - .setNegativeButton(getString(R.string.you_cannot_do_it)) { _, _ -> - settings.doNotShowBatteryOptimisationWarning = true - } - .create() - .show() - } } } /** * Check and request notification permission on Android 13+ (API 33). * This is required for the app to post notifications. + * + * After this permission flow completes (granted, denied, or already had permission), + * the battery optimization check will be triggered. */ protected fun checkNotificationPermission() { if (!PermissionsManager.hasNotificationPermission(this)) { @@ -189,16 +177,63 @@ abstract class MainActivityBase : AppCompatActivity() { .setMessage(R.string.notification_permission_explanation) .setCancelable(false) .setPositiveButton(android.R.string.ok) { _, _ -> + // Set flag so battery check runs after system permission dialog + pendingBatteryOptimizationCheck = true PermissionsManager.requestNotificationPermission(this) } .setNegativeButton(R.string.cancel) { _, _ -> - // User declined, they won't get notifications + // User declined - show feedback and proceed to battery check + Toast.makeText( + this, + R.string.notification_permission_denied_toast, + Toast.LENGTH_LONG + ).show() + checkBatteryOptimization() } .create() .show() } else { + // No rationale needed - request permission directly + // Set flag so battery check runs after system permission dialog + pendingBatteryOptimizationCheck = true PermissionsManager.requestNotificationPermission(this) } + } else { + // Notification permission already granted - proceed to battery check + checkBatteryOptimization() + } + } + + /** + * Check for battery optimization exemption and show dialog if needed. + * This is called after the notification permission flow completes to avoid + * stacking multiple dialogs. + */ + protected fun checkBatteryOptimization() { + if (!settings.doNotShowBatteryOptimisationWarning && + !powerManager.isIgnoringBatteryOptimizations(BuildConfig.APPLICATION_ID)) { + AlertDialog.Builder(this) + .setTitle(getString(R.string.battery_optimisation_title)) + .setMessage(getString(R.string.battery_optimisation_details)) + .setPositiveButton(getString(R.string.you_can_do_it)) { _, _ -> + val intent = Intent() + .setAction(android.provider.Settings.ACTION_REQUEST_IGNORE_BATTERY_OPTIMIZATIONS) + .setData(Uri.parse("package:" + BuildConfig.APPLICATION_ID)) + startActivity(intent) + } + .setNeutralButton(getString(R.string.you_can_do_it_later)) { _, _ -> + // User chose to defer - show gentle reminder + Toast.makeText( + this, + R.string.battery_optimization_later_toast, + Toast.LENGTH_SHORT + ).show() + } + .setNegativeButton(getString(R.string.you_cannot_do_it)) { _, _ -> + settings.doNotShowBatteryOptimisationWarning = true + } + .create() + .show() } } @@ -207,12 +242,49 @@ abstract class MainActivityBase : AppCompatActivity() { @NotNull permissions: Array, @NotNull grantResults: IntArray ) { - for (result in grantResults) { - if (result != PackageManager.PERMISSION_GRANTED) { - DevLog.error(LOG_TAG, "Permission is not granted!") + super.onRequestPermissionsResult(requestCode, permissions, grantResults) + + when (requestCode) { + PermissionsManager.PERMISSION_REQUEST_NOTIFICATIONS -> { + // Notification permission flow completed - now check battery optimization + if (pendingBatteryOptimizationCheck) { + pendingBatteryOptimizationCheck = false + + // Check if permission was denied and show feedback + val granted = grantResults.isNotEmpty() && + grantResults[0] == PackageManager.PERMISSION_GRANTED + if (!granted) { + DevLog.warn(LOG_TAG, "Notification permission denied by user") + Toast.makeText( + this, + R.string.notification_permission_denied_toast, + Toast.LENGTH_LONG + ).show() + } + + checkBatteryOptimization() + } + } + PermissionsManager.PERMISSION_REQUEST_CALENDAR -> { + // Calendar permission result - if granted, continue permission flow + val granted = grantResults.isNotEmpty() && + grantResults.all { it == PackageManager.PERMISSION_GRANTED } + if (granted) { + DevLog.info(LOG_TAG, "Calendar permission granted, continuing permission flow") + checkNotificationPermission() + } else { + DevLog.error(LOG_TAG, "Calendar permission denied!") + } + } + else -> { + // Log other permission results + for (result in grantResults) { + if (result != PackageManager.PERMISSION_GRANTED) { + DevLog.error(LOG_TAG, "Permission is not granted!") + } + } } } - super.onRequestPermissionsResult(requestCode, permissions, grantResults) } /** diff --git a/android/app/src/main/res/values/strings.xml b/android/app/src/main/res/values/strings.xml index 2ba192112..7e8bd6b54 100644 --- a/android/app/src/main/res/values/strings.xml +++ b/android/app/src/main/res/values/strings.xml @@ -457,8 +457,12 @@ Notification Permission This app needs permission to show notifications for your calendar events. Without this permission, you won\'t receive any calendar reminders. + Notifications disabled - you won\'t receive calendar reminders LATER DISMISS + + + Reminders may be delayed by battery optimization Always collapse Always collapse multiple notifications into large summary one diff --git a/docs/dev_todo/permission_dialogs_stacking.md b/docs/dev_todo/permission_dialogs_stacking.md new file mode 100644 index 000000000..3f4d37584 --- /dev/null +++ b/docs/dev_todo/permission_dialogs_stacking.md @@ -0,0 +1,104 @@ +# Permission Dialogs Stacking Fix + +**Issue:** [#114 - Bug: Perms dialogs may stack causing confusing UI](https://github.com/williscool/CalendarNotification/issues/114) + +**Status:** Implemented (awaiting testing) + +## Problem + +On fresh app install (Android 13+), multiple permission dialogs can appear simultaneously: + +1. After calendar permission is granted, `checkNotificationPermission()` is called (non-blocking) +2. Immediately after, battery optimization dialog check runs +3. Both dialogs stack on top of each other, confusing users + +The user sees a mess of overlapping dialogs and may: +- Accidentally dismiss the wrong one +- Get frustrated and force-close the app +- Deny everything just to make dialogs go away + +## Current Permission Flow + +``` +onResume() + └── checkPermissions() + ├── Calendar not granted → Show calendar dialog/request (BLOCKS) + └── Calendar granted + ├── checkNotificationPermission() ← NON-BLOCKING + └── Battery optimization check ← RUNS IMMEDIATELY (BUG!) +``` + +## Solution + +### Phase 1: Sequential Permission Flow (This PR) + +Make permission dialogs appear one at a time: + +``` +onResume() + └── checkPermissions() + ├── Calendar not granted → Show calendar dialog/request (BLOCKS) + └── Calendar granted → checkNotificationPermission() + ├── Already granted → checkBatteryOptimization() + ├── Shows rationale dialog → onDismiss → checkBatteryOptimization() + └── Shows system dialog → onRequestPermissionsResult → checkBatteryOptimization() +``` + +**Implementation:** +1. Extract battery check to `checkBatteryOptimization()` method +2. Add `pendingBatteryOptimizationCheck` flag +3. Modify `checkNotificationPermission()` to: + - If permission already granted, call `checkBatteryOptimization()` directly + - If showing rationale dialog, call `checkBatteryOptimization()` in dismiss callbacks + - If requesting system permission, set flag for `onRequestPermissionsResult` +4. In `onRequestPermissionsResult`, check flag and call `checkBatteryOptimization()` + +### Phase 2: User Feedback for Missing Permissions + +When user dismisses/denies permissions, provide clear feedback: + +**Notification Permission Denied:** +- Show toast: "Notifications disabled - you won't receive calendar reminders" +- On subsequent opens, show non-modal banner at top of event list + +**Calendar Permission Denied:** +- Already handled: Shows "Application has no calendar permissions" message +- App is essentially non-functional without this + +**Battery Optimization Not Exempted:** +- Show toast when "Later" tapped: "Reminders may be delayed by battery optimization" +- Already has "Don't show again" option + +### String Resources Needed + +```xml + +Notifications disabled - you won\'t receive calendar reminders +Reminders may be delayed by battery optimization + + +Please enable notifications to use this feature +``` + +## Testing Considerations + +The permission flow is difficult to test in instrumented tests because: +- System permission dialogs can't be easily controlled +- `shouldShowRequestPermissionRationale` behavior is OS-controlled + +Testing approach: +- Unit test the sequencing logic with mocked permission states +- Manual testing on fresh install +- Robolectric tests for permission state checking + +## Files to Modify + +- `android/app/src/main/java/com/github/quarck/calnotify/ui/MainActivityBase.kt` - Main fix +- `android/app/src/main/res/values/strings.xml` - New string resources +- Test files as needed + +## Notes + +- Calendar permission is already properly blocking (in if/else structure) +- The "persistent asking" on each resume is intentional - permissions ARE essential +- Battery optimization "Don't show again" is preserved - user can opt out permanently