Fix desktop tray unread indicator: clear only on genuine room entry

The tray indicator must stay visible while a message is unread and
clear only when the user actually opens the room containing it --
never merely because the window was shown, focused, restored, or
opened from the tray.

Adds window.dsDesktop.roomOpened() (called exactly once per genuine
transition to a real room id, via useDesktopRoomBridge) and wires up
window.dsDesktop.setUnreadCount() (declared but never actually called
before this) to fire whenever the unread-room count changes. Both are
no-ops in browser builds, matching this bridge's existing optional-
chaining convention.

Frontend unread state stays authoritative -- clearRoomIndicators is
pulled out of ChatShellPage as a pure function (same behavior, now
directly testable) so opening one room only clears that room's
unread/mention flags, leaving every other room's state untouched.

Also adds a frontend test runner (vitest + React Testing Library) --
none existed in this project before. 10 new tests cover: roomOpened
firing once on entry, not refiring on an unrelated rerender with the
same room id, refiring on a genuine room-to-room transition, never
firing with no room id, setUnreadCount firing independently of
roomOpened, clearing one room preserving every other room's unread
state, and the desktop bridge remaining fully optional.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
ksmithandClaude Sonnet 5 committed 2026-09-28 17:21:45 -06:00
1 parent 047f47d524
commit 3349cadea2
9 files changed
+1155 -16

No files matched your search

@@ -0,0 +1,68 @@
import { renderHook } from '@testing-library/react'
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
import * as desktopBridge from '../lib/desktopBridge'
import { useDesktopRoomBridge } from './useDesktopRoomBridge'
describe('useDesktopRoomBridge', () => {
beforeEach(() => {
vi.spyOn(desktopBridge, 'notifyRoomOpened')
vi.spyOn(desktopBridge, 'notifyUnreadCount')
})
afterEach(() => {
vi.restoreAllMocks()
})
it('calls roomOpened once when entering a room', () => {
renderHook(({ roomId, count }) => useDesktopRoomBridge(roomId, count), {
initialProps: { roomId: 'room-1', count: 0 },
})
expect(desktopBridge.notifyRoomOpened).toHaveBeenCalledTimes(1)
})
it('does not call roomOpened again on a rerender with the same roomId', () => {
const { rerender } = renderHook(({ roomId, count }) => useDesktopRoomBridge(roomId, count), {
initialProps: { roomId: 'room-1', count: 0 },
})
expect(desktopBridge.notifyRoomOpened).toHaveBeenCalledTimes(1)
// Unrelated rerender -- unreadRoomCount changed, roomId did not.
rerender({ roomId: 'room-1', count: 3 })
rerender({ roomId: 'room-1', count: 3 })
expect(desktopBridge.notifyRoomOpened).toHaveBeenCalledTimes(1)
})
it('calls roomOpened again on a genuine transition to a different room', () => {
const { rerender } = renderHook(({ roomId, count }) => useDesktopRoomBridge(roomId, count), {
initialProps: { roomId: 'room-1', count: 0 },
})
rerender({ roomId: 'room-2', count: 0 })
expect(desktopBridge.notifyRoomOpened).toHaveBeenCalledTimes(2)
})
it('does not call roomOpened when there is no room id', () => {
renderHook(({ roomId, count }) => useDesktopRoomBridge(roomId, count), {
initialProps: { roomId: undefined as string | undefined, count: 0 },
})
expect(desktopBridge.notifyRoomOpened).not.toHaveBeenCalled()
})
it('sends the unread count whenever it changes, independent of roomOpened', () => {
const { rerender } = renderHook(({ roomId, count }) => useDesktopRoomBridge(roomId, count), {
initialProps: { roomId: undefined as string | undefined, count: 0 },
})
expect(desktopBridge.notifyUnreadCount).toHaveBeenCalledWith(0)
rerender({ roomId: undefined, count: 2 })
expect(desktopBridge.notifyUnreadCount).toHaveBeenCalledWith(2)
expect(desktopBridge.notifyRoomOpened).not.toHaveBeenCalled()
})
})
describe('desktop bridge functions with no window.dsDesktop', () => {
it('remain optional -- calling them does not throw when the bridge is absent', () => {
expect(window.dsDesktop).toBeUndefined()
expect(() => desktopBridge.notifyRoomOpened()).not.toThrow()
expect(() => desktopBridge.notifyUnreadCount(5)).not.toThrow()
})
})
@@ -0,0 +1,26 @@
import { useEffect } from 'react'
import { notifyRoomOpened, notifyUnreadCount } from '../lib/desktopBridge'
// Tells the Electron desktop wrapper (a) when the user has genuinely
// entered a room, so it can clear that room's tray indicator, and (b) the
// current unread-room count, so the tray badge stays accurate. Both are
// no-ops in every browser build (see desktopBridge.ts).
//
// roomOpened must fire only on a genuine transition to a real room id --
// never merely because the window was shown, focused, restored, or opened
// from the tray, and never when no room is open at all. Depending on just
// the id (a primitive, not the room object) is what makes this safe to
// call from a component that rerenders often: an unrelated rerender that
// leaves activeRoomId unchanged doesn't rerun the effect, so there's no
// separate de-duplication to get wrong here -- it falls out of React's own
// effect-dependency comparison.
export function useDesktopRoomBridge(activeRoomId: string | undefined, unreadRoomCount: number): void {
useEffect(() => {
if (!activeRoomId) return
notifyRoomOpened()
}, [activeRoomId])
useEffect(() => {
notifyUnreadCount(unreadRoomCount)
}, [unreadRoomCount])
}
+16
View File
@@ -15,6 +15,12 @@ declare global {
interface Window {
dsDesktop?: {
setUnreadCount?(unreadRoomCount: number): void
// Tells the tray/dock icon the user has genuinely entered a room --
// the desktop wrapper's own signal to clear that room's tray
// indicator. Deliberately not fired just because the window was
// shown/focused/restored, or opened from the tray: none of those mean
// the user actually looked at the room the unread message is in.
roomOpened?(): void
showNotification?(notification: DesktopNotificationRequest): void
onNotificationClick?(callback: (roomId: string) => void): () => void
}
@@ -38,6 +44,16 @@ export function isDesktopClickListenerSupported(): boolean {
return typeof window.dsDesktop?.onNotificationClick === 'function'
}
// Both no-op silently if the bridge or this specific method isn't present,
// same as showDesktopNotification below -- callers never need to guard.
export function notifyRoomOpened(): void {
window.dsDesktop?.roomOpened?.()
}
export function notifyUnreadCount(unreadRoomCount: number): void {
window.dsDesktop?.setUnreadCount?.(unreadRoomCount)
}
// No-ops silently if the bridge or this specific method isn't present --
// callers don't need to guard, matching the rest of this module's
// capability-detect-per-method philosophy (see #49's "feature-test each
+56
View File
@@ -0,0 +1,56 @@
import { describe, expect, it } from 'vitest'
import type { MyRoomItem } from '../types'
import { clearRoomIndicators } from './roomIndicators'
function makeRoom(overrides: Partial<MyRoomItem> & { id: string }): MyRoomItem {
return {
name: overrides.id,
description: null,
is_private: false,
is_dm: false,
is_archived: false,
owner_id: 'owner',
created_at: '2026-01-01T00:00:00Z',
role: 'member',
has_unread: false,
has_mention: false,
dm_partner: null,
email_notifications: false,
...overrides,
}
}
describe('clearRoomIndicators', () => {
it('clears has_unread and has_mention on the matching room', () => {
const rooms = [makeRoom({ id: 'room-1', has_unread: true, has_mention: true })]
const result = clearRoomIndicators(rooms, 'room-1')
expect(result[0].has_unread).toBe(false)
expect(result[0].has_mention).toBe(false)
})
it('preserves unread state on every other room', () => {
const rooms = [
makeRoom({ id: 'room-1', has_unread: true, has_mention: true }),
makeRoom({ id: 'room-2', has_unread: true, has_mention: false }),
makeRoom({ id: 'room-3', has_unread: false, has_mention: false }),
]
const result = clearRoomIndicators(rooms, 'room-1')
const room2 = result.find((r) => r.id === 'room-2')
const room3 = result.find((r) => r.id === 'room-3')
expect(room2?.has_unread).toBe(true)
expect(room3?.has_unread).toBe(false)
})
it('leaves untouched rooms as the same object reference', () => {
const room2 = makeRoom({ id: 'room-2', has_unread: true })
const rooms = [makeRoom({ id: 'room-1', has_unread: true }), room2]
const result = clearRoomIndicators(rooms, 'room-1')
expect(result[1]).toBe(room2)
})
it('is a no-op if the room id is not found', () => {
const rooms = [makeRoom({ id: 'room-1', has_unread: true })]
const result = clearRoomIndicators(rooms, 'does-not-exist')
expect(result[0].has_unread).toBe(true)
})
})
+9
View File
@@ -0,0 +1,9 @@
import type { MyRoomItem } from '../types'
// Pulled out of ChatShellPage as a pure function purely so it's directly
// unit-testable without rendering the whole page -- behavior is unchanged.
// Only touches the matching room; every other room's object reference (and
// therefore its unread state) is left untouched.
export function clearRoomIndicators(rooms: MyRoomItem[], roomId: string): MyRoomItem[] {
return rooms.map((r) => (r.id === roomId ? { ...r, has_unread: false, has_mention: false } : r))
}
+9 -4
View File
@@ -1,4 +1,4 @@
import { useCallback, useEffect, useState } from 'react'
import { useCallback, useEffect, useMemo, useState } from 'react'
import { useNavigate, useParams } from 'react-router-dom'
import { NetworkError } from '../api/client'
import { listMyRooms, listRoomMembers } from '../api/rooms'
@@ -12,7 +12,9 @@ import { Sidebar } from '../components/Sidebar'
import { TopBar } from '../components/TopBar'
import { useAuth } from '../context/AuthContext'
import { useChatSocketContext } from '../context/ChatSocketContext'
import { useDesktopRoomBridge } from '../hooks/useDesktopRoomBridge'
import { MOBILE_BREAKPOINT, useWindowWidth } from '../hooks/useWindowWidth'
import { clearRoomIndicators } from '../lib/roomIndicators'
import type { MyRoomItem, RoomMember } from '../types'
import './ChatShellPage.css'
@@ -60,10 +62,13 @@ export function ChatShellPage() {
const socket = useChatSocketContext()
const clearRoomIndicators = useCallback((id: string) => {
setRooms((prev) => prev.map((r) => (r.id === id ? { ...r, has_unread: false, has_mention: false } : r)))
const handleRoomRead = useCallback((id: string) => {
setRooms((prev) => clearRoomIndicators(prev, id))
}, [])
const unreadRoomCount = useMemo(() => rooms.filter((r) => r.has_unread).length, [rooms])
useDesktopRoomBridge(activeRoom?.id, unreadRoomCount)
useEffect(
() =>
socket.subscribe((envelope) => {
@@ -134,7 +139,7 @@ export function ChatShellPage() {
onToggleInfo={() => setInfoOpen((v) => !v)}
infoOpen={infoOpen}
socket={socket}
onRoomRead={clearRoomIndicators}
onRoomRead={handleRoomRead}
/>
) : (
!isMobile && (