From 47019ae458a654df84b6ab37656544c4d97ea3cb Mon Sep 17 00:00:00 2001 From: Jinwoo Hong <73622457+Jinwoo-H@users.noreply.github.com> Date: Thu, 8 Oct 2026 16:09:56 -0400 Subject: [PATCH] fix(editor): route zoom gestures and app zoom to the PDF viewer (#26652) * fix(editor): route zoom gestures and app zoom to the PDF viewer Ctrl-wheel / trackpad pinch over a PDF scrolled it instead of zooming, and Cmd/Ctrl +/-/0 changed the (invisible) editor font zoom because the main process intercepts zoom shortcuts before the PDF's keydown listener runs. - The pdf.js session owns a non-passive ctrl-wheel listener for its container and zooms to an absolute scale anchored at the pointer. - App zoom commands on an editor tab are first offered to the PDF in the command-owning (focused) editor pane; unclaimed commands keep editor font zoom. - Wheel, toolbar and shortcut zoom all record the persisted scale. Co-authored-by: Infinity_X * fix(editor): make pinch zoom track the trackpad more closely Halve the wheel-zoom divisor so pinch zoom in PDFs and images moves twice as fast, and halve the per-event delta cap so a single event still zooms less than 2x. Co-authored-by: Infinity_X * test(editor): type the PDF ownership test's document fixture * fix(editor): keep fast pinch bursts anchored to the pointer pdf.js refreshes its saved location on the next frame, so a second wheel event in the same frame zoomed around the old location and drifted. Call viewer.update() after the anchor scroll. Also make the ownership test's reset assertion depend on reset running. --------- Co-authored-by: Infinity_X --- .../src/components/editor/EditorPanel.tsx | 116 +++++++++--------- .../src/components/editor/PdfViewer.tsx | 58 +++++---- .../editor/editor-command-owner-context.ts | 4 + .../editor/image-viewer-zoom.test.ts | 4 +- .../components/editor/image-viewer-zoom.ts | 4 +- .../editor/pdf-scale-preference.test.ts | 62 +++++++++- .../components/editor/pdf-scale-preference.ts | 47 +++++++ .../components/editor/pdf-viewer-session.ts | 22 +++- .../editor/pdf-viewer-zoom-ownership.test.tsx | 77 ++++++++++++ .../editor/pdf-zoom-request.test.ts | 18 +++ .../src/components/editor/pdf-zoom-request.ts | 24 ++++ .../src/hooks/ipc-events/zoom-ipc-bridge.ts | 5 + src/renderer/src/hooks/zoom-routing.test.ts | 31 ++++- 13 files changed, 382 insertions(+), 90 deletions(-) create mode 100644 src/renderer/src/components/editor/editor-command-owner-context.ts create mode 100644 src/renderer/src/components/editor/pdf-viewer-zoom-ownership.test.tsx create mode 100644 src/renderer/src/components/editor/pdf-zoom-request.test.ts create mode 100644 src/renderer/src/components/editor/pdf-zoom-request.ts diff --git a/src/renderer/src/components/editor/EditorPanel.tsx b/src/renderer/src/components/editor/EditorPanel.tsx index de466cd7074..118b14a42bb 100644 --- a/src/renderer/src/components/editor/EditorPanel.tsx +++ b/src/renderer/src/components/editor/EditorPanel.tsx @@ -9,6 +9,7 @@ import { exportActiveMarkdownToPdf } from './export-active-markdown' import type { EditorToggleValue } from './EditorViewToggle' import { EditorPanelShell } from './EditorPanelShell' import { DiffNavigationProvider } from './diff-navigation-context' +import { EditorCommandOwnerContext } from './editor-command-owner-context' import { canUseChangesModeForFile } from './editor-panel-file-mode' import { getEditorPanelRenderModel } from './editor-panel-render-model' import { useEditorCmdSaveRequest } from './useEditorCmdSaveRequest' @@ -317,61 +318,66 @@ function EditorPanelInner({ ) return ( - // Why: each split pane needs an isolated bridge between its diff editor and header controls. - - void handleCopyPath()} - onOpenDiffTargetFile={handleOpenDiffTargetFile} - onOpenPreviewToSide={handleOpenPreviewToSide} - onOpenMarkdownPreview={handleOpenMarkdownPreview} - onToggleSideBySide={() => setSideBySide((prev) => !prev)} - onEditorToggleChange={handleEditorToggleChange} - onToggleMarkdownTableOfContents={() => - setMarkdownTableOfContentsVisible( - markdownDocumentStateFileId, - !isMarkdownTableOfContentsVisible - ) - } - onToggleMarkdownFrontmatter={() => - setMarkdownFrontmatterVisible(markdownDocumentStateFileId, !isMarkdownFrontmatterVisible) - } - onExportMarkdownToPdf={() => - void exportActiveMarkdownToPdf({ fileId: activeFile.id, root: panelRef.current }) - } - createMarkdownArtifactRequest={ - activeMarkdownContent === null ? undefined : createActiveMarkdownArtifactRequest - } - onContentChange={handleContentChange} - onContentChangeForFile={handleContentChangeForFile} - onDirtyStateHint={handleDirtyStateHint} - onSave={handleSave} - onSaveForFile={handleSaveForFile} - onReloadContent={reloadContent} - onCloseMarkdownTableOfContents={() => - setMarkdownTableOfContentsVisible(markdownDocumentStateFileId, false) - } - onCloseRenameDialog={closeRenameDialog} - onRenameConfirm={handleRenameConfirm} - markdownAnnotationsEnabled={markdownAnnotationsEnabled && markdownReviewNotesEnabled} - /> - + + {/* Why: each split pane needs an isolated bridge between its diff editor and header controls. */} + + void handleCopyPath()} + onOpenDiffTargetFile={handleOpenDiffTargetFile} + onOpenPreviewToSide={handleOpenPreviewToSide} + onOpenMarkdownPreview={handleOpenMarkdownPreview} + onToggleSideBySide={() => setSideBySide((prev) => !prev)} + onEditorToggleChange={handleEditorToggleChange} + onToggleMarkdownTableOfContents={() => + setMarkdownTableOfContentsVisible( + markdownDocumentStateFileId, + !isMarkdownTableOfContentsVisible + ) + } + onToggleMarkdownFrontmatter={() => + setMarkdownFrontmatterVisible( + markdownDocumentStateFileId, + !isMarkdownFrontmatterVisible + ) + } + onExportMarkdownToPdf={() => + void exportActiveMarkdownToPdf({ fileId: activeFile.id, root: panelRef.current }) + } + createMarkdownArtifactRequest={ + activeMarkdownContent === null ? undefined : createActiveMarkdownArtifactRequest + } + onContentChange={handleContentChange} + onContentChangeForFile={handleContentChangeForFile} + onDirtyStateHint={handleDirtyStateHint} + onSave={handleSave} + onSaveForFile={handleSaveForFile} + onReloadContent={reloadContent} + onCloseMarkdownTableOfContents={() => + setMarkdownTableOfContentsVisible(markdownDocumentStateFileId, false) + } + onCloseRenameDialog={closeRenameDialog} + onRenameConfirm={handleRenameConfirm} + markdownAnnotationsEnabled={markdownAnnotationsEnabled && markdownReviewNotesEnabled} + /> + + ) } diff --git a/src/renderer/src/components/editor/PdfViewer.tsx b/src/renderer/src/components/editor/PdfViewer.tsx index b63ce8ef1ff..1243f2664a2 100644 --- a/src/renderer/src/components/editor/PdfViewer.tsx +++ b/src/renderer/src/components/editor/PdfViewer.tsx @@ -1,5 +1,5 @@ /* oxlint-disable react-doctor/no-adjust-state-on-prop-change -- Why: PDF loading drives pdf.js document/viewer instances and decode errors through an external worker lifecycle. */ -import { type JSX, useCallback, useEffect, useMemo, useRef, useState } from 'react' +import { type JSX, useCallback, useContext, useEffect, useMemo, useRef, useState } from 'react' import { Image as ImageIcon, RotateCcw, Search, ZoomIn, ZoomOut } from 'lucide-react' import * as pdfjsLib from 'pdfjs-dist' import type { @@ -24,6 +24,8 @@ import { type PdfScalePreference } from './pdf-scale-preference' import { readPdfScalePreference, writePdfScalePreference } from './pdf-scale-preference-storage' +import { EditorCommandOwnerContext } from './editor-command-owner-context' +import { listenForPdfZoomRequests } from './pdf-zoom-request' pdfjsLib.GlobalWorkerOptions.workerSrc = workerUrl @@ -66,6 +68,7 @@ function PdfDocumentViewer({ const [scale, setScale] = useState(1) const keybindings = useAppStore((state) => state.keybindings) const findShortcutLabel = useShortcutLabel('editor.find') + const isCommandOwner = useContext(EditorCommandOwnerContext) const eventBusRef = useRef | null>(null) const findControllerRef = useRef | null>(null) const pdfViewerRef = useRef | null>(null) @@ -76,6 +79,18 @@ function PdfDocumentViewer({ const filename = useMemo(() => filePath.split(/[/\\]/).pop() || filePath, [filePath]) const cleanedContent = useMemo(() => content.replace(/\s/g, ''), [content]) + // Why: every zoom entry point (toolbar, app zoom command, wheel/pinch) must record the + // scale preference so the next content reload restores it (see scalePreferenceRef). + const recordScalePreference = useCallback( + (preference: PdfScalePreference) => { + scalePreferenceRef.current = preference + if (preferenceKey) { + writePdfScalePreference(preferenceKey, preference) + } + }, + [preferenceKey] + ) + useEffect(() => { const container = containerRef.current const viewerDiv = viewerDivRef.current @@ -97,7 +112,8 @@ function PdfDocumentViewer({ scrollCacheKey, scalePreference: scalePreferenceRef.current, scaleBounds: SCALE_BOUNDS, - onScaleChanging: setScale + onScaleChanging: setScale, + onWheelZoom: recordScalePreference }) eventBusRef.current = session.eventBus findControllerRef.current = session.findController @@ -119,7 +135,7 @@ function PdfDocumentViewer({ loaderRef.current = null loader.dispose() } - }, [filePath, preferenceKey, scrollCacheKey]) + }, [filePath, preferenceKey, recordScalePreference, scrollCacheKey]) useEffect(() => { loaderRef.current?.load(cleanedContent) @@ -134,8 +150,6 @@ function PdfDocumentViewer({ setFindOpen(false) }, []) - // Why: every zoom entry point (toolbar + keyboard) must record the scale - // preference so the next content reload restores it (see scalePreferenceRef). const stepZoom = useCallback( (direction: 'in' | 'out') => { const viewer = pdfViewerRef.current @@ -144,12 +158,9 @@ function PdfDocumentViewer({ } const next = stepPdfScalePreference(viewer.currentScale, direction, SCALE_BOUNDS) viewer.currentScale = next.scale - scalePreferenceRef.current = next.preference - if (preferenceKey) { - writePdfScalePreference(preferenceKey, next.preference) - } + recordScalePreference(next.preference) }, - [preferenceKey] + [recordScalePreference] ) const zoomIn = useCallback(() => stepZoom('in'), [stepZoom]) @@ -160,12 +171,18 @@ function PdfDocumentViewer({ if (!viewer) { return } - scalePreferenceRef.current = 'page-width' applyPdfScalePreference(viewer, 'page-width', SCALE_BOUNDS) - if (preferenceKey) { - writePdfScalePreference(preferenceKey, 'page-width') + recordScalePreference('page-width') + }, [recordScalePreference]) + + useEffect(() => { + if (!isCommandOwner) { + return } - }, [preferenceKey]) + return listenForPdfZoomRequests((direction) => + direction === 'reset' ? zoomReset() : stepZoom(direction) + ) + }, [isCommandOwner, stepZoom, zoomReset]) useEffect(() => { const handleKeyDown = (e: KeyboardEvent): void => { @@ -174,22 +191,11 @@ function PdfDocumentViewer({ e.preventDefault() e.stopPropagation() setFindOpen(true) - return - } - if (keybindingMatchesAction('zoom.in', e, platform, keybindings)) { - e.preventDefault() - zoomIn() - } else if (keybindingMatchesAction('zoom.out', e, platform, keybindings)) { - e.preventDefault() - zoomOut() - } else if (keybindingMatchesAction('zoom.reset', e, platform, keybindings)) { - e.preventDefault() - zoomReset() } } window.addEventListener('keydown', handleKeyDown, true) return () => window.removeEventListener('keydown', handleKeyDown, true) - }, [keybindings, zoomIn, zoomOut, zoomReset]) + }, [keybindings]) const zoomPercent = Math.round(scale * 100) diff --git a/src/renderer/src/components/editor/editor-command-owner-context.ts b/src/renderer/src/components/editor/editor-command-owner-context.ts new file mode 100644 index 00000000000..e78988a642c --- /dev/null +++ b/src/renderer/src/components/editor/editor-command-owner-context.ts @@ -0,0 +1,4 @@ +import { createContext } from 'react' + +// Why: split panes and hidden worktrees keep editors mounted; only the focused pane answers app commands. +export const EditorCommandOwnerContext = createContext(false) diff --git a/src/renderer/src/components/editor/image-viewer-zoom.test.ts b/src/renderer/src/components/editor/image-viewer-zoom.test.ts index 367c4f365e2..c67aa8899c3 100644 --- a/src/renderer/src/components/editor/image-viewer-zoom.test.ts +++ b/src/renderer/src/components/editor/image-viewer-zoom.test.ts @@ -43,8 +43,8 @@ describe('image viewer zoom helpers', () => { }) it('bounds very large per-event zoom factors', () => { - expect(getPinchZoomFactor(-10_000, 0)).toBeCloseTo(getPinchZoomFactor(-200, 0)) - expect(getPinchZoomFactor(10_000, 0)).toBeCloseTo(getPinchZoomFactor(200, 0)) + expect(getPinchZoomFactor(-10_000, 0)).toBeCloseTo(getPinchZoomFactor(-100, 0)) + expect(getPinchZoomFactor(10_000, 0)).toBeCloseTo(getPinchZoomFactor(100, 0)) expect(getPinchZoomFactor(-10_000, 0)).toBeLessThan(2) expect(getPinchZoomFactor(10_000, 0)).toBeGreaterThan(0.5) }) diff --git a/src/renderer/src/components/editor/image-viewer-zoom.ts b/src/renderer/src/components/editor/image-viewer-zoom.ts index 3f6d2c30e08..f9896bf29da 100644 --- a/src/renderer/src/components/editor/image-viewer-zoom.ts +++ b/src/renderer/src/components/editor/image-viewer-zoom.ts @@ -7,8 +7,8 @@ const DOM_DELTA_LINE = 1 const DOM_DELTA_PAGE = 2 const PIXELS_PER_LINE = 16 const PIXELS_PER_PAGE = 800 -const MAX_NORMALIZED_WHEEL_DELTA = 200 -const WHEEL_ZOOM_SENSITIVITY = 300 +const MAX_NORMALIZED_WHEEL_DELTA = 100 +const WHEEL_ZOOM_SENSITIVITY = 150 type ImageZoomWheelEventLike = { ctrlKey: boolean diff --git a/src/renderer/src/components/editor/pdf-scale-preference.test.ts b/src/renderer/src/components/editor/pdf-scale-preference.test.ts index 6d3ac4a7584..51ecc701c2d 100644 --- a/src/renderer/src/components/editor/pdf-scale-preference.test.ts +++ b/src/renderer/src/components/editor/pdf-scale-preference.test.ts @@ -1,9 +1,11 @@ -import { describe, expect, it } from 'vitest' +import { describe, expect, it, vi } from 'vitest' import { applyPdfScalePreference, clampPdfScale, - stepPdfScalePreference + stepPdfScalePreference, + zoomPdfViewerWithWheel } from './pdf-scale-preference' +import { getPinchZoomFactor } from './image-viewer-zoom' const BOUNDS = { min: 0.25, max: 5, step: 1.25 } @@ -46,3 +48,59 @@ describe('stepPdfScalePreference', () => { expect(zoomedOut.scale).toBe(1) }) }) + +describe('zoomPdfViewerWithWheel', () => { + function wheel(deltaY: number, ctrlKey = true) { + return { ctrlKey, deltaY, deltaMode: 0, clientX: 300, clientY: 250, preventDefault: vi.fn() } + } + function viewerAt(scale: number) { + return { + currentScale: scale, + container: { + scrollLeft: 100, + scrollTop: 400, + getBoundingClientRect: () => ({ left: 200, top: 50 }) + }, + update: vi.fn() + } + } + + it('leaves plain wheel scrolling alone', () => { + const viewer = viewerAt(1) + const event = wheel(-30, false) + expect(zoomPdfViewerWithWheel(viewer, event, BOUNDS)).toBeNull() + expect(event.preventDefault).not.toHaveBeenCalled() + expect(viewer.currentScale).toBe(1) + }) + + it('keeps the content under the pointer in place', () => { + const viewer = viewerAt(1) + const event = wheel(-30) + expect(zoomPdfViewerWithWheel(viewer, event, BOUNDS)).toBe(viewer.currentScale) + expect(event.preventDefault).toHaveBeenCalled() + const growth = viewer.currentScale - 1 + expect(growth).toBeGreaterThan(0) + // Pointer sits 100px right of and 200px below the container's top-left. + expect(viewer.container.scrollLeft).toBeCloseTo(100 + 100 * growth) + expect(viewer.container.scrollTop).toBeCloseTo(400 + 200 * growth) + // The next same-frame event must anchor to this scroll position. + expect(viewer.update).toHaveBeenCalledOnce() + }) + + it('accumulates slow trackpad pinches instead of rounding them away', () => { + const viewer = viewerAt(0.5) + for (let i = 0; i < 30; i++) { + zoomPdfViewerWithWheel(viewer, wheel(-1), BOUNDS) + } + expect(viewer.currentScale).toBeCloseTo(0.5 * getPinchZoomFactor(-1, 0) ** 30) + }) + + it('claims the gesture but stops at the scale bounds', () => { + const viewer = viewerAt(BOUNDS.max) + const event = wheel(-30) + expect(zoomPdfViewerWithWheel(viewer, event, BOUNDS)).toBeNull() + expect(event.preventDefault).toHaveBeenCalled() + expect(viewer.container.scrollLeft).toBe(100) + expect(viewer.update).not.toHaveBeenCalled() + }) +}) diff --git a/src/renderer/src/components/editor/pdf-scale-preference.ts b/src/renderer/src/components/editor/pdf-scale-preference.ts index abdba00c6a9..27f50254a28 100644 --- a/src/renderer/src/components/editor/pdf-scale-preference.ts +++ b/src/renderer/src/components/editor/pdf-scale-preference.ts @@ -1,3 +1,5 @@ +import { getPinchZoomFactor, shouldHandleImageZoomWheel } from './image-viewer-zoom' + export type PdfScalePreference = 'page-width' | number export function clampPdfScale(scale: number, min: number, max: number): number { @@ -29,3 +31,48 @@ export function stepPdfScalePreference( : clampPdfScale(currentScale / bounds.step, bounds.min, bounds.max) return { scale: next, preference: next } } + +type PdfWheelZoomEvent = Pick< + WheelEvent, + 'ctrlKey' | 'deltaY' | 'deltaMode' | 'clientX' | 'clientY' | 'preventDefault' +> + +/** Ctrl-wheel / trackpad-pinch zoom that keeps the content under the pointer in place. */ +export function zoomPdfViewerWithWheel( + viewer: { + currentScale: number + container: { + scrollLeft: number + scrollTop: number + getBoundingClientRect: () => { left: number; top: number } + } + update: () => void + }, + event: PdfWheelZoomEvent, + bounds: { min: number; max: number } +): number | null { + if (!shouldHandleImageZoomWheel(event)) { + return null + } + event.preventDefault() + const previous = viewer.currentScale + const next = clampPdfScale( + previous * getPinchZoomFactor(event.deltaY, event.deltaMode), + bounds.min, + bounds.max + ) + if (next === previous) { + return null + } + const { container } = viewer + const rect = container.getBoundingClientRect() + // Why: an absolute scale avoids updateScale's 0.01 rounding, which swallows slow pinches. + viewer.currentScale = next + // Why: pdf.js keeps the viewport's top-left fixed, so shift by the pointer's grown offset. + const growth = next / previous - 1 + container.scrollLeft += (event.clientX - rect.left) * growth + container.scrollTop += (event.clientY - rect.top) * growth + // Why: pdf.js re-reads its location on the next frame; a faster pinch event would anchor to the old one. + viewer.update() + return next +} diff --git a/src/renderer/src/components/editor/pdf-viewer-session.ts b/src/renderer/src/components/editor/pdf-viewer-session.ts index dfa80fff68b..52b14b3eefe 100644 --- a/src/renderer/src/components/editor/pdf-viewer-session.ts +++ b/src/renderer/src/components/editor/pdf-viewer-session.ts @@ -5,7 +5,11 @@ import { PDFLinkService, PDFViewer as PdfJsViewer } from 'pdfjs-dist/web/pdf_viewer.mjs' -import { applyPdfScalePreference, type PdfScalePreference } from './pdf-scale-preference' +import { + applyPdfScalePreference, + zoomPdfViewerWithWheel, + type PdfScalePreference +} from './pdf-scale-preference' import { pdfViewPositionCache, setWithLRU } from '@/lib/scroll-cache' import { buildPdfScrollDestination, @@ -33,7 +37,8 @@ export function createPdfViewerSession({ scrollCacheKey, scalePreference, scaleBounds, - onScaleChanging + onScaleChanging, + onWheelZoom }: { container: HTMLDivElement viewerDiv: HTMLDivElement @@ -42,6 +47,7 @@ export function createPdfViewerSession({ scalePreference: PdfScalePreference scaleBounds: { min: number; max: number } onScaleChanging: (scale: number) => void + onWheelZoom: (scale: number) => void }): { viewer: InstanceType eventBus: InstanceType @@ -77,6 +83,18 @@ export function createPdfViewerSession({ } eventBus.on('scalechanging', handleScaleChanging) + // Why: Chromium reports trackpad pinch as ctrl-wheel; unclaimed, it only scrolls the PDF. + container.addEventListener( + 'wheel', + (event) => { + const scale = zoomPdfViewerWithWheel(viewer, event, scaleBounds) + if (scale !== null) { + onWheelZoom(scale) + } + }, + { passive: false, signal: abortController.signal } + ) + // Why: each displayed document owns its scroll recorder and cache key. const recorder = scrollCacheKey ? createPdfViewPositionRecorder({ diff --git a/src/renderer/src/components/editor/pdf-viewer-zoom-ownership.test.tsx b/src/renderer/src/components/editor/pdf-viewer-zoom-ownership.test.tsx new file mode 100644 index 00000000000..ff4c6340046 --- /dev/null +++ b/src/renderer/src/components/editor/pdf-viewer-zoom-ownership.test.tsx @@ -0,0 +1,77 @@ +// @vitest-environment happy-dom +import { act, cleanup, render } from '@testing-library/react' +import { afterEach, describe, expect, it, vi } from 'vitest' +import PdfViewer from './PdfViewer' +import { EditorCommandOwnerContext } from './editor-command-owner-context' +import { requestPdfZoom } from './pdf-zoom-request' +import { readPdfScalePreference } from './pdf-scale-preference-storage' + +type PdfDocumentFixture = { name: string } + +const viewers = vi.hoisted((): { currentScale: number; currentScaleValue: string }[] => []) + +vi.mock('pdfjs-dist', () => ({ GlobalWorkerOptions: {} })) +vi.mock('pdfjs-dist/web/pdf_viewer.css', () => ({})) +vi.mock('pdfjs-dist/build/pdf.worker.min.mjs?url', () => ({ default: '' })) +vi.mock('@/i18n/i18n', () => ({ translate: (_key: string, fallback: string) => fallback })) +vi.mock('@/hooks/useShortcutLabel', () => ({ useShortcutLabel: () => '' })) +vi.mock('@/store', () => ({ + useAppStore: (selector: (state: { keybindings: Record }) => unknown) => + selector({ keybindings: {} }) +})) +vi.mock('./pdf-document-loader', () => ({ + createPdfDocumentLoader: ({ display }: { display: (doc: PdfDocumentFixture) => () => void }) => ({ + load: () => display({ name: 'report.pdf' }), + dispose: () => {} + }) +})) +vi.mock('./pdf-viewer-session', () => ({ + createPdfViewerSession: () => { + const viewer = { currentScale: 1, currentScaleValue: 'page-width' } + viewers.push(viewer) + return { viewer, eventBus: null, findController: null, dispose: () => {} } + } +})) + +afterEach(() => { + cleanup() + viewers.length = 0 + localStorage.clear() +}) + +function renderPdf(isCommandOwner: boolean): void { + render( + + + + ) +} + +describe('PdfViewer app zoom ownership', () => { + it('zooms the PDF in the command-owning editor pane', () => { + renderPdf(true) + + let claimed = false + act(() => { + claimed = requestPdfZoom('in') + }) + + expect(claimed).toBe(true) + expect(viewers[0].currentScale).toBe(1.25) + expect(readPdfScalePreference('report')).toBe(1.25) + + viewers[0].currentScaleValue = '1.25' + act(() => { + requestPdfZoom('reset') + }) + expect(viewers[0].currentScaleValue).toBe('page-width') + expect(readPdfScalePreference('report')).toBe('page-width') + }) + + it('leaves zoom to the focused pane when this PDF is in an unfocused split or hidden worktree', () => { + renderPdf(false) + + expect(requestPdfZoom('in')).toBe(false) + expect(viewers[0].currentScale).toBe(1) + }) +}) diff --git a/src/renderer/src/components/editor/pdf-zoom-request.test.ts b/src/renderer/src/components/editor/pdf-zoom-request.test.ts new file mode 100644 index 00000000000..b8b33bf6876 --- /dev/null +++ b/src/renderer/src/components/editor/pdf-zoom-request.test.ts @@ -0,0 +1,18 @@ +// @vitest-environment happy-dom +import { describe, expect, it, vi } from 'vitest' +import { listenForPdfZoomRequests, requestPdfZoom } from './pdf-zoom-request' + +describe('requestPdfZoom', () => { + it('reports whether a PDF took the zoom command', () => { + expect(requestPdfZoom('in')).toBe(false) + + const onZoom = vi.fn() + const stop = listenForPdfZoomRequests(onZoom) + expect(requestPdfZoom('out')).toBe(true) + expect(onZoom).toHaveBeenCalledWith('out') + + stop() + expect(requestPdfZoom('reset')).toBe(false) + expect(onZoom).toHaveBeenCalledTimes(1) + }) +}) diff --git a/src/renderer/src/components/editor/pdf-zoom-request.ts b/src/renderer/src/components/editor/pdf-zoom-request.ts new file mode 100644 index 00000000000..5f46b78176a --- /dev/null +++ b/src/renderer/src/components/editor/pdf-zoom-request.ts @@ -0,0 +1,24 @@ +export type PdfZoomDirection = 'in' | 'out' | 'reset' + +const PDF_ZOOM_REQUEST_EVENT = 'orca:pdf-zoom-request' + +/** Offers an app zoom command to the focused pane's PDF; true when a PDF took it. */ +export function requestPdfZoom(direction: PdfZoomDirection): boolean { + const event = new CustomEvent(PDF_ZOOM_REQUEST_EVENT, { detail: direction, cancelable: true }) + window.dispatchEvent(event) + return event.defaultPrevented +} + +export function listenForPdfZoomRequests( + onZoom: (direction: PdfZoomDirection) => void +): () => void { + const listener = (event: Event): void => { + const direction: unknown = event instanceof CustomEvent ? event.detail : null + if (direction === 'in' || direction === 'out' || direction === 'reset') { + event.preventDefault() + onZoom(direction) + } + } + window.addEventListener(PDF_ZOOM_REQUEST_EVENT, listener) + return () => window.removeEventListener(PDF_ZOOM_REQUEST_EVENT, listener) +} diff --git a/src/renderer/src/hooks/ipc-events/zoom-ipc-bridge.ts b/src/renderer/src/hooks/ipc-events/zoom-ipc-bridge.ts index 6d233790909..763c553fcfd 100644 --- a/src/renderer/src/hooks/ipc-events/zoom-ipc-bridge.ts +++ b/src/renderer/src/hooks/ipc-events/zoom-ipc-bridge.ts @@ -6,6 +6,7 @@ import { dispatchZoomLevelChanged } from '@/lib/zoom-events' import { stepUIZoomLevel } from '../../../../shared/ui-zoom-level' import { useAppStore } from '../../store' import { resolveZoomTarget } from '../resolve-zoom-target' +import { requestPdfZoom } from '@/components/editor/pdf-zoom-request' export function registerZoomIpcBridge(unsubs: (() => void)[]): void { // Zoom handling for menu accelerators and keyboard fallback paths. @@ -29,6 +30,10 @@ export function registerZoomIpcBridge(unsubs: (() => void)[]): void { return } if (target === 'editor') { + // Why: a PDF owns its page zoom; editor font zoom would change nothing visible. + if (requestPdfZoom(direction)) { + return + } const next = nextEditorFontZoomLevel(editorFontZoomLevel, direction) setEditorFontZoomLevel(next) void window.api.ui.set({ editorFontZoomLevel: next }) diff --git a/src/renderer/src/hooks/zoom-routing.test.ts b/src/renderer/src/hooks/zoom-routing.test.ts index 370e229fb01..4ee5624e26b 100644 --- a/src/renderer/src/hooks/zoom-routing.test.ts +++ b/src/renderer/src/hooks/zoom-routing.test.ts @@ -113,6 +113,7 @@ describe('registerZoomIpcBridge', () => { uiZoomLevel?: number nativeChatAppearance?: NativeChatAppearanceSettings editorFontZoomLevel?: number + pdfTakesZoom?: boolean } = {} ) { const { @@ -121,7 +122,8 @@ describe('registerZoomIpcBridge', () => { activeElement = makeTarget({ editorClosest: true, hasXtermClass: true }), uiZoomLevel = 0, editorFontZoomLevel = 0, - nativeChatAppearance + nativeChatAppearance, + pdfTakesZoom = false } = args const applyUIZoom = vi.fn() @@ -133,6 +135,8 @@ describe('registerZoomIpcBridge', () => { Object.assign(settings, updates) }) + const requestPdfZoom = vi.fn(() => pdfTakesZoom) + vi.doMock('@/components/editor/pdf-zoom-request', () => ({ requestPdfZoom })) vi.doMock('@/lib/ui-zoom', () => ({ applyUIZoom })) vi.doMock('@/lib/zoom-events', () => ({ dispatchZoomLevelChanged })) vi.doMock('../store', () => ({ @@ -176,6 +180,7 @@ describe('registerZoomIpcBridge', () => { } return { fire, + requestPdfZoom, applyUIZoom, dispatchZoomLevelChanged, setEditorFontZoomLevel, @@ -264,6 +269,30 @@ describe('registerZoomIpcBridge', () => { expect(zoom.applyUIZoom).not.toHaveBeenCalled() }) + it('hands editor-tab zoom to the PDF it shows instead of editor font zoom', async () => { + const zoom = await mountZoomBridge({ + activeTabType: 'editor', + activeElement: makeTarget({}), + pdfTakesZoom: true + }) + + zoom.fire('out') + + expect(zoom.requestPdfZoom).toHaveBeenCalledWith('out') + expect(zoom.setEditorFontZoomLevel).not.toHaveBeenCalled() + expect(zoom.dispatchZoomLevelChanged).not.toHaveBeenCalled() + expect(zoom.applyUIZoom).not.toHaveBeenCalled() + }) + + it('never offers app zoom outside editor tabs to a PDF', async () => { + const zoom = await mountZoomBridge({ activeTabType: 'browser', pdfTakesZoom: true }) + + zoom.fire('in') + + expect(zoom.requestPdfZoom).not.toHaveBeenCalled() + expect(zoom.applyUIZoom).toHaveBeenCalledWith(0.5) + }) + it('clamps app zoom at the supported maximum', async () => { const zoom = await mountZoomBridge({ uiZoomLevel: UI_ZOOM_MAX })