From 1cad6d2996f79a3518ded30dc247627ca054306d Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 27 Feb 2026 17:05:33 +0000 Subject: [PATCH 01/10] feat: add Find in PDF (Cmd+F) text search Add in-document text search to the PDF viewer. Pressing Cmd+F opens a search bar that finds and highlights matches across all rendered pages. - Case-insensitive search through pdfjs text layer spans - Yellow highlight overlays on all matches, orange for current match - Navigate with Enter/Shift+Enter or Cmd+G/Cmd+Shift+G - Match counter showing "N of M" results - Auto-scrolls to bring current match into view - Escape or close button dismisses the search bar - Dark mode support for highlight colors Closes #99 https://claude.ai/code/session_01PtqSxhcyfUARLFvQPjMbqt --- src/renderer/components/PdfViewer.tsx | 18 +- src/renderer/components/pdf/PdfSearchBar.tsx | 236 +++++++++++++++++++ src/renderer/styles/globals.css | 23 ++ 3 files changed, 274 insertions(+), 3 deletions(-) create mode 100644 src/renderer/components/pdf/PdfSearchBar.tsx diff --git a/src/renderer/components/PdfViewer.tsx b/src/renderer/components/PdfViewer.tsx index 43138c1..a7a9b8b 100644 --- a/src/renderer/components/PdfViewer.tsx +++ b/src/renderer/components/PdfViewer.tsx @@ -3,6 +3,7 @@ import { useCallback, useEffect, useLayoutEffect, useRef, useState } from 'react import { ConfirmPopup } from './ConfirmPopup'; import { HighlightIcon, StickyNoteIcon } from './Icons'; import { PdfPage, selectionRectsToQuadPoints } from './pdf/PdfPage'; +import { PdfSearchBar } from './pdf/PdfSearchBar'; import { usePdfDocument } from './pdf/usePdfDocument'; import { ShortcutHint } from './ShortcutHint'; @@ -198,6 +199,7 @@ export function PdfViewer({ paperId, pdfUrl, arxivId }: { paperId?: string; pdfU const [highlightToolbar, setHighlightToolbar] = useState(null); const [stickyNotePopup, setStickyNotePopup] = useState(null); const [deleteConfirm, setDeleteConfirm] = useState(null); + const [showSearch, setShowSearch] = useState(false); const isPinching = useRef(false); const contentRef = useRef(null); @@ -409,10 +411,16 @@ export function PdfViewer({ paperId, pdfUrl, arxivId }: { paperId?: string; pdfU setVisualScale(1.0); }, [prepareScaleCommit]); + const closeSearch = useCallback(() => setShowSearch(false), []); + // Cmd+/Cmd- keyboard shortcuts useEffect(() => { const handleKeyDown = (event: KeyboardEvent) => { if (event.key === 'Escape') { + if (showSearch) { + setShowSearch(false); + return; + } setAnnotationMode('read'); setHighlightToolbar(null); setStickyNotePopup(null); @@ -420,7 +428,10 @@ export function PdfViewer({ paperId, pdfUrl, arxivId }: { paperId?: string; pdfU return; } if (!event.metaKey) return; - if (event.key === '=' || event.key === '+') { + if (event.key === 'f') { + event.preventDefault(); + setShowSearch(true); + } else if (event.key === '=' || event.key === '+') { event.preventDefault(); zoomIn(); } else if (event.key === '-') { @@ -433,7 +444,7 @@ export function PdfViewer({ paperId, pdfUrl, arxivId }: { paperId?: string; pdfU }; window.addEventListener('keydown', handleKeyDown); return () => window.removeEventListener('keydown', handleKeyDown); - }, [zoomIn, zoomOut, zoomReset]); + }, [zoomIn, zoomOut, zoomReset, showSearch]); // Pinch-to-zoom useEffect(() => { @@ -745,7 +756,8 @@ export function PdfViewer({ paperId, pdfUrl, arxivId }: { paperId?: string; pdfU )} -
+
+ {showSearch && }
{pages.map((page, index) => ( containing this match */ + span: HTMLElement; + /** Character offset within the span's textContent where the match starts */ + startOffset: number; + /** Length of the matched text */ + length: number; + /** Page index (0-based) */ + pageIndex: number; +} + +interface PdfSearchBarProps { + containerRef: React.RefObject; + onClose: () => void; +} + +function findMatchesInTextLayers(container: HTMLElement, query: string): SearchMatch[] { + if (!query) return []; + + const lowerQuery = query.toLowerCase(); + const matches: SearchMatch[] = []; + + // Get all page wrappers in order + const pageWrappers = container.querySelectorAll('[data-page-index]'); + + for (const wrapper of pageWrappers) { + const pageIndex = Number.parseInt(wrapper.getAttribute('data-page-index') || '0', 10); + const textLayer = wrapper.querySelector('.textLayer'); + if (!textLayer) continue; + + const spans = textLayer.querySelectorAll('span'); + for (const span of spans) { + const text = span.textContent || ''; + const lowerText = text.toLowerCase(); + let searchFrom = 0; + + while (searchFrom < lowerText.length) { + const idx = lowerText.indexOf(lowerQuery, searchFrom); + if (idx === -1) break; + matches.push({ span, startOffset: idx, length: query.length, pageIndex }); + searchFrom = idx + 1; + } + } + } + + return matches; +} + +function clearHighlights(container: HTMLElement) { + for (const el of container.querySelectorAll('.pdf-search-highlight')) { + el.remove(); + } +} + +function highlightMatches(matches: SearchMatch[], currentIndex: number) { + // Group by page wrapper to batch DOM reads + for (let i = 0; i < matches.length; i++) { + const match = matches[i]; + const span = match.span; + const pageWrapper = span.closest('[data-page-index]') as HTMLElement; + if (!pageWrapper) continue; + + // We need to measure where the matched substring is within the span. + // Use a Range to get the bounding rect of the matched characters. + const textNode = span.firstChild; + if (!textNode || textNode.nodeType !== Node.TEXT_NODE) continue; + + const range = document.createRange(); + try { + range.setStart(textNode, match.startOffset); + range.setEnd(textNode, match.startOffset + match.length); + } catch { + continue; + } + + const rects = range.getClientRects(); + const wrapperRect = pageWrapper.getBoundingClientRect(); + + for (const rect of rects) { + if (rect.width === 0 || rect.height === 0) continue; + const highlight = document.createElement('div'); + highlight.className = + i === currentIndex ? 'pdf-search-highlight pdf-search-highlight-current' : 'pdf-search-highlight'; + highlight.style.position = 'absolute'; + highlight.style.left = `${rect.left - wrapperRect.left}px`; + highlight.style.top = `${rect.top - wrapperRect.top}px`; + highlight.style.width = `${rect.width}px`; + highlight.style.height = `${rect.height}px`; + highlight.style.pointerEvents = 'none'; + if (i === currentIndex) { + highlight.setAttribute('data-current', 'true'); + } + pageWrapper.appendChild(highlight); + } + } +} + +export function PdfSearchBar({ containerRef, onClose }: PdfSearchBarProps) { + const [query, setQuery] = useState(''); + const [matches, setMatches] = useState([]); + const [currentIndex, setCurrentIndex] = useState(0); + const inputRef = useRef(null); + + // Focus input on mount + useEffect(() => { + inputRef.current?.focus(); + inputRef.current?.select(); + }, []); + + // Find matches when query changes + useEffect(() => { + const container = containerRef.current; + if (!container) return; + + if (!query) { + setMatches([]); + setCurrentIndex(0); + return; + } + + const found = findMatchesInTextLayers(container, query); + setMatches(found); + setCurrentIndex(0); + }, [query, containerRef]); + + // Highlight matches and scroll to current + useEffect(() => { + const container = containerRef.current; + if (!container) return; + + clearHighlights(container); + if (matches.length === 0) return; + + highlightMatches(matches, currentIndex); + + const currentHighlight = container.querySelector('.pdf-search-highlight-current'); + if (currentHighlight) { + currentHighlight.scrollIntoView({ block: 'center', behavior: 'smooth' }); + } + }, [currentIndex, matches, containerRef]); + + const goToNext = useCallback(() => { + if (matches.length === 0) return; + setCurrentIndex((prev) => (prev + 1) % matches.length); + }, [matches.length]); + + const goToPrev = useCallback(() => { + if (matches.length === 0) return; + setCurrentIndex((prev) => (prev - 1 + matches.length) % matches.length); + }, [matches.length]); + + const handleKeyDown = useCallback( + (event: React.KeyboardEvent) => { + if (event.key === 'Escape') { + event.preventDefault(); + event.stopPropagation(); + onClose(); + } else if (event.key === 'Enter' && event.shiftKey) { + event.preventDefault(); + goToPrev(); + } else if (event.key === 'Enter') { + event.preventDefault(); + goToNext(); + } else if (event.key === 'g' && event.metaKey && event.shiftKey) { + event.preventDefault(); + goToPrev(); + } else if (event.key === 'g' && event.metaKey) { + event.preventDefault(); + goToNext(); + } + }, + [onClose, goToNext, goToPrev], + ); + + // Clean up highlights on unmount + useEffect(() => { + return () => { + const container = containerRef.current; + if (container) clearHighlights(container); + }; + }, [containerRef]); + + return ( +
+ setQuery(e.target.value)} + placeholder="Find in PDF..." + className="w-48 text-mac-small bg-transparent border-none outline-none placeholder-gray-400" + /> + {query && ( + + {matches.length > 0 ? `${currentIndex + 1} of ${matches.length}` : 'No matches'} + + )} +
+ + +
+ +
+ ); +} diff --git a/src/renderer/styles/globals.css b/src/renderer/styles/globals.css index 95071a4..de8823c 100644 --- a/src/renderer/styles/globals.css +++ b/src/renderer/styles/globals.css @@ -88,6 +88,29 @@ font-size: 12px; } +/* PDF text search highlights */ +.pdf-search-highlight { + background: rgba(255, 200, 0, 0.35); + border-radius: 1px; + mix-blend-mode: multiply; +} + +.pdf-search-highlight-current { + background: rgba(255, 150, 0, 0.6); +} + +@media (prefers-color-scheme: dark) { + .pdf-search-highlight { + background: rgba(255, 200, 0, 0.3); + mix-blend-mode: screen; + } + + .pdf-search-highlight-current { + background: rgba(255, 150, 0, 0.5); + mix-blend-mode: screen; + } +} + @keyframes toast-slide-up { from { opacity: 0; From 7c6ec5c9405d0cfff6ea1d75b7c4c29140f2beb7 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 27 Feb 2026 18:19:00 +0000 Subject: [PATCH 02/10] test: add e2e test for PDF search with screenshots Playwright test that seeds a paper with a generated test PDF, opens it, triggers Cmd+F search, types a query, and captures screenshots at each step. Screenshots are saved to test-results/ and uploaded as CI artifacts. https://claude.ai/code/session_01PtqSxhcyfUARLFvQPjMbqt --- e2e/helpers/create-test-pdf.ts | 44 ++++++++++++++++++ e2e/pdf-search.spec.ts | 81 ++++++++++++++++++++++++++++++++++ 2 files changed, 125 insertions(+) create mode 100644 e2e/helpers/create-test-pdf.ts create mode 100644 e2e/pdf-search.spec.ts diff --git a/e2e/helpers/create-test-pdf.ts b/e2e/helpers/create-test-pdf.ts new file mode 100644 index 0000000..e9dd20a --- /dev/null +++ b/e2e/helpers/create-test-pdf.ts @@ -0,0 +1,44 @@ +import fs from 'fs'; +import path from 'path'; +import { PDFDocument, rgb, StandardFonts } from 'pdf-lib'; + +/** + * Generate a minimal test PDF with searchable text content. + * Returns the path to the generated file. + */ +export async function createTestPdf(dir: string, filename = 'test-paper.pdf'): Promise { + const doc = await PDFDocument.create(); + const font = await doc.embedFont(StandardFonts.Helvetica); + const page = doc.addPage([612, 792]); // US Letter + + const lines = [ + 'Attention Is All You Need', + '', + 'Abstract', + '', + 'The dominant sequence transduction models are based on complex recurrent', + 'or convolutional neural networks that include an encoder and a decoder.', + 'The best performing models also connect the encoder and decoder through', + 'an attention mechanism. We propose a new simple network architecture,', + 'the Transformer, based solely on attention mechanisms, dispensing with', + 'recurrence and convolutions entirely.', + ]; + + let y = 720; + for (const line of lines) { + const size = line === lines[0] ? 18 : 11; + page.drawText(line, { + x: 72, + y, + size, + font, + color: rgb(0, 0, 0), + }); + y -= size + 6; + } + + const pdfBytes = await doc.save(); + const filePath = path.join(dir, filename); + fs.writeFileSync(filePath, pdfBytes); + return filePath; +} diff --git a/e2e/pdf-search.spec.ts b/e2e/pdf-search.spec.ts new file mode 100644 index 0000000..4c0b562 --- /dev/null +++ b/e2e/pdf-search.spec.ts @@ -0,0 +1,81 @@ +import fs from 'fs'; +import os from 'os'; +import path from 'path'; +import { expect, test } from './fixtures'; +import { createTestPdf } from './helpers/create-test-pdf'; + +let pdfDir: string; +let pdfPath: string; + +test.beforeAll(async () => { + pdfDir = fs.mkdtempSync(path.join(os.tmpdir(), 'papershelf-pdf-')); + pdfPath = await createTestPdf(pdfDir); +}); + +test.afterAll(() => { + fs.rmSync(pdfDir, { recursive: true, force: true }); +}); + +test('Cmd+F opens search bar in PDF viewer', async ({ electronApp, window }) => { + // Seed a paper with a local PDF + await electronApp.evaluate( + async ({ pdfFilePath }) => { + const db = (global as Record).__papershelf_db; + db.insertPaper({ + arxivId: '1706.03762', + title: 'Attention Is All You Need', + authors: ['Ashish Vaswani', 'Noam Shazeer'], + abstract: 'The dominant sequence transduction models are based on complex recurrent neural networks.', + publishedDate: '2017-06-12T00:00:00Z', + updatedDate: '2017-06-12T00:00:00Z', + categories: ['cs.CL', 'cs.AI'], + arxivUrl: 'https://arxiv.org/abs/1706.03762', + pdfUrl: 'https://arxiv.org/pdf/1706.03762', + pdfPath: pdfFilePath, + fullText: null, + }); + }, + { pdfFilePath: pdfPath }, + ); + + // Navigate to library and click the paper + await window.getByRole('button', { name: 'My Library' }).click(); + await expect(window.getByText('Attention Is All You Need').first()).toBeVisible({ timeout: 5000 }); + await window.getByText('Attention Is All You Need').first().click(); + + // Wait for PDF to render (text layer spans appear) + await window.locator('.textLayer span').first().waitFor({ timeout: 10000 }); + + // Take a screenshot of the PDF viewer before search + await window.screenshot({ path: 'test-results/pdf-viewer-before-search.png' }); + + // Open search with Cmd+F + await window.keyboard.press('Meta+f'); + + // Search bar should be visible + const searchInput = window.getByPlaceholder('Find in PDF...'); + await expect(searchInput).toBeVisible({ timeout: 3000 }); + + // Take screenshot showing search bar open + await window.screenshot({ path: 'test-results/pdf-search-bar-open.png' }); + + // Type a search query + await searchInput.fill('attention'); + + // Wait for match counter to appear + await expect(window.getByText(/\d+ of \d+/)).toBeVisible({ timeout: 3000 }); + + // Take screenshot showing search results + await window.screenshot({ path: 'test-results/pdf-search-results.png' }); + + // Navigate to next match with Enter + await searchInput.press('Enter'); + await window.screenshot({ path: 'test-results/pdf-search-next-match.png' }); + + // Close search with Escape + await searchInput.press('Escape'); + await expect(searchInput).not.toBeVisible(); + + // Take screenshot after closing search (highlights should be gone) + await window.screenshot({ path: 'test-results/pdf-search-closed.png' }); +}); From 9c9c4e6d9b87ea3aafbbbfe76c7bed08481a4f1b Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 27 Feb 2026 18:26:57 +0000 Subject: [PATCH 03/10] fix: rewrite pdf-search e2e test for CI reliability Create the test PDF inside electronApp.evaluate (main process) to eliminate cross-process file path issues. Add intermediate waits for "Loading PDF..." state and explicit error checking before waiting for text layer. Increase timeouts. Remove unused helper file. https://claude.ai/code/session_01PtqSxhcyfUARLFvQPjMbqt --- e2e/helpers/create-test-pdf.ts | 44 -------------- e2e/pdf-search.spec.ts | 103 ++++++++++++++++++++------------- 2 files changed, 63 insertions(+), 84 deletions(-) delete mode 100644 e2e/helpers/create-test-pdf.ts diff --git a/e2e/helpers/create-test-pdf.ts b/e2e/helpers/create-test-pdf.ts deleted file mode 100644 index e9dd20a..0000000 --- a/e2e/helpers/create-test-pdf.ts +++ /dev/null @@ -1,44 +0,0 @@ -import fs from 'fs'; -import path from 'path'; -import { PDFDocument, rgb, StandardFonts } from 'pdf-lib'; - -/** - * Generate a minimal test PDF with searchable text content. - * Returns the path to the generated file. - */ -export async function createTestPdf(dir: string, filename = 'test-paper.pdf'): Promise { - const doc = await PDFDocument.create(); - const font = await doc.embedFont(StandardFonts.Helvetica); - const page = doc.addPage([612, 792]); // US Letter - - const lines = [ - 'Attention Is All You Need', - '', - 'Abstract', - '', - 'The dominant sequence transduction models are based on complex recurrent', - 'or convolutional neural networks that include an encoder and a decoder.', - 'The best performing models also connect the encoder and decoder through', - 'an attention mechanism. We propose a new simple network architecture,', - 'the Transformer, based solely on attention mechanisms, dispensing with', - 'recurrence and convolutions entirely.', - ]; - - let y = 720; - for (const line of lines) { - const size = line === lines[0] ? 18 : 11; - page.drawText(line, { - x: 72, - y, - size, - font, - color: rgb(0, 0, 0), - }); - y -= size + 6; - } - - const pdfBytes = await doc.save(); - const filePath = path.join(dir, filename); - fs.writeFileSync(filePath, pdfBytes); - return filePath; -} diff --git a/e2e/pdf-search.spec.ts b/e2e/pdf-search.spec.ts index 4c0b562..b2c8353 100644 --- a/e2e/pdf-search.spec.ts +++ b/e2e/pdf-search.spec.ts @@ -1,50 +1,73 @@ -import fs from 'fs'; -import os from 'os'; -import path from 'path'; import { expect, test } from './fixtures'; -import { createTestPdf } from './helpers/create-test-pdf'; - -let pdfDir: string; -let pdfPath: string; - -test.beforeAll(async () => { - pdfDir = fs.mkdtempSync(path.join(os.tmpdir(), 'papershelf-pdf-')); - pdfPath = await createTestPdf(pdfDir); -}); - -test.afterAll(() => { - fs.rmSync(pdfDir, { recursive: true, force: true }); -}); test('Cmd+F opens search bar in PDF viewer', async ({ electronApp, window }) => { - // Seed a paper with a local PDF - await electronApp.evaluate( - async ({ pdfFilePath }) => { - const db = (global as Record).__papershelf_db; - db.insertPaper({ - arxivId: '1706.03762', - title: 'Attention Is All You Need', - authors: ['Ashish Vaswani', 'Noam Shazeer'], - abstract: 'The dominant sequence transduction models are based on complex recurrent neural networks.', - publishedDate: '2017-06-12T00:00:00Z', - updatedDate: '2017-06-12T00:00:00Z', - categories: ['cs.CL', 'cs.AI'], - arxivUrl: 'https://arxiv.org/abs/1706.03762', - pdfUrl: 'https://arxiv.org/pdf/1706.03762', - pdfPath: pdfFilePath, - fullText: null, - }); - }, - { pdfFilePath: pdfPath }, - ); + // Create a test PDF and seed a paper — all inside the main process to avoid path issues + await electronApp.evaluate(async () => { + const { PDFDocument, StandardFonts, rgb } = await import('pdf-lib'); + const fs = await import('fs'); + const path = await import('path'); + const os = await import('os'); + + const doc = await PDFDocument.create(); + const font = await doc.embedFont(StandardFonts.Helvetica); + const page = doc.addPage([612, 792]); + const lines = [ + { text: 'Attention Is All You Need', size: 18 }, + { text: '', size: 11 }, + { text: 'Abstract', size: 14 }, + { text: '', size: 11 }, + { text: 'The dominant sequence transduction models are based on complex recurrent', size: 11 }, + { text: 'or convolutional neural networks that include an encoder and a decoder.', size: 11 }, + { text: 'We propose a new simple network architecture, the Transformer, based', size: 11 }, + { text: 'solely on attention mechanisms, dispensing with recurrence entirely.', size: 11 }, + ]; + let y = 720; + for (const line of lines) { + page.drawText(line.text, { x: 72, y, size: line.size, font, color: rgb(0, 0, 0) }); + y -= line.size + 6; + } + const pdfBytes = await doc.save(); + + const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'papershelf-pdf-')); + const pdfPath = path.join(tmpDir, 'test-paper.pdf'); + fs.writeFileSync(pdfPath, pdfBytes); + + const db = (global as Record).__papershelf_db; + db.insertPaper({ + arxivId: '1706.03762', + title: 'Attention Is All You Need', + authors: ['Ashish Vaswani', 'Noam Shazeer'], + abstract: 'The dominant sequence transduction models are based on complex recurrent neural networks.', + publishedDate: '2017-06-12T00:00:00Z', + updatedDate: '2017-06-12T00:00:00Z', + categories: ['cs.CL', 'cs.AI'], + arxivUrl: 'https://arxiv.org/abs/1706.03762', + pdfUrl: 'https://arxiv.org/pdf/1706.03762', + pdfPath, + fullText: null, + }); + }); // Navigate to library and click the paper await window.getByRole('button', { name: 'My Library' }).click(); await expect(window.getByText('Attention Is All You Need').first()).toBeVisible({ timeout: 5000 }); await window.getByText('Attention Is All You Need').first().click(); - // Wait for PDF to render (text layer spans appear) - await window.locator('.textLayer span').first().waitFor({ timeout: 10000 }); + // Wait for PDF to fully render — "Loading PDF..." should disappear + await expect(window.getByText('Loading PDF...')).toBeVisible({ timeout: 5000 }); + await expect(window.getByText('Loading PDF...')).not.toBeVisible({ timeout: 15000 }); + + // Verify we didn't get an error (e.g., "PDF file not found") + const errorVisible = await window + .getByText('PDF file not found') + .isVisible() + .catch(() => false); + if (errorVisible) { + throw new Error('PDF failed to load — file not found'); + } + + // Wait for text layer to render + await window.locator('.textLayer span').first().waitFor({ timeout: 15000 }); // Take a screenshot of the PDF viewer before search await window.screenshot({ path: 'test-results/pdf-viewer-before-search.png' }); @@ -63,9 +86,9 @@ test('Cmd+F opens search bar in PDF viewer', async ({ electronApp, window }) => await searchInput.fill('attention'); // Wait for match counter to appear - await expect(window.getByText(/\d+ of \d+/)).toBeVisible({ timeout: 3000 }); + await expect(window.getByText(/\d+ of \d+/)).toBeVisible({ timeout: 5000 }); - // Take screenshot showing search results + // Take screenshot showing search results with highlights await window.screenshot({ path: 'test-results/pdf-search-results.png' }); // Navigate to next match with Enter From e6da3355baeb2ad79c42a8f5baa6105299e551fc Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 27 Feb 2026 18:31:43 +0000 Subject: [PATCH 04/10] fix: use require() instead of import() in e2e evaluate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Electron main process is CJS — dynamic import() fails with "A dynamic import callback was not specified". Switch to require(). https://claude.ai/code/session_01PtqSxhcyfUARLFvQPjMbqt --- e2e/pdf-search.spec.ts | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/e2e/pdf-search.spec.ts b/e2e/pdf-search.spec.ts index b2c8353..feb799d 100644 --- a/e2e/pdf-search.spec.ts +++ b/e2e/pdf-search.spec.ts @@ -2,11 +2,12 @@ import { expect, test } from './fixtures'; test('Cmd+F opens search bar in PDF viewer', async ({ electronApp, window }) => { // Create a test PDF and seed a paper — all inside the main process to avoid path issues + // Main process is CJS so we must use require(), not dynamic import() await electronApp.evaluate(async () => { - const { PDFDocument, StandardFonts, rgb } = await import('pdf-lib'); - const fs = await import('fs'); - const path = await import('path'); - const os = await import('os'); + const { PDFDocument, StandardFonts, rgb } = require('pdf-lib'); + const fs = require('fs'); + const path = require('path'); + const os = require('os'); const doc = await PDFDocument.create(); const font = await doc.embedFont(StandardFonts.Helvetica); From 9c111c324a7c711b9af796ae57f11c5f2920b39f Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 27 Feb 2026 19:22:03 +0000 Subject: [PATCH 05/10] fix: move PDF creation out of electronApp.evaluate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The sandboxed evaluate context has neither require() nor import(). Create the PDF in the test's Node.js context and pass the path into evaluate for DB seeding only — matching the pattern used by other e2e tests. https://claude.ai/code/session_01PtqSxhcyfUARLFvQPjMbqt --- e2e/pdf-search.spec.ts | 96 +++++++++++++++++++++--------------------- 1 file changed, 49 insertions(+), 47 deletions(-) diff --git a/e2e/pdf-search.spec.ts b/e2e/pdf-search.spec.ts index feb799d..de408fb 100644 --- a/e2e/pdf-search.spec.ts +++ b/e2e/pdf-search.spec.ts @@ -1,53 +1,55 @@ +import fs from 'fs'; +import os from 'os'; +import path from 'path'; +import { PDFDocument, rgb, StandardFonts } from 'pdf-lib'; import { expect, test } from './fixtures'; test('Cmd+F opens search bar in PDF viewer', async ({ electronApp, window }) => { - // Create a test PDF and seed a paper — all inside the main process to avoid path issues - // Main process is CJS so we must use require(), not dynamic import() - await electronApp.evaluate(async () => { - const { PDFDocument, StandardFonts, rgb } = require('pdf-lib'); - const fs = require('fs'); - const path = require('path'); - const os = require('os'); - - const doc = await PDFDocument.create(); - const font = await doc.embedFont(StandardFonts.Helvetica); - const page = doc.addPage([612, 792]); - const lines = [ - { text: 'Attention Is All You Need', size: 18 }, - { text: '', size: 11 }, - { text: 'Abstract', size: 14 }, - { text: '', size: 11 }, - { text: 'The dominant sequence transduction models are based on complex recurrent', size: 11 }, - { text: 'or convolutional neural networks that include an encoder and a decoder.', size: 11 }, - { text: 'We propose a new simple network architecture, the Transformer, based', size: 11 }, - { text: 'solely on attention mechanisms, dispensing with recurrence entirely.', size: 11 }, - ]; - let y = 720; - for (const line of lines) { - page.drawText(line.text, { x: 72, y, size: line.size, font, color: rgb(0, 0, 0) }); - y -= line.size + 6; - } - const pdfBytes = await doc.save(); - - const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'papershelf-pdf-')); - const pdfPath = path.join(tmpDir, 'test-paper.pdf'); - fs.writeFileSync(pdfPath, pdfBytes); - - const db = (global as Record).__papershelf_db; - db.insertPaper({ - arxivId: '1706.03762', - title: 'Attention Is All You Need', - authors: ['Ashish Vaswani', 'Noam Shazeer'], - abstract: 'The dominant sequence transduction models are based on complex recurrent neural networks.', - publishedDate: '2017-06-12T00:00:00Z', - updatedDate: '2017-06-12T00:00:00Z', - categories: ['cs.CL', 'cs.AI'], - arxivUrl: 'https://arxiv.org/abs/1706.03762', - pdfUrl: 'https://arxiv.org/pdf/1706.03762', - pdfPath, - fullText: null, - }); - }); + // Create the test PDF in the test's Node.js context (where imports work) + const doc = await PDFDocument.create(); + const font = await doc.embedFont(StandardFonts.Helvetica); + const page = doc.addPage([612, 792]); + const lines = [ + { text: 'Attention Is All You Need', size: 18 }, + { text: '', size: 11 }, + { text: 'Abstract', size: 14 }, + { text: '', size: 11 }, + { text: 'The dominant sequence transduction models are based on complex recurrent', size: 11 }, + { text: 'or convolutional neural networks that include an encoder and a decoder.', size: 11 }, + { text: 'We propose a new simple network architecture, the Transformer, based', size: 11 }, + { text: 'solely on attention mechanisms, dispensing with recurrence entirely.', size: 11 }, + ]; + let y = 720; + for (const line of lines) { + page.drawText(line.text, { x: 72, y, size: line.size, font, color: rgb(0, 0, 0) }); + y -= line.size + 6; + } + const pdfBytes = await doc.save(); + + const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'papershelf-pdf-')); + const pdfPath = path.join(tmpDir, 'test-paper.pdf'); + fs.writeFileSync(pdfPath, pdfBytes); + + // Seed the paper in the main process DB (only DB access needs evaluate) + await electronApp.evaluate( + async (_params: { pdfPath: string }) => { + const db = (global as Record).__papershelf_db; + db.insertPaper({ + arxivId: '1706.03762', + title: 'Attention Is All You Need', + authors: ['Ashish Vaswani', 'Noam Shazeer'], + abstract: 'The dominant sequence transduction models are based on complex recurrent neural networks.', + publishedDate: '2017-06-12T00:00:00Z', + updatedDate: '2017-06-12T00:00:00Z', + categories: ['cs.CL', 'cs.AI'], + arxivUrl: 'https://arxiv.org/abs/1706.03762', + pdfUrl: 'https://arxiv.org/pdf/1706.03762', + pdfPath: _params.pdfPath, + fullText: null, + }); + }, + { pdfPath }, + ); // Navigate to library and click the paper await window.getByRole('button', { name: 'My Library' }).click(); From 763d77bc041290ca75af735aeb1a14ace508b866 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 27 Feb 2026 19:48:15 +0000 Subject: [PATCH 06/10] fix: remove flaky "Loading PDF..." assertion in e2e test The loading indicator is transient and may disappear before the assertion runs, especially in CI. Wait directly for the text layer to render instead. https://claude.ai/code/session_01PtqSxhcyfUARLFvQPjMbqt --- e2e/pdf-search.spec.ts | 18 +++--------------- 1 file changed, 3 insertions(+), 15 deletions(-) diff --git a/e2e/pdf-search.spec.ts b/e2e/pdf-search.spec.ts index de408fb..8d15b9f 100644 --- a/e2e/pdf-search.spec.ts +++ b/e2e/pdf-search.spec.ts @@ -56,21 +56,9 @@ test('Cmd+F opens search bar in PDF viewer', async ({ electronApp, window }) => await expect(window.getByText('Attention Is All You Need').first()).toBeVisible({ timeout: 5000 }); await window.getByText('Attention Is All You Need').first().click(); - // Wait for PDF to fully render — "Loading PDF..." should disappear - await expect(window.getByText('Loading PDF...')).toBeVisible({ timeout: 5000 }); - await expect(window.getByText('Loading PDF...')).not.toBeVisible({ timeout: 15000 }); - - // Verify we didn't get an error (e.g., "PDF file not found") - const errorVisible = await window - .getByText('PDF file not found') - .isVisible() - .catch(() => false); - if (errorVisible) { - throw new Error('PDF failed to load — file not found'); - } - - // Wait for text layer to render - await window.locator('.textLayer span').first().waitFor({ timeout: 15000 }); + // Wait for PDF to fully render — the loading indicator is transient and may + // disappear before we can assert on it, so just wait for the text layer + await window.locator('.textLayer span').first().waitFor({ timeout: 30000 }); // Take a screenshot of the PDF viewer before search await window.screenshot({ path: 'test-results/pdf-viewer-before-search.png' }); From 6ec71058274e0a93a442a185017ea37388c54bd6 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 28 Feb 2026 08:41:58 +0000 Subject: [PATCH 07/10] fix: add missing _electron param in evaluate callback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit electronApp.evaluate passes the Electron module as the first arg to the callback — the data arg comes second. Without _electron as the first parameter, _params received the Electron module object and _params.pdfPath was undefined, so the PDF viewer never loaded. Also bumps test timeout to 60s for PDF rendering in CI. https://claude.ai/code/session_01PtqSxhcyfUARLFvQPjMbqt --- e2e/pdf-search.spec.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/e2e/pdf-search.spec.ts b/e2e/pdf-search.spec.ts index 8d15b9f..417b04e 100644 --- a/e2e/pdf-search.spec.ts +++ b/e2e/pdf-search.spec.ts @@ -4,7 +4,7 @@ import path from 'path'; import { PDFDocument, rgb, StandardFonts } from 'pdf-lib'; import { expect, test } from './fixtures'; -test('Cmd+F opens search bar in PDF viewer', async ({ electronApp, window }) => { +test('Cmd+F opens search bar in PDF viewer', { timeout: 60_000 }, async ({ electronApp, window }) => { // Create the test PDF in the test's Node.js context (where imports work) const doc = await PDFDocument.create(); const font = await doc.embedFont(StandardFonts.Helvetica); @@ -31,8 +31,10 @@ test('Cmd+F opens search bar in PDF viewer', async ({ electronApp, window }) => fs.writeFileSync(pdfPath, pdfBytes); // Seed the paper in the main process DB (only DB access needs evaluate) + // Note: electronApp.evaluate passes the Electron module as the first arg; + // the data argument comes second (see take-screenshots.ts for the pattern) await electronApp.evaluate( - async (_params: { pdfPath: string }) => { + async (_electron, _params: { pdfPath: string }) => { const db = (global as Record).__papershelf_db; db.insertPaper({ arxivId: '1706.03762', From 8d830edcdacb4383dfe0743bf53f4b144613ea49 Mon Sep 17 00:00:00 2001 From: Daniel Klevebring Date: Mon, 2 Mar 2026 21:59:21 +0100 Subject: [PATCH 08/10] polish: find-in-PDF UX, toolbar layout, shortcuts, tooltips MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Fix search bar disappearing on zoom by moving it outside scrollable container - Delay auto-search for single-char queries (require Enter) - Unify header: title+zoom on row 1, authors+icons on row 2 - Add search icon with ShortcutHint (⌘F) to toolbar - Wire keyboard shortcuts through store so remapping works - Add grouped Cmd-hold hint pills for zoom and find next/prev - Add Tooltip component and hover tooltips for all toolbar buttons - Fix PDF font warnings by providing cMapUrl to pdf.js - Bump max zoom from 300% to 1000% - Add findInPdf to default shortcuts - Add shortcut store unit tests (20 tests) Co-Authored-By: Claude Opus 4.6 --- src/renderer/__tests__/shortcut-store.test.ts | 175 ++++++++++++ src/renderer/components/PaperDetail.tsx | 254 +++++++++--------- src/renderer/components/PdfViewer.tsx | 234 ++++++++++------ src/renderer/components/ShortcutHint.tsx | 63 ++++- src/renderer/components/pdf/PdfSearchBar.tsx | 32 ++- src/renderer/components/pdf/usePdfDocument.ts | 5 +- src/renderer/stores/shortcutStore.ts | 1 + 7 files changed, 550 insertions(+), 214 deletions(-) create mode 100644 src/renderer/__tests__/shortcut-store.test.ts diff --git a/src/renderer/__tests__/shortcut-store.test.ts b/src/renderer/__tests__/shortcut-store.test.ts new file mode 100644 index 0000000..4d488e7 --- /dev/null +++ b/src/renderer/__tests__/shortcut-store.test.ts @@ -0,0 +1,175 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +vi.stubGlobal('localStorage', { + getItem: vi.fn(() => null), + setItem: vi.fn(), + removeItem: vi.fn(), + clear: vi.fn(), + length: 0, + key: vi.fn(() => null), +}); + +// Mock electronAPI for store methods that call it +vi.stubGlobal('window', { + ...globalThis.window, + electronAPI: { + getShortcutOverrides: vi.fn(() => Promise.resolve({})), + saveShortcutOverrides: vi.fn(), + }, +}); + +import { buildKeyString, formatKeys, getDefaultKeys, useShortcutStore } from '../stores/shortcutStore'; + +function fakeKeyEvent(overrides: Partial): KeyboardEvent { + return { + key: '', + metaKey: false, + ctrlKey: false, + altKey: false, + shiftKey: false, + ...overrides, + } as KeyboardEvent; +} + +beforeEach(() => { + useShortcutStore.getState().resetAll(); +}); + +// --- buildKeyString --- + +describe('buildKeyString', () => { + it('returns null for lone modifier presses', () => { + expect(buildKeyString(fakeKeyEvent({ key: 'Meta', metaKey: true }))).toBeNull(); + expect(buildKeyString(fakeKeyEvent({ key: 'Shift', shiftKey: true }))).toBeNull(); + expect(buildKeyString(fakeKeyEvent({ key: 'Alt', altKey: true }))).toBeNull(); + expect(buildKeyString(fakeKeyEvent({ key: 'Control', ctrlKey: true }))).toBeNull(); + }); + + it('returns null when no modifier is held', () => { + expect(buildKeyString(fakeKeyEvent({ key: 'f' }))).toBeNull(); + expect(buildKeyString(fakeKeyEvent({ key: 'Enter' }))).toBeNull(); + }); + + it('builds Meta+key strings', () => { + expect(buildKeyString(fakeKeyEvent({ key: 'f', metaKey: true }))).toBe('Meta+f'); + expect(buildKeyString(fakeKeyEvent({ key: 'b', metaKey: true }))).toBe('Meta+b'); + }); + + it('lowercases single-char keys', () => { + expect(buildKeyString(fakeKeyEvent({ key: 'F', metaKey: true }))).toBe('Meta+f'); + }); + + it('preserves multi-char key names', () => { + expect(buildKeyString(fakeKeyEvent({ key: 'Enter', metaKey: true }))).toBe('Meta+Enter'); + expect(buildKeyString(fakeKeyEvent({ key: 'Escape', ctrlKey: true }))).toBe('Control+Escape'); + }); + + it('combines multiple modifiers in order', () => { + expect(buildKeyString(fakeKeyEvent({ key: 'g', metaKey: true, shiftKey: true }))).toBe('Meta+Shift+g'); + expect(buildKeyString(fakeKeyEvent({ key: 'z', metaKey: true, altKey: true }))).toBe('Meta+Alt+z'); + }); +}); + +// --- formatKeys --- + +describe('formatKeys', () => { + it('formats Meta as ⌘', () => { + expect(formatKeys('Meta+f')).toBe('⌘F'); + }); + + it('formats Shift as ⇧', () => { + expect(formatKeys('Meta+Shift+g')).toBe('⌘⇧G'); + }); + + it('formats Alt as ⌥', () => { + expect(formatKeys('Meta+Alt+z')).toBe('⌘⌥Z'); + }); + + it('formats Control as ⌃', () => { + expect(formatKeys('Control+c')).toBe('⌃C'); + }); +}); + +// --- Default shortcuts --- + +describe('default shortcuts', () => { + it('includes findInPdf shortcut', () => { + const shortcut = useShortcutStore.getState().getShortcut('findInPdf'); + expect(shortcut).toBeDefined(); + expect(shortcut?.keys).toBe('Meta+f'); + expect(shortcut?.label).toBe('Find in PDF'); + }); + + it('includes highlightSelection shortcut', () => { + const shortcut = useShortcutStore.getState().getShortcut('highlightSelection'); + expect(shortcut).toBeDefined(); + expect(shortcut?.keys).toBe('Meta+e'); + }); + + it('returns undefined for unknown shortcut IDs', () => { + expect(useShortcutStore.getState().getShortcut('nonexistent')).toBeUndefined(); + }); + + it('getDefaultKeys returns the default for known IDs', () => { + expect(getDefaultKeys('findInPdf')).toBe('Meta+f'); + expect(getDefaultKeys('toggleSidebar')).toBe('Meta+b'); + }); + + it('getDefaultKeys returns undefined for unknown IDs', () => { + expect(getDefaultKeys('nonexistent')).toBeUndefined(); + }); +}); + +// --- Shortcut remapping --- + +describe('shortcut remapping', () => { + it('remaps a shortcut and finds it by new keys', () => { + const store = useShortcutStore.getState(); + const result = store.setShortcutKeys('findInPdf', 'Meta+Shift+f'); + expect(result.success).toBe(true); + + const updated = useShortcutStore.getState().getShortcut('findInPdf'); + expect(updated?.keys).toBe('Meta+Shift+f'); + }); + + it('detects conflicts when remapping to an existing key', () => { + const store = useShortcutStore.getState(); + // Try to remap findInPdf to Meta+b which is toggleSidebar + const result = store.setShortcutKeys('findInPdf', 'Meta+b'); + expect(result.success).toBe(false); + expect(result.conflict).toBe('Toggle Sidebar'); + }); + + it('resets a shortcut to default', () => { + const store = useShortcutStore.getState(); + store.setShortcutKeys('findInPdf', 'Meta+Shift+f'); + expect(useShortcutStore.getState().getShortcut('findInPdf')?.keys).toBe('Meta+Shift+f'); + + useShortcutStore.getState().resetShortcut('findInPdf'); + expect(useShortcutStore.getState().getShortcut('findInPdf')?.keys).toBe('Meta+f'); + }); + + it('resetAll restores all defaults', () => { + const store = useShortcutStore.getState(); + store.setShortcutKeys('findInPdf', 'Meta+Shift+f'); + store.setShortcutKeys('toggleSidebar', 'Meta+Shift+b'); + + useShortcutStore.getState().resetAll(); + + expect(useShortcutStore.getState().getShortcut('findInPdf')?.keys).toBe('Meta+f'); + expect(useShortcutStore.getState().getShortcut('toggleSidebar')?.keys).toBe('Meta+b'); + }); + + it('remapped shortcut is found via key lookup (simulating keyboard dispatch)', () => { + useShortcutStore.getState().setShortcutKeys('findInPdf', 'Meta+Shift+f'); + + const keyString = buildKeyString(fakeKeyEvent({ key: 'f', metaKey: true, shiftKey: true })); + const match = useShortcutStore.getState().shortcuts.find((s) => s.keys === keyString); + expect(match?.id).toBe('findInPdf'); + + // Old key should no longer match findInPdf + const oldKeyString = buildKeyString(fakeKeyEvent({ key: 'f', metaKey: true })); + const oldMatch = useShortcutStore.getState().shortcuts.find((s) => s.keys === oldKeyString); + expect(oldMatch?.id).not.toBe('findInPdf'); + }); +}); diff --git a/src/renderer/components/PaperDetail.tsx b/src/renderer/components/PaperDetail.tsx index c54d866..3132f02 100644 --- a/src/renderer/components/PaperDetail.tsx +++ b/src/renderer/components/PaperDetail.tsx @@ -5,8 +5,8 @@ import { formatKeys, useShortcutStore } from '../stores/shortcutStore'; import { useUIStore } from '../stores/uiStore'; import { ConfirmPopup } from './ConfirmPopup'; import { FolderPlusIcon, InfoCircleIcon, StarIcon, StarOutlineIcon, TagPlusIcon } from './Icons'; - import { PdfViewer } from './PdfViewer'; +import { Tooltip } from './ShortcutHint'; function formatDate(dateStr: string): string { return new Date(dateStr).toLocaleDateString('en-US', { @@ -117,131 +117,132 @@ export function PaperDetail() { return (
- {/* Compact header */} -
-
- {isLibraryPaper && ( - - )} - -

{paper.title}

- - {isLibraryPaper && ( - <> -
- - {showHeaderCollectionPicker && collections.length > 0 && ( -
- {collections.map((col) => { - const isIn = paperCollections.some((c) => c.id === col.id); - return ( + {/* Content: always PDF */} +
+ {hasPdf ? ( + + {isLibraryPaper && ( + + )} +

{paper.title}

+
+ } + headerActions={ + <> + {isLibraryPaper && ( + <> +
+ - ); - })} -
- )} -
- -
- - {showHeaderTagPicker && tags.length > 0 && ( -
- {tags.map((tag) => { - const has = paperTags.some((t) => t.id === tag.id); - return ( + + {showHeaderCollectionPicker && collections.length > 0 && ( +
+ {collections.map((col) => { + const isIn = paperCollections.some((c) => c.id === col.id); + return ( + + ); + })} +
+ )} +
+ +
+ - ); - })} -
+ + {showHeaderTagPicker && tags.length > 0 && ( +
+ {tags.map((tag) => { + const has = paperTags.some((t) => t.id === tag.id); + return ( + + ); + })} +
+ )} +
+ )} -
- - )} - - { - setShowInfoPopover((v) => !v); - setShowHeaderCollectionPicker(false); - setShowHeaderTagPicker(false); - }} - onClose={closeInfoPopover} - paper={paper} - isLibraryPaper={isLibraryPaper} - paperCollections={paperCollections} - paperTags={paperTags} - collections={collections} - tags={tags} - showCollectionPicker={showCollectionPicker} - setShowCollectionPicker={setShowCollectionPicker} - showTagPicker={showTagPicker} - setShowTagPicker={setShowTagPicker} - handleToggleCollection={handleToggleCollection} - handleToggleTag={handleToggleTag} - onDelete={paperId ? () => deletePaper(paperId) : undefined} - /> -
-

{paper.authors.join(', ')}

-
- - {/* Content: always PDF */} -
- {hasPdf ? ( - { + setShowInfoPopover((v) => !v); + setShowHeaderCollectionPicker(false); + setShowHeaderTagPicker(false); + }} + onClose={closeInfoPopover} + paper={paper} + isLibraryPaper={isLibraryPaper} + paperCollections={paperCollections} + paperTags={paperTags} + collections={collections} + tags={tags} + showCollectionPicker={showCollectionPicker} + setShowCollectionPicker={setShowCollectionPicker} + showTagPicker={showTagPicker} + setShowTagPicker={setShowTagPicker} + handleToggleCollection={handleToggleCollection} + handleToggleTag={handleToggleTag} + onDelete={paperId ? () => deletePaper(paperId) : undefined} + /> + + } /> ) : (
No PDF available
@@ -375,14 +376,15 @@ function InfoPopoverButton({ return (
- + + + {open && (
s.commandDown); const [scale, setScale] = useState(1.0); const [visualScale, setVisualScale] = useState(1.0); const [pdfVersion, setPdfVersion] = useState(0); @@ -413,39 +429,6 @@ export function PdfViewer({ paperId, pdfUrl, arxivId }: { paperId?: string; pdfU const closeSearch = useCallback(() => setShowSearch(false), []); - // Cmd+/Cmd- keyboard shortcuts - useEffect(() => { - const handleKeyDown = (event: KeyboardEvent) => { - if (event.key === 'Escape') { - if (showSearch) { - setShowSearch(false); - return; - } - setAnnotationMode('read'); - setHighlightToolbar(null); - setStickyNotePopup(null); - setDeleteConfirm(null); - return; - } - if (!event.metaKey) return; - if (event.key === 'f') { - event.preventDefault(); - setShowSearch(true); - } else if (event.key === '=' || event.key === '+') { - event.preventDefault(); - zoomIn(); - } else if (event.key === '-') { - event.preventDefault(); - zoomOut(); - } else if (event.key === '0') { - event.preventDefault(); - zoomReset(); - } - }; - window.addEventListener('keydown', handleKeyDown); - return () => window.removeEventListener('keydown', handleKeyDown); - }, [zoomIn, zoomOut, zoomReset, showSearch]); - // Pinch-to-zoom useEffect(() => { let latestVisualScale = scale; @@ -685,6 +668,55 @@ export function PdfViewer({ paperId, pdfUrl, arxivId }: { paperId?: string; pdfU setDeleteConfirm(null); }, []); + // Keyboard shortcuts (reads from shortcut store so remapping works) + useEffect(() => { + const handleKeyDown = (event: KeyboardEvent) => { + if (event.key === 'Escape') { + if (showSearch) { + setShowSearch(false); + return; + } + setAnnotationMode('read'); + setHighlightToolbar(null); + setStickyNotePopup(null); + setDeleteConfirm(null); + return; + } + // Zoom: fixed Cmd+/- shortcuts (not remappable) + if (event.metaKey) { + if (event.key === '=' || event.key === '+') { + zoomIn(); + return; + } + if (event.key === '-') { + zoomOut(); + return; + } + if (event.key === '0') { + zoomReset(); + return; + } + } + // Remappable shortcuts via store + const keyString = buildKeyString(event); + if (!keyString) return; + const shortcut = useShortcutStore.getState().shortcuts.find((s) => s.keys === keyString); + if (!shortcut) return; + switch (shortcut.id) { + case 'findInPdf': + event.preventDefault(); + setShowSearch(true); + break; + case 'highlightSelection': + event.preventDefault(); + toggleHighlightMode(); + break; + } + }; + window.addEventListener('keydown', handleKeyDown); + return () => window.removeEventListener('keydown', handleKeyDown); + }, [showSearch, toggleHighlightMode, zoomIn, zoomOut, zoomReset]); + if (loading) { return (
@@ -703,61 +735,104 @@ export function PdfViewer({ paperId, pdfUrl, arxivId }: { paperId?: string; pdfU return (
-
- - - - {numPages > 0 && {numPages} pages} - - {!readOnly && ( -
- +
+
+
{headerTitle}
+
+ + + + {numPages > 0 && {numPages}p} +
+
+
+ {authors && authors.length > 0 && ( +

{authors.join(', ')}

+ )} + {commandDown && ( +
+ + ⌘+/⌘− + Zoom + + {showSearch && ( + + ⌘G + Find Next/Prev + + )} +
+ )} +
+ {headerActions} +
+ - + {!readOnly && ( + <> + + + + + + + + )}
- )} +
-
+
{showSearch && } +
{pages.map((page, index) => ( ))}
+
{highlightToolbar && ( diff --git a/src/renderer/components/ShortcutHint.tsx b/src/renderer/components/ShortcutHint.tsx index 2f884e3..72f20a9 100644 --- a/src/renderer/components/ShortcutHint.tsx +++ b/src/renderer/components/ShortcutHint.tsx @@ -1,7 +1,7 @@ import { type ReactNode, useCallback, useRef, useState } from 'react'; import { formatKeys, useShortcutStore } from '../stores/shortcutStore'; -const HOVER_DELAY_MS = 600; +const HOVER_DELAY_MS = 400; interface ShortcutHintProps { shortcutId: string; @@ -112,3 +112,64 @@ export function ShortcutHint({ ); } + +interface TooltipProps { + label: string; + children: ReactNode; + position?: 'below' | 'above'; + align?: 'center' | 'end'; +} + +export function Tooltip({ label, children, position = 'below', align = 'center' }: TooltipProps) { + const [hovered, setHovered] = useState(false); + const timerRef = useRef | null>(null); + + const onMouseEnter = useCallback(() => { + timerRef.current = setTimeout(() => setHovered(true), HOVER_DELAY_MS); + }, []); + + const onMouseLeave = useCallback(() => { + if (timerRef.current) clearTimeout(timerRef.current); + setHovered(false); + }, []); + + const isEnd = align === 'end'; + const posClass = position === 'above' + ? (isEnd ? 'bottom-full right-0 mb-0.5' : 'bottom-full left-1/2 mb-0.5') + : (isEnd ? 'top-full right-0 mt-0.5' : 'top-full left-1/2 mt-0.5'); + const transform = position === 'above' || position === 'below' + ? (isEnd ? undefined : 'translateX(-50%)') + : undefined; + const flexClass = position === 'above' + ? (isEnd ? 'flex-col items-end' : 'flex-col items-center') + : (isEnd ? 'flex-col items-end' : 'flex-col items-center'); + const arrow = position === 'above' ? ARROW_DOWN : ARROW; + + return ( + + {children} + {hovered && ( + + {position === 'above' ? ( + <> + + {label} + + {arrow} + + ) : ( + <> + {arrow} + + {label} + + + )} + + )} + + ); +} diff --git a/src/renderer/components/pdf/PdfSearchBar.tsx b/src/renderer/components/pdf/PdfSearchBar.tsx index 473f079..ed31e81 100644 --- a/src/renderer/components/pdf/PdfSearchBar.tsx +++ b/src/renderer/components/pdf/PdfSearchBar.tsx @@ -99,6 +99,7 @@ function highlightMatches(matches: SearchMatch[], currentIndex: number) { export function PdfSearchBar({ containerRef, onClose }: PdfSearchBarProps) { const [query, setQuery] = useState(''); + const [activeQuery, setActiveQuery] = useState(''); const [matches, setMatches] = useState([]); const [currentIndex, setCurrentIndex] = useState(0); const inputRef = useRef(null); @@ -109,21 +110,30 @@ export function PdfSearchBar({ containerRef, onClose }: PdfSearchBarProps) { inputRef.current?.select(); }, []); - // Find matches when query changes + // Auto-commit query when 2+ characters; clear when empty + useEffect(() => { + if (query.length >= 2) { + setActiveQuery(query); + } else if (query.length === 0) { + setActiveQuery(''); + } + }, [query]); + + // Find matches when activeQuery changes useEffect(() => { const container = containerRef.current; if (!container) return; - if (!query) { + if (!activeQuery) { setMatches([]); setCurrentIndex(0); return; } - const found = findMatchesInTextLayers(container, query); + const found = findMatchesInTextLayers(container, activeQuery); setMatches(found); setCurrentIndex(0); - }, [query, containerRef]); + }, [activeQuery, containerRef]); // Highlight matches and scroll to current useEffect(() => { @@ -159,10 +169,18 @@ export function PdfSearchBar({ containerRef, onClose }: PdfSearchBarProps) { onClose(); } else if (event.key === 'Enter' && event.shiftKey) { event.preventDefault(); - goToPrev(); + if (query.length === 1 && activeQuery !== query) { + setActiveQuery(query); + } else { + goToPrev(); + } } else if (event.key === 'Enter') { event.preventDefault(); - goToNext(); + if (query.length === 1 && activeQuery !== query) { + setActiveQuery(query); + } else { + goToNext(); + } } else if (event.key === 'g' && event.metaKey && event.shiftKey) { event.preventDefault(); goToPrev(); @@ -171,7 +189,7 @@ export function PdfSearchBar({ containerRef, onClose }: PdfSearchBarProps) { goToNext(); } }, - [onClose, goToNext, goToPrev], + [onClose, goToNext, goToPrev, query, activeQuery], ); // Clean up highlights on unmount diff --git a/src/renderer/components/pdf/usePdfDocument.ts b/src/renderer/components/pdf/usePdfDocument.ts index a23b9da..e21cba6 100644 --- a/src/renderer/components/pdf/usePdfDocument.ts +++ b/src/renderer/components/pdf/usePdfDocument.ts @@ -1,6 +1,9 @@ import type { PDFDocumentProxy } from 'pdfjs-dist'; import * as pdfjsLib from 'pdfjs-dist'; import pdfjsWorkerUrl from 'pdfjs-dist/build/pdf.worker.min.mjs?url'; +// PDF.js appends CMap filenames to this URL prefix to fetch them on demand. +import cMapSample from 'pdfjs-dist/cmaps/78-H.bcmap?url'; +const cMapUrl = cMapSample.replace(/[^/]+$/, ''); import { useEffect, useRef, useState } from 'react'; pdfjsLib.GlobalWorkerOptions.workerSrc = pdfjsWorkerUrl; @@ -53,7 +56,7 @@ export function usePdfDocument(paperId: string | null, pdfVersion: number, pdfUr try { const prevDoc = documentRef.current; - const doc = await pdfjsLib.getDocument({ data: copy }).promise; + const doc = await pdfjsLib.getDocument({ data: copy, cMapUrl, cMapPacked: true }).promise; if (cancelled) { doc.destroy(); return; diff --git a/src/renderer/stores/shortcutStore.ts b/src/renderer/stores/shortcutStore.ts index fc0b377..850912d 100644 --- a/src/renderer/stores/shortcutStore.ts +++ b/src/renderer/stores/shortcutStore.ts @@ -32,6 +32,7 @@ const DEFAULT_SHORTCUTS: Shortcut[] = [ { id: 'highlightSelection', label: 'Highlight Selection', keys: 'Meta+e' }, { id: 'importPdfs', label: 'Import PDFs', keys: 'Meta+i' }, { id: 'toggleMcp', label: 'Toggle MCP Server', keys: 'Meta+t' }, + { id: 'findInPdf', label: 'Find in PDF', keys: 'Meta+f' }, ]; function buildOverridesFromShortcuts(shortcuts: Shortcut[]): Record { From 5fa649b7abb24fd19548c8e22b944ced288ca5c0 Mon Sep 17 00:00:00 2001 From: Daniel Klevebring Date: Tue, 3 Mar 2026 00:53:08 +0100 Subject: [PATCH 09/10] fix: address 3 Greptile review bugs in PDF search 1. Stale matches on zoom: MutationObserver re-runs search when text layers are rebuilt after zoom commit 2. Search state persists across papers: key={paperId} remounts PdfSearchBar when switching papers 3. Cmd+G only works with focus: window-level keyboard handler in PdfViewer now handles Cmd+G/Cmd+Shift+G via searchNavRef exposed through onNavigate callback Co-Authored-By: Claude Opus 4.6 --- src/renderer/components/PdfViewer.tsx | 24 +++++++++++++++-- src/renderer/components/pdf/PdfSearchBar.tsx | 27 +++++++++++++++++++- 2 files changed, 48 insertions(+), 3 deletions(-) diff --git a/src/renderer/components/PdfViewer.tsx b/src/renderer/components/PdfViewer.tsx index dcfdeec..6fead25 100644 --- a/src/renderer/components/PdfViewer.tsx +++ b/src/renderer/components/PdfViewer.tsx @@ -427,7 +427,17 @@ export function PdfViewer({ setVisualScale(1.0); }, [prepareScaleCommit]); - const closeSearch = useCallback(() => setShowSearch(false), []); + const closeSearch = useCallback(() => { + setShowSearch(false); + searchNavRef.current = null; + }, []); + const searchNavRef = useRef<{ goToNext: () => void; goToPrev: () => void } | null>(null); + const handleSearchNavigate = useCallback( + (handlers: { goToNext: () => void; goToPrev: () => void }) => { + searchNavRef.current = handlers; + }, + [], + ); // Pinch-to-zoom useEffect(() => { @@ -697,6 +707,16 @@ export function PdfViewer({ return; } } + // Cmd+G / Cmd+Shift+G — find next/prev (window-level so it works without search bar focus) + if (event.metaKey && event.key === 'g') { + event.preventDefault(); + if (event.shiftKey) { + searchNavRef.current?.goToPrev(); + } else { + searchNavRef.current?.goToNext(); + } + return; + } // Remappable shortcuts via store const keyString = buildKeyString(event); if (!keyString) return; @@ -831,7 +851,7 @@ export function PdfViewer({
- {showSearch && } + {showSearch && }
{pages.map((page, index) => ( diff --git a/src/renderer/components/pdf/PdfSearchBar.tsx b/src/renderer/components/pdf/PdfSearchBar.tsx index ed31e81..438249a 100644 --- a/src/renderer/components/pdf/PdfSearchBar.tsx +++ b/src/renderer/components/pdf/PdfSearchBar.tsx @@ -14,6 +14,7 @@ interface SearchMatch { interface PdfSearchBarProps { containerRef: React.RefObject; onClose: () => void; + onNavigate?: (handlers: { goToNext: () => void; goToPrev: () => void }) => void; } function findMatchesInTextLayers(container: HTMLElement, query: string): SearchMatch[] { @@ -97,7 +98,7 @@ function highlightMatches(matches: SearchMatch[], currentIndex: number) { } } -export function PdfSearchBar({ containerRef, onClose }: PdfSearchBarProps) { +export function PdfSearchBar({ containerRef, onClose, onNavigate }: PdfSearchBarProps) { const [query, setQuery] = useState(''); const [activeQuery, setActiveQuery] = useState(''); const [matches, setMatches] = useState([]); @@ -161,6 +162,30 @@ export function PdfSearchBar({ containerRef, onClose }: PdfSearchBarProps) { setCurrentIndex((prev) => (prev - 1 + matches.length) % matches.length); }, [matches.length]); + // Expose navigation handlers to parent for window-level Cmd+G + useEffect(() => { + onNavigate?.({ goToNext, goToPrev }); + }, [onNavigate, goToNext, goToPrev]); + + // Re-run search when text layers are rebuilt (e.g. after zoom commit) + useEffect(() => { + const container = containerRef.current; + if (!container || !activeQuery) return; + + const observer = new MutationObserver((mutations) => { + const hasChildListChange = mutations.some((m) => m.type === 'childList'); + if (!hasChildListChange) return; + const current = containerRef.current; + if (!current) return; + const found = findMatchesInTextLayers(current, activeQuery); + setMatches(found); + setCurrentIndex((prev) => Math.min(prev, Math.max(0, found.length - 1))); + }); + + observer.observe(container, { childList: true, subtree: true }); + return () => observer.disconnect(); + }, [activeQuery, containerRef]); + const handleKeyDown = useCallback( (event: React.KeyboardEvent) => { if (event.key === 'Escape') { From 1a05ec7f82dfa5f53e6c7f080372d40b93b03cc1 Mon Sep 17 00:00:00 2001 From: Daniel Klevebring Date: Tue, 3 Mar 2026 08:12:30 +0100 Subject: [PATCH 10/10] fix: address remaining Greptile review issues - Add event.preventDefault() to zoom shortcuts to prevent browser default zoom - Guard highlight shortcut from firing in readonly mode - Use paperId ?? arxivId ?? pdfUrl as key fallback for PdfSearchBar remount - Add aria-label to search input for accessibility - Clean up e2e temp directory with try/finally - Fix biome lint/formatting issues Co-Authored-By: Claude Opus 4.6 --- e2e/pdf-search.spec.ts | 4 + src/renderer/components/PdfViewer.tsx | 89 ++++++++++++------- src/renderer/components/ShortcutHint.tsx | 26 ++++-- src/renderer/components/pdf/PdfSearchBar.tsx | 1 + src/renderer/components/pdf/usePdfDocument.ts | 2 + 5 files changed, 81 insertions(+), 41 deletions(-) diff --git a/e2e/pdf-search.spec.ts b/e2e/pdf-search.spec.ts index 417b04e..75830aa 100644 --- a/e2e/pdf-search.spec.ts +++ b/e2e/pdf-search.spec.ts @@ -30,6 +30,7 @@ test('Cmd+F opens search bar in PDF viewer', { timeout: 60_000 }, async ({ elect const pdfPath = path.join(tmpDir, 'test-paper.pdf'); fs.writeFileSync(pdfPath, pdfBytes); + try { // Seed the paper in the main process DB (only DB access needs evaluate) // Note: electronApp.evaluate passes the Electron module as the first arg; // the data argument comes second (see take-screenshots.ts for the pattern) @@ -94,4 +95,7 @@ test('Cmd+F opens search bar in PDF viewer', { timeout: 60_000 }, async ({ elect // Take screenshot after closing search (highlights should be gone) await window.screenshot({ path: 'test-results/pdf-search-closed.png' }); + } finally { + fs.rmSync(tmpDir, { recursive: true, force: true }); + } }); diff --git a/src/renderer/components/PdfViewer.tsx b/src/renderer/components/PdfViewer.tsx index 6fead25..df22182 100644 --- a/src/renderer/components/PdfViewer.tsx +++ b/src/renderer/components/PdfViewer.tsx @@ -1,12 +1,12 @@ import type { PDFPageProxy } from 'pdfjs-dist'; import { useCallback, useEffect, useLayoutEffect, useRef, useState } from 'react'; +import { buildKeyString, useShortcutStore } from '../stores/shortcutStore'; import { ConfirmPopup } from './ConfirmPopup'; import { HighlightIcon, StickyNoteIcon } from './Icons'; import { PdfPage, selectionRectsToQuadPoints } from './pdf/PdfPage'; import { PdfSearchBar } from './pdf/PdfSearchBar'; import { usePdfDocument } from './pdf/usePdfDocument'; import { ShortcutHint, Tooltip } from './ShortcutHint'; -import { buildKeyString, useShortcutStore } from '../stores/shortcutStore'; const MIN_SCALE = 0.5; const MAX_SCALE = 10.0; @@ -432,12 +432,9 @@ export function PdfViewer({ searchNavRef.current = null; }, []); const searchNavRef = useRef<{ goToNext: () => void; goToPrev: () => void } | null>(null); - const handleSearchNavigate = useCallback( - (handlers: { goToNext: () => void; goToPrev: () => void }) => { - searchNavRef.current = handlers; - }, - [], - ); + const handleSearchNavigate = useCallback((handlers: { goToNext: () => void; goToPrev: () => void }) => { + searchNavRef.current = handlers; + }, []); // Pinch-to-zoom useEffect(() => { @@ -695,14 +692,17 @@ export function PdfViewer({ // Zoom: fixed Cmd+/- shortcuts (not remappable) if (event.metaKey) { if (event.key === '=' || event.key === '+') { + event.preventDefault(); zoomIn(); return; } if (event.key === '-') { + event.preventDefault(); zoomOut(); return; } if (event.key === '0') { + event.preventDefault(); zoomReset(); return; } @@ -728,14 +728,16 @@ export function PdfViewer({ setShowSearch(true); break; case 'highlightSelection': - event.preventDefault(); - toggleHighlightMode(); + if (!readOnly) { + event.preventDefault(); + toggleHighlightMode(); + } break; } }; window.addEventListener('keydown', handleKeyDown); return () => window.removeEventListener('keydown', handleKeyDown); - }, [showSearch, toggleHighlightMode, zoomIn, zoomOut, zoomReset]); + }, [showSearch, readOnly, toggleHighlightMode, zoomIn, zoomOut, zoomReset]); if (loading) { return ( @@ -787,7 +789,10 @@ export function PdfViewer({

{authors.join(', ')}

)} {commandDown && ( -
+
⌘+/⌘− Zoom @@ -812,7 +817,16 @@ export function PdfViewer({ : 'hover:bg-gray-100 dark:hover:bg-gray-800 text-gray-600 dark:text-gray-400' }`} > - + @@ -832,7 +846,11 @@ export function PdfViewer({ - +
- {showSearch && } + {showSearch && ( + + )}
-
- {pages.map((page, index) => ( - - ))} -
+
+ {pages.map((page, index) => ( + + ))} +
diff --git a/src/renderer/components/ShortcutHint.tsx b/src/renderer/components/ShortcutHint.tsx index 72f20a9..39ef83d 100644 --- a/src/renderer/components/ShortcutHint.tsx +++ b/src/renderer/components/ShortcutHint.tsx @@ -134,15 +134,23 @@ export function Tooltip({ label, children, position = 'below', align = 'center' }, []); const isEnd = align === 'end'; - const posClass = position === 'above' - ? (isEnd ? 'bottom-full right-0 mb-0.5' : 'bottom-full left-1/2 mb-0.5') - : (isEnd ? 'top-full right-0 mt-0.5' : 'top-full left-1/2 mt-0.5'); - const transform = position === 'above' || position === 'below' - ? (isEnd ? undefined : 'translateX(-50%)') - : undefined; - const flexClass = position === 'above' - ? (isEnd ? 'flex-col items-end' : 'flex-col items-center') - : (isEnd ? 'flex-col items-end' : 'flex-col items-center'); + const posClass = + position === 'above' + ? isEnd + ? 'bottom-full right-0 mb-0.5' + : 'bottom-full left-1/2 mb-0.5' + : isEnd + ? 'top-full right-0 mt-0.5' + : 'top-full left-1/2 mt-0.5'; + const transform = position === 'above' || position === 'below' ? (isEnd ? undefined : 'translateX(-50%)') : undefined; + const flexClass = + position === 'above' + ? isEnd + ? 'flex-col items-end' + : 'flex-col items-center' + : isEnd + ? 'flex-col items-end' + : 'flex-col items-center'; const arrow = position === 'above' ? ARROW_DOWN : ARROW; return ( diff --git a/src/renderer/components/pdf/PdfSearchBar.tsx b/src/renderer/components/pdf/PdfSearchBar.tsx index 438249a..2c23037 100644 --- a/src/renderer/components/pdf/PdfSearchBar.tsx +++ b/src/renderer/components/pdf/PdfSearchBar.tsx @@ -236,6 +236,7 @@ export function PdfSearchBar({ containerRef, onClose, onNavigate }: PdfSearchBar value={query} onChange={(e) => setQuery(e.target.value)} placeholder="Find in PDF..." + aria-label="Search PDF text" className="w-48 text-mac-small bg-transparent border-none outline-none placeholder-gray-400" /> {query && ( diff --git a/src/renderer/components/pdf/usePdfDocument.ts b/src/renderer/components/pdf/usePdfDocument.ts index e21cba6..2aed260 100644 --- a/src/renderer/components/pdf/usePdfDocument.ts +++ b/src/renderer/components/pdf/usePdfDocument.ts @@ -3,7 +3,9 @@ import * as pdfjsLib from 'pdfjs-dist'; import pdfjsWorkerUrl from 'pdfjs-dist/build/pdf.worker.min.mjs?url'; // PDF.js appends CMap filenames to this URL prefix to fetch them on demand. import cMapSample from 'pdfjs-dist/cmaps/78-H.bcmap?url'; + const cMapUrl = cMapSample.replace(/[^/]+$/, ''); + import { useEffect, useRef, useState } from 'react'; pdfjsLib.GlobalWorkerOptions.workerSrc = pdfjsWorkerUrl;