diff --git a/package-lock.json b/package-lock.json index 7d7b277..6338592 100644 --- a/package-lock.json +++ b/package-lock.json @@ -8452,7 +8452,7 @@ }, "packages/cli": { "name": "diffity", - "version": "0.9.15", + "version": "0.9.16", "license": "MIT", "dependencies": { "commander": "^14.0.3", @@ -8476,7 +8476,7 @@ }, "packages/git": { "name": "@diffity/git", - "version": "0.9.15", + "version": "0.9.16", "devDependencies": { "@types/node": "^25.5.0", "typescript": "^5.9.3", @@ -8485,7 +8485,7 @@ }, "packages/github": { "name": "@diffity/github", - "version": "0.9.15", + "version": "0.9.16", "dependencies": { "@diffity/parser": "*" }, @@ -8497,7 +8497,7 @@ }, "packages/parser": { "name": "@diffity/parser", - "version": "0.9.15", + "version": "0.9.16", "devDependencies": { "typescript": "^5.9.3", "vitest": "^4.1.0" @@ -8505,7 +8505,7 @@ }, "packages/ui": { "name": "@diffity/ui", - "version": "0.9.15", + "version": "0.9.16", "dependencies": { "@react-router/node": "^7.13.2", "@tailwindcss/vite": "^4.2.1", diff --git a/packages/cli/package.json b/packages/cli/package.json index 3632baa..f6f4b3b 100644 --- a/packages/cli/package.json +++ b/packages/cli/package.json @@ -1,6 +1,6 @@ { "name": "diffity", - "version": "0.9.15", + "version": "0.9.16", "description": "GitHub-style git diff viewer in the browser", "type": "module", "bin": { diff --git a/packages/git/package.json b/packages/git/package.json index 84e391a..42d43be 100644 --- a/packages/git/package.json +++ b/packages/git/package.json @@ -1,6 +1,6 @@ { "name": "@diffity/git", - "version": "0.9.15", + "version": "0.9.16", "private": true, "type": "module", "main": "./dist/index.js", diff --git a/packages/github/package.json b/packages/github/package.json index 1100600..56e71b4 100644 --- a/packages/github/package.json +++ b/packages/github/package.json @@ -1,6 +1,6 @@ { "name": "@diffity/github", - "version": "0.9.15", + "version": "0.9.16", "private": true, "type": "module", "main": "./dist/index.js", diff --git a/packages/parser/package.json b/packages/parser/package.json index cbcf4fb..d28e666 100644 --- a/packages/parser/package.json +++ b/packages/parser/package.json @@ -1,6 +1,6 @@ { "name": "@diffity/parser", - "version": "0.9.15", + "version": "0.9.16", "private": true, "type": "module", "main": "./dist/index.js", diff --git a/packages/ui/package.json b/packages/ui/package.json index 6c4836d..8459b15 100644 --- a/packages/ui/package.json +++ b/packages/ui/package.json @@ -1,6 +1,6 @@ { "name": "@diffity/ui", - "version": "0.9.15", + "version": "0.9.16", "type": "module", "private": true, "scripts": { diff --git a/packages/ui/src/components/diff/diff-page.tsx b/packages/ui/src/components/diff/diff-page.tsx index 04ec311..0d39bd4 100644 --- a/packages/ui/src/components/diff/diff-page.tsx +++ b/packages/ui/src/components/diff/diff-page.tsx @@ -17,7 +17,8 @@ import { readReadingPosition, writeReadingPosition } from '../../lib/reading-pos import { staleMessage } from '../../lib/stale-files'; import { canAskAgent, canActOnCode } from '../../lib/live-mode'; import { patchDiffFile } from '../../lib/patch-diff-file'; -import { newAnswers, dropSeenAlerts, positionForAlert, type AnswerAlert } from '../../lib/answer-alerts'; +import { newAnswers, dropSeenAlerts, positionForAlert, unreadAlerts, type AnswerAlert } from '../../lib/answer-alerts'; +import { useFaviconBadge } from '../../hooks/use-favicon-badge'; import { whereIsThread, type ThreadPosition } from '../../lib/thread-visibility'; import { AnswerBubble } from '../layout/answer-bubble'; import { fetchDiffFile } from '../../lib/api'; @@ -552,6 +553,10 @@ export function DiffPage() { ]); }, [threads]); + // Both lists, so the count does not appear to rise when a note merely stops being shown. + const unread = useMemo(() => unreadAlerts(answerAlerts, unseenAlerts), [answerAlerts, unseenAlerts]); + useFaviconBadge(unread.length > 0); + const [alertPosition, setAlertPosition] = useState('below'); const handleAlertsExpired = useCallback(() => { @@ -686,7 +691,7 @@ export function DiffPage() { githubDetails={githubDetails} reviewInProgress={!!info?.review?.inProgress} live={liveStatus} - unreadAnswers={unseenAlerts} + unreadAnswers={unread} onGoToAnswer={handleGoToAnswer} sessionId={sessionId} onGitHubPulled={() => queryClient.invalidateQueries({ queryKey: ['threads'] })} diff --git a/packages/ui/src/hooks/use-favicon-badge.ts b/packages/ui/src/hooks/use-favicon-badge.ts new file mode 100644 index 0000000..92e8f28 --- /dev/null +++ b/packages/ui/src/hooks/use-favicon-badge.ts @@ -0,0 +1,46 @@ +import { useEffect, useRef } from 'react'; +import { addBadge, toHref, FAVICON_HREF } from '../lib/favicon-badge'; + +/** + * Marks the browser tab while something is unread, so a reader looking at another window can tell + * there is an answer waiting without switching to find out. + */ +export function useFaviconBadge(hasUnread: boolean): void { + const plainSvg = useRef(null); + + useEffect(() => { + const link = document.querySelector('link[rel="icon"]'); + if (!link) { + return; + } + + if (!hasUnread) { + link.href = FAVICON_HREF; + return; + } + + let cancelled = false; + const badge = (svg: string): void => { + if (!cancelled) { + link.href = toHref(addBadge(svg)); + } + }; + + if (plainSvg.current !== null) { + badge(plainSvg.current); + } else { + // The icon is served from the same origin, so this is a cache hit in practice. + void fetch(FAVICON_HREF) + .then(res => res.text()) + .then(svg => { + plainSvg.current = svg; + badge(svg); + }) + .catch(() => {}); + } + + return () => { + cancelled = true; + }; + }, [hasUnread]); +} diff --git a/packages/ui/src/lib/answer-alerts.ts b/packages/ui/src/lib/answer-alerts.ts index be3664d..c796f69 100644 --- a/packages/ui/src/lib/answer-alerts.ts +++ b/packages/ui/src/lib/answer-alerts.ts @@ -100,3 +100,20 @@ export function positionForAlert( return alertAt < readerAt ? 'above' : 'below'; } + +/** + * Everything the reader has not dealt with yet, whether or not a note is still on screen for it. + * + * The bubble and what it leaves behind are two lists, because a note that has had its time is no + * longer in the way but is still unread. The count has to span both, or it appears to go up when a + * note expires — the reader sees the number change at the moment nothing actually happened. + */ +export function unreadAlerts(shown: AnswerAlert[], expired: AnswerAlert[]): AnswerAlert[] { + const byThread = new Map(); + + for (const alert of [...expired, ...shown]) { + byThread.set(alert.threadId, alert); + } + + return [...byThread.values()]; +} diff --git a/packages/ui/src/lib/favicon-badge.ts b/packages/ui/src/lib/favicon-badge.ts new file mode 100644 index 0000000..6ae1a15 --- /dev/null +++ b/packages/ui/src/lib/favicon-badge.ts @@ -0,0 +1,25 @@ +export const FAVICON_HREF = '/favicon.svg'; + +/** + * The same icon with an unread mark on it, the way a chat app marks a tab you are not looking at. + * + * Done as SVG text rather than drawn on a canvas because the icon already is an SVG: the mark + * inherits its scaling, and the ring can follow the same colour-scheme rules the icon uses, so it + * reads on a light tab strip and a dark one. + */ +export function addBadge(svg: string): string { + const closing = svg.lastIndexOf(''); + if (closing === -1) { + return svg; + } + + const mark = '' + + ''; + + return svg.slice(0, closing) + mark + svg.slice(closing); +} + +export function toHref(svg: string): string { + return `data:image/svg+xml,${encodeURIComponent(svg)}`; +} diff --git a/packages/ui/tests/favicon-badge-hook.test.tsx b/packages/ui/tests/favicon-badge-hook.test.tsx new file mode 100644 index 0000000..0133690 --- /dev/null +++ b/packages/ui/tests/favicon-badge-hook.test.tsx @@ -0,0 +1,73 @@ +import { describe, it, expect, afterEach, beforeEach, vi } from 'vitest'; +import { render, cleanup, waitFor } from '@testing-library/react'; +import { useFaviconBadge } from '../src/hooks/use-favicon-badge'; + +const ICON = ''; + +function Page(props: { hasUnread: boolean }) { + useFaviconBadge(props.hasUnread); + return null; +} + +function iconLink(): HTMLLinkElement { + return document.querySelector('link[rel="icon"]')!; +} + +beforeEach(() => { + const link = document.createElement('link'); + link.rel = 'icon'; + link.href = '/favicon.svg'; + document.head.append(link); + vi.stubGlobal('fetch', vi.fn(() => Promise.resolve(new Response(ICON)))); +}); + +afterEach(() => { + cleanup(); + iconLink()?.remove(); + vi.restoreAllMocks(); +}); + +describe('useFaviconBadge', () => { + it('leaves the tab alone while there is nothing unread', () => { + render(); + + expect(iconLink().href).toContain('/favicon.svg'); + }); + + it('marks the tab once something is unread', async () => { + render(); + + await waitFor(() => expect(iconLink().href).toContain('data:image/svg+xml')); + expect(decodeURIComponent(iconLink().href)).toContain(' { + const { rerender } = render(); + await waitFor(() => expect(iconLink().href).toContain('data:')); + + rerender(); + + expect(iconLink().href).toContain('/favicon.svg'); + }); + + // The icon is fetched once and kept, so a count that changes repeatedly does not refetch it. + it('does not fetch the icon again when the mark comes back', async () => { + const { rerender } = render(); + await waitFor(() => expect(iconLink().href).toContain('data:')); + const fetched = (fetch as unknown as { mock: { calls: unknown[] } }).mock.calls.length; + + rerender(); + rerender(); + + await waitFor(() => expect(iconLink().href).toContain('data:')); + expect((fetch as unknown as { mock: { calls: unknown[] } }).mock.calls.length).toBe(fetched); + }); + + it('leaves the tab as it was when the icon cannot be read', async () => { + vi.stubGlobal('fetch', vi.fn(() => Promise.reject(new Error('offline')))); + render(); + + await new Promise(r => setTimeout(r, 10)); + expect(iconLink().href).toContain('/favicon.svg'); + }); +}); diff --git a/packages/ui/tests/favicon-badge.test.ts b/packages/ui/tests/favicon-badge.test.ts new file mode 100644 index 0000000..d3a6874 --- /dev/null +++ b/packages/ui/tests/favicon-badge.test.ts @@ -0,0 +1,44 @@ +import { describe, it, expect } from 'vitest'; +import { addBadge, toHref, FAVICON_HREF } from '../src/lib/favicon-badge'; + +const ICON = ''; + +describe('addBadge', () => { + it('puts the mark inside the icon, so it scales with it', () => { + const badged = addBadge(ICON); + + expect(badged.endsWith('')).toBe(true); + expect(badged).toContain('')); + }); + + it('keeps what was already there', () => { + expect(addBadge(ICON)).toContain(''); + }); + + // A tab strip is light in one theme and dark in the other, and the icon itself already flips. + it('gives the mark a ring that follows the colour scheme', () => { + const badged = addBadge(ICON); + + expect(badged).toContain('prefers-color-scheme: dark'); + expect(badged).toContain('stroke'); + }); + + it('leaves something that is not an icon alone rather than corrupting it', () => { + expect(addBadge('not an svg')).toBe('not an svg'); + }); +}); + +describe('toHref', () => { + it('is usable as a link href', () => { + expect(toHref('')).toBe('data:image/svg+xml,%3Csvg%2F%3E'); + }); + + it('escapes what would otherwise end the attribute', () => { + expect(toHref('')).not.toContain('"'); + }); + + it('knows where the plain icon lives', () => { + expect(FAVICON_HREF).toBe('/favicon.svg'); + }); +}); diff --git a/packages/ui/tests/notification-bell.test.tsx b/packages/ui/tests/notification-bell.test.tsx index 54d0f98..92d284c 100644 --- a/packages/ui/tests/notification-bell.test.tsx +++ b/packages/ui/tests/notification-bell.test.tsx @@ -1,6 +1,7 @@ import { describe, it, expect, afterEach, vi } from 'vitest'; import { render, cleanup, screen, fireEvent } from '@testing-library/react'; import { NotificationBell } from '../src/components/layout/notification-bell'; +import { unreadAlerts } from '../src/lib/answer-alerts'; afterEach(cleanup); @@ -76,3 +77,24 @@ describe('a long list', () => { expect(list.className).toContain('overflow-y-auto'); }); }) + +// The two lists the page keeps — a note still on screen, and what one leaves behind — reach the +// bell as one, so what the reader sees is "how many answers are waiting" rather than "how many +// notes have timed out". +describe('the count against the two lists behind it', () => { + it('includes a note that is still on screen', () => { + render(); + + expect(screen.getByRole('button', { name: /1 unread/i })).toBeTruthy(); + }); + + it('does not move when that note times out', () => { + const { unmount } = render(); + expect(screen.getByText('2')).toBeTruthy(); + unmount(); + + render(); + + expect(screen.getByText('2')).toBeTruthy(); + }); +}); diff --git a/packages/ui/tests/unread-alerts.test.ts b/packages/ui/tests/unread-alerts.test.ts new file mode 100644 index 0000000..84b8b15 --- /dev/null +++ b/packages/ui/tests/unread-alerts.test.ts @@ -0,0 +1,33 @@ +import { describe, it, expect } from 'vitest'; +import { unreadAlerts } from '../src/lib/answer-alerts'; +import type { AnswerAlert } from '../src/lib/answer-alerts'; + +const alert = (threadId: string): AnswerAlert => + ({ threadId, filePath: 'src/a.ts', preview: 'an answer' }) as AnswerAlert; + +describe('unreadAlerts', () => { + it('counts a note that is still on screen', () => { + expect(unreadAlerts([alert('a')], []).map(a => a.threadId)).toEqual(['a']); + }); + + it('counts one whose time has run out', () => { + expect(unreadAlerts([], [alert('a')]).map(a => a.threadId)).toEqual(['a']); + }); + + // The bug this is for: a note moves from shown to expired, and the count must not budge, because + // nothing happened that the reader did not already know about. + it('does not change when a note stops being shown', () => { + const shown = unreadAlerts([alert('a'), alert('b')], []); + const afterExpiry = unreadAlerts([], [alert('a'), alert('b')]); + + expect(afterExpiry.length).toBe(shown.length); + }); + + it('counts a thread once when it is in both lists', () => { + expect(unreadAlerts([alert('a')], [alert('a')]).length).toBe(1); + }); + + it('is empty when there is nothing unread', () => { + expect(unreadAlerts([], [])).toEqual([]); + }); +});