Skip to content

feat(editor): click-to-zoom image viewer like Chromium, Alt-drag to pan - #36898

Merged
nwparker merged 4 commits into
mainfrom
nwparker/svg-img-viewer-behavior
Nov 1, 2026
Merged

nwparker merged 4 commits into
mainfrom
nwparker/svg-img-viewer-behavior

Conversation

@nwparker

@nwparker nwparker commented Nov 1, 2026 •

Copy link
Copy Markdown
Contributor
Files Added Deleted Net
Test 2 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​53 $\color{#cf222e}{\Huge{\mathbf{−}}}$​7 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​46
Prod 3 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​106 $\color{#cf222e}{\Huge{\mathbf{−}}}$​20 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​86

ELI5

Image and SVG previews now behave like Chromium's image view. An image opens fitted to the pane. Clicking zooms to actual size (100%) at the point you clicked, and clicking again fits it back. Hold Alt (Option on macOS) and the cursor turns into a grab hand so you can drag to pan.

What Changed

  • Before (feat(editor): SVG preview + source, and a drag-to-pan image viewer #36576): every press-drag panned, and the cursor was always a grab hand.
  • After:
    • Click switches between fit and actual size, anchored at the cursor. The cursor shows zoom-in or zoom-out depending on which way the click goes. A fitted SVG that was enlarged past its natural size gets zoom-out. An image whose fit is already 100% gets no zoom cursor, and clicking it does nothing (Chromium does the same).
    • From any other zoom (wheel, pinch, toolbar), a click fits.
    • Alt/Option held shows grab, and dragging pans (grabbing). The cursor follows the Alt key (keydown/keyup, pointer altKey, cleared on window blur).
    • A plain drag no longer pans. Wheel and trackpad panning and Ctrl/pinch zoom are unchanged.
    • Touch and pen still drag to pan, since they have no Alt key. A tap toggles zoom.
  • This applies everywhere usePanZoomViewport is used: the editor pane image/SVG viewer and the expanded pan/zoom dialog (diff images, Mermaid, chat images).

Validation

  • pnpm tc:web, oxlint, and pnpm test PanZoomImageDialog.test.tsx ImageViewer.test.tsx. A new test covers click → 100% at the anchor, plain drag not panning, Alt cursor and Alt-drag panning, and click → fit.
  • Dev Electron app over CDP with real mouse and keyboard input:
    • 3200×2400 PNG: fits at 34% (zoom-in). A click goes to 100% (zoom-out). A plain drag does not move it. With Alt held the cursor is grab, and dragging pans by exactly the pointer delta (grabbing). Clicking again fits.
    • 64×64 SVG: fits enlarged (zoom-out). A click goes to 64px (zoom-in), and clicking again fits.
    • 256×256 PNG, already at 100%: no zoom cursor, and clicks do nothing.

Notes

  • On some Linux window managers, Alt+drag moves the window and never reaches the app. Wheel/trackpad panning still works there.

@coderabbitai

coderabbitai Bot commented Nov 1, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review.

Recent review info
Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 269434dc-c714-4c72-8ecf-8ea8b4f070d6
Commits

Reviewing files that changed from the base of the PR and between d8d67cb and 6d0114b.

Files selected for processing: 1
  • src/renderer/src/components/editor/DiagramViewerModal.test.tsx

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 3 remain after this review.

Walkthrough

The image viewer toggles between fitted and actual-size zoom when the user clicks. Alt-drag pans with a mouse, while touch and pen input can pan by dragging. Pointer movement under four pixels counts as a click. The viewport hook exposes a cursor state, and the image surface maps that state to CSS cursor classes. Tests cover the zoom, cursor, and pan interactions.

Merge Risk: Low · up to 6d011

Multi-contact gestures may jump or stop unexpectedly, and touch or pen drags that return near their starting point may zoom the image. The impact is limited, but the gesture handling should be corrected as follow-up.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Description check Warning The description is thorough and follows the template. It covers ELI5, What Changed (before/after with mechanism and edge cases), Why is implied but not explicit, Validation (automated and manual testi… Add a BEFORE and AFTER visual (screenshot or video) for the click-to-zoom and Alt-drag behavior, or write N/A with a reason if no visual applies. This PR changes UI behavior, so attach the visuals. Replace the empty "Fixes #" with the linke…
Passed Passed checks: 4 passed
Check name Status Explanation
Title check Passed The title names the main change: a click-to-zoom image viewer that toggles fit and actual size, plus Alt-drag panning. The raw summary confirms these changes in the pan/zoom viewport hook, the surface…
Docstring Coverage Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat…
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Title check

Explanation

The title names the main change: a click-to-zoom image viewer that toggles fit and actual size, plus Alt-drag panning. The raw summary confirms these changes in the pan/zoom viewport hook, the surface cursor mapping, and the tests. The title uses the conventional feat(scope) form and is specific enough for a teammate scanning history.

Full details: Docstring Coverage

Explanation

Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 3 functions across 5 files.

Full details: Description check

Explanation

The description is thorough and follows the template. It covers ELI5, What Changed (before/after with mechanism and edge cases), Why is implied but not explicit, Validation (automated and manual testing with specifics), and Notes on Linux Alt-drag behavior. The Visual Proof section is missing: the template requires BEFORE/AFTER visuals for UI behavior changes, and the description has no screenshots or videos. The Linked Issue section is also empty ("Fixes #" with no number), and the template says there should always be one for outside contributors. The Testing checklist boxes are unticked, the Agent skill upstream boundary checkbox is unticked, and the Checklist is absent. The Why section lacks an explicit rationale for choosing this approach over alternatives.

Resolution

Add a BEFORE and AFTER visual (screenshot or video) for the click-to-zoom and Alt-drag behavior, or write N/A with a reason if no visual applies. This PR changes UI behavior, so attach the visuals. Replace the empty "Fixes #" with the linked issue number. Add a Why section that explains the problem and why this approach beats the alternatives. Complete the Testing checkboxes and state which platforms you tested (macOS, Linux, Windows, SSH). Complete the Agent skill upstream boundary checkbox, and add the Checklist section with the items checked or explained.

Finishing Touches
Generate docstrings
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

Nitpick comments: 1
src/renderer/src/components/editor/PanZoomImageDialog.test.tsx: 1

97-130: Trivial · Maintainability & Code Quality · Quick win

Click-to-zoom test does not detect centered zoom.

The new click assertion uses [500, 300]. That point is the center of the 1000×600 fixture canvas. Centered zoom and pointer-anchored zoom both produce translate(-500px, -200px). A regression that centers the zoom while keeping the correct 100% size would still pass this test. The toolbar zoom paths also anchor at the center, so no test shows that the pointer position is used.

Click at an off-center point in the same test, such as one that maps to a distinct image coordinate. Derive the expected transform from the pointer anchor. Keep the zoom-out and Alt-pan assertions on a point that still tests their own behavior.


Review info
Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 1104f5a5-c4d9-41c8-bf7e-fe172bee019f
Commits

Reviewing files that changed from the base of the PR and between 7ed5185 and f69061c.

Files selected for processing: 4
  • src/renderer/src/components/editor/ImageViewer.tsx
  • src/renderer/src/components/editor/PanZoomImageDialog.test.tsx
  • src/renderer/src/components/editor/PanZoomImageSurface.tsx
  • src/renderer/src/components/editor/use-pan-zoom-viewport.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 4 remain after this review.

const point = { x: event.clientX, y: event.clientY }
// Touch and pen have no Alt key, so they keep dragging to pan.
const pan = event.altKey || event.pointerType === 'touch' || event.pointerType === 'pen'
gestureRef.current = { start: point, last: point, pan }

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor · Functional Correctness · Quick win

Supported by static analysis

Script executed:

git diff 018b21789c61c210e8e4e44a0d7656239fd7ce55 f69061c5238a7ea573b850aead61aad33072bb42 -- src/renderer/src/components/editor/use-pan-zoom-viewport.ts
sed -n '215,330p' src/renderer/src/components/editor/use-pan-zoom-viewport.ts

Repository: stablyai/orca

Length of output: 10694


Script executed:

set -o pipefail
printf '%s\n' '--- head handler and type ---'
nl -ba src/renderer/src/components/editor/use-pan-zoom-viewport.ts | sed -n '1,340p'
printf '%s\n' '--- base handler ---'
git show 018b21789c61c210e8e4e44a0d7656239fd7ce55:src/renderer/src/components/editor/use-pan-zoom-viewport.ts | nl -ba | sed -n '1,300p'
printf '%s\n' '--- full PR diff for hook and direct users ---'
git diff --unified=80 018b21789c61c210e8e4e44a0d7656239fd7ce55 f69061c5238a7ea573b850aead61aad33072bb42 -- src/renderer/src/components/editor/use-pan-zoom-viewport.ts src/renderer/src/components/editor src/renderer/src/components | sed -n '1,700p'
printf '%s\n' '--- pointer handler and cursor references ---'
rg -n -F --glob '*.{ts,tsx}' -e 'canvasHandlers' -e 'onPointerDown' -e 'onPointerMove' -e 'onPointerUp' -e 'onPointerCancel' -e 'onLostPointerCapture' -e 'PanZoomViewport' src/renderer/src/components/editor src/renderer/src/components | sed -n '1,500p'
printf '%s\n' '--- relevant tests ---'
rg -n -F --glob '*.{test,spec}.{ts,tsx}' -e 'pointer' -e 'touch' -e 'zoom' src/renderer/src/components/editor src/renderer/src/components | sed -n '1,500p'

Repository: stablyai/orca

Length of output: 42238


Keep the gesture with its primary pointer.

The merge base already allowed mixed pointers to pan or end the shared drag. This change adds click-to-zoom, so a second touch can replace gestureRef.current; the other touch can then end the gesture or trigger the new zoom toggle. Ignore non-primary pointers and accept move, up, cancel, and capture-loss events only when event.pointerId matches the active gesture.

Suggested fix
-type PointerGesture = { start: PanZoomPoint; last: PanZoomPoint; pan: boolean }
+type PointerGesture = {
+  pointerId: number
+  start: PanZoomPoint
+  last: PanZoomPoint
+  pan: boolean
+}
...
-        if (event.button !== 0 || !geometry) {
+        if (event.button !== 0 || !event.isPrimary || !geometry) {
           return
         }
...
-        gestureRef.current = { start: point, last: point, pan }
+        gestureRef.current = { pointerId: event.pointerId, start: point, last: point, pan }
...
-        if (!gesture?.pan || !geometry) {
+        if (!gesture?.pan || gesture.pointerId !== event.pointerId || !geometry) {
           return
         }
...
       onPointerUp: (event) => {
         const gesture = gestureRef.current
+        if (!gesture || gesture.pointerId !== event.pointerId) {
+          return
+        }
         endDrag()
-        if (!gesture || (gesture.pan && event.altKey)) {
+        if (gesture.pan && event.altKey) {
           return
         }
...
-      onPointerCancel: endDrag,
-      onLostPointerCapture: endDrag
+      onPointerCancel: (event) => {
+        if (gestureRef.current?.pointerId === event.pointerId) {
+          endDrag()
+        }
+      },
+      onLostPointerCapture: (event) => {
+        if (gestureRef.current?.pointerId === event.pointerId) {
+          endDrag()
+        }
+      }

Comment on lines +293 to +294
const moved = Math.hypot(event.clientX - gesture.start.x, event.clientY - gesture.start.y)
if (moved < CLICK_SLOP) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor · Functional Correctness · Quick win

Count movement during the whole gesture.

If a touch pans more than four pixels and returns near its start, the press-to-release distance is still below CLICK_SLOP. Releasing then toggles zoom after the pan. Record the maximum distance from gesture.start during pointer moves, and toggle only if that distance stays below the threshold. (raw.githubusercontent.com)

@nwparker
nwparker merged commit 706774d into main Nov 1, 2026
48 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant