Tray: no notifications switch of its own - Do not disturb is that, and now silences an account's desktop notifications and the taskbar flash (#330)

Co-Authored-By: Claude Sonnet 5.5 <[email protected]>

scripts/tray-check.mjs

@@ -10,6 +10,7 @@
  *
  *   npm run test:tray        (builds first; needs the daemon built too)
  */
+import { execFileSync } from 'node:child_process'
 import fs from 'node:fs'
 import { cdp, freePort, launchApp, sleep, until } from './lib/harness.mjs'
 
@@ -95,11 +96,8 @@ try {
   await until(page, 'window.__trayGot.length', 'the status to reach the window')
   check(JSON.stringify(await page.evaluate('window.__trayGot[0]')) === '["status","dnd"]', 'a status is handed to the window')
 
-  // Desktop notifications is a real setting, shared with Settings.
-  await tray('t.click(["Desktop notifications"])')
-  await until(page, 'window.moho.prefs.getAll().then((p) => p["notifications.desktop"] === false)', 'the setting to change')
-  check(true, 'Desktop notifications toggles the setting')
-  check((await tray('t.menu().find((i) => i.label === "Desktop notifications").checked')) === false, 'and the menu shows it off')
+  // There is no notifications switch of its own: Do not disturb is that.
+  check(!labels.some((l) => /notification/i.test(l)), 'the menu has no notifications switch of its own', labels.join(' | '))
 
   // Restart brings back moho and a daemon of its own.
   const before = daemons(scratchDir)
@@ -136,13 +134,18 @@ try {
   console.log('FAIL', e.message)
   failures++
 } finally {
-  await stop()
   // The relaunched copy is not the one the harness started, so it is not in the
-  // group stop() ended. It is found by the home it was given, which everything
-  // it starts - its daemon, and the helpers Chromium runs that name no port -
-  // carries in its environment; twice, since it may still be starting.
+  // group stop() ends. It is found by the debugging port it was told to use -
+  // it shows no environment to be found by - and what it started goes when it
+  // does. Ended first, and twice since it may still be starting, because it
+  // writes into the directory stop() is about to remove.
   for (let i = 0; i < 2; i++) {
     await sleep(1500)
+    try {
+      execFileSync('pkill', ['-9', '-f', `remote-debugging-port=${port}`])
+    } catch {
+      /* none left */
+    }
     for (const entry of fs.readdirSync('/proc')) {
       if (!/^\d+$/.test(entry) || Number(entry) === process.pid || !scratchDir) continue
       try {
@@ -152,7 +155,8 @@ try {
       }
     }
   }
-  await sleep(500)
+  await sleep(1500)
+  await stop().catch(() => {})
   if (scratchDir) fs.rmSync(scratchDir, { recursive: true, force: true })
 }
 console.log(failures ? `${failures} failed` : 'all passed')

src/main/index.ts

@@ -204,7 +204,6 @@ function applyPref(key: string, value: unknown, except?: Electron.WebContents):
   prefs.set(key, value)
   // The badge and the flash are decided from these as they stand.
   if (key === 'notifications.badge' || key === 'notifications.flash') updateTray(lastTray)
-  if (key === 'notifications.desktop') refreshTrayMenu()
   // Every window keeps its own cache of these, so a setting changed in one
   // is stale in the others until they are told. That is not cosmetic once
   // there are several windows: muting a conversation, or switching the log
@@ -622,8 +621,11 @@ function trayIcon(unread: number): Electron.NativeImage {
   return img.isEmpty() ? nativeImage.createFromPath(resourcePath('icons', 'moho.png')) : img
 }
 
-/** What the window last said the accounts' status is, so the menu can check it. */
+/** What the window last said each connected account's status is. */
+let accountStatus: Record<string, string> = {}
+/** The status the menu shows as checked: the first account's, since the menu sets one on all of them. */
 let trayStatus: TrayStatus | null = null
+const isTrayStatus = (v: unknown): v is TrayStatus => v === 'online' || v === 'idle' || v === 'dnd' || v === 'invisible'
 
 /**
  * Asks the window to do something, showing it first: the tray's entries that
@@ -664,8 +666,7 @@ function refreshTrayMenu(): void {
     {
       state: lastTray,
       windowVisible: !!mainWindow && !mainWindow.isDestroyed() && mainWindow.isVisible(),
-      status: trayStatus,
-      notifications: prefs.get<boolean>('notifications.desktop', true)
+      status: trayStatus
     },
     {
       toggleWindow,
@@ -678,7 +679,6 @@ function refreshTrayMenu(): void {
         trayStatus = status
         tellWindow('status', status)
       },
-      setNotifications: (on) => applyPref('notifications.desktop', on),
       restart: restartMoho,
       quit: () => app.quit()
     }
@@ -708,7 +708,7 @@ function updateTray(state: TrayState): void {
   // The taskbar flash: asking for attention while the window is not in front.
   const flash = prefs.get<boolean>('notifications.flash', true)
   if (mainWindow && !mainWindow.isDestroyed()) {
-    mainWindow.flashFrame(flash && state.unread > 0 && !mainWindow.isFocused())
+    mainWindow.flashFrame(flash && state.unread > 0 && trayStatus !== 'dnd' && !mainWindow.isFocused())
   }
   refreshTrayMenu()
   if (!tray) return
@@ -768,10 +768,15 @@ function wireIpc(): void {
   ipcMain.handle(IPC.prefsSet, (e, key: string, value: unknown) => applyPref(key, value, e.sender))
 
   // The window says what status its accounts are at, for the tray's menu to check.
-  ipcMain.on(IPC.trayStatus, (_e, status: TrayStatus | null) => {
-    if (status === trayStatus) return
-    trayStatus = status
+  ipcMain.on(IPC.trayStatus, (_e, statuses: Record<string, string>) => {
+    accountStatus = statuses && typeof statuses === 'object' ? statuses : {}
+    const first = Object.values(accountStatus)[0]
+    const next = isTrayStatus(first) ? first : null
+    if (next === trayStatus) return
+    trayStatus = next
     refreshTrayMenu()
+    // Do not disturb is asking for quiet, including from the taskbar.
+    updateTray(lastTray)
   })
 
   ipcMain.handle(IPC.markBufferRead, (_e, bufferId: string) => notifier.clear(bufferId))
@@ -1232,7 +1237,8 @@ app.whenReady().then(() => {
     updateTray,
     openConversation,
     () => mainWindow?.webContents ?? null,
-    (bufferId) => popoutState().watched.includes(bufferId)
+    (bufferId) => popoutState().watched.includes(bufferId),
+    (accountId) => accountStatus[accountId] === 'dnd'
   )
 
   client.on('link', (up) => {

src/main/notifications.test.ts

@@ -23,10 +23,10 @@ import type { TrayState } from './tray'
 
 const payload = { accountId: 'a', bufferId: 'b', title: 'alice', body: 'hi' }
 
-function notifier(settings: Record<string, unknown>): { n: Notifier; alerts: TrayState[] } {
+function notifier(settings: Record<string, unknown>, dnd: string[] = []): { n: Notifier; alerts: TrayState[] } {
   const alerts: TrayState[] = []
   const prefs = { get: <T>(key: string, fallback: T): T => (key in settings ? (settings[key] as T) : fallback) }
-  const n = new Notifier(prefs as never, (state) => alerts.push(state), () => {})
+  const n = new Notifier(prefs as never, (state) => alerts.push(state), () => {}, () => null, () => false, (account) => dnd.includes(account))
   return { n, alerts }
 }
 
@@ -78,3 +78,22 @@ describe('what the tray is told is waiting', () => {
     expect(n.latestUnread()).toBeNull()
   })
 })
+
+describe('Do not disturb', () => {
+  beforeEach(() => {
+    shown.length = 0
+  })
+
+  it('shows no notification for an account that is set to it, but still counts the conversation', async () => {
+    const { n, alerts } = notifier({}, ['a'])
+    await n.handle(payload)
+    expect(shown).toHaveLength(0)
+    expect(alerts.at(-1)?.unread).toBe(1)
+  })
+
+  it('leaves the other accounts alone', async () => {
+    const { n } = notifier({}, ['somebody-else'])
+    await n.handle(payload)
+    expect(shown).toHaveLength(1)
+  })
+})

src/main/notifications.ts

@@ -61,7 +61,16 @@ export class Notifier {
      * them what they can already see, and a tray badge counting it as unread
      * is simply wrong.
      */
-    private isWatched: (bufferId: string) => boolean = () => false
+    private isWatched: (bufferId: string) => boolean = () => false,
+    /**
+     * Whether this account has been set to Do not disturb.
+     *
+     * That is the one switch for silence: an account that says so gets no popup
+     * and no sound, however its conversations behave. What it is sent still
+     * counts as unread - the tray and the badge keep their numbers, as Discord's
+     * own do - and is there to be read when it is looked at.
+     */
+    private isDnd: (accountId: string) => boolean = () => false
   ) {}
 
   trackBuffer(buffer: ChatBuffer, removed: boolean): void {
@@ -104,6 +113,8 @@ export class Notifier {
       this.publish()
     }
 
+    if (this.isDnd(payload.accountId)) return
+
     // Switched off in Settings: the conversation still counts as unread above,
     // it just does not pop up.
     if (!this.prefs.get<boolean>('notifications.desktop', true)) return

src/main/tray.test.ts

@@ -6,7 +6,6 @@ const acts = (): TrayActions => ({
   openLatest: vi.fn(),
   openSettings: vi.fn(),
   setStatus: vi.fn(),
-  setNotifications: vi.fn(),
   restart: vi.fn(),
   quit: vi.fn()
 })
@@ -40,7 +39,7 @@ describe('what is said about what is waiting', () => {
 })
 
 describe('the menu', () => {
-  const model = { state: NOTHING_WAITING, windowVisible: true, status: 'idle' as const, notifications: true }
+  const model = { state: NOTHING_WAITING, windowVisible: true, status: 'idle' as const }
   const labels = (items: ReturnType<typeof trayMenu>): string[] => items.map((i) => i.label ?? '-')
 
   it('offers no daemon plumbing, and a restart that is of moho as a whole', () => {
@@ -64,6 +63,12 @@ describe('the menu', () => {
     expect(a.openLatest).toHaveBeenCalled()
   })
 
+  it('has no notifications switch of its own: Do not disturb is that', () => {
+    expect(labels(trayMenu(model, acts())).join('|')).not.toMatch(/notification/i)
+    const status = trayMenu(model, acts()).find((i) => i.label === 'Status')
+    expect((status?.submenu as { label: string }[]).map((s) => s.label)).toContain('Do not disturb')
+  })
+
   it('asks for the window to be shown or hidden as it stands', () => {
     expect(labels(trayMenu({ ...model, windowVisible: true }, acts()))).toContain('Hide moho')
     expect(labels(trayMenu({ ...model, windowVisible: false }, acts()))).toContain('Show moho')

src/main/tray.ts

@@ -53,7 +53,6 @@ export interface TrayActions {
   openLatest: () => void
   openSettings: () => void
   setStatus: (status: TrayStatus) => void
-  setNotifications: (on: boolean) => void
   restart: () => void
   quit: () => void
 }
@@ -63,7 +62,6 @@ export interface TrayModel {
   windowVisible: boolean
   /** The status the accounts are at, where the window has said. */
   status: TrayStatus | null
-  notifications: boolean
 }
 
 const STATUSES: { id: TrayStatus; label: string }[] = [
@@ -75,7 +73,8 @@ const STATUSES: { id: TrayStatus; label: string }[] = [
 
 /**
  * The right-click menu: what is new, how to get to moho, how to say what you
- * are doing, and how to leave. Daemon plumbing - restart it, stop it - is
+ * are doing - which includes Do not disturb, the one switch for silencing it -
+ * and how to leave. Daemon plumbing - restart it, stop it - is
  * deliberately not here; it is a thing for Settings, and "restart moho"
  * covers what somebody reaching for it from a tray actually wants.
  */
@@ -96,12 +95,6 @@ export function trayMenu(model: TrayModel, act: TrayActions): MenuItemConstructo
         click: () => act.setStatus(s.id)
       }))
     },
-    {
-      label: 'Desktop notifications',
-      type: 'checkbox',
-      checked: model.notifications,
-      click: (item) => act.setNotifications(item.checked)
-    },
     { label: 'Settings', click: act.openSettings },
     { type: 'separator' },
     { label: 'Restart moho', click: act.restart },

src/preload/index.ts

@@ -90,9 +90,12 @@ const api = {
     return () => ipcRenderer.off(IPC.trayCommand, handler)
   },
 
-  /** Says what status the accounts are at, for the tray menu to show as checked. */
-  setTrayStatus(status: string | null): void {
-    ipcRenderer.send(IPC.trayStatus, status)
+  /**
+   * Says what status each connected account is at: for the tray menu to show as
+   * checked, and for main to stay quiet for an account on Do not disturb.
+   */
+  setAccountStatuses(statuses: Record<string, string>): void {
+    ipcRenderer.send(IPC.trayStatus, statuses)
   },
 
   onMaximizeChange(cb: (maximized: boolean) => void): () => void {

src/renderer/src/state/store.ts

@@ -1183,14 +1183,14 @@ export class ChatStore {
   }
 
   /**
-   * Tells main what status the accounts are at, so the tray menu's Status
-   * entry can show the one that is set. The first connected account's, since
-   * the menu sets one status on all of them and so they are alike unless one
-   * was changed on its own.
+   * Tells main what status each connected account is at. The tray menu checks
+   * the first one's, since it sets one status on all of them, and main stays
+   * quiet - no popup, no sound - for an account that is on Do not disturb.
    */
   private reportTrayStatus(): void {
-    const status = this.state.accounts.find((a) => a.state === 'connected')?.status
-    window.moho.setTrayStatus(status === 'online' || status === 'idle' || status === 'dnd' || status === 'invisible' ? status : null)
+    const statuses: Record<string, string> = {}
+    for (const a of this.state.accounts) if (a.state === 'connected' && a.status) statuses[a.id] = a.status
+    window.moho.setAccountStatuses(statuses)
   }
 
   // --- lifecycle ------------------------------------------------------

src/shared/ipc.ts

@@ -55,7 +55,7 @@ export const IPC = {
   popoutsChanged: 'moho:popout:changed',
   /** main -> window: something from the tray menu that the window carries out. */
   trayCommand: 'moho:tray:command',
-  /** window -> main: the status the accounts are at, for the tray menu to check. */
+  /** window -> main: the status each connected account is at. */
   trayStatus: 'moho:tray:status'
 } as const