From d53aea40323e6089eeca5936552f6d61bcfb4741 Mon Sep 17 00:00:00 2001 From: Nameless 7778777 <7778777@7778777.online> Date: Mon, 3 Aug 2026 20:44:26 +0200 Subject: [PATCH 1/3] Add open-tab system notifications Real OS Notification banners whenever Impro is open in a tab (foreground or background), driven entirely by the existing 10s polling in NotificationService/ChatNotificationService. No backend, no new dependencies. SystemNotificationService dedupes on the last-seen count so it only fires when a count goes up, gated by an opt-in localStorage flag, actual browser permission, and the existing snooze setting. New Settings > Notifications page primes the browser's permission prompt with an explanatory confirm dialog first, since permission prompts must originate from a user gesture and shouldn't surprise anyone. Bluesky's own app.bsky.notification.registerPush was evaluated and ruled out for this: it only forwards registration to a service DID the caller operates, so it doesn't tell a third party when something actually happens - it wouldn't have saved us from needing this kind of client-side detection regardless. Co-Authored-By: Claude Sonnet 5 --- src/js/app.js | 24 ++ src/js/systemNotificationService.js | 92 +++++++ src/js/views/settings.view.js | 7 + src/js/views/settings/notifications.view.js | 110 ++++++++ tests/e2e/specs/views/settings.view.test.js | 7 +- .../views/settingsNotifications.view.test.js | 148 ++++++++++ .../specs/systemNotificationService.test.js | 254 ++++++++++++++++++ 7 files changed, 640 insertions(+), 2 deletions(-) create mode 100644 src/js/systemNotificationService.js create mode 100644 src/js/views/settings/notifications.view.js create mode 100644 tests/e2e/specs/views/settingsNotifications.view.test.js create mode 100644 tests/unit/specs/systemNotificationService.test.js diff --git a/src/js/app.js b/src/js/app.js index c63b9e8b..29795c92 100644 --- a/src/js/app.js +++ b/src/js/app.js @@ -24,6 +24,7 @@ import settingsMutedWordsView from "/js/views/settings/mutedWords.view.js"; import settingsBlockedAccountsView from "/js/views/settings/blockedAccounts.view.js"; import settingsMutedAccountsView from "/js/views/settings/mutedAccounts.view.js"; import settingsAdvancedView from "/js/views/settings/advanced.view.js"; +import settingsNotificationsView from "/js/views/settings/notifications.view.js"; import installedPluginsView from "/js/views/installedPlugins.view.js"; import pluginSettingsView from "/js/views/pluginSettings.view.js"; import communityPluginsView from "/js/views/communityPlugins.view.js"; @@ -42,6 +43,7 @@ import { Api } from "/js/api.js"; import { auth } from "/js/auth.js"; import { NotificationService } from "/js/notificationService.js"; import { ChatNotificationService } from "/js/chatNotificationService.js"; +import { SystemNotificationService } from "/js/systemNotificationService.js"; import { PostComposerService } from "/js/postComposerService.js"; import { AccountSwitcherService } from "/js/accountSwitcherService.js"; import { ReportService } from "/js/reportService.js"; @@ -118,6 +120,13 @@ export async function main() { const chatNotificationService = session ? new ChatNotificationService(api) : null; + const systemNotificationService = + notificationService && chatNotificationService + ? new SystemNotificationService( + notificationService, + chatNotificationService, + ) + : null; const postComposerService = session ? new PostComposerService(dataLayer, identityResolver, pluginService, { draftsEnabled: await checkDraftsEnabled(), @@ -191,6 +200,7 @@ export async function main() { identityResolver, notificationService, chatNotificationService, + systemNotificationService, postComposerService, accountSwitcherService, reportService, @@ -233,6 +243,15 @@ export async function main() { }); } + if (systemNotificationService) { + effect( + () => { + systemNotificationService.checkForUpdates(); + }, + { debugName: "systemNotifications" }, + ); + } + router.addRoute(["/", "/intent/compose"], () => homeView, { layoutOptions: { activeNavItem: "home" }, }); @@ -307,6 +326,11 @@ export async function main() { () => settingsAppearanceView, settingsRouteOptions, ); + router.addRoute( + "/settings/notifications", + () => settingsNotificationsView, + settingsRouteOptions, + ); router.addRoute( "/settings/muted-words", () => settingsMutedWordsView, diff --git a/src/js/systemNotificationService.js b/src/js/systemNotificationService.js new file mode 100644 index 00000000..10dbea5a --- /dev/null +++ b/src/js/systemNotificationService.js @@ -0,0 +1,92 @@ +const STORAGE_KEY = "system-notifications-enabled"; +const ICON_URL = "/img/impro-logo-192.png"; + +export class SystemNotificationService { + constructor(notificationService, chatNotificationService) { + this.notificationService = notificationService; + this.chatNotificationService = chatNotificationService; + this._lastSeenActivityCount = + notificationService.$numNotifications.get() ?? 0; + this._lastSeenChatCount = + chatNotificationService.$numNotifications.get() ?? 0; + } + + get isSupported() { + return typeof Notification !== "undefined"; + } + + get isEnabled() { + return localStorage.getItem(STORAGE_KEY) === "true"; + } + + get permissionState() { + return this.isSupported ? Notification.permission : "unsupported"; + } + + async requestPermission() { + if (!this.isSupported) return "unsupported"; + const result = await Notification.requestPermission(); + if (result === "granted") { + localStorage.setItem(STORAGE_KEY, "true"); + } + return result; + } + + disable() { + localStorage.removeItem(STORAGE_KEY); + } + + notify({ title, body, tag, url }) { + if ( + !this.isSupported || + !this.isEnabled || + Notification.permission !== "granted" || + this.notificationService.isSnoozed + ) { + return; + } + const notification = new Notification(title, { + body, + icon: ICON_URL, + badge: ICON_URL, + tag, + }); + notification.onclick = () => { + window.focus(); + window.location.href = url; + notification.close(); + }; + } + + checkForUpdates() { + const activityCount = this.notificationService.$numNotifications.get() ?? 0; + const chatCount = this.chatNotificationService.$numNotifications.get() ?? 0; + + if (activityCount > this._lastSeenActivityCount) { + this.notify({ + title: "New activity on Impro", + body: + activityCount === 1 + ? "You have 1 new notification" + : `You have ${activityCount} new notifications`, + tag: "impro-activity", + url: "/notifications", + }); + } + + if (chatCount > this._lastSeenChatCount) { + this.notify({ + title: "New message on Impro", + body: + chatCount === 1 + ? "You have 1 unread conversation" + : `You have ${chatCount} unread conversations`, + tag: "impro-chat", + url: "/messages", + }); + } + + this._lastSeenActivityCount = activityCount; + this._lastSeenChatCount = chatCount; + } +} diff --git a/src/js/views/settings.view.js b/src/js/views/settings.view.js index dadf5924..99173fd6 100644 --- a/src/js/views/settings.view.js +++ b/src/js/views/settings.view.js @@ -2,6 +2,7 @@ import { View } from "/js/views/view.js"; import { pageEffect, bindToPage, bindPageTitle } from "/js/router.js"; import { html, render } from "/js/lib/lit-html.js"; import { eyeIconTemplate } from "/js/templates/icons/eyeIcon.template.js"; +import { notificationsIconTemplate } from "/js/templates/icons/notificationsIcon.template.js"; import { eyeSlashIconTemplate } from "/js/templates/icons/eyeSlashIcon.template.js"; import { mutedWordIconTemplate } from "/js/templates/icons/mutedWordIcon.template.js"; import { restrictedIconTemplate } from "/js/templates/icons/restrictedIcon.template.js"; @@ -188,6 +189,12 @@ class SettingsView extends View { label: "Appearance", url: "/settings/appearance", }, + { + key: "notifications", + icon: notificationsIconTemplate, + label: "Notifications", + url: "/settings/notifications", + }, { key: "muted-words", icon: mutedWordIconTemplate, diff --git a/src/js/views/settings/notifications.view.js b/src/js/views/settings/notifications.view.js new file mode 100644 index 00000000..b7fe1d29 --- /dev/null +++ b/src/js/views/settings/notifications.view.js @@ -0,0 +1,110 @@ +import { View } from "/js/views/view.js"; +import { html, render } from "/js/lib/lit-html.js"; +import { pageEffect, bindToPage, bindPageTitle } from "/js/router.js"; +import { headerTemplate } from "/js/templates/header.template.js"; +import { auth } from "/js/auth.js"; +import { confirmModal } from "/js/modals/confirm.modal.js"; +import { showToast } from "/js/toasts.js"; +import { Signal } from "/js/signals.js"; +import "/js/components/toggle-switch.js"; + +class SettingsNotificationsView extends View { + async render({ + root, + router, + layout, + context: { systemNotificationService }, + }) { + await auth.requireAuth(); + + const $enabled = new Signal.State( + systemNotificationService?.isEnabled ?? false, + ); + + async function handleToggle(checked) { + if (!systemNotificationService) return; + if (!checked) { + systemNotificationService.disable(); + $enabled.set(false); + return; + } + const confirmed = await confirmModal( + "Impro will ask your browser for permission to show notifications. You can turn this off again at any time.", + { title: "Enable notifications?", confirmButtonText: "Continue" }, + ); + if (!confirmed) return; + const result = await systemNotificationService.requestPermission(); + if (result === "granted") { + $enabled.set(true); + } else { + $enabled.set(false); + if (result === "denied") { + showToast( + "Notifications are blocked for this site. Re-enable them in your browser's site settings.", + { style: "error" }, + ); + } + } + } + + bindToPage(root, layout, "active-nav-click", (event) => { + event.preventDefault(); + router.go("/settings"); + }); + + bindPageTitle(root, () => "Notifications"); + + pageEffect(root, () => { + const enabled = $enabled.get(); + const isSupported = systemNotificationService?.isSupported ?? false; + const permissionState = + systemNotificationService?.permissionState ?? "unsupported"; + const isDenied = permissionState === "denied"; + + let description = + "Get notified when you have new activity or messages while Impro is open in a tab or window."; + if (!isSupported) { + description = "Your browser doesn't support notifications."; + } else if (isDenied) { + description = + "Notifications are blocked for this site. Re-enable them in your browser's site settings to turn this on."; + } + + render( + html`
+ ${headerTemplate({ + title: "Notifications", + backButtonFallbackRoute: "/settings", + })} +
+
+
+

Enable notifications

+

${description}

+
+
+ handleToggle(event.detail.checked)} + > +
+
+
+
`, + root, + ); + }); + + root.addEventListener("page-restore", () => { + window.scrollTo(0, 0); + }); + } +} + +export default new SettingsNotificationsView(); diff --git a/tests/e2e/specs/views/settings.view.test.js b/tests/e2e/specs/views/settings.view.test.js index 4c2feb69..33de65c8 100644 --- a/tests/e2e/specs/views/settings.view.test.js +++ b/tests/e2e/specs/views/settings.view.test.js @@ -19,13 +19,16 @@ test.describe("Settings view", () => { ); const nav = view.locator(".vertical-nav"); - // 5 menu items + Switch account toggle + Sign out (accounts list is collapsed by default) - await expect(nav.locator(".vertical-nav-item")).toHaveCount(7, { + // 6 menu items + Switch account toggle + Sign out (accounts list is collapsed by default) + await expect(nav.locator(".vertical-nav-item")).toHaveCount(8, { timeout: 10000, }); await expect( nav.locator('[data-testid="settings-nav-appearance"]'), ).toBeVisible(); + await expect( + nav.locator('[data-testid="settings-nav-notifications"]'), + ).toBeVisible(); await expect( nav.locator('[data-testid="settings-nav-muted-words"]'), ).toBeVisible(); diff --git a/tests/e2e/specs/views/settingsNotifications.view.test.js b/tests/e2e/specs/views/settingsNotifications.view.test.js new file mode 100644 index 00000000..fe081cb8 --- /dev/null +++ b/tests/e2e/specs/views/settingsNotifications.view.test.js @@ -0,0 +1,148 @@ +import { test, expect } from "../../base.js"; +import { login } from "../../helpers.js"; +import { MockServer } from "../../mockServer.js"; + +// Stubs the Notification API so permission prompts are deterministic in CI +// (headless browsers have no real OS notification center to grant/deny). +// `initial` is the permission state on page load; `onPrompt` is what the +// mock resolves to the first time requestPermission() is actually called +// (only takes effect starting from "default" -- real browsers never +// re-prompt once a decision has already been made). +async function stubNotificationPermission( + page, + { initial = "default", onPrompt = "granted" } = {}, +) { + await page.addInitScript( + ({ initialPermission, promptResult }) => { + class MockNotification { + static permission = initialPermission; + static async requestPermission() { + if (MockNotification.permission === "default") { + MockNotification.permission = promptResult; + } + return MockNotification.permission; + } + constructor() {} + close() {} + } + window.Notification = MockNotification; + }, + { initialPermission: initial, promptResult: onPrompt }, + ); +} + +test.describe("Settings > Notifications view", () => { + test("toggle starts unchecked and enables after granting permission", async ({ + page, + }) => { + const mockServer = new MockServer(); + await mockServer.setup(page); + await stubNotificationPermission(page, { + initial: "default", + onPrompt: "granted", + }); + await login(page); + await page.goto("/settings/notifications"); + + const toggle = page.locator('[data-testid="system-notifications-toggle"]'); + await expect(toggle).toBeVisible({ timeout: 10000 }); + await expect(toggle).not.toHaveAttribute("checked", ""); + + await toggle.click(); + + const confirmModal = page.locator('[data-testid="confirm-modal"]'); + await expect(confirmModal).toBeVisible(); + await confirmModal.locator('[data-testid="modal-confirm-button"]').click(); + + await expect(toggle).toHaveAttribute("checked", "", { timeout: 10000 }); + await expect + .poll(() => + page.evaluate(() => + localStorage.getItem("system-notifications-enabled"), + ), + ) + .toBe("true"); + }); + + test("declining the confirm dialog leaves notifications disabled", async ({ + page, + }) => { + const mockServer = new MockServer(); + await mockServer.setup(page); + await stubNotificationPermission(page, { initial: "default" }); + await login(page); + await page.goto("/settings/notifications"); + + const toggle = page.locator('[data-testid="system-notifications-toggle"]'); + await expect(toggle).toBeVisible({ timeout: 10000 }); + + await toggle.click(); + + const confirmModal = page.locator('[data-testid="confirm-modal"]'); + await expect(confirmModal).toBeVisible(); + await confirmModal.locator('[data-testid="modal-cancel-button"]').click(); + + await expect(toggle).not.toHaveAttribute("checked", ""); + expect( + await page.evaluate(() => + localStorage.getItem("system-notifications-enabled"), + ), + ).toBeNull(); + }); + + test("shows an error toast when the browser denies permission", async ({ + page, + }) => { + const mockServer = new MockServer(); + await mockServer.setup(page); + await stubNotificationPermission(page, { + initial: "default", + onPrompt: "denied", + }); + await login(page); + await page.goto("/settings/notifications"); + + const toggle = page.locator('[data-testid="system-notifications-toggle"]'); + await expect(toggle).toBeVisible({ timeout: 10000 }); + + await toggle.click(); + await page + .locator( + '[data-testid="confirm-modal"] [data-testid="modal-confirm-button"]', + ) + .click(); + + await expect(page.locator('[data-testid="toast"]')).toBeVisible({ + timeout: 10000, + }); + await expect(toggle).not.toHaveAttribute("checked", ""); + }); + + test("turning the toggle off clears the stored preference without a confirm dialog", async ({ + page, + }) => { + const mockServer = new MockServer(); + await mockServer.setup(page); + await stubNotificationPermission(page, { initial: "granted" }); + await login(page); + await page.addInitScript(() => { + localStorage.setItem("system-notifications-enabled", "true"); + }); + await page.goto("/settings/notifications"); + + const toggle = page.locator('[data-testid="system-notifications-toggle"]'); + await expect(toggle).toHaveAttribute("checked", "", { timeout: 10000 }); + + await toggle.click(); + + await expect(page.locator('[data-testid="confirm-modal"]')).toHaveCount(0); + await expect(toggle).not.toHaveAttribute("checked", ""); + await expect + .poll(() => + page.evaluate(() => + localStorage.getItem("system-notifications-enabled"), + ), + ) + .toBeNull(); + }); +}); diff --git a/tests/unit/specs/systemNotificationService.test.js b/tests/unit/specs/systemNotificationService.test.js new file mode 100644 index 00000000..a04ccf3d --- /dev/null +++ b/tests/unit/specs/systemNotificationService.test.js @@ -0,0 +1,254 @@ +import { describe, it, beforeEach, afterEach } from "node:test"; +import assert from "node:assert/strict"; +import { Signal } from "/js/signals.js"; +import { SystemNotificationService } from "/js/systemNotificationService.js"; + +function createMockNotificationService({ + numNotifications = 0, + isSnoozed = false, +} = {}) { + return { + $numNotifications: new Signal.State(numNotifications), + isSnoozed, + }; +} + +function createMockChatNotificationService({ numNotifications = 0 } = {}) { + return { + $numNotifications: new Signal.State(numNotifications), + }; +} + +function enable() { + localStorage.setItem("system-notifications-enabled", "true"); +} + +describe("SystemNotificationService", () => { + let instances; + let originalNotification; + + beforeEach(() => { + instances = []; + originalNotification = globalThis.Notification; + globalThis.Notification = class { + static permission = "granted"; + static async requestPermission() { + return globalThis.Notification.permission; + } + constructor(title, options) { + this.title = title; + this.options = options; + instances.push(this); + } + close() {} + }; + localStorage.clear(); + }); + + afterEach(() => { + globalThis.Notification = originalNotification; + localStorage.clear(); + }); + + describe("constructor", () => { + it("seeds last-seen counts without firing", () => { + const notificationService = createMockNotificationService({ + numNotifications: 5, + }); + const chatNotificationService = createMockChatNotificationService({ + numNotifications: 2, + }); + const service = new SystemNotificationService( + notificationService, + chatNotificationService, + ); + enable(); + + service.checkForUpdates(); + + assert.deepEqual(instances.length, 0); + }); + }); + + describe("checkForUpdates", () => { + it("notifies when the activity count increases", () => { + const notificationService = createMockNotificationService(); + const chatNotificationService = createMockChatNotificationService(); + const service = new SystemNotificationService( + notificationService, + chatNotificationService, + ); + enable(); + + notificationService.$numNotifications.set(3); + service.checkForUpdates(); + + assert.deepEqual(instances.length, 1); + assert.deepEqual(instances[0].options.tag, "impro-activity"); + }); + + it("does not notify again for an unchanged count", () => { + const notificationService = createMockNotificationService(); + const chatNotificationService = createMockChatNotificationService(); + const service = new SystemNotificationService( + notificationService, + chatNotificationService, + ); + enable(); + + notificationService.$numNotifications.set(3); + service.checkForUpdates(); + service.checkForUpdates(); + + assert.deepEqual(instances.length, 1); + }); + + it("notifies again when the count increases further", () => { + const notificationService = createMockNotificationService(); + const chatNotificationService = createMockChatNotificationService(); + const service = new SystemNotificationService( + notificationService, + chatNotificationService, + ); + enable(); + + notificationService.$numNotifications.set(3); + service.checkForUpdates(); + notificationService.$numNotifications.set(5); + service.checkForUpdates(); + + assert.deepEqual(instances.length, 2); + }); + + it("does not notify when the count decreases", () => { + const notificationService = createMockNotificationService({ + numNotifications: 5, + }); + const chatNotificationService = createMockChatNotificationService(); + const service = new SystemNotificationService( + notificationService, + chatNotificationService, + ); + enable(); + + notificationService.$numNotifications.set(0); + service.checkForUpdates(); + + assert.deepEqual(instances.length, 0); + }); + + it("notifies when the chat count increases", () => { + const notificationService = createMockNotificationService(); + const chatNotificationService = createMockChatNotificationService(); + const service = new SystemNotificationService( + notificationService, + chatNotificationService, + ); + enable(); + + chatNotificationService.$numNotifications.set(2); + service.checkForUpdates(); + + assert.deepEqual(instances.length, 1); + assert.deepEqual(instances[0].options.tag, "impro-chat"); + }); + }); + + describe("notify gating", () => { + it("does not notify when disabled", () => { + const notificationService = createMockNotificationService(); + const chatNotificationService = createMockChatNotificationService(); + const service = new SystemNotificationService( + notificationService, + chatNotificationService, + ); + + notificationService.$numNotifications.set(3); + service.checkForUpdates(); + + assert.deepEqual(instances.length, 0); + }); + + it("does not notify when permission is not granted", () => { + const notificationService = createMockNotificationService(); + const chatNotificationService = createMockChatNotificationService(); + const service = new SystemNotificationService( + notificationService, + chatNotificationService, + ); + enable(); + globalThis.Notification.permission = "default"; + + notificationService.$numNotifications.set(3); + service.checkForUpdates(); + + assert.deepEqual(instances.length, 0); + }); + + it("does not notify when snoozed", () => { + const notificationService = createMockNotificationService({ + isSnoozed: true, + }); + const chatNotificationService = createMockChatNotificationService(); + const service = new SystemNotificationService( + notificationService, + chatNotificationService, + ); + enable(); + + notificationService.$numNotifications.set(3); + service.checkForUpdates(); + + assert.deepEqual(instances.length, 0); + }); + }); + + describe("requestPermission", () => { + it("sets the storage flag when granted", async () => { + const notificationService = createMockNotificationService(); + const chatNotificationService = createMockChatNotificationService(); + const service = new SystemNotificationService( + notificationService, + chatNotificationService, + ); + globalThis.Notification.permission = "granted"; + + const result = await service.requestPermission(); + + assert.deepEqual(result, "granted"); + assert.deepEqual(service.isEnabled, true); + }); + + it("does not set the storage flag when denied", async () => { + const notificationService = createMockNotificationService(); + const chatNotificationService = createMockChatNotificationService(); + const service = new SystemNotificationService( + notificationService, + chatNotificationService, + ); + globalThis.Notification.permission = "denied"; + + const result = await service.requestPermission(); + + assert.deepEqual(result, "denied"); + assert.deepEqual(service.isEnabled, false); + }); + }); + + describe("disable", () => { + it("clears the storage flag", () => { + const notificationService = createMockNotificationService(); + const chatNotificationService = createMockChatNotificationService(); + const service = new SystemNotificationService( + notificationService, + chatNotificationService, + ); + enable(); + assert.deepEqual(service.isEnabled, true); + + service.disable(); + + assert.deepEqual(service.isEnabled, false); + }); + }); +}); -- 2.51.2 From 0918eeee55ae8bfe0c7b2816dbaa02e19b7350bf Mon Sep 17 00:00:00 2001 From: Grace Kind Date: Mon, 3 Aug 2026 16:47:09 -0500 Subject: [PATCH 2/3] Move effect into service and tweak settings copy --- src/js/app.js | 13 +- src/js/systemNotificationService.js | 70 +++++----- src/js/views/settings/notifications.view.js | 4 +- .../specs/systemNotificationService.test.js | 132 ++++++++++-------- 4 files changed, 117 insertions(+), 102 deletions(-) diff --git a/src/js/app.js b/src/js/app.js index 29795c92..abbf4720 100644 --- a/src/js/app.js +++ b/src/js/app.js @@ -193,6 +193,10 @@ export async function main() { chatNotificationService.startPolling(); } + if (systemNotificationService) { + systemNotificationService.start(); + } + const context = { isAuthenticated: !!session, api, @@ -243,15 +247,6 @@ export async function main() { }); } - if (systemNotificationService) { - effect( - () => { - systemNotificationService.checkForUpdates(); - }, - { debugName: "systemNotifications" }, - ); - } - router.addRoute(["/", "/intent/compose"], () => homeView, { layoutOptions: { activeNavItem: "home" }, }); diff --git a/src/js/systemNotificationService.js b/src/js/systemNotificationService.js index 10dbea5a..6a9f5d6b 100644 --- a/src/js/systemNotificationService.js +++ b/src/js/systemNotificationService.js @@ -1,3 +1,5 @@ +import { effect } from "/js/signals.js"; + const STORAGE_KEY = "system-notifications-enabled"; const ICON_URL = "/img/impro-logo-192.png"; @@ -11,6 +13,42 @@ export class SystemNotificationService { chatNotificationService.$numNotifications.get() ?? 0; } + start() { + return effect(() => { + const activityCount = + this.notificationService.$numNotifications.get() ?? 0; + const chatCount = + this.chatNotificationService.$numNotifications.get() ?? 0; + + if (activityCount > this._lastSeenActivityCount) { + this.notify({ + title: "New activity on Impro", + body: + activityCount === 1 + ? "You have 1 new notification" + : `You have ${activityCount} new notifications`, + tag: "impro-activity", + url: "/notifications", + }); + } + + if (chatCount > this._lastSeenChatCount) { + this.notify({ + title: "New message on Impro", + body: + chatCount === 1 + ? "You have 1 unread conversation" + : `You have ${chatCount} unread conversations`, + tag: "impro-chat", + url: "/messages", + }); + } + + this._lastSeenActivityCount = activityCount; + this._lastSeenChatCount = chatCount; + }); + } + get isSupported() { return typeof Notification !== "undefined"; } @@ -57,36 +95,4 @@ export class SystemNotificationService { notification.close(); }; } - - checkForUpdates() { - const activityCount = this.notificationService.$numNotifications.get() ?? 0; - const chatCount = this.chatNotificationService.$numNotifications.get() ?? 0; - - if (activityCount > this._lastSeenActivityCount) { - this.notify({ - title: "New activity on Impro", - body: - activityCount === 1 - ? "You have 1 new notification" - : `You have ${activityCount} new notifications`, - tag: "impro-activity", - url: "/notifications", - }); - } - - if (chatCount > this._lastSeenChatCount) { - this.notify({ - title: "New message on Impro", - body: - chatCount === 1 - ? "You have 1 unread conversation" - : `You have ${chatCount} unread conversations`, - tag: "impro-chat", - url: "/messages", - }); - } - - this._lastSeenActivityCount = activityCount; - this._lastSeenChatCount = chatCount; - } } diff --git a/src/js/views/settings/notifications.view.js b/src/js/views/settings/notifications.view.js index b7fe1d29..6789c253 100644 --- a/src/js/views/settings/notifications.view.js +++ b/src/js/views/settings/notifications.view.js @@ -62,7 +62,7 @@ class SettingsNotificationsView extends View { const isDenied = permissionState === "denied"; let description = - "Get notified when you have new activity or messages while Impro is open in a tab or window."; + "Get notified when you have new activity while Impro is open in a tab or window."; if (!isSupported) { description = "Your browser doesn't support notifications."; } else if (isDenied) { @@ -82,7 +82,7 @@ class SettingsNotificationsView extends View { data-testid="settings-section-system-notifications" >
-

Enable notifications

+

Enable system notifications

${description}

diff --git a/tests/unit/specs/systemNotificationService.test.js b/tests/unit/specs/systemNotificationService.test.js index a04ccf3d..795cab85 100644 --- a/tests/unit/specs/systemNotificationService.test.js +++ b/tests/unit/specs/systemNotificationService.test.js @@ -19,6 +19,12 @@ function createMockChatNotificationService({ numNotifications = 0 } = {}) { }; } +function flushEffects() { + return new Promise((resolve) => + requestAnimationFrame(() => requestAnimationFrame(resolve)), + ); +} + function enable() { localStorage.setItem("system-notifications-enabled", "true"); } @@ -26,9 +32,21 @@ function enable() { describe("SystemNotificationService", () => { let instances; let originalNotification; + let disposers; + + function startService(notificationService, chatNotificationService) { + const service = new SystemNotificationService( + notificationService, + chatNotificationService, + ); + const dispose = service.start(); + disposers.push(dispose); + return { service, dispose }; + } beforeEach(() => { instances = []; + disposers = []; originalNotification = globalThis.Notification; globalThis.Notification = class { static permission = "granted"; @@ -46,158 +64,154 @@ describe("SystemNotificationService", () => { }); afterEach(() => { + for (const dispose of disposers) { + dispose(); + } globalThis.Notification = originalNotification; localStorage.clear(); }); - describe("constructor", () => { - it("seeds last-seen counts without firing", () => { + describe("start", () => { + it("seeds last-seen counts without firing", async () => { const notificationService = createMockNotificationService({ numNotifications: 5, }); const chatNotificationService = createMockChatNotificationService({ numNotifications: 2, }); - const service = new SystemNotificationService( - notificationService, - chatNotificationService, - ); enable(); - service.checkForUpdates(); + startService(notificationService, chatNotificationService); + await flushEffects(); assert.deepEqual(instances.length, 0); }); - }); - describe("checkForUpdates", () => { - it("notifies when the activity count increases", () => { + it("notifies when the activity count increases", async () => { const notificationService = createMockNotificationService(); const chatNotificationService = createMockChatNotificationService(); - const service = new SystemNotificationService( - notificationService, - chatNotificationService, - ); enable(); + startService(notificationService, chatNotificationService); notificationService.$numNotifications.set(3); - service.checkForUpdates(); + await flushEffects(); assert.deepEqual(instances.length, 1); assert.deepEqual(instances[0].options.tag, "impro-activity"); }); - it("does not notify again for an unchanged count", () => { + it("does not notify again for an unchanged count", async () => { const notificationService = createMockNotificationService(); const chatNotificationService = createMockChatNotificationService(); - const service = new SystemNotificationService( - notificationService, - chatNotificationService, - ); enable(); + startService(notificationService, chatNotificationService); notificationService.$numNotifications.set(3); - service.checkForUpdates(); - service.checkForUpdates(); + await flushEffects(); + chatNotificationService.$numNotifications.set(1); + await flushEffects(); - assert.deepEqual(instances.length, 1); + assert.deepEqual(instances.length, 2); + assert.deepEqual(instances[1].options.tag, "impro-chat"); }); - it("notifies again when the count increases further", () => { + it("notifies again when the count increases further", async () => { const notificationService = createMockNotificationService(); const chatNotificationService = createMockChatNotificationService(); - const service = new SystemNotificationService( - notificationService, - chatNotificationService, - ); enable(); + startService(notificationService, chatNotificationService); notificationService.$numNotifications.set(3); - service.checkForUpdates(); + await flushEffects(); notificationService.$numNotifications.set(5); - service.checkForUpdates(); + await flushEffects(); assert.deepEqual(instances.length, 2); }); - it("does not notify when the count decreases", () => { + it("does not notify when the count decreases", async () => { const notificationService = createMockNotificationService({ numNotifications: 5, }); const chatNotificationService = createMockChatNotificationService(); - const service = new SystemNotificationService( - notificationService, - chatNotificationService, - ); enable(); + startService(notificationService, chatNotificationService); notificationService.$numNotifications.set(0); - service.checkForUpdates(); + await flushEffects(); assert.deepEqual(instances.length, 0); }); - it("notifies when the chat count increases", () => { + it("notifies when the chat count increases", async () => { const notificationService = createMockNotificationService(); const chatNotificationService = createMockChatNotificationService(); - const service = new SystemNotificationService( - notificationService, - chatNotificationService, - ); enable(); + startService(notificationService, chatNotificationService); chatNotificationService.$numNotifications.set(2); - service.checkForUpdates(); + await flushEffects(); assert.deepEqual(instances.length, 1); assert.deepEqual(instances[0].options.tag, "impro-chat"); }); - }); - describe("notify gating", () => { - it("does not notify when disabled", () => { + it("stops notifying after the effect is disposed", async () => { const notificationService = createMockNotificationService(); const chatNotificationService = createMockChatNotificationService(); - const service = new SystemNotificationService( + enable(); + const { dispose } = startService( notificationService, chatNotificationService, ); + notificationService.$numNotifications.set(1); + await flushEffects(); + assert.deepEqual(instances.length, 1); + + dispose(); + chatNotificationService.$numNotifications.set(3); + await flushEffects(); + + assert.deepEqual(instances.length, 1); + }); + }); + + describe("notify gating", () => { + it("does not notify when disabled", async () => { + const notificationService = createMockNotificationService(); + const chatNotificationService = createMockChatNotificationService(); + startService(notificationService, chatNotificationService); + notificationService.$numNotifications.set(3); - service.checkForUpdates(); + await flushEffects(); assert.deepEqual(instances.length, 0); }); - it("does not notify when permission is not granted", () => { + it("does not notify when permission is not granted", async () => { const notificationService = createMockNotificationService(); const chatNotificationService = createMockChatNotificationService(); - const service = new SystemNotificationService( - notificationService, - chatNotificationService, - ); enable(); globalThis.Notification.permission = "default"; + startService(notificationService, chatNotificationService); notificationService.$numNotifications.set(3); - service.checkForUpdates(); + await flushEffects(); assert.deepEqual(instances.length, 0); }); - it("does not notify when snoozed", () => { + it("does not notify when snoozed", async () => { const notificationService = createMockNotificationService({ isSnoozed: true, }); const chatNotificationService = createMockChatNotificationService(); - const service = new SystemNotificationService( - notificationService, - chatNotificationService, - ); enable(); + startService(notificationService, chatNotificationService); notificationService.$numNotifications.set(3); - service.checkForUpdates(); + await flushEffects(); assert.deepEqual(instances.length, 0); }); -- 2.51.2 From 69166981ba3af450192bb242819d9bf52144128c Mon Sep 17 00:00:00 2001 From: Grace Kind Date: Mon, 3 Aug 2026 17:02:40 -0500 Subject: [PATCH 3/3] Switch to router --- src/js/systemNotificationService.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/js/systemNotificationService.js b/src/js/systemNotificationService.js index 6a9f5d6b..df424374 100644 --- a/src/js/systemNotificationService.js +++ b/src/js/systemNotificationService.js @@ -25,8 +25,8 @@ export class SystemNotificationService { title: "New activity on Impro", body: activityCount === 1 - ? "You have 1 new notification" - : `You have ${activityCount} new notifications`, + ? "You have 1 unread notification" + : `You have ${activityCount} unread notifications`, tag: "impro-activity", url: "/notifications", }); @@ -91,7 +91,7 @@ export class SystemNotificationService { }); notification.onclick = () => { window.focus(); - window.location.href = url; + window.router.go(url); notification.close(); }; } -- 2.51.2