Merge PR: fix(desktop): make Enable OS notifications button actually enable them

Merges nostr proposal 82e72369 into main:
- Provide LocalNotificationSettings / LocalNotificationReadState at the app
  root. Both CompositionLocals were already consumed by NotificationsScreen
  and NotificationSettingsScreen but never provided, so each screen fell back
  to its own PreferencesNotificationSettings(); disk prefs stayed in sync but
  the in-memory StateFlow updates never crossed instances.
- Flip the master toggle when the OS permission is granted, so the "Enable OS
  notifications" button matches its label end to end.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Vitor Pamplona
2026-07-28 19:06:05 -04:00
co-authored by Claude Opus 5
2 changed files with 35 additions and 1 deletions
@@ -1262,11 +1262,32 @@ private fun AppInner(
// Auto-dispatcher: subscribes to newEventBundles and fires OS toasts.
// Only starts once the user is logged in — pubKey and settings must exist.
val loggedIn = accountState as? AccountState.LoggedIn
// Single, process-wide NotificationSettings instance. Hoisted here (not
// inside NotificationSettingsScreen / NotificationsScreen) so the
// settings UI, the inbox column's OS-toasts banner, and the auto-
// dispatcher all read + write the same MutableStateFlow. Without this,
// each screen fell back to its own PreferencesNotificationSettings()
// instance — the disk-backed prefs stayed in sync, but the in-memory
// StateFlow updates never crossed instances, so toggling the master
// switch or granting permission never actually notified the auto-
// dispatcher until the app restarted.
val notifSettings =
remember {
com.vitorpamplona.amethyst.commons.moderation.notifications
.PreferencesNotificationSettings()
}
// Per-account read-state for the notification inbox. Keyed on
// pubKey so switching accounts gets a fresh cursor. `remember` here
// is intentionally keyed on pubKeyHex; a null pubKey (Loading /
// LoggedOut branches never render the settings screen anyway, but
// guard against a stale ReadState leaking between accounts).
val notifReadState =
remember(loggedIn?.pubKeyHex) {
loggedIn?.pubKeyHex?.let { pk ->
com.vitorpamplona.amethyst.commons.moderation.notifications
.PreferencesNotificationReadState(pk)
}
}
DisposableEffect(loggedIn?.pubKeyHex, notifDispatcher, localCache) {
val myPk = loggedIn?.pubKeyHex
val autoDispatcherJob =
@@ -1294,6 +1315,8 @@ private fun AppInner(
com.vitorpamplona.amethyst.desktop.service.drafts.LocalNoteDraftStore provides noteDraftStore,
LocalHashtagSpamSettings provides hashtagSpamSettings,
com.vitorpamplona.amethyst.desktop.ui.notifications.LocalNotificationDispatcher provides notifDispatcher,
com.vitorpamplona.amethyst.desktop.ui.notifications.LocalNotificationSettings provides notifSettings,
com.vitorpamplona.amethyst.desktop.ui.notifications.LocalNotificationReadState provides notifReadState,
) {
when (accountState) {
is AccountState.Loading -> {
@@ -183,9 +183,20 @@ fun NotificationSettingsScreen(onBack: (() -> Unit)? = null) {
} finally {
requestingPermission = false
}
// Match the button label: "Enable OS notifications"
// must actually enable them end-to-end. Prior to this,
// clicking through the OS prompt granted permission
// but left the master toggle OFF, so the auto-
// dispatcher stayed muted and the user had to hunt
// for the switch above. Flip it here on grant — the
// switch UI observes settings.enabled and
// recomposes automatically.
if (newState == PermissionState.Granted && !enabled) {
settings.setEnabled(true)
}
testStatus =
when (newState) {
PermissionState.Granted -> "Permission granted. Try the test toast below."
PermissionState.Granted -> "Permission granted. Notifications are on — try the test toast below."
PermissionState.Denied -> "Permission denied. Enable in System Settings if you change your mind."
PermissionState.BundleRequired -> "Notifications need a bundled app — run `./gradlew :desktopApp:runDistributable`."
else -> null