Skip to content
This repository was archived by the owner on Mar 10, 2026. It is now read-only.

fix: notification settings UX improvements and in-app banner fixes#239

Closed
Just-Insane wants to merge 6 commits into7w1:devfrom
CloudHub-Social:pr/notifications-settings
Closed

fix: notification settings UX improvements and in-app banner fixes#239
Just-Insane wants to merge 6 commits into7w1:devfrom
CloudHub-Social:pr/notifications-settings

Conversation

@Just-Insane
Copy link
Collaborator

@Just-Insane Just-Insane commented Mar 8, 2026

Summary

Overhaul notification settings UX and fix several notification bugs.

Bug fixes

  • Push notifications always silentresolveSilent in the service worker now always returns false, leaving sound/vibration decisions entirely to the OS and Sygnal push gateway. The in-app sound setting no longer affects push behaviour.
  • In-app banner not appearing on desktop — The banner was erroneously gated behind a mobileOrTablet() check. It now fires on all platforms when In-App Notifications is enabled.
  • In-app banner placement — Rendered as position: fixed in ClientLayout (single render point, full viewport width, z-index: 9999). Doesn't displace any page content.
  • In-app banner shows "sent an encrypted message" — Events reaching the banner are already decrypted by the SDK. isEncryptedRoom: false is now passed so the actual message body is always shown when message content preview is enabled.
  • Desktop OS notifications not firing when page is hidden — The OS notification block now runs before the visibilityState !== 'visible' guard, which only gates the in-app banner and audio. Notifications now fire even when the browser window is minimised.
  • iOS lock screen media player — After HTMLAudioElement.play(), mediaSession.playbackState is cleared after a short delay, dismissing the lock screen widget. If in-app media (e.g. a video in a room) has since registered its own metadata the media session is left untouched.

Settings page improvements

  • Move badge display settings (Show Message Counts, Direct Messages Only, Always Count Mentions) from Appearance → Notifications.
  • Rename "Mobile In-App Notifications" → "In-App Notifications"; show the toggle on all platforms.
  • Rename "Notification Sound" → "In-App Notification Sound" to clarify it only controls in-page audio, not push sound.
  • Fix "System Notifications" description (removed incorrect claim that mobile uses the in-app banner).
  • Add notification levels info button (ⓘ) to All Messages, Special Messages, and Keyword Messages headings explaining Disable / Notify Silent / Notify Loud.
  • Add descriptive text under each notification section heading.
  • Collapse the two @room push rules into a single "Mention @room" control: routes to IsRoomMention (m.mentions, MSC3952) on modern servers, or AtRoomNotification on older servers. Synapse mirrors both rules so showing them separately caused an apparent sync loop where setting one immediately reset the other.
  • Add "Follows your global notification rules" subtitle to the "Default" option in the per-room notification switcher.
  • Default "In-App Notifications" to off for all platforms — users can opt in per-device in Notifications settings.
  • Default "Show Message Counts" (badge numbers) to off — badges show a number only for direct mentions by default (Discord-style).

Changed files

File Change
src/app/pages/client/ClientNonUIFeatures.tsx iOS media session clear; OS notifications before visibility guard; isEncryptedRoom: false for banner; remove mobileOrTablet() gate
src/app/pages/client/ClientLayout.tsx Single <NotificationBanner> render point
src/app/components/notification-banner/NotificationBanner.css.ts position: fixed; top: 0; left: 0; right: 0
src/app/features/settings/notifications/SystemNotification.tsx In-App toggle on all platforms; rename labels; add Badges section
src/app/features/settings/notifications/AllMessages.tsx Descriptive text + levels hint
src/app/features/settings/notifications/SpecialMessages.tsx Descriptive text + levels hint; collapse @room to single control
src/app/features/settings/notifications/KeywordMessages.tsx Descriptive text + levels hint
src/app/features/settings/notifications/NotificationLevelsHint.tsx New component — ⓘ popover
src/app/components/RoomNotificationSwitcher.tsx "Follows your global rules" subtitle on Default
src/app/features/settings/cosmetics/Themes.tsx Remove badge settings (moved to Notifications)
src/app/state/settings.ts useInAppNotifications: false; showUnreadCounts: false defaults
src/sw/pushNotification.ts resolveSilent = () => false — OS handles sound
src/sw.ts Remove notificationSoundEnabled from SW postMessage payload

@Just-Insane Just-Insane force-pushed the pr/notifications-settings branch 9 times, most recently from 4af672b to e5bf4a5 Compare March 8, 2026 07:22
@Just-Insane Just-Insane marked this pull request as ready for review March 8, 2026 07:24
@Just-Insane Just-Insane requested a review from 7w1 as a code owner March 8, 2026 07:24
@Just-Insane Just-Insane force-pushed the pr/notifications-settings branch 14 times, most recently from 02f731a to 498df6c Compare March 8, 2026 16:24
Evie Gauthier added 4 commits March 8, 2026 12:28
… audio, media session)

- resolveSilent = () => false — OS/Sygnal handles push sound entirely; the
  in-app sound setting no longer affects push notification sound
- remove notificationSoundEnabled from SW postMessage payload
- move OS notification block before visibilityState guard so desktop
  notifications fire even when the browser window is minimised
- pass isEncryptedRoom:false for in-app banner preview — events are already
  decrypted by the SDK so the actual message body is always shown
- call clearMediaSessionQuickly() after play() to dismiss the iOS lock screen
  media player; only clears playbackState if no real media metadata is set
- render NotificationBanner in ClientLayout so it spans the full viewport
  as position:fixed and doesn't displace page content
- remove the old banner render from Room.tsx
- rename "Mobile In-App Notifications" to "In-App Notifications"; show on
  all platforms (was mobile-only)
- rename "Notification Sound" to "In-App Notification Sound" to clarify it
  only controls in-page audio, not push sound
- move badge display settings (Show Unread Counts, Badge Counts for DMs
  Only, Show Unread Ping Counts) from Appearance to Notifications
- add NotificationLevelsHint info popover to All/Special/Keyword sections
  explaining Disable / Notify Silent / Notify Loud
- add descriptive text under each notification section heading
- show a single Mention @room control (IsRoomMention on modern servers,
  AtRoomNotification on old servers) — they are spec-mandated mirrors and
  exposing both as independent controls was confusing
- clarify @room push rule labels and descriptions
- add "Follows your global notification rules" subtitle to the Default
  option in the per-room notification switcher
@Just-Insane Just-Insane force-pushed the pr/notifications-settings branch from 498df6c to 43a86d4 Compare March 8, 2026 16:29
@Just-Insane Just-Insane force-pushed the pr/notifications-settings branch from 8268513 to 332dcfd Compare March 8, 2026 16:37
@7w1
Copy link
Owner

7w1 commented Mar 8, 2026

In-app notifs should probably be enabled by default on mobile. Also the changeset needs to be like 1 one, or multiple files detailing the individual updates. Tbh, this might be better split up into multiple PRs it's kind of a lot of changes all at once for things that very likely have edge cases.

@Just-Insane
Copy link
Collaborator Author

Just-Insane commented Mar 8, 2026

Sounds good, I was thinking it was quite large.

This PR has been split into four focused draft PRs, each with its own changeset:

PR Scope
#252 — fix: notification delivery bugs SW resolveSilent, OS notification timing, in-app banner platform gate, iOS media session, encrypted banner text
#253 — fix: in-app notification banner placement Move NotificationBanner to ClientLayout as position: fixed, remove from Room.tsx
#254 — feat: notification settings page improvements Label renames, badge settings move, @room rule collapse, levels hint, descriptive text, per-room switcher subtitle
#255 — fix: default badge unread counts to off showUnreadCounts: true → false (Discord-style); useInAppNotifications left as mobileOrTablet() per your feedback

This PR (#239) can be closed once the individual ones are reviewed.

@Just-Insane
Copy link
Collaborator Author

Closed as superseded (split out into smaller PRs)

@Just-Insane Just-Insane closed this Mar 8, 2026
@Just-Insane Just-Insane deleted the pr/notifications-settings branch March 8, 2026 18:49
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants