fix: turn on notification defaults and prompt on first DM visit #284

2026-05-26T06:10:52Z

userAdityaa commented

2026-05-26 06:10:52 +00:00

Summary

Default notification settings (push, sound, badge) are now enabled for new users; they can disable them under Settings → Alerts.
On the first visit to a direct message conversation, if notifications are not actually working (settings off or OS permission/token missing), show a modal to enable notifications via the existing Push.request() flow.

Screen shot

First time dming someone, if notification not enabled

closes #257

### Summary - Default notification settings (`push`, `sound`, `badge`) are now enabled for new users; they can disable them under Settings → Alerts. - On the first visit to a direct message conversation, if notifications are not actually working (settings off or OS permission/token missing), show a modal to enable notifications via the existing `Push.request()` flow. ### Screen shot * First time dming someone, if notification not enabled <img width="1439" alt="Screenshot 2026-05-26 at 11.39.49 AM.png" src="attachments/0a6a8133-e59b-413d-a2d9-e27809031112"> closes #257

Screenshot 2026-05-26 at 11.39.49 AM.png

202 KiB

userAdityaa force-pushed fix/notification-defaults-dm-prompt from 0bbff458bd to 36b09eb8d5

2026-05-26 06:14:14 +00:00

Compare

hodlbod reviewed 2026-05-26 15:49:32 +00:00

src/app/util/notifications.ts Outdated

						
				@@ -21,1 +23,3 @@

				export {Push} from "@app/util/push"

				import {Push} from "@app/util/push"

				export {Push}

hodlbod commented

2026-05-26 15:39:26 +00:00

Never re-export, always update imports

👍 1

src/app/util/notifications.ts Outdated

						
				@@ -22,0 +36,4 @@

				  }

				  return Notification?.permission === "granted"

				}

hodlbod commented

2026-05-26 15:43:26 +00:00

This is very badly named, because it checks messages specifically. That logic should be inlined, this should only check push/token/permission. This should also live in push, not notifications (which handles badges/checked state).

This is very badly named, because it checks messages specifically. That logic should be inlined, this should only check push/token/permission. This should also live in `push`, not `notifications` (which handles badges/checked state).

👍 1

src/app/util/notifications.ts Outdated

						
				@@ -22,0 +56,4 @@

				  })

				  return true

				}

hodlbod commented

2026-05-26 15:45:59 +00:00

Also badly named, because this only enables messaging notifications. Just inline all this logic.

👍 1

src/app/util/notifications.ts Outdated

						
				@@ -22,0 +62,4 @@

				  key: "dmNotificationsPrompted",

				  defaultValue: false,

				  storage: kv,

				})

hodlbod commented

2026-05-26 15:47:27 +00:00

Put this in a script context="module" in the component where it's used.

Put this in a `script context="module"` in the component where it's used.

👍 1

src/routes/chat/[chat]/+page.svelte Outdated

						
				@@ -11,0 +17,4 @@

				  onMount(() => {

				    if (!areNotificationsEnabled() && !get(dmNotificationsPrompted)) {

				      dmNotificationsPrompted.set(true)

				      pushModal(EnableNotificationsPrompt)

hodlbod commented

2026-05-26 15:49:29 +00:00

I don't think we need the modal component, just do:

if (!$dmNotificationsPrompted) {
  dmNotificationsPrompted.set(true)
  enableDMNotificationsAndShowToast() // inline the logic
}

I don't think we need the modal component, just do: ``` if (!$dmNotificationsPrompted) { dmNotificationsPrompted.set(true) enableDMNotificationsAndShowToast() // inline the logic } ```

👍 1

hodlbod marked this conversation as resolved

userAdityaa force-pushed fix/notification-defaults-dm-prompt from 36b09eb8d5 to a63734fa1d

2026-05-27 15:02:29 +00:00

Compare

hodlbod reviewed 2026-05-27 17:41:23 +00:00

src/app/core/state.ts

						
				@@ -422,1 +420,3 @@

				    badge: false,

				    push: true,

				    sound: true,

				    badge: true,

hodlbod commented

2026-05-27 17:38:32 +00:00

This can't default to true, because push is only true if we've successfully requested permissions.

👍 1

hodlbod commented

2026-05-28 22:25:35 +00:00

Let's keep sound off too, that's an optional add on. Web users will still be notified about push

hodlbod marked this conversation as resolved

src/routes/chat/[chat]/+page.svelte Outdated

						
				@@ -10,1 +27,4 @@

				  const pubkeys = uniq(append($pubkey!, splitChatId(chat)))

				  onMount(() => {

				    if (!get(dmNotificationsPrompted)) {

hodlbod commented

2026-05-27 17:37:19 +00:00

This can be $dmNotificationsPrompted if it's in a svelte component

This can be `$dmNotificationsPrompted` if it's in a svelte component

👍 1

hodlbod marked this conversation as resolved

src/routes/chat/[chat]/+page.svelte

						
				@@ -11,0 +31,4 @@

				      dmNotificationsPrompted.set(true)

				      const enableDMNotificationsAndShowToast = async () => {

				        const {push, messages} = notificationSettings.get()

hodlbod commented

2026-05-27 17:38:46 +00:00

$notificationSettings. But don't actually check this, we want to turn these on here; this guard will make this function a no-op.

`$notificationSettings`. But don't actually check this, we want to turn these on here; this guard will make this function a no-op.

👍 1

hodlbod marked this conversation as resolved

src/routes/chat/[chat]/+page.svelte

						
				@@ -11,0 +39,4 @@

				          } else {

				            if (Notification?.permission === "granted") return

				          }

				        }

hodlbod commented

2026-05-27 17:40:02 +00:00

All of this should be handled by the push adapters. This should be as simple as calling Push.request() below.

All of this should be handled by the push adapters. This should be as simple as calling `Push.request()` below.

👍 1

hodlbod marked this conversation as resolved

src/routes/chat/[chat]/+page.svelte

						
				@@ -11,0 +58,4 @@

				          badge: true,

				          sound: Capacitor.isNativePlatform() ? current.sound : true,

				          messages: true,

				        })

hodlbod commented

2026-05-27 17:40:23 +00:00

notificationSettings.update would be more idiomatic here

`notificationSettings.update` would be more idiomatic here

👍 1

src/routes/chat/[chat]/+page.svelte

						
				@@ -11,0 +63,4 @@

				        pushToast({message: "Notifications enabled!"})

				      }

				      enableDMNotificationsAndShowToast()

hodlbod commented

2026-05-27 17:38:12 +00:00

Unless we need to return an unsubscribe function from onMount, we can just make the closure async and avoid the define/call function pattern.

Unless we need to return an unsubscribe function from onMount, we can just make the closure `async` and avoid the define/call function pattern.

👍 1

userAdityaa force-pushed fix/notification-defaults-dm-prompt from a63734fa1d to f3763eee7e

2026-05-28 06:43:24 +00:00

Compare

hodlbod reviewed 2026-05-28 22:26:55 +00:00

hodlbod left a comment

Looks pretty good, just a couple small things.

src/routes/chat/[chat]/+page.svelte

						
				@@ -11,0 +42,4 @@

				        ...current,

				        push: true,

				        badge: true,

				        sound: Capacitor.isNativePlatform() ? current.sound : true,

hodlbod commented

2026-05-28 22:26:42 +00:00

Leave badge and sound alone I think, push is enough.

👍 1

hodlbod marked this conversation as resolved

userAdityaa force-pushed fix/notification-defaults-dm-prompt from f3763eee7e to ddf9358703

2026-05-29 05:05:33 +00:00

Compare

hodlbod added 1 commit 2026-05-29 15:12:50 +00:00

fix: turn on notification defaults and prompt on first DM visit cbe311262a

hodlbod force-pushed fix/notification-defaults-dm-prompt from ddf9358703 to cbe311262a

2026-05-29 15:12:50 +00:00

Compare

hodlbod merged commit 8dd278f47c into dev

2026-05-29 15:13:11 +00:00

hodlbod deleted branch fix/notification-defaults-dm-prompt

2026-05-29 15:13:11 +00:00

Sign in to join this conversation.

2 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: coracle/flotilla#284

Allow edits from maintainers