fix(editor): let Cmd/Ctrl +/- zoom an open image (#32184)

* fix(editor): let Cmd/Ctrl +/- zoom an open image

The zoom shortcut on an image tab changed the hidden editor font zoom,
because only PdfViewer took the app zoom request. ImageViewer in the
command-owning pane now takes it too: it zooms the popup while that is
open and the inline image otherwise. Intrinsic-layout images in a
combined diff keep editor font zoom. The request module gets a
viewer-neutral name now that it serves PDFs and images.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* fix(editor): zoom a combined-diff image's popup from the shortcut

Intrinsic-layout images skipped the app zoom request even while their
popup was open, so Cmd/Ctrl +/- left the popup unchanged in a combined
diff. Skip them only while the popup is closed.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* fix(editor): let Cmd/Ctrl +/- zoom an open image

The zoom shortcut on an image tab changed the hidden editor font zoom,
because only PdfViewer took the app zoom request. ImageViewer in the
command-owning pane now takes it too: it zooms the popup while that is
open and the inline image otherwise. Intrinsic-layout images in a
combined diff keep editor font zoom. The request module gets a
viewer-neutral name now that it serves PDFs and images.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* fix(editor): zoom a combined-diff image's popup from the shortcut

Intrinsic-layout images skipped the app zoom request even while their
popup was open, so Cmd/Ctrl +/- left the popup unchanged in a combined
diff. Skip them only while the popup is closed.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* fix(editor): give image popups exclusive app zoom

* fix(editor): scope viewer zoom to the focused workspace surface

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Co-authored-by: Neil <neil@stably.ai>
This commit is contained in:
김보경
2026-10-23 19:21:28 -07:00
committed by GitHub
co-authored by Claude Opus 5.5 Neil
parent 24431a201b
commit ba9eb7fcae
11 changed files with 317 additions and 62 deletions
@@ -18,6 +18,9 @@ vi.mock('react', async () => {
useEffect() {
return undefined
},
useContext() {
return false
},
useRef<T>(initial: T) {
return { current: initial }
},
@@ -1,8 +1,10 @@
/* oxlint-disable react-doctor/no-adjust-state-on-prop-change -- Why: image surface size is measured with ResizeObserver and DOM refs, which are external layout systems outside render derivation. */
import { Image as ImageIcon, RotateCcw, ZoomIn, ZoomOut } from 'lucide-react'
import { type JSX, useCallback, useEffect, useMemo, useRef, useState } from 'react'
import { type JSX, useCallback, useContext, useEffect, useMemo, useRef, useState } from 'react'
import { cn } from '@/lib/utils'
import ImageViewerPopup from './ImageViewerPopup'
import { EditorCommandOwnerContext } from './editor-command-owner-context'
import { listenForViewerZoomRequests } from './viewer-zoom-request'
import PdfViewer from './PdfViewer'
import {
type ApplyImageViewerZoomChange,
@@ -54,6 +56,7 @@ export default function ImageViewer({
const [popupSurfaceSize, setPopupSurfaceSize] = useState<ImageViewerSurfaceSize | null>(null)
const [imageDimensions, setImageDimensions] = useState<ImageViewerImageDimensions | null>(null)
const [failedPreviewSrc, setFailedPreviewSrc] = useState<string | null>(null)
const isCommandOwner = useContext(EditorCommandOwnerContext)
const filename = useMemo(() => filePath.split(/[/\\]/).pop() || filePath, [filePath])
const cleanedContent = useMemo(() => content.replace(/\s/g, ''), [content])
@@ -217,6 +220,34 @@ export default function ImageViewer({
return () => observer.disconnect()
}, [isPopupOpen])
useEffect(() => {
// Why: PDFs own their zoom; combined diff images yield to editor text until a popup opens.
if (!isCommandOwner || isPdf || (isIntrinsicLayout && !isPopupOpen)) {
return
}
const applyZoomChange = isPopupOpen ? applyPopupZoomChange : applyInlineZoomChange
return listenForViewerZoomRequests(
(direction) =>
applyZoomChange((currentZoom) => {
if (direction === 'reset') {
return 1
}
return direction === 'in'
? currentZoom * IMAGE_VIEWER_ZOOM_STEP
: currentZoom / IMAGE_VIEWER_ZOOM_STEP
}),
isPopupOpen ? popupSurfaceRef : inlineSurfaceRef,
isPopupOpen ? 'popup' : 'pane'
)
}, [
applyInlineZoomChange,
applyPopupZoomChange,
isCommandOwner,
isIntrinsicLayout,
isPdf,
isPopupOpen
])
if (isPdf) {
return (
<PdfViewer
@@ -24,7 +24,7 @@ import {
} 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'
import { listenForViewerZoomRequests } from './viewer-zoom-request'
pdfjsLib.GlobalWorkerOptions.workerSrc = workerUrl
@@ -179,8 +179,9 @@ function PdfDocumentViewer({
if (!isCommandOwner) {
return
}
return listenForPdfZoomRequests((direction) =>
direction === 'reset' ? zoomReset() : stepZoom(direction)
return listenForViewerZoomRequests(
(direction) => (direction === 'reset' ? zoomReset() : stepZoom(direction)),
rootRef
)
}, [isCommandOwner, stepZoom, zoomReset])
@@ -0,0 +1,184 @@
// @vitest-environment happy-dom
import { act, cleanup, fireEvent, render, screen, within } from '@testing-library/react'
import { afterEach, describe, expect, it, vi } from 'vitest'
import ImageViewer from './ImageViewer'
import ImageDiffViewer from './ImageDiffViewer'
import { EditorCommandOwnerContext } from './editor-command-owner-context'
import { requestViewerZoom } from './viewer-zoom-request'
vi.mock('@/i18n/i18n', () => ({ translate: (_key: string, fallback: string) => fallback }))
vi.mock('./PdfViewer', () => ({ default: () => null }))
// 1x1 PNG
const PNG =
'iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mNkYAAAAAYAAjCB0C8AAAAASUVORK5CYII='
afterEach(cleanup)
function renderImage(isCommandOwner: boolean, layout: 'fill' | 'intrinsic' = 'fill'): void {
render(
<EditorCommandOwnerContext value={isCommandOwner}>
<ImageViewer content={PNG} filePath="/repo/shot.png" mimeType="image/png" layout={layout} />
</EditorCommandOwnerContext>
)
}
function zoom(direction: 'in' | 'out' | 'reset'): boolean {
let claimed = false
act(() => {
claimed = requestViewerZoom(direction)
})
return claimed
}
describe('ImageViewer app zoom ownership', () => {
it('zooms the image in the command-owning editor pane', () => {
renderImage(true)
expect(zoom('in')).toBe(true)
expect(screen.getByText('125%')).toBeTruthy()
zoom('in')
zoom('out')
expect(screen.getByText('125%')).toBeTruthy()
zoom('reset')
expect(screen.getByText('100%')).toBeTruthy()
})
it('zooms the open popup instead of the hidden inline image', () => {
renderImage(true)
fireEvent.click(screen.getByTitle('Open image in popup'))
zoom('in')
expect(within(screen.getByRole('dialog')).getByText('125%')).toBeTruthy()
expect(screen.getAllByText('100%')).toHaveLength(1)
})
it('releases zoom ownership when focus moves and removes its listener on unmount', () => {
const image = (ownsCommands: boolean) => (
<EditorCommandOwnerContext value={ownsCommands}>
<ImageViewer content={PNG} filePath="/repo/shot.png" mimeType="image/png" />
</EditorCommandOwnerContext>
)
const viewer = render(image(true))
expect(zoom('in')).toBe(true)
viewer.rerender(image(false))
expect(zoom('in')).toBe(false)
expect(screen.getByText('125%')).toBeTruthy()
viewer.rerender(image(true))
expect(zoom('in')).toBe(true)
expect(screen.getByText('156%')).toBeTruthy()
viewer.unmount()
expect(zoom('in')).toBe(false)
})
it('zooms a diff popup without changing the other image behind it', () => {
render(
<EditorCommandOwnerContext value={true}>
<ImageDiffViewer
originalContent={PNG}
modifiedContent={PNG}
filePath="/repo/shot.png"
mimeType="image/png"
sideBySide={true}
/>
</EditorCommandOwnerContext>
)
fireEvent.click(screen.getAllByTitle('Open image in popup')[1])
expect(zoom('in')).toBe(true)
expect(within(screen.getByRole('dialog')).getByText('125%')).toBeTruthy()
expect(screen.getAllByText('100%')).toHaveLength(2)
fireEvent.click(within(screen.getByRole('dialog')).getByRole('button', { name: 'Close' }))
expect(zoom('in')).toBe(true)
expect(screen.getAllByText('125%')).toHaveLength(2)
})
it('routes between main and floating image owners using DOM focus', () => {
const image = (filePath: string) => (
<EditorCommandOwnerContext value={true}>
<ImageViewer content={PNG} filePath={filePath} mimeType="image/png" />
</EditorCommandOwnerContext>
)
render(
<>
<div data-testid="main" tabIndex={-1}>
{image('/repo/main.png')}
</div>
<div data-testid="floating" data-floating-terminal-panel="" tabIndex={-1}>
{image('/repo/floating.png')}
</div>
</>
)
const main = screen.getByTestId('main')
const floating = screen.getByTestId('floating')
floating.focus()
expect(zoom('in')).toBe(true)
expect(within(floating).getByText('125%')).toBeTruthy()
expect(within(main).getByText('100%')).toBeTruthy()
main.focus()
expect(zoom('in')).toBe(true)
expect(within(main).getByText('125%')).toBeTruthy()
expect(within(floating).getByText('125%')).toBeTruthy()
floating.focus()
fireEvent.click(within(floating).getByTitle('Open image in popup'))
expect(zoom('in')).toBe(true)
expect(within(screen.getByRole('dialog')).getByText('156%')).toBeTruthy()
expect(within(main).getByText('125%')).toBeTruthy()
expect(within(floating).getByText('125%')).toBeTruthy()
})
it.each([true, false])(
'leaves text zoom unclaimed across surfaces (floating image: %s)',
(floating) => {
render(
<>
<div data-floating-terminal-panel={floating ? '' : undefined}>
<EditorCommandOwnerContext value={true}>
<ImageViewer content={PNG} filePath="/repo/shot.png" mimeType="image/png" />
</EditorCommandOwnerContext>
</div>
<div data-floating-terminal-panel={floating ? undefined : ''}>
<textarea aria-label="Other editor" />
</div>
</>
)
screen.getByRole('textbox', { name: 'Other editor' }).focus()
expect(zoom('in')).toBe(false)
expect(screen.getByText('100%')).toBeTruthy()
}
)
it('leaves zoom to the focused pane when this image is in an unfocused split or hidden worktree', () => {
renderImage(false)
expect(zoom('in')).toBe(false)
expect(screen.getByText('100%')).toBeTruthy()
})
it('leaves editor font zoom alone in a combined diff of intrinsic images', () => {
renderImage(true, 'intrinsic')
expect(zoom('in')).toBe(false)
expect(screen.getByText('100%')).toBeTruthy()
})
it('zooms the popup a combined-diff image opened', () => {
renderImage(true, 'intrinsic')
fireEvent.click(screen.getByTitle('Open image in popup'))
expect(zoom('in')).toBe(true)
expect(within(screen.getByRole('dialog')).getByText('125%')).toBeTruthy()
expect(screen.getAllByText('100%')).toHaveLength(1)
})
})
@@ -3,7 +3,7 @@ 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 { requestViewerZoom } from './viewer-zoom-request'
import { readPdfScalePreference } from './pdf-scale-preference-storage'
type PdfDocumentFixture = { name: string }
@@ -53,7 +53,7 @@ describe('PdfViewer app zoom ownership', () => {
let claimed = false
act(() => {
claimed = requestPdfZoom('in')
claimed = requestViewerZoom('in')
})
expect(claimed).toBe(true)
@@ -62,16 +62,30 @@ describe('PdfViewer app zoom ownership', () => {
viewers[0].currentScaleValue = '1.25'
act(() => {
requestPdfZoom('reset')
requestViewerZoom('reset')
})
expect(viewers[0].currentScaleValue).toBe('page-width')
expect(readPdfScalePreference('report')).toBe('page-width')
})
it('leaves zoom unclaimed when text in the floating workspace has focus', () => {
renderPdf(true)
const floating = document.createElement('textarea')
floating.dataset.floatingTerminalPanel = ''
document.body.appendChild(floating)
try {
floating.focus()
expect(requestViewerZoom('in')).toBe(false)
expect(viewers[0].currentScale).toBe(1)
} finally {
floating.remove()
}
})
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(requestViewerZoom('in')).toBe(false)
expect(viewers[0].currentScale).toBe(1)
})
})
@@ -1,18 +0,0 @@
// @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)
})
})
@@ -1,24 +0,0 @@
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)
}
@@ -0,0 +1,18 @@
// @vitest-environment happy-dom
import { describe, expect, it, vi } from 'vitest'
import { listenForViewerZoomRequests, requestViewerZoom } from './viewer-zoom-request'
describe('requestViewerZoom', () => {
it('reports whether a viewer took the zoom command', () => {
expect(requestViewerZoom('in')).toBe(false)
const onZoom = vi.fn()
const stop = listenForViewerZoomRequests(onZoom, { current: document.body })
expect(requestViewerZoom('out')).toBe(true)
expect(onZoom).toHaveBeenCalledWith('out')
stop()
expect(requestViewerZoom('reset')).toBe(false)
expect(onZoom).toHaveBeenCalledTimes(1)
})
})
@@ -0,0 +1,46 @@
import { isEventTargetInsideFloatingWorkspacePanel } from '@/lib/floating-workspace-terminal-actions'
export type ViewerZoomDirection = 'in' | 'out' | 'reset'
const VIEWER_ZOOM_REQUEST_EVENT = 'orca:viewer-zoom-request'
/** Offers an app zoom command to the focused pane's PDF or image; true when a viewer took it. */
export function requestViewerZoom(direction: ViewerZoomDirection): boolean {
const event = new CustomEvent(VIEWER_ZOOM_REQUEST_EVENT, {
detail: direction,
cancelable: true,
bubbles: true
})
// Why: dispatch below window so popup capture precedes pane listeners.
document.dispatchEvent(event)
return event.defaultPrevented
}
export function listenForViewerZoomRequests(
onZoom: (direction: ViewerZoomDirection) => void,
rootRef: { readonly current: HTMLElement | null },
priority: 'pane' | 'popup' = 'pane'
): () => void {
const listener = (event: Event): void => {
const root = rootRef.current
if (
!root ||
(priority !== 'popup' &&
isEventTargetInsideFloatingWorkspacePanel(document.activeElement) !==
isEventTargetInsideFloatingWorkspacePanel(root))
) {
return
}
const direction: unknown = event instanceof CustomEvent ? event.detail : null
if (direction === 'in' || direction === 'out' || direction === 'reset') {
event.preventDefault()
if (priority === 'popup') {
event.stopImmediatePropagation()
}
onZoom(direction)
}
}
const capture = priority === 'popup'
window.addEventListener(VIEWER_ZOOM_REQUEST_EVENT, listener, capture)
return () => window.removeEventListener(VIEWER_ZOOM_REQUEST_EVENT, listener, capture)
}
@@ -7,7 +7,7 @@ import { stepUIZoomLevel } from '../../../../shared/ui-zoom-level'
import { useAppStore } from '../../store'
import { focusedBrowserPageId, resolveZoomTarget } from '../resolve-zoom-target'
import { dispatchBrowserPageZoom } from '@/components/browser-pane/host-guest/browser-page-zoom'
import { requestPdfZoom } from '@/components/editor/pdf-zoom-request'
import { requestViewerZoom } from '@/components/editor/viewer-zoom-request'
export function registerZoomIpcBridge(unsubs: (() => void)[]): void {
// Zoom handling for menu accelerators and keyboard fallback paths.
@@ -38,8 +38,8 @@ 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)) {
// Why: a PDF or image owns its zoom; editor font zoom would change nothing visible.
if (requestViewerZoom(direction)) {
return
}
const next = nextEditorFontZoomLevel(editorFontZoomLevel, direction)
+9 -9
View File
@@ -139,7 +139,7 @@ describe('registerZoomIpcBridge', () => {
uiZoomLevel?: number
nativeChatAppearance?: NativeChatAppearanceSettings
editorFontZoomLevel?: number
pdfTakesZoom?: boolean
viewerTakesZoom?: boolean
} = {}
) {
const {
@@ -149,7 +149,7 @@ describe('registerZoomIpcBridge', () => {
uiZoomLevel = 0,
editorFontZoomLevel = 0,
nativeChatAppearance,
pdfTakesZoom = false
viewerTakesZoom = false
} = args
const applyUIZoom = vi.fn()
@@ -161,12 +161,12 @@ describe('registerZoomIpcBridge', () => {
Object.assign(settings, updates)
})
const requestPdfZoom = vi.fn(() => pdfTakesZoom)
const requestViewerZoom = vi.fn(() => viewerTakesZoom)
const dispatchBrowserPageZoom = vi.fn()
vi.doMock('@/components/browser-pane/host-guest/browser-page-zoom', () => ({
dispatchBrowserPageZoom
}))
vi.doMock('@/components/editor/pdf-zoom-request', () => ({ requestPdfZoom }))
vi.doMock('@/components/editor/viewer-zoom-request', () => ({ requestViewerZoom }))
vi.doMock('@/lib/ui-zoom', () => ({ applyUIZoom }))
vi.doMock('@/lib/zoom-events', () => ({ dispatchZoomLevelChanged }))
vi.doMock('../store', () => ({
@@ -211,7 +211,7 @@ describe('registerZoomIpcBridge', () => {
return {
fire,
dispatchBrowserPageZoom,
requestPdfZoom,
requestViewerZoom,
applyUIZoom,
dispatchZoomLevelChanged,
setEditorFontZoomLevel,
@@ -334,23 +334,23 @@ describe('registerZoomIpcBridge', () => {
const zoom = await mountZoomBridge({
activeTabType: 'editor',
activeElement: makeTarget({}),
pdfTakesZoom: true
viewerTakesZoom: true
})
zoom.fire('out')
expect(zoom.requestPdfZoom).toHaveBeenCalledWith('out')
expect(zoom.requestViewerZoom).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 })
const zoom = await mountZoomBridge({ activeTabType: 'browser', viewerTakesZoom: true })
zoom.fire('in')
expect(zoom.requestPdfZoom).not.toHaveBeenCalled()
expect(zoom.requestViewerZoom).not.toHaveBeenCalled()
expect(zoom.applyUIZoom).toHaveBeenCalledWith(0.5)
})