diff --git a/core/highlights/repo.test.ts b/core/highlights/repo.test.ts new file mode 100644 index 0000000..76d890c --- /dev/null +++ b/core/highlights/repo.test.ts @@ -0,0 +1,152 @@ +import type { Database } from 'better-sqlite3' +import { afterEach, beforeEach, describe, expect, it } from 'vitest' + +import { upsertPaper } from '../library/repo.js' +import { openDb } from '../store/db.js' +import { createHighlight, deleteHighlight, listHighlights } from './repo.js' + +describe('highlights repo', () => { + let db: Database + + beforeEach(() => { + db = openDb({ path: ':memory:' }) + upsertPaper(db, { + slug: 'attention-is-all-you-need', + title: 'Attention Is All You Need', + authors: ['A. Vaswani'], + addedAt: new Date().toISOString(), + }) + }) + + afterEach(() => { + db.close() + }) + + it('returns an empty list when a paper has no highlights yet', () => { + expect(listHighlights(db, 'attention-is-all-you-need')).toEqual([]) + }) + + it('creates a highlight and reads it back', () => { + const created = createHighlight(db, { + id: 'h1', + paperSlug: 'attention-is-all-you-need', + page: 3, + color: 'yellow', + quote: 'scaled dot-product attention', + anchor: '{"start":10,"end":40}', + createdAt: '2026-01-01T00:00:00.000Z', + }) + + expect(created).toEqual({ + id: 'h1', + paperSlug: 'attention-is-all-you-need', + page: 3, + color: 'yellow', + quote: 'scaled dot-product attention', + anchor: '{"start":10,"end":40}', + createdAt: '2026-01-01T00:00:00.000Z', + }) + + const highlights = listHighlights(db, 'attention-is-all-you-need') + expect(highlights).toEqual([created]) + }) + + it('lists multiple highlights for a paper ordered by page then created_at ascending', () => { + createHighlight(db, { + id: 'h-page2-later', + paperSlug: 'attention-is-all-you-need', + page: 2, + color: 'green', + quote: 'second page, later', + anchor: '{}', + createdAt: '2026-01-02T00:00:00.000Z', + }) + createHighlight(db, { + id: 'h-page1', + paperSlug: 'attention-is-all-you-need', + page: 1, + color: 'blue', + quote: 'first page', + anchor: '{}', + createdAt: '2026-01-03T00:00:00.000Z', + }) + createHighlight(db, { + id: 'h-page2-earlier', + paperSlug: 'attention-is-all-you-need', + page: 2, + color: 'pink', + quote: 'second page, earlier', + anchor: '{}', + createdAt: '2026-01-01T00:00:00.000Z', + }) + + const highlights = listHighlights(db, 'attention-is-all-you-need') + expect(highlights.map((h) => h.id)).toEqual(['h-page1', 'h-page2-earlier', 'h-page2-later']) + }) + + it('only returns highlights for the requested paper', () => { + upsertPaper(db, { + slug: 'other-paper', + title: 'Other Paper', + authors: [], + addedAt: new Date().toISOString(), + }) + createHighlight(db, { + id: 'h-mine', + paperSlug: 'attention-is-all-you-need', + page: 1, + color: 'yellow', + quote: 'mine', + anchor: '{}', + createdAt: '2026-01-01T00:00:00.000Z', + }) + createHighlight(db, { + id: 'h-other', + paperSlug: 'other-paper', + page: 1, + color: 'yellow', + quote: 'other', + anchor: '{}', + createdAt: '2026-01-01T00:00:00.000Z', + }) + + const highlights = listHighlights(db, 'attention-is-all-you-need') + expect(highlights.map((h) => h.id)).toEqual(['h-mine']) + }) + + it('deletes a highlight', () => { + createHighlight(db, { + id: 'h1', + paperSlug: 'attention-is-all-you-need', + page: 1, + color: 'yellow', + quote: 'to be deleted', + anchor: '{}', + createdAt: '2026-01-01T00:00:00.000Z', + }) + + deleteHighlight(db, 'h1') + expect(listHighlights(db, 'attention-is-all-you-need')).toEqual([]) + }) + + it('deleting a highlight that does not exist is a no-op', () => { + expect(() => deleteHighlight(db, 'no-such-id')).not.toThrow() + }) + + it('deleting the paper cascades and removes its highlights', () => { + createHighlight(db, { + id: 'h1', + paperSlug: 'attention-is-all-you-need', + page: 1, + color: 'yellow', + quote: 'cascade me', + anchor: '{}', + createdAt: '2026-01-01T00:00:00.000Z', + }) + + db.prepare('DELETE FROM papers WHERE slug = ?').run('attention-is-all-you-need') + + const row = db.prepare('SELECT * FROM highlights WHERE id = ?').get('h1') + expect(row).toBeUndefined() + }) +}) diff --git a/core/highlights/repo.ts b/core/highlights/repo.ts new file mode 100644 index 0000000..cf5cde4 --- /dev/null +++ b/core/highlights/repo.ts @@ -0,0 +1,93 @@ +// Highlights CRUD against the `highlights` table (core/store/schema.ts +// SCHEMA_V4_HIGHLIGHTS, migration 4). [P2-02] Highlight tool + Annotations +// tab: multiple highlights per paper, each pinned to a PDF page. +// +// Unlike notes (one row per paper, `paper_slug` as PRIMARY KEY), a paper can +// have many highlights, so this table uses a synthetic, caller-supplied `id` +// (uuid) as its primary key. The caller also supplies `createdAt` (ISO +// string) rather than this module stamping `new Date()` itself, to keep +// creation deterministic and testable. +// +// `anchor` is an opaque JSON string produced by the renderer's text-layer +// selection logic (start/end offsets etc. needed to re-locate the highlight +// on the page). This module stores it verbatim and never parses it — same +// "state vs. content" boundary as everywhere else in core/store. `quote` is +// the exact selected text, kept alongside for the Annotations list UI and as +// a fallback re-find anchor. +// +// `ON DELETE CASCADE` on the papers FK means purging a paper drops its +// highlights automatically. There is no on-disk file counterpart for +// highlights (they only reference page numbers into the existing paper.pdf), +// so there's nothing for the file layer to coordinate on purge here. + +import type { Database } from 'better-sqlite3' + +export interface HighlightRecord { + id: string + paperSlug: string + page: number + color: string + quote: string + anchor: string + createdAt: string +} + +export interface CreateHighlightInput { + id: string + paperSlug: string + page: number + color: string + quote: string + anchor: string + createdAt: string +} + +interface HighlightRow { + id: string + paper_slug: string + page: number + color: string + quote: string + anchor: string + created_at: string +} + +function toRecord(row: HighlightRow): HighlightRecord { + return { + id: row.id, + paperSlug: row.paper_slug, + page: row.page, + color: row.color, + quote: row.quote, + anchor: row.anchor, + createdAt: row.created_at, + } +} + +/** Insert a new highlight. `id` and `createdAt` are caller-supplied for testability. */ +export function createHighlight(db: Database, input: CreateHighlightInput): HighlightRecord { + db.prepare( + `INSERT INTO highlights (id, paper_slug, page, color, quote, anchor, created_at) + VALUES (@id, @paperSlug, @page, @color, @quote, @anchor, @createdAt)`, + ).run(input) + + return { ...input } +} + +/** List all highlights for a paper, ordered by page then creation time ascending. */ +export function listHighlights(db: Database, paperSlug: string): HighlightRecord[] { + const rows = db + .prepare( + `SELECT * FROM highlights + WHERE paper_slug = ? + ORDER BY page ASC, created_at ASC`, + ) + .all(paperSlug) as HighlightRow[] + + return rows.map(toRecord) +} + +/** Delete a highlight by id. No-op if it doesn't exist. */ +export function deleteHighlight(db: Database, id: string): void { + db.prepare('DELETE FROM highlights WHERE id = ?').run(id) +} diff --git a/core/store/db.test.ts b/core/store/db.test.ts index 08ffeef..0dd0c48 100644 --- a/core/store/db.test.ts +++ b/core/store/db.test.ts @@ -60,7 +60,7 @@ describe('openDb', () => { const row = db2.prepare('SELECT * FROM papers WHERE slug = ?').get('p1') expect(row).toBeTruthy() const version = db2.pragma('user_version', { simple: true }) - expect(version).toBe(3) + expect(version).toBe(4) } finally { db2.close() } diff --git a/core/store/migrate.test.ts b/core/store/migrate.test.ts index ec635ed..bdf2d5c 100644 --- a/core/store/migrate.test.ts +++ b/core/store/migrate.test.ts @@ -27,6 +27,7 @@ describe('runMigrations', () => { 'chat_sessions', 'chat_messages', 'notes', + 'highlights', ]), ) }) @@ -36,7 +37,19 @@ describe('runMigrations', () => { runMigrations(db) const version = db.pragma('user_version', { simple: true }) - expect(version).toBe(3) + expect(version).toBe(4) + }) + + it('creates the highlights table index', () => { + db = new Database(':memory:') + runMigrations(db) + + const indexes = db + .prepare("SELECT name FROM sqlite_master WHERE type = 'index' AND tbl_name = 'highlights'") + .all() + .map((row) => (row as { name: string }).name) + + expect(indexes).toContain('idx_highlights_paper') }) it('re-running is a no-op: applying twice does not error or reset data', () => { @@ -51,7 +64,7 @@ describe('runMigrations', () => { runMigrations(db) const version = db.pragma('user_version', { simple: true }) - expect(version).toBe(3) + expect(version).toBe(4) const row = db.prepare('SELECT * FROM papers WHERE slug = ?').get('a') expect(row).toBeTruthy() diff --git a/core/store/migrate.ts b/core/store/migrate.ts index 2694f9c..ac13407 100644 --- a/core/store/migrate.ts +++ b/core/store/migrate.ts @@ -12,7 +12,7 @@ import type { Database } from 'better-sqlite3' -import { SCHEMA_V1, SCHEMA_V3_NOTES } from './schema.js' +import { SCHEMA_V1, SCHEMA_V3_NOTES, SCHEMA_V4_HIGHLIGHTS } from './schema.js' export interface Migration { version: number @@ -24,6 +24,7 @@ export const MIGRATIONS: Migration[] = [ { version: 1, sql: SCHEMA_V1 }, { version: 2, sql: 'ALTER TABLE papers ADD COLUMN summary TEXT;' }, { version: 3, sql: SCHEMA_V3_NOTES }, // [P2-01] notes table + { version: 4, sql: SCHEMA_V4_HIGHLIGHTS }, // [P2-02] highlights table ] /** diff --git a/core/store/schema.ts b/core/store/schema.ts index 649b736..a57dbed 100644 --- a/core/store/schema.ts +++ b/core/store/schema.ts @@ -74,3 +74,25 @@ CREATE TABLE IF NOT EXISTS notes ( updated_at TEXT NOT NULL ); ` + +// [P2-02] Highlight tool + Annotations tab: multiple highlights per paper, so +// (unlike notes) this table uses a synthetic caller-supplied `id` rather than +// `paper_slug` as the primary key. `anchor` is an opaque JSON string produced +// by the renderer's text-layer selection logic (offsets etc.) — this module +// stores it verbatim and never parses it, same "state vs. content" boundary +// as everywhere else in this file. `ON DELETE CASCADE` on the papers FK means +// purging a paper drops its highlights automatically (coordinate the on-disk +// side, if any, with the file layer — this table has none, highlights are +// pure DB state referencing paper.pdf pages). +export const SCHEMA_V4_HIGHLIGHTS = ` +CREATE TABLE IF NOT EXISTS highlights ( + id TEXT PRIMARY KEY, + paper_slug TEXT NOT NULL REFERENCES papers(slug) ON DELETE CASCADE, + page INTEGER NOT NULL, + color TEXT NOT NULL, + quote TEXT NOT NULL, + anchor TEXT NOT NULL, + created_at TEXT NOT NULL +); +CREATE INDEX IF NOT EXISTS idx_highlights_paper ON highlights(paper_slug); +` diff --git a/electron/main.ts b/electron/main.ts index 1e03525..47dc9b0 100644 --- a/electron/main.ts +++ b/electron/main.ts @@ -18,6 +18,8 @@ import { getPaper, listPapers } from '../core/library/repo.js' import type { ListPapersOptions, PaperRecord, PaperSortColumn } from '../core/library/repo.js' import { deleteNote, getNote, upsertNote } from '../core/notes/repo.js' import type { NoteRecord } from '../core/notes/repo.js' +import { createHighlight, deleteHighlight, listHighlights } from '../core/highlights/repo.js' +import type { HighlightRecord } from '../core/highlights/repo.js' import { openDb } from '../core/store/db.js' const __dirname = dirname(fileURLToPath(import.meta.url)) @@ -172,6 +174,65 @@ ipcMain.handle('vellum:notes-delete', (_event, slug: unknown): void => { deleteNote(getDb(), requireSlug(slug, 'vellum:notes-delete')) }) +// [P2-02] Highlight tool + Annotations tab. --------------------------------- +// +// `id`/`createdAt` are stamped here (not by the renderer) — same convention +// as `vellum:ask-start`'s requestId — so storage stays deterministic and +// testable (core/highlights/repo.ts takes both as caller-supplied input). +// `anchor`/`quote`/`color` are opaque strings from the renderer's text-layer +// selection logic; validated only for type/shape, never parsed here. +function parseHighlightsCreateParams(value: unknown): { + slug: string + page: number + color: string + quote: string + anchor: string +} { + if (typeof value !== 'object' || value === null) { + throw new Error('vellum:highlights-create: params must be an object') + } + const candidate = value as Record + const page = candidate['page'] + if (typeof page !== 'number' || !Number.isInteger(page) || page < 1) { + throw new Error('vellum:highlights-create: page must be a positive integer') + } + const color = candidate['color'] + const quote = candidate['quote'] + const anchor = candidate['anchor'] + if (typeof color !== 'string' || typeof quote !== 'string' || typeof anchor !== 'string') { + throw new Error('vellum:highlights-create: color, quote, and anchor must be strings') + } + return { slug: requireSlug(candidate['slug'], 'vellum:highlights-create'), page, color, quote, anchor } +} + +ipcMain.handle('vellum:highlights-create', (_event, params: unknown): HighlightRecord => { + const { slug, page, color, quote, anchor } = parseHighlightsCreateParams(params) + return createHighlight(getDb(), { + id: randomUUID(), + paperSlug: slug, + page, + color, + quote, + anchor, + createdAt: new Date().toISOString(), + }) +}) + +ipcMain.handle('vellum:highlights-list', (_event, slug: unknown): HighlightRecord[] => { + return listHighlights(getDb(), requireSlug(slug, 'vellum:highlights-list')) +}) + +function requireNonEmptyId(value: unknown, channel: string): string { + if (typeof value !== 'string' || value.trim().length === 0) { + throw new Error(`${channel}: id must be a non-empty string`) + } + return value +} + +ipcMain.handle('vellum:highlights-delete', (_event, id: unknown): void => { + deleteHighlight(getDb(), requireNonEmptyId(id, 'vellum:highlights-delete')) +}) + // [P1-10] Ask tab — grounded chat over ACP. ------------------------------- // // One ChatManager for the process lifetime: it caches an AcpSession per diff --git a/electron/preload.ts b/electron/preload.ts index 87f2453..513cbf3 100644 --- a/electron/preload.ts +++ b/electron/preload.ts @@ -4,6 +4,7 @@ import type { AskOpenResult, AskStartParams, AskStreamEvent } from '../core/chat import type { IngestResult } from '../core/ingest/index.js' import type { ListPapersOptions, PaperRecord } from '../core/library/repo.js' import type { NoteRecord } from '../core/notes/repo.js' +import type { HighlightRecord } from '../core/highlights/repo.js' import type { AcpBackend } from '../core/acp/client.js' export interface AskUpdatePayload { @@ -41,6 +42,19 @@ const api = { ipcRenderer.invoke('vellum:notes-save', params), // Delete half of Notes CRUD — "Clear note" action. notesDelete: (slug: string): Promise => ipcRenderer.invoke('vellum:notes-delete', slug), + // [P2-02] Highlight tool + Annotations tab. `anchor` is an opaque JSON + // string the renderer produces from the pdf.js text-layer selection (see + // Reader.tsx's `anchorFromRange`/`rangeFromAnchor`) and consumes to + // re-locate the highlight later — main/core never parse it. + highlightsCreate: (params: { + slug: string + page: number + color: string + quote: string + anchor: string + }): Promise => ipcRenderer.invoke('vellum:highlights-create', params), + highlightsList: (slug: string): Promise => ipcRenderer.invoke('vellum:highlights-list', slug), + highlightsDelete: (id: string): Promise => ipcRenderer.invoke('vellum:highlights-delete', id), // [P1-10] Ask tab — grounded chat over ACP. ----------------------------- // Open (or reload) the most recent chat session + history for a paper. askOpen: (slug: string): Promise => ipcRenderer.invoke('vellum:ask-open', slug), diff --git a/src/app/AnnotationsPanel.module.css b/src/app/AnnotationsPanel.module.css new file mode 100644 index 0000000..184c43d --- /dev/null +++ b/src/app/AnnotationsPanel.module.css @@ -0,0 +1,77 @@ +.annotationsPanel { + list-style: none; + margin: 0; + padding: 0; + overflow-y: auto; + height: 100%; +} + +.row { + display: flex; + align-items: center; + gap: 4px; + border-bottom: 1px solid var(--vellum-border, #2a2a2a); +} + +.rowMain { + flex: 1; + display: flex; + align-items: center; + gap: 8px; + min-width: 0; + border: none; + background: transparent; + color: var(--vellum-text, #eaeaea); + text-align: left; + padding: 10px 8px; + cursor: pointer; +} + +.rowMain:hover { + background: var(--vellum-surface-hover, #222222); +} + +.swatch { + flex-shrink: 0; + width: 10px; + height: 10px; + border-radius: 50%; +} + +.quote { + flex: 1; + min-width: 0; + overflow: hidden; + text-overflow: ellipsis; + white-space: nowrap; + font-size: 13px; +} + +.page { + flex-shrink: 0; + font-size: 11px; + color: var(--vellum-text-dim, #8a8a8a); +} + +.deleteButton { + flex-shrink: 0; + border: none; + background: transparent; + color: var(--vellum-text-dim, #8a8a8a); + font-size: 16px; + line-height: 1; + padding: 8px; + cursor: pointer; +} + +.deleteButton:hover { + color: var(--vellum-text, #eaeaea); +} + +.placeholder { + padding: 24px 16px; + margin: 0; + font-size: 13px; + color: var(--vellum-text-dim, #8a8a8a); + text-align: center; +} diff --git a/src/app/AnnotationsPanel.test.tsx b/src/app/AnnotationsPanel.test.tsx new file mode 100644 index 0000000..d0e186e --- /dev/null +++ b/src/app/AnnotationsPanel.test.tsx @@ -0,0 +1,103 @@ +// @vitest-environment jsdom +import '@testing-library/jest-dom/vitest' +import { act, cleanup, render, screen } from '@testing-library/react' +import userEvent from '@testing-library/user-event' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { AnnotationsPanel } from './AnnotationsPanel' + +let highlightsList: ReturnType +let highlightsDelete: ReturnType + +const HIGHLIGHTS = [ + { id: 'h1', paperSlug: 'p1', page: 2, color: 'yellow', quote: 'first highlight', anchor: '{}', createdAt: 't1' }, + { id: 'h2', paperSlug: 'p1', page: 5, color: 'blue', quote: 'second highlight', anchor: '{}', createdAt: 't2' }, +] + +beforeEach(() => { + highlightsList = vi.fn().mockResolvedValue(HIGHLIGHTS) + highlightsDelete = vi.fn().mockResolvedValue(undefined) + + Object.defineProperty(window, 'vellum', { + configurable: true, + value: { highlightsList, highlightsDelete }, + }) +}) + +afterEach(() => { + cleanup() +}) + +async function flushPromises(): Promise { + await act(async () => { + await Promise.resolve() + }) +} + +describe('AnnotationsPanel', () => { + it('shows a placeholder and never calls window.vellum when no paper is open', () => { + render() + + expect(screen.getByText(/open a paper/i)).toBeInTheDocument() + expect(highlightsList).not.toHaveBeenCalled() + }) + + it('loads and lists highlights for the open paper (color, quote, page)', async () => { + render() + await flushPromises() + + expect(highlightsList).toHaveBeenCalledWith('p1') + expect(screen.getByText('first highlight')).toBeInTheDocument() + expect(screen.getByText('Page 2')).toBeInTheDocument() + expect(screen.getByText('second highlight')).toBeInTheDocument() + expect(screen.getByText('Page 5')).toBeInTheDocument() + }) + + it('shows an empty state when the paper has no highlights', async () => { + highlightsList.mockResolvedValue([]) + + render() + await flushPromises() + + expect(screen.getByText(/no highlights yet/i)).toBeInTheDocument() + }) + + it('calls onJump with the highlight when a row is clicked', async () => { + const user = userEvent.setup() + const onJump = vi.fn() + render() + await flushPromises() + + await user.click(screen.getByText('first highlight')) + + expect(onJump).toHaveBeenCalledWith(HIGHLIGHTS[0]) + }) + + it('deletes a highlight and refreshes the list', async () => { + const user = userEvent.setup() + render() + await flushPromises() + + highlightsList.mockResolvedValue([HIGHLIGHTS[1]]) + await user.click(screen.getByRole('button', { name: /delete highlight: first highlight/i })) + await flushPromises() + + expect(highlightsDelete).toHaveBeenCalledWith('h1') + expect(highlightsList).toHaveBeenCalledTimes(2) + expect(screen.queryByText('first highlight')).not.toBeInTheDocument() + expect(screen.getByText('second highlight')).toBeInTheDocument() + }) + + it('reloads highlights when switching to a different paper', async () => { + highlightsList.mockResolvedValueOnce(HIGHLIGHTS).mockResolvedValueOnce([]) + + const { rerender } = render() + await flushPromises() + expect(screen.getByText('first highlight')).toBeInTheDocument() + + rerender() + await flushPromises() + + expect(highlightsList).toHaveBeenCalledWith('p2') + expect(screen.getByText(/no highlights yet/i)).toBeInTheDocument() + }) +}) diff --git a/src/app/AnnotationsPanel.tsx b/src/app/AnnotationsPanel.tsx new file mode 100644 index 0000000..2029ee1 --- /dev/null +++ b/src/app/AnnotationsPanel.tsx @@ -0,0 +1,77 @@ +// AnnotationsPanel — [P2-02] Annotations tab. Lists this paper's highlights +// (color swatch, quote, page), lets the user jump the Reader to one and +// delete it. Talks only to `window.vellum` (preload bridge) — never imports +// Node/core directly, per AGENTS.md. Mirrors NotesPanel's slug-driven +// load/reset shape. +import { useCallback, useEffect, useState } from 'react' +import type { HighlightRecord } from '../../core/highlights/repo' +import styles from './AnnotationsPanel.module.css' + +interface AnnotationsPanelProps { + /** Slug of the currently open paper, if any. Undefined = no paper open. */ + slug?: string + /** Drives the Reader to a highlight's page (and flashes it) — wired by + * App, since Reader and this panel are siblings under it. */ + onJump?: (highlight: HighlightRecord) => void +} + +export function AnnotationsPanel({ slug, onJump }: AnnotationsPanelProps): JSX.Element { + const [highlights, setHighlights] = useState([]) + const [loading, setLoading] = useState(false) + + const load = useCallback((paperSlug: string) => { + setLoading(true) + window.vellum + .highlightsList(paperSlug) + .then((records) => setHighlights(records)) + .catch(() => setHighlights([])) + .finally(() => setLoading(false)) + }, []) + + useEffect(() => { + if (!slug) { + setHighlights([]) + return + } + load(slug) + }, [slug, load]) + + const deleteHighlight = useCallback( + (id: string) => { + if (!slug) return + window.vellum + .highlightsDelete(id) + .then(() => load(slug)) + .catch(() => undefined) + }, + [slug, load], + ) + + if (!slug) return

Open a paper to view its highlights.

+ + if (!loading && highlights.length === 0) { + return

No highlights yet — turn on Highlight mode and select text to add one.

+ } + + return ( +
    + {highlights.map((highlight) => ( +
  • + + +
    • + ))} +
+ ) +} diff --git a/src/app/App.test.tsx b/src/app/App.test.tsx index a5b2833..969b258 100644 --- a/src/app/App.test.tsx +++ b/src/app/App.test.tsx @@ -41,6 +41,11 @@ beforeEach(() => { // state without asserting on it. askOpen: vi.fn(() => new Promise(() => {})), onAskUpdate: vi.fn(() => () => {}), + // [P2-02] Reader/AnnotationsPanel both load a paper's highlights once + // opened — this test only cares about tab/pane shell behavior. + highlightsList: vi.fn().mockResolvedValue([]), + highlightsCreate: vi.fn(), + highlightsDelete: vi.fn().mockResolvedValue(undefined), }, }) }) diff --git a/src/app/App.tsx b/src/app/App.tsx index b274c20..ac5de2a 100644 --- a/src/app/App.tsx +++ b/src/app/App.tsx @@ -3,11 +3,13 @@ import { IngestModal } from './IngestModal' import { Library } from './Library' import { Reader } from './Reader' import { ReaderToolbar } from './ReaderToolbar' +import type { HighlightColor } from './ReaderToolbar' import { Sidebar } from './Sidebar' import type { NavItem } from './Sidebar' import { TabStrip } from './TabStrip' import type { PaperTab } from './TabStrip' import { RightPanel } from './RightPanel' +import type { HighlightRecord } from '../../core/highlights/repo' import styles from './App.module.css' // Vellum shell — [P1-07] anara-style frame, [P1-08] wires in real data: @@ -24,12 +26,21 @@ import styles from './App.module.css' // grid and switching tabs both funnel through the same `openPaper`/ // `selectTab` handlers. [P1-14] adds the reader toolbar's highlight stub to // the center pane so it's visible even before a paper is open. +// +// [P2-02] Highlight tool state (active/color) and the Annotations-tab "jump +// to this highlight's page" seam both live here too, for the same reason: +// ReaderToolbar/Reader/RightPanel are siblings, not parent/child, so +// anything shared between them funnels through App's state rather than a +// prop drilled through an unrelated tree or a global event bus. export function App(): JSX.Element { const [pong, setPong] = useState('…') const [tabs, setTabs] = useState([]) const [activeTabId, setActiveTabId] = useState(null) const [view, setView] = useState<'reader' | 'library'>('reader') const [isIngestOpen, setIsIngestOpen] = useState(false) + const [highlightActive, setHighlightActive] = useState(false) + const [highlightColor, setHighlightColor] = useState('yellow') + const [jumpTarget, setJumpTarget] = useState<{ page: number; highlightId: string; nonce: number } | null>(null) useEffect(() => { window.vellum?.ping().then(setPong).catch(() => setPong('no-bridge')) @@ -54,6 +65,14 @@ export function App(): JSX.Element { } } + // [P2-02] Annotations tab -> Reader jump seam. `nonce` (not just page/id) + // so clicking the same annotation twice in a row re-triggers the flash — + // Reader's jump effect keys off this whole object changing, not just its + // page/highlightId fields. + function jumpToHighlight(highlight: HighlightRecord): void { + setJumpTarget({ page: highlight.page, highlightId: highlight.id, nonce: Date.now() }) + } + return (
@@ -64,12 +83,21 @@ export function App(): JSX.Element { ) : ( <> - - + setHighlightActive((current) => !current)} + onColorChange={setHighlightColor} + /> + )} - +
/paper.pdf` on disk. import '@testing-library/jest-dom/vitest' -import { cleanup, render, screen, waitFor } from '@testing-library/react' +import { act, cleanup, fireEvent, render, screen, waitFor } from '@testing-library/react' import userEvent from '@testing-library/user-event' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' -import { clampPage, clampScale, countOccurrences, Reader } from './Reader' +import { anchorFromRange, clampPage, clampScale, countOccurrences, rangeFromAnchor, Reader } from './Reader' interface FakePage { getViewport: () => { width: number; height: number } @@ -41,10 +41,29 @@ function fakePdfDoc(): unknown { const getDocumentMock = vi.fn() +// TextLayer's mock actually appends one per text item into the given +// container on render() (unlike a true no-op) — [P2-02]'s highlight-capture +// tests need real DOM text nodes inside the text layer to build a Selection/ +// Range against, the same way pdf.js's real TextLayer would populate it. +// Page-nav/zoom/search/TOC tests above don't touch this DOM at all, so +// making the mock slightly more realistic doesn't affect them. vi.mock('pdfjs-dist', () => ({ GlobalWorkerOptions: {}, TextLayer: class { + private readonly container: HTMLElement + private readonly items: Array<{ str: string }> + + constructor(opts: { textContentSource: { items: Array<{ str: string }> }; container: HTMLElement }) { + this.container = opts.container + this.items = opts.textContentSource.items + } + render(): Promise { + for (const item of this.items) { + const span = document.createElement('span') + span.textContent = item.str + this.container.appendChild(span) + } return Promise.resolve() } }, @@ -56,11 +75,30 @@ vi.mock('pdfjs-dist/build/pdf.worker.mjs?url', () => ({ default: 'worker-url' }) beforeEach(() => { getDocumentMock.mockReset() getDocumentMock.mockReturnValue({ promise: Promise.resolve(fakePdfDoc()) }) + + // jsdom implements Element.getBoundingClientRect (zero rect) but doesn't + // implement Range.prototype.getClientRects at all — Reader's highlight + // overlay effect calls it to position rects. Stubbed here (test-env + // limitation, not a Reader.tsx bug: real Chromium implements this + // natively) so overlay-rendering tests don't crash on a missing DOM API. + if (!Range.prototype.getClientRects) { + Range.prototype.getClientRects = function (): DOMRectList { + const rect = { left: 0, top: 0, right: 10, bottom: 10, width: 10, height: 10, x: 0, y: 0, toJSON: () => ({}) } + return [rect] as unknown as DOMRectList + } + } + Object.defineProperty(window, 'vellum', { configurable: true, value: { ping: vi.fn().mockResolvedValue('pong'), readPaperFile: vi.fn().mockResolvedValue(new ArrayBuffer(8)), + // [P2-02] Reader loads a paper's highlights alongside its PDF bytes; + // an empty list keeps these pagination/zoom/search-focused tests from + // touching overlay rendering at all. + highlightsList: vi.fn().mockResolvedValue([]), + highlightsCreate: vi.fn(), + highlightsDelete: vi.fn(), }, }) }) @@ -90,6 +128,87 @@ describe('pure helpers', () => { }) }) +describe('anchorFromRange / rangeFromAnchor [P2-02]', () => { + // Builds a text-layer-shaped container: multiple text nodes, the + // way pdf.js's TextLayer renders one span per text item. + function buildContainer(spans: string[]): HTMLDivElement { + const container = document.createElement('div') + for (const text of spans) { + const span = document.createElement('span') + span.textContent = text + container.appendChild(span) + } + document.body.appendChild(container) + return container + } + + it('serializes a single-span selection into character offsets', () => { + const container = buildContainer(['Vellum reads PDFs offline']) + const textNode = container.querySelector('span')!.firstChild! + const range = document.createRange() + range.setStart(textNode, 7) + range.setEnd(textNode, 12) + + const anchor = anchorFromRange(container, range) + + expect(anchor).toEqual({ start: 7, end: 12 }) + }) + + it('serializes a selection spanning multiple spans into container-relative offsets', () => { + const container = buildContainer(['Vellum reads', 'PDFs offline']) + const firstText = container.querySelectorAll('span')[0].firstChild! + const secondText = container.querySelectorAll('span')[1].firstChild! + const range = document.createRange() + range.setStart(firstText, 7) // "reads" starts at offset 7 in span 1 + range.setEnd(secondText, 4) // "PDFs" ends at offset 4 in span 2 (offset 12 + 4 = 16 overall) + + const anchor = anchorFromRange(container, range) + + expect(anchor).toEqual({ start: 7, end: 16 }) + }) + + it('returns null for a range whose containers are not inside the container', () => { + const container = buildContainer(['Vellum']) + const outside = document.createElement('div') + outside.textContent = 'not in container' + document.body.appendChild(outside) + const range = document.createRange() + range.selectNodeContents(outside) + + expect(anchorFromRange(container, range)).toBeNull() + }) + + it('rangeFromAnchor reconstructs the same text a Range previously anchored', () => { + const container = buildContainer(['Vellum reads', 'PDFs offline']) + const anchor = { start: 7, end: 16 } + + const range = rangeFromAnchor(container, anchor) + + expect(range).not.toBeNull() + expect(range!.toString()).toBe('readsPDFs') + }) + + it('rangeFromAnchor returns null when the anchor no longer fits the current text', () => { + const container = buildContainer(['short']) + + expect(rangeFromAnchor(container, { start: 0, end: 999 })).toBeNull() + }) + + it('round-trips: anchorFromRange then rangeFromAnchor recovers the original selected text', () => { + const container = buildContainer(['The quick brown fox jumps']) + const textNode = container.querySelector('span')!.firstChild! + const original = document.createRange() + original.setStart(textNode, 4) + original.setEnd(textNode, 15) + expect(original.toString()).toBe('quick brown') + + const anchor = anchorFromRange(container, original)! + const recovered = rangeFromAnchor(container, anchor)! + + expect(recovered.toString()).toBe('quick brown') + }) +}) + describe('Reader empty state', () => { it('shows the empty state and never touches window.vellum when no slug is open', () => { render() @@ -190,3 +309,130 @@ describe('Reader with a loaded document', () => { await waitFor(() => expect(screen.getByLabelText('Search matches')).toHaveTextContent('0 / 0')) }) }) + +// Locates the rendered text-layer div: DOM order in Reader.tsx's pageLayer is +// ,
,
— no +// data-testid is added to production code for this, so tests navigate that +// fixed structural order instead. +function getTextLayer(container: HTMLElement): HTMLElement { + return container.querySelector('canvas')!.nextElementSibling as HTMLElement +} + +function getOverlayContainer(container: HTMLElement): HTMLElement { + return getTextLayer(container).nextElementSibling as HTMLElement +} + +describe('Reader highlight capture [P2-02]', () => { + it('creates a highlight from a text-layer selection while the highlight tool is active, and renders the returned record as an overlay', async () => { + const record = { + id: 'h1', + paperSlug: 'my-paper', + page: 1, + color: 'yellow', + quote: 'Introduction', + anchor: JSON.stringify({ start: 0, end: 12 }), + createdAt: 't', + } + window.vellum.highlightsCreate = vi.fn().mockResolvedValue(record) + + const { container } = render() + await waitFor(() => expect(screen.getByLabelText('Page')).toHaveTextContent('1 of 2')) + + const textLayer = getTextLayer(container) + await waitFor(() => expect(textLayer.querySelector('span')).not.toBeNull()) + + // PAGE_TEXT[0] is 'Introduction to Vellum' — select "Introduction" (0..12). + const textNode = textLayer.querySelector('span')!.firstChild! + const range = document.createRange() + range.setStart(textNode, 0) + range.setEnd(textNode, 12) + const selection = window.getSelection()! + selection.removeAllRanges() + selection.addRange(range) + + fireEvent.mouseUp(textLayer) + + await waitFor(() => + expect(window.vellum.highlightsCreate).toHaveBeenCalledWith({ + slug: 'my-paper', + page: 1, + color: 'yellow', + quote: 'Introduction', + anchor: JSON.stringify({ start: 0, end: 12 }), + }), + ) + + // The record highlightsCreate resolves with is appended to Reader's + // highlight state, which the overlay-recompute effect turns into a + // rendered rect. + const overlayContainer = getOverlayContainer(container) + await waitFor(() => expect(overlayContainer.children.length).toBe(1)) + }) + + it('does not create a highlight on mouseup when the highlight tool is inactive', async () => { + const { container } = render() + await waitFor(() => expect(screen.getByLabelText('Page')).toHaveTextContent('1 of 2')) + + const textLayer = getTextLayer(container) + await waitFor(() => expect(textLayer.querySelector('span')).not.toBeNull()) + + const textNode = textLayer.querySelector('span')!.firstChild! + const range = document.createRange() + range.setStart(textNode, 0) + range.setEnd(textNode, 12) + const selection = window.getSelection()! + selection.removeAllRanges() + selection.addRange(range) + + fireEvent.mouseUp(textLayer) + + expect(window.vellum.highlightsCreate).not.toHaveBeenCalled() + }) +}) + +describe('Reader jump/flash seam [P2-02]', () => { + beforeEach(() => { + vi.useFakeTimers({ shouldAdvanceTime: true }) + }) + + afterEach(() => { + vi.useRealTimers() + }) + + it('navigates to the jump target page, flashes the highlight, then clears the flash after FLASH_DURATION_MS', async () => { + const highlight = { + id: 'h1', + paperSlug: 'my-paper', + page: 2, + color: 'yellow', + quote: 'PDFs', + anchor: JSON.stringify({ start: 0, end: 4 }), + createdAt: 't', + } + window.vellum.highlightsList = vi.fn().mockResolvedValue([highlight]) + + const { container, rerender } = render() + await waitFor(() => expect(screen.getByLabelText('Page')).toHaveTextContent('1 of 2')) + + rerender() + + // Navigated to the jump target's page. + await waitFor(() => expect(screen.getByLabelText('Page')).toHaveTextContent('2 of 2')) + + const overlayContainer = getOverlayContainer(container) + await waitFor(() => expect(overlayContainer.children.length).toBe(1)) + + // Flash applies a second CSS-module class (`highlightRect` + + // `highlightRectFlash`) to the overlay rect — checked by class-token + // count rather than a specific (hashed) class name. + const rect = overlayContainer.firstElementChild as HTMLElement + await waitFor(() => expect(rect.className.trim().split(/\s+/)).toHaveLength(2)) + + // FLASH_DURATION_MS in Reader.tsx is 1500ms. + await act(async () => { + vi.advanceTimersByTime(1500) + }) + + await waitFor(() => expect(rect.className.trim().split(/\s+/)).toHaveLength(1)) + }) +}) diff --git a/src/app/Reader.tsx b/src/app/Reader.tsx index aaa99fb..2c1cb4b 100644 --- a/src/app/Reader.tsx +++ b/src/app/Reader.tsx @@ -21,6 +21,8 @@ import { useEffect, useRef, useState } from 'react' import { GlobalWorkerOptions, TextLayer, getDocument, type PDFDocumentProxy } from 'pdfjs-dist' import workerSrc from 'pdfjs-dist/build/pdf.worker.mjs?url' +import type { HighlightColor } from './ReaderToolbar' +import type { HighlightRecord } from '../../core/highlights/repo' import styles from './Reader.module.css' GlobalWorkerOptions.workerSrc = workerSrc @@ -55,6 +57,73 @@ export function countOccurrences(text: string, query: string): number { return count } +/** [P2-02] Opaque anchor shape stored (JSON-stringified) as `HighlightRecord.anchor` — + * character offsets into the current page's text layer container, walking + * text nodes in DOM order. Re-locatable via `rangeFromAnchor` as long as the + * page's text layer renders the same text content in the same order (true + * for a fixed PDF page — scale/zoom only affects layout, not text-node + * order). Kept as plain offsets (not e.g. a CSS selector) so it survives + * across zoom levels without re-derivation. */ +export interface HighlightAnchor { + start: number + end: number +} + +/** + * Serialize a DOM `Range` inside `container` into character offsets over + * `container`'s text nodes (document order) — the opaque anchor a highlight + * is stored under and later re-located from (`rangeFromAnchor`). Returns + * null if the range's start/end containers aren't found while walking + * `container`'s text nodes (e.g. the selection isn't actually inside it). + */ +export function anchorFromRange(container: Node, range: Range): HighlightAnchor | null { + const walker = document.createTreeWalker(container, NodeFilter.SHOW_TEXT) + let offset = 0 + let start = -1 + let end = -1 + let node: Node | null = walker.nextNode() + while (node) { + const length = (node as Text).data.length + if (start === -1 && node === range.startContainer) start = offset + range.startOffset + if (node === range.endContainer) end = offset + range.endOffset + offset += length + node = walker.nextNode() + } + if (start === -1 || end === -1 || end <= start) return null + return { start, end } +} + +/** + * Inverse of `anchorFromRange`: reconstruct a `Range` covering `anchor`'s + * character span inside `container`'s current text nodes. Returns null if + * the anchor no longer fits (e.g. the page re-rendered with different/less + * text than when the highlight was created) rather than throwing — a stale + * highlight should be silently skipped, not crash the reader. + */ +export function rangeFromAnchor(container: Node, anchor: HighlightAnchor): Range | null { + const walker = document.createTreeWalker(container, NodeFilter.SHOW_TEXT) + let offset = 0 + let startSet = false + let endSet = false + const range = document.createRange() + let node: Node | null = walker.nextNode() + while (node && !(startSet && endSet)) { + const length = (node as Text).data.length + if (!startSet && anchor.start >= offset && anchor.start <= offset + length) { + range.setStart(node, anchor.start - offset) + startSet = true + } + if (!endSet && anchor.end >= offset && anchor.end <= offset + length) { + range.setEnd(node, anchor.end - offset) + endSet = true + } + offset += length + node = walker.nextNode() + } + if (!startSet || !endSet) return null + return range +} + export interface OutlineItem { title: string pageNumber: number | null @@ -141,12 +210,34 @@ export async function searchDocument(pdf: PDFDocumentProxy, query: string): Prom return results } +/** One highlight's rendered overlay rect, positioned relative to the text + * layer container (`pageLayer`'s coordinate space). */ +interface OverlayRect { + id: string + color: string + flash: boolean + left: number + top: number + width: number + height: number +} + interface ReaderProps { /** Paper slug (data/papers//). Undefined = no paper open (empty state). */ slug?: string + /** [P2-02] Highlight tool state, lifted to App so ReaderToolbar (a sibling) + * can drive it. `undefined`/inactive = plain text selection, no capture. */ + highlightTool?: { active: boolean; color: HighlightColor } + /** [P2-02] Jump seam: set by the Annotations tab (via App) to drive the + * reader to a highlight's page and briefly flash it. `nonce` makes + * re-jumping to the same target (clicked twice in a row) re-trigger the + * effect even though `page`/`highlightId` didn't change. */ + jumpTarget?: { page: number; highlightId: string; nonce: number } | null } -export function Reader({ slug }: ReaderProps): JSX.Element { +const FLASH_DURATION_MS = 1500 + +export function Reader({ slug, highlightTool, jumpTarget }: ReaderProps): JSX.Element { const [doc, setDoc] = useState(null) const [numPages, setNumPages] = useState(0) const [pageNumber, setPageNumber] = useState(1) @@ -159,6 +250,10 @@ export function Reader({ slug }: ReaderProps): JSX.Element { const [searchResults, setSearchResults] = useState([]) const [searchIndex, setSearchIndex] = useState(0) const [searching, setSearching] = useState(false) + const [highlights, setHighlights] = useState([]) + const [textLayerVersion, setTextLayerVersion] = useState(0) + const [overlays, setOverlays] = useState([]) + const [flashId, setFlashId] = useState(null) const canvasRef = useRef(null) const textLayerRef = useRef(null) @@ -175,6 +270,7 @@ export function Reader({ slug }: ReaderProps): JSX.Element { setSearchResults([]) setSearchIndex(0) setError(null) + setHighlights([]) if (!slug) return @@ -202,11 +298,33 @@ export function Reader({ slug }: ReaderProps): JSX.Element { } })() + void window.vellum + .highlightsList(slug) + .then((records) => { + if (!cancelled) setHighlights(records) + }) + .catch(() => { + if (!cancelled) setHighlights([]) + }) + return () => { cancelled = true } }, [slug]) + // Jump seam: drive the reader to a highlight's page and flash it briefly. + // Fires on every `jumpTarget` change (the caller bumps `nonce` so clicking + // the same annotation twice re-triggers the flash even though page/id + // didn't change). + useEffect(() => { + if (!jumpTarget) return + setPageNumber(clampPage(jumpTarget.page, numPages)) + setFlashId(jumpTarget.highlightId) + const timer = setTimeout(() => setFlashId(null), FLASH_DURATION_MS) + return () => clearTimeout(timer) + // eslint-disable-next-line react-hooks/exhaustive-deps -- numPages intentionally excluded: re-clamping on every doc load isn't the intent, only on an actual jump request. + }, [jumpTarget]) + // Render the current page (canvas + selectable text layer) on page/zoom change. useEffect(() => { if (!doc) return @@ -243,6 +361,11 @@ export function Reader({ slug }: ReaderProps): JSX.Element { viewport, }) await textLayer.render() + if (cancelled) return + // Bumps the overlay-recompute effect below — the text layer's DOM + // (needed to re-locate highlight anchors) only exists after this + // await resolves. + setTextLayerVersion((version) => version + 1) } })() @@ -251,6 +374,44 @@ export function Reader({ slug }: ReaderProps): JSX.Element { } }, [doc, pageNumber, scale]) + // Recompute highlight overlay rects whenever the highlight list, current + // page, or the rendered text layer itself changes. Anchors that no longer + // resolve (stale page content) are silently skipped — see `rangeFromAnchor`. + useEffect(() => { + const container = textLayerRef.current + if (!container) { + setOverlays([]) + return + } + + const next: OverlayRect[] = [] + const containerRect = container.getBoundingClientRect() + for (const highlight of highlights) { + if (highlight.page !== pageNumber) continue + let anchor: HighlightAnchor + try { + anchor = JSON.parse(highlight.anchor) as HighlightAnchor + } catch { + continue + } + const range = rangeFromAnchor(container, anchor) + if (!range) continue + for (const rect of Array.from(range.getClientRects())) { + next.push({ + id: highlight.id, + color: highlight.color, + flash: highlight.id === flashId, + left: rect.left - containerRect.left, + top: rect.top - containerRect.top, + width: rect.width, + height: rect.height, + }) + } + } + setOverlays(next) + // eslint-disable-next-line react-hooks/exhaustive-deps -- textLayerVersion is the "has the DOM actually re-rendered" signal; highlights/pageNumber/flashId are the real deps. + }, [highlights, pageNumber, textLayerVersion, flashId]) + function goToPage(next: number): void { setPageNumber(clampPage(next, numPages)) } @@ -259,6 +420,36 @@ export function Reader({ slug }: ReaderProps): JSX.Element { setScale((current) => clampScale(current + delta)) } + // [P2-02] Highlight capture: when the highlight tool is active, a mouseup + // inside the text layer with a non-collapsed selection creates a + // highlight from that selection (page, quote, anchor) and clears the + // native selection so it doesn't linger visually once the overlay paints. + function handleTextLayerMouseUp(): void { + if (!highlightTool?.active || !slug) return + const selection = window.getSelection() + if (!selection || selection.isCollapsed || selection.rangeCount === 0) return + + const container = textLayerRef.current + if (!container) return + const range = selection.getRangeAt(0) + if (!container.contains(range.commonAncestorContainer)) return + + const quote = selection.toString().trim() + if (!quote) return + const anchor = anchorFromRange(container, range) + if (!anchor) return + + const page = pageNumber + const color = highlightTool.color + void window.vellum + .highlightsCreate({ slug, page, color, quote, anchor: JSON.stringify(anchor) }) + .then((record) => { + setHighlights((current) => [...current, record]) + selection.removeAllRanges() + }) + .catch(() => undefined) + } + async function runSearch(query: string): Promise { setSearchQuery(query) if (!doc || !query.trim()) { @@ -385,7 +576,27 @@ export function Reader({ slug }: ReaderProps): JSX.Element { {loading ?

Loading…

: null}
-
+
+
diff --git a/src/app/ReaderToolbar.module.css b/src/app/ReaderToolbar.module.css index bd266d8..3c3288c 100644 --- a/src/app/ReaderToolbar.module.css +++ b/src/app/ReaderToolbar.module.css @@ -33,3 +33,16 @@ background: var(--vellum-surface-hover, #222222); color: var(--vellum-text, #eaeaea); } + +.colorSwatch { + width: 18px; + height: 18px; + border-radius: 50%; + border: 2px solid transparent; + cursor: pointer; + padding: 0; +} + +.colorSwatchActive { + border-color: var(--vellum-text, #eaeaea); +} diff --git a/src/app/ReaderToolbar.test.tsx b/src/app/ReaderToolbar.test.tsx index 69d243c..fd9b05b 100644 --- a/src/app/ReaderToolbar.test.tsx +++ b/src/app/ReaderToolbar.test.tsx @@ -2,7 +2,7 @@ import '@testing-library/jest-dom/vitest' import { cleanup, render, screen } from '@testing-library/react' import userEvent from '@testing-library/user-event' -import { afterEach, describe, expect, it } from 'vitest' +import { afterEach, describe, expect, it, vi } from 'vitest' import { ReaderToolbar } from './ReaderToolbar' afterEach(() => { @@ -10,18 +10,43 @@ afterEach(() => { }) describe('ReaderToolbar', () => { - it('renders the Highlight button', () => { - render() - expect(screen.getByRole('button', { name: /highlight/i })).toBeInTheDocument() + it('renders the Highlight button, inactive by default', () => { + render() + + expect(screen.getByRole('button', { name: /highlight/i })).toHaveAttribute('aria-pressed', 'false') }) - it('shows a "coming soon" stub linking [P2-02] on click', async () => { + it('calls onToggle when the Highlight button is clicked', async () => { const user = userEvent.setup() - render() + const onToggle = vi.fn() + render() await user.click(screen.getByRole('button', { name: /highlight/i })) - expect(screen.getByText(/Highlight — coming soon/i)).toBeInTheDocument() - expect(screen.getByText(/P2-02/)).toBeInTheDocument() + expect(onToggle).toHaveBeenCalledTimes(1) + }) + + it('shows a color swatch per highlight color only while active', () => { + const { rerender } = render() + expect(screen.queryByLabelText(/highlight color/i)).not.toBeInTheDocument() + + rerender() + expect(screen.getByLabelText('yellow highlight color')).toBeInTheDocument() + expect(screen.getByLabelText('green highlight color')).toBeInTheDocument() + expect(screen.getByLabelText('blue highlight color')).toBeInTheDocument() + expect(screen.getByLabelText('pink highlight color')).toBeInTheDocument() + }) + + it('marks the currently selected color swatch and calls onColorChange on click', async () => { + const user = userEvent.setup() + const onColorChange = vi.fn() + render() + + expect(screen.getByLabelText('yellow highlight color')).toHaveAttribute('aria-pressed', 'true') + expect(screen.getByLabelText('green highlight color')).toHaveAttribute('aria-pressed', 'false') + + await user.click(screen.getByLabelText('green highlight color')) + + expect(onColorChange).toHaveBeenCalledWith('green') }) }) diff --git a/src/app/ReaderToolbar.tsx b/src/app/ReaderToolbar.tsx index 103b165..b9c281b 100644 --- a/src/app/ReaderToolbar.tsx +++ b/src/app/ReaderToolbar.tsx @@ -1,27 +1,49 @@ -// Reader toolbar — sits above the paper pane. The reader itself lands in -// [P1-09]; this card only needs the highlight-tool affordance visible before -// then, per anara's chrome. Click shows a "coming soon" stub instead of a -// dead button (real highlighting + Annotations tab: [P2-02]). -import { useState } from 'react' -import { ComingSoon } from './ComingSoon' +// Reader toolbar — sits above the paper pane. [P2-02]: the highlight button +// is a real tool toggle (not a "coming soon" stub anymore) — activating it +// switches the Reader's text layer into highlight-capture mode (see +// Reader.tsx's `handleTextLayerMouseUp`), and a color swatch row lets the +// user pick which of four highlight colors new highlights use. Tool +// active/color state is lifted to App (the shell's one source of truth, +// same pattern as tabs/view) since ReaderToolbar and Reader are siblings +// under App, not parent/child. import styles from './ReaderToolbar.module.css' -export function ReaderToolbar(): JSX.Element { - const [open, setOpen] = useState(false) +export const HIGHLIGHT_COLORS = ['yellow', 'green', 'blue', 'pink'] as const +export type HighlightColor = (typeof HIGHLIGHT_COLORS)[number] +interface ReaderToolbarProps { + /** Whether the highlight tool is currently active (text-layer selections + * create highlights instead of just selecting text). */ + active: boolean + /** Color new highlights are created with while the tool is active. */ + color: HighlightColor + onToggle: () => void + onColorChange: (color: HighlightColor) => void +} + +export function ReaderToolbar({ active, color, onToggle, onColorChange }: ReaderToolbarProps): JSX.Element { return (
- + {active + ? HIGHLIGHT_COLORS.map((swatch) => ( +
- {open ? : null}
) } diff --git a/src/app/RightPanel.test.tsx b/src/app/RightPanel.test.tsx index 849c389..6129559 100644 --- a/src/app/RightPanel.test.tsx +++ b/src/app/RightPanel.test.tsx @@ -2,9 +2,19 @@ import '@testing-library/jest-dom/vitest' import { cleanup, render, screen } from '@testing-library/react' import userEvent from '@testing-library/user-event' -import { afterEach, describe, expect, it } from 'vitest' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import { RightPanel } from './RightPanel' +beforeEach(() => { + // Annotations tab ([P2-02]) calls highlightsList when a paper is open; + // these tests render with no slug by default so it's unused, but mocked + // for the tests below that do open a paper. + Object.defineProperty(window, 'vellum', { + configurable: true, + value: { highlightsList: vi.fn().mockResolvedValue([]), highlightsDelete: vi.fn().mockResolvedValue(undefined) }, + }) +}) + afterEach(() => { cleanup() }) @@ -34,13 +44,50 @@ describe('RightPanel', () => { expect(screen.getByText(/metadata will show here/i)).toBeInTheDocument() }) - it('renders a visible "coming soon" stub for deferred tabs (Annotations)', async () => { + it('switches to Annotations and shows its no-paper-open placeholder', async () => { const user = userEvent.setup() render() await user.click(screen.getByRole('tab', { name: 'Annotations' })) - expect(screen.getByText(/Annotations — coming soon/i)).toBeInTheDocument() - expect(screen.getByText(/P2-02/)).toBeInTheDocument() + + expect(screen.getByRole('tab', { name: 'Annotations' })).toHaveAttribute('aria-selected', 'true') + expect(screen.getByText(/open a paper to view its highlights/i)).toBeInTheDocument() + expect(window.vellum.highlightsList).not.toHaveBeenCalled() + }) + + it('renders AnnotationsPanel (not a stub) for an open paper', async () => { + window.vellum.highlightsList = vi + .fn() + .mockResolvedValue([{ id: 'h1', paperSlug: 'p1', page: 3, color: 'yellow', quote: 'a quote', anchor: '{}', createdAt: 't' }]) + // RightPanel defaults to the Ask tab, which mounts AskPanel bound to + // `slug` before we switch to Annotations — a never-resolving promise + // keeps it harmlessly loading (same pattern as App.test.tsx). + window.vellum.askOpen = vi.fn(() => new Promise(() => {})) + window.vellum.onAskUpdate = vi.fn(() => () => {}) + const user = userEvent.setup() + render() + + await user.click(screen.getByRole('tab', { name: 'Annotations' })) + + expect(await screen.findByText('a quote')).toBeInTheDocument() + expect(screen.getByText('Page 3')).toBeInTheDocument() + }) + + it('threads onJumpToHighlight through to AnnotationsPanel: clicking an annotation row fires it with that highlight', async () => { + const highlight = { id: 'h1', paperSlug: 'p1', page: 3, color: 'yellow', quote: 'a quote', anchor: '{}', createdAt: 't' } + window.vellum.highlightsList = vi.fn().mockResolvedValue([highlight]) + window.vellum.askOpen = vi.fn(() => new Promise(() => {})) + window.vellum.onAskUpdate = vi.fn(() => () => {}) + const onJumpToHighlight = vi.fn() + const user = userEvent.setup() + + render() + await user.click(screen.getByRole('tab', { name: 'Annotations' })) + + const row = await screen.findByText('a quote') + await user.click(row) + + expect(onJumpToHighlight).toHaveBeenCalledWith(highlight) }) it('switches to Notes and shows its no-paper-open placeholder', async () => { diff --git a/src/app/RightPanel.tsx b/src/app/RightPanel.tsx index a79b1e3..504300c 100644 --- a/src/app/RightPanel.tsx +++ b/src/app/RightPanel.tsx @@ -5,31 +5,34 @@ // still cover the no-paper-open case. // - Notes: [P2-01] per-paper markdown note (NotesPanel), bound to slug. // - Details renders an empty-state placeholder (real logic: [P1-12]). -// - Annotations is Phase-2 and renders a visible "coming soon" stub linking -// its wiki card — never a dead/invisible button. +// - Annotations: [P2-02] this paper's highlights (AnnotationsPanel), bound +// to slug; clicking one jumps the Reader to its page via +// `onJumpToHighlight` (threaded down from App — Reader and this panel +// are siblings under it, not parent/child). import { useState } from 'react' +import { AnnotationsPanel } from './AnnotationsPanel' import { AskInput } from './AskInput' import { AskPanel } from './AskPanel' -import { ComingSoon } from './ComingSoon' import { DetailsPanel } from './DetailsPanel' import { NotesPanel } from './NotesPanel' +import type { HighlightRecord } from '../../core/highlights/repo' import styles from './RightPanel.module.css' const RIGHT_PANEL_TABS = ['Ask', 'Notes', 'Details', 'Annotations'] as const export type RightPanelTab = (typeof RIGHT_PANEL_TABS)[number] -const DEFERRED_TABS: Partial> = { - Annotations: { note: 'Highlights + annotations — wiki card [P2-02]' }, -} - interface RightPanelProps { defaultTab?: RightPanelTab - /** Slug of the currently open paper, if any — scopes the Ask tab's chat. - * Undefined = no paper open, Ask shows the [P1-07] empty-state instead. */ + /** Slug of the currently open paper, if any — scopes the Ask/Notes/ + * Annotations tabs. Undefined = no paper open, each tab shows its own + * empty state instead. */ slug?: string + /** [P2-02] Drives the Reader to a clicked annotation's page. Undefined = + * jump seam not wired (e.g. tests rendering RightPanel standalone). */ + onJumpToHighlight?: (highlight: HighlightRecord) => void } -export function RightPanel({ defaultTab = 'Ask', slug }: RightPanelProps): JSX.Element { +export function RightPanel({ defaultTab = 'Ask', slug, onJumpToHighlight }: RightPanelProps): JSX.Element { const [activeTab, setActiveTab] = useState(defaultTab) return ( @@ -50,21 +53,18 @@ export function RightPanel({ defaultTab = 'Ask', slug }: RightPanelProps): JSX.E
- {renderTabContent(activeTab, slug)} + {renderTabContent(activeTab, slug, onJumpToHighlight)}
) } -function renderTabContent(tab: RightPanelTab, slug: string | undefined): JSX.Element { - const deferred = DEFERRED_TABS[tab] - if (deferred) { - return - } - - // Only Ask/Notes/Details reach here — Annotations is handled above via - // DEFERRED_TABS. - switch (tab as 'Ask' | 'Notes' | 'Details') { +function renderTabContent( + tab: RightPanelTab, + slug: string | undefined, + onJumpToHighlight: ((highlight: HighlightRecord) => void) | undefined, +): JSX.Element { + switch (tab) { case 'Ask': if (slug) return return ( @@ -77,5 +77,7 @@ function renderTabContent(tab: RightPanelTab, slug: string | undefined): JSX.Ele return case 'Details': return + case 'Annotations': + return } }