mirror of
https://github.com/vitorpamplona/amethyst.git
synced 2026-08-08 23:54:39 +00:00
fix(desktop): make Enable OS notifications button actually enable them
Two independent gaps meant the desktop notifications flow silently did nothing after the user clicked the settings button: 1. `LocalNotificationSettings` and `LocalNotificationReadState` were declared but never `.provides()`'d anywhere. Both `NotificationSettingsScreen` and `NotificationsScreen` fell back to a fresh `PreferencesNotificationSettings()` / `PreferencesNotification- ReadState()` instance each time — the java.util.prefs backing store kept the values consistent across cold restarts, but each Composable held its own `MutableStateFlow`, so a `setEnabled(true)` in Settings never fired the collectors in the inbox banner or in the auto- dispatcher. The user could toggle the master switch back and forth and nothing observable happened until the app was restarted. The auto-dispatcher in Main.kt was already using its own hoisted `notifSettings` instance too — just never handed down through the composition — so it never even saw the toggle. 2. The "Enable OS notifications" button (only shown on macOS when permission == NotRequested) called `dispatcher.requestPermission()` and updated `permissionState`, but it never touched `settings.enabled`. So the OS prompt appeared, user clicked Allow, the UI cheerfully said "Permission granted" — and toasts still didn't fire because the master switch was still off (defaults to false, first-launch UX choice). The button label promised the whole flow; the code did half of it. Fix: - Hoist `notifSettings` (already existed for the auto-dispatcher) into the app-level `CompositionLocalProvider` as `LocalNotificationSettings provides notifSettings`, so every consumer reads/writes the same instance. Same-instance sharing means StateFlow emissions actually propagate. - Add a per-account `PreferencesNotificationReadState`, keyed on `loggedIn?.pubKeyHex` via `remember(pubKeyHex)`, provided through `LocalNotificationReadState`. This also fixes the "Mark all as read" button in the inbox, which was `enabled = false` because the composition-local was always null. - In `NotificationSettingsScreen`, if `requestPermission()` returns Granted and the master switch is currently off, call `settings.setEnabled(true)` alongside setting the status message. The master switch UI observes `settings.enabled` and recomposes automatically, so clicking one button now does the whole handshake. Behaviour on other platforms is unchanged: Windows / Linux start in `PermissionState.NotApplicable`, so the "Enable OS notifications" branch never renders there — they see the "Send a test toast" button directly. The auto-enable is guarded by `!enabled`, so users who deliberately disabled the master switch and then re-granted permission (e.g. after denying in System Settings) don't get overridden if the switch was still on. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
@@ -1262,11 +1262,32 @@ private fun AppInner(
|
|||||||
// Auto-dispatcher: subscribes to newEventBundles and fires OS toasts.
|
// Auto-dispatcher: subscribes to newEventBundles and fires OS toasts.
|
||||||
// Only starts once the user is logged in — pubKey and settings must exist.
|
// Only starts once the user is logged in — pubKey and settings must exist.
|
||||||
val loggedIn = accountState as? AccountState.LoggedIn
|
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 =
|
val notifSettings =
|
||||||
remember {
|
remember {
|
||||||
com.vitorpamplona.amethyst.commons.moderation.notifications
|
com.vitorpamplona.amethyst.commons.moderation.notifications
|
||||||
.PreferencesNotificationSettings()
|
.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) {
|
DisposableEffect(loggedIn?.pubKeyHex, notifDispatcher, localCache) {
|
||||||
val myPk = loggedIn?.pubKeyHex
|
val myPk = loggedIn?.pubKeyHex
|
||||||
val autoDispatcherJob =
|
val autoDispatcherJob =
|
||||||
@@ -1294,6 +1315,8 @@ private fun AppInner(
|
|||||||
com.vitorpamplona.amethyst.desktop.service.drafts.LocalNoteDraftStore provides noteDraftStore,
|
com.vitorpamplona.amethyst.desktop.service.drafts.LocalNoteDraftStore provides noteDraftStore,
|
||||||
LocalHashtagSpamSettings provides hashtagSpamSettings,
|
LocalHashtagSpamSettings provides hashtagSpamSettings,
|
||||||
com.vitorpamplona.amethyst.desktop.ui.notifications.LocalNotificationDispatcher provides notifDispatcher,
|
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) {
|
when (accountState) {
|
||||||
is AccountState.Loading -> {
|
is AccountState.Loading -> {
|
||||||
|
|||||||
+12
-1
@@ -183,9 +183,20 @@ fun NotificationSettingsScreen(onBack: (() -> Unit)? = null) {
|
|||||||
} finally {
|
} finally {
|
||||||
requestingPermission = false
|
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 =
|
testStatus =
|
||||||
when (newState) {
|
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.Denied -> "Permission denied. Enable in System Settings if you change your mind."
|
||||||
PermissionState.BundleRequired -> "Notifications need a bundled app — run `./gradlew :desktopApp:runDistributable`."
|
PermissionState.BundleRequired -> "Notifications need a bundled app — run `./gradlew :desktopApp:runDistributable`."
|
||||||
else -> null
|
else -> null
|
||||||
|
|||||||
Reference in New Issue
Block a user