Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion packages/cli/package.json
Original file line numberDiff line numberDiff line change
@@ -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": {
Expand Down
2 changes: 1 addition & 1 deletion packages/git/package.json
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
{
"name": "@diffity/git",
"version": "0.9.15",
"version": "0.9.16",
"private": true,
"type": "module",
"main": "./dist/index.js",
Expand Down
2 changes: 1 addition & 1 deletion packages/github/package.json
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
{
"name": "@diffity/github",
"version": "0.9.15",
"version": "0.9.16",
"private": true,
"type": "module",
"main": "./dist/index.js",
Expand Down
2 changes: 1 addition & 1 deletion packages/parser/package.json
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
{
"name": "@diffity/parser",
"version": "0.9.15",
"version": "0.9.16",
"private": true,
"type": "module",
"main": "./dist/index.js",
Expand Down
2 changes: 1 addition & 1 deletion packages/ui/package.json
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
{
"name": "@diffity/ui",
"version": "0.9.15",
"version": "0.9.16",
"type": "module",
"private": true,
"scripts": {
Expand Down
9 changes: 7 additions & 2 deletions packages/ui/src/components/diff/diff-page.tsx
Original file line numberDiff line numberDiff line change
Expand Up@@ -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';
Expand DownExpand Up@@ -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<ThreadPosition>('below');

const handleAlertsExpired = useCallback(() => {
Expand DownExpand Up@@ -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'] })}
Expand Down
46 changes: 46 additions & 0 deletions packages/ui/src/hooks/use-favicon-badge.ts
Original file line numberDiff line numberDiff line change
@@ -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<string | null>(null);

useEffect(() => {
const link = document.querySelector<HTMLLinkElement>('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]);
}
17 changes: 17 additions & 0 deletions packages/ui/src/lib/answer-alerts.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -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<string, AnswerAlert>();

for (const alert of [...expired, ...shown]) {
byThread.set(alert.threadId, alert);
}

return [...byThread.values()];
}
25 changes: 25 additions & 0 deletions packages/ui/src/lib/favicon-badge.ts
Original file line numberDiff line numberDiff line change
@@ -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('</svg>');
if (closing === -1) {
return svg;
}

const mark = '<style>.unread-ring{stroke:#fff}'
+ '@media (prefers-color-scheme: dark){.unread-ring{stroke:#000}}</style>'
+ '<circle class="unread-ring" cx="300" cy="110" r="88" fill="#e5484d" stroke-width="24"/>';

return svg.slice(0, closing) + mark + svg.slice(closing);
}

export function toHref(svg: string): string {
return `data:image/svg+xml,${encodeURIComponent(svg)}`;
}
73 changes: 73 additions & 0 deletions packages/ui/tests/favicon-badge-hook.test.tsx
Original file line numberDiff line numberDiff line change
@@ -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 = '<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 411 395"><path d="M0 0"/></svg>';

function Page(props: { hasUnread: boolean }) {
useFaviconBadge(props.hasUnread);
return null;
}

function iconLink(): HTMLLinkElement {
return document.querySelector<HTMLLinkElement>('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(<Page hasUnread={false} />);

expect(iconLink().href).toContain('/favicon.svg');
});

it('marks the tab once something is unread', async () => {
render(<Page hasUnread={true} />);

await waitFor(() => expect(iconLink().href).toContain('data:image/svg+xml'));
expect(decodeURIComponent(iconLink().href)).toContain('<circle');
});

it('puts the plain icon back once nothing is', async () => {
const { rerender } = render(<Page hasUnread={true} />);
await waitFor(() => expect(iconLink().href).toContain('data:'));

rerender(<Page hasUnread={false} />);

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(<Page hasUnread={true} />);
await waitFor(() => expect(iconLink().href).toContain('data:'));
const fetched = (fetch as unknown as { mock: { calls: unknown[] } }).mock.calls.length;

rerender(<Page hasUnread={false} />);
rerender(<Page hasUnread={true} />);

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(<Page hasUnread={true} />);

await new Promise(r => setTimeout(r, 10));
expect(iconLink().href).toContain('/favicon.svg');
});
});
44 changes: 44 additions & 0 deletions packages/ui/tests/favicon-badge.test.ts
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
import { describe, it, expect } from 'vitest';
import { addBadge, toHref, FAVICON_HREF } from '../src/lib/favicon-badge';

const ICON = '<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 411 395"><path d="M0 0"/></svg>';

describe('addBadge', () => {
it('puts the mark inside the icon, so it scales with it', () => {
const badged = addBadge(ICON);

expect(badged.endsWith('</svg>')).toBe(true);
expect(badged).toContain('<circle');
expect(badged.indexOf('<circle')).toBeLessThan(badged.indexOf('</svg>'));
});

it('keeps what was already there', () => {
expect(addBadge(ICON)).toContain('<path d="M0 0"/>');
});

// 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('<svg/>')).toBe('data:image/svg+xml,%3Csvg%2F%3E');
});

it('escapes what would otherwise end the attribute', () => {
expect(toHref('<svg a="b"/>')).not.toContain('"');
});

it('knows where the plain icon lives', () => {
expect(FAVICON_HREF).toBe('/favicon.svg');
});
});
22 changes: 22 additions & 0 deletions packages/ui/tests/notification-bell.test.tsx
Original file line numberDiff line numberDiff line change
@@ -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);

Expand DownExpand Up@@ -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(<NotificationBell alerts={unreadAlerts([two[0]], [])} onGo={vi.fn()} />);

expect(screen.getByRole('button', { name: /1 unread/i })).toBeTruthy();
});

it('does not move when that note times out', () => {
const { unmount } = render(<NotificationBell alerts={unreadAlerts(two, [])} onGo={vi.fn()} />);
expect(screen.getByText('2')).toBeTruthy();
unmount();

render(<NotificationBell alerts={unreadAlerts([], two)} onGo={vi.fn()} />);

expect(screen.getByText('2')).toBeTruthy();
});
});
33 changes: 33 additions & 0 deletions packages/ui/tests/unread-alerts.test.ts
Original file line numberDiff line numberDiff line change
@@ -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([]);
});
});