From 53fb0f34cebf4e9759ba1c2ffd0101a2d2e8a699 Mon Sep 17 00:00:00 2001 From: Salastil Date: Fri, 9 Oct 2026 11:07:50 -0400 Subject: [PATCH] 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 --- scripts/tray-check.mjs | 24 ++++++++++++++---------- src/main/index.ts | 26 ++++++++++++++++---------- src/main/notifications.test.ts | 23 +++++++++++++++++++++-- src/main/notifications.ts | 13 ++++++++++++- src/main/tray.test.ts | 9 +++++++-- src/main/tray.ts | 11 ++--------- src/preload/index.ts | 9 ++++++--- src/renderer/src/state/store.ts | 12 ++++++------ src/shared/ipc.ts | 2 +- 9 files changed, 85 insertions(+), 44 deletions(-) diff --git a/scripts/tray-check.mjs b/scripts/tray-check.mjs index c9945e0..8a945d0 100644 --- a/scripts/tray-check.mjs +++ b/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') diff --git a/src/main/index.ts b/src/main/index.ts index f46b5ca..575c5e5 100644 --- a/src/main/index.ts +++ b/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 = {} +/** 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('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('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) => { + 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) => { diff --git a/src/main/notifications.test.ts b/src/main/notifications.test.ts index 7b195bf..b32dd8c 100644 --- a/src/main/notifications.test.ts +++ b/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): { n: Notifier; alerts: TrayState[] } { +function notifier(settings: Record, dnd: string[] = []): { n: Notifier; alerts: TrayState[] } { const alerts: TrayState[] = [] const prefs = { get: (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) + }) +}) diff --git a/src/main/notifications.ts b/src/main/notifications.ts index afefb83..0de3093 100644 --- a/src/main/notifications.ts +++ b/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('notifications.desktop', true)) return diff --git a/src/main/tray.test.ts b/src/main/tray.test.ts index 1264171..778585f 100644 --- a/src/main/tray.test.ts +++ b/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): 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') diff --git a/src/main/tray.ts b/src/main/tray.ts index c3d3fed..fec1366 100644 --- a/src/main/tray.ts +++ b/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 }, diff --git a/src/preload/index.ts b/src/preload/index.ts index ee6ba31..3078884 100644 --- a/src/preload/index.ts +++ b/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): void { + ipcRenderer.send(IPC.trayStatus, statuses) }, onMaximizeChange(cb: (maximized: boolean) => void): () => void { diff --git a/src/renderer/src/state/store.ts b/src/renderer/src/state/store.ts index 445f971..8cbc108 100644 --- a/src/renderer/src/state/store.ts +++ b/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 = {} + for (const a of this.state.accounts) if (a.state === 'connected' && a.status) statuses[a.id] = a.status + window.moho.setAccountStatuses(statuses) } // --- lifecycle ------------------------------------------------------ diff --git a/src/shared/ipc.ts b/src/shared/ipc.ts index 5164eae..df05cd3 100644 --- a/src/shared/ipc.ts +++ b/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