Repository navigation
feat(desktop): add pan, pinch, and zoom to the image file pane - #6285
Merged
Merged
Conversation
Contributor
📝 WalkthroughWalkthrough
ChangesInteractive image viewing
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant ImageView
participant usePanZoom
participant ImageSurface
User->>ImageSurface: Wheel, pointer, or keyboard input
ImageSurface->>usePanZoom: Invoke interaction handlers
usePanZoom->>ImageView: Return transform and drag state
ImageView->>ImageSurface: Apply scale and translation
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/hooks/usePaneRegistry/components/FilePane/registry/views/ImageView/ImageView.tsx:
- Around line 94-135: Update the ImageView pan/zoom surface and its related
controls to provide an always-available keyboard-accessible path for zooming,
panning, and resetting the transform. Add documented keyboard handlers or
persistent accessible controls that invoke the existing transform logic, while
preserving pointer behavior and retaining equivalent keyboard support before
removing the noStaticElementInteractions suppression.
- Around line 101-133: Update the ImageView pointer handlers so an active drag
in dragRef cannot be replaced by another pointer: ignore onPointerDown events
while dragRef.current is set, and in onPointerUp and onPointerCancel clear the
drag state only when the ending event’s pointerId matches the active drag’s
pointerId. Preserve existing movement and dragging-state behavior for the
initiating pointer.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: db8c1a1e-63f0-460c-9dba-3d5dee7eb53a
📒 Files selected for processing (1)
apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/FilePane/registry/views/ImageView/ImageView.tsx
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/hooks/usePaneRegistry/components/FilePane/registry/views/ImageView/hooks/usePanZoom/usePanZoom.ts:
- Around line 85-117: Update the usePanZoom drag handlers so only the active
pointer can end the drag: have endDrag receive the pointer event, compare its
pointerId with dragRef.current.pointerId, and ignore mismatches. Ensure
onPointerDown accepts a primary pointer only, preventing additional touches from
replacing the active drag.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: e43f3152-49a0-42ea-a901-04b64761a74b
📒 Files selected for processing (3)
apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/FilePane/registry/views/ImageView/ImageView.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/FilePane/registry/views/ImageView/hooks/usePanZoom/index.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/FilePane/registry/views/ImageView/hooks/usePanZoom/usePanZoom.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/FilePane/registry/views/ImageView/ImageView.tsx
Comment on lines
+85
to
+117
| const endDrag = () => { | ||
| dragRef.current = null; | ||
| setIsDragging(false); | ||
| }; | ||
|
|
||
| const handlers = { | ||
| onPointerDown: (event: ReactPointerEvent<HTMLDivElement>) => { | ||
| if (event.button !== 0) { | ||
| return; | ||
| } | ||
| event.currentTarget.setPointerCapture(event.pointerId); | ||
| dragRef.current = { | ||
| pointerId: event.pointerId, | ||
| startX: event.clientX, | ||
| startY: event.clientY, | ||
| originX: transform.x, | ||
| originY: transform.y, | ||
| }; | ||
| setIsDragging(true); | ||
| }, | ||
| onPointerMove: (event: ReactPointerEvent<HTMLDivElement>) => { | ||
| const drag = dragRef.current; | ||
| if (!drag || drag.pointerId !== event.pointerId) { | ||
| return; | ||
| } | ||
| setTransform((prev) => ({ | ||
| ...prev, | ||
| x: drag.originX + event.clientX - drag.startX, | ||
| y: drag.originY + event.clientY - drag.startY, | ||
| })); | ||
| }, | ||
| onPointerUp: endDrag, | ||
| onPointerCancel: endDrag, |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Keep the active drag when another pointer ends.
onPointerDown accepts every primary-button pointer, and endDrag does not compare pointer IDs. A second touch can replace dragRef.current, then the first touch pointerup can clear the second drag. Accept only the primary pointer and ignore end events from other pointers.
Proposed fix
- const endDrag = () => {
+ const endDrag = (event: ReactPointerEvent<HTMLDivElement>) => {
+ const drag = dragRef.current;
+ if (!drag || drag.pointerId !== event.pointerId) {
+ return;
+ }
dragRef.current = null;
setIsDragging(false);
};
const handlers = {
onPointerDown: (event: ReactPointerEvent<HTMLDivElement>) => {
- if (event.button !== 0) {
+ if (event.button !== 0 || !event.isPrimary) {
return;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const endDrag = () => { | |
| dragRef.current = null; | |
| setIsDragging(false); | |
| }; | |
| const handlers = { | |
| onPointerDown: (event: ReactPointerEvent<HTMLDivElement>) => { | |
| if (event.button !== 0) { | |
| return; | |
| } | |
| event.currentTarget.setPointerCapture(event.pointerId); | |
| dragRef.current = { | |
| pointerId: event.pointerId, | |
| startX: event.clientX, | |
| startY: event.clientY, | |
| originX: transform.x, | |
| originY: transform.y, | |
| }; | |
| setIsDragging(true); | |
| }, | |
| onPointerMove: (event: ReactPointerEvent<HTMLDivElement>) => { | |
| const drag = dragRef.current; | |
| if (!drag || drag.pointerId !== event.pointerId) { | |
| return; | |
| } | |
| setTransform((prev) => ({ | |
| ...prev, | |
| x: drag.originX + event.clientX - drag.startX, | |
| y: drag.originY + event.clientY - drag.startY, | |
| })); | |
| }, | |
| onPointerUp: endDrag, | |
| onPointerCancel: endDrag, | |
| const endDrag = (event: ReactPointerEvent<HTMLDivElement>) => { | |
| const drag = dragRef.current; | |
| if (!drag || drag.pointerId !== event.pointerId) { | |
| return; | |
| } | |
| dragRef.current = null; | |
| setIsDragging(false); | |
| }; | |
| const handlers = { | |
| onPointerDown: (event: ReactPointerEvent<HTMLDivElement>) => { | |
| if (event.button !== 0 || !event.isPrimary) { | |
| return; | |
| } | |
| event.currentTarget.setPointerCapture(event.pointerId); | |
| dragRef.current = { | |
| pointerId: event.pointerId, | |
| startX: event.clientX, | |
| startY: event.clientY, | |
| originX: transform.x, | |
| originY: transform.y, | |
| }; | |
| setIsDragging(true); | |
| }, | |
| onPointerMove: (event: ReactPointerEvent<HTMLDivElement>) => { | |
| const drag = dragRef.current; | |
| if (!drag || drag.pointerId !== event.pointerId) { | |
| return; | |
| } | |
| setTransform((prev) => ({ | |
| ...prev, | |
| x: drag.originX + event.clientX - drag.startX, | |
| y: drag.originY + event.clientY - drag.startY, | |
| })); | |
| }, | |
| onPointerUp: endDrag, | |
| onPointerCancel: endDrag, |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/hooks/usePaneRegistry/components/FilePane/registry/views/ImageView/hooks/usePanZoom/usePanZoom.ts
around lines 85 - 117, Update the usePanZoom drag handlers so only the active
pointer can end the drag: have endDrag receive the pointer event, compare its
pointerId with dragRef.current.pointerId, and ignore mismatches. Ensure
onPointerDown accepts a primary pointer only, preventing additional touches from
replacing the active drag.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
1 issue found across 3 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/FilePane/registry/views/ImageView/hooks/usePanZoom/usePanZoom.ts">
<violation number="1" location="apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/FilePane/registry/views/ImageView/hooks/usePanZoom/usePanZoom.ts:85">
P3: endDrag (used for both onPointerUp and onPointerCancel) clears dragRef.current unconditionally without checking the event's pointerId. If a second pointer (e.g., another touch) starts while dragging, it can replace dragRef.current, and then the pointerup/pointercancel from the original pointer will incorrectly end the second drag. Consider checking event.pointerId against dragRef.current.pointerId in endDrag and requiring event.isPrimary in onPointerDown.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| return () => container.removeEventListener("wheel", handleWheel); | ||
| }, []); | ||
|
|
||
| const endDrag = () => { |
Contributor
There was a problem hiding this comment.
P3: endDrag (used for both onPointerUp and onPointerCancel) clears dragRef.current unconditionally without checking the event's pointerId. If a second pointer (e.g., another touch) starts while dragging, it can replace dragRef.current, and then the pointerup/pointercancel from the original pointer will incorrectly end the second drag. Consider checking event.pointerId against dragRef.current.pointerId in endDrag and requiring event.isPrimary in onPointerDown.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/FilePane/registry/views/ImageView/hooks/usePanZoom/usePanZoom.ts, line 85:
<comment>endDrag (used for both onPointerUp and onPointerCancel) clears dragRef.current unconditionally without checking the event's pointerId. If a second pointer (e.g., another touch) starts while dragging, it can replace dragRef.current, and then the pointerup/pointercancel from the original pointer will incorrectly end the second drag. Consider checking event.pointerId against dragRef.current.pointerId in endDrag and requiring event.isPrimary in onPointerDown.</comment>
<file context>
@@ -0,0 +1,132 @@
+ return () => container.removeEventListener("wheel", handleWheel);
+ }, []);
+
+ const endDrag = () => {
+ dragRef.current = null;
+ setIsDragging(false);
</file context>
Contributor
There was a problem hiding this comment.
1 issue found across 2 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/FilePane/registry/views/ImageView/ImageView.tsx">
<violation number="1" location="apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/FilePane/registry/views/ImageView/ImageView.tsx:35">
P3: role="application" disables normal screen-reader navigation (virtual cursor) inside the pane, so the image's alt text and the reset button stop being announced/reachable through standard reading keys. tabIndex={0} already supplies the focusable element the keyboard pan/zoom handlers need, so the region can typically be labeled without application — consider a more permissive role (or aria-roledescription on the focusable region) so the inner img/button stay accessible, unless full screen-reader key interception is intentional.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| </div> | ||
| <div | ||
| ref={containerRef} | ||
| role="application" |
Contributor
There was a problem hiding this comment.
P3: role="application" disables normal screen-reader navigation (virtual cursor) inside the pane, so the image's alt text and the reset button stop being announced/reachable through standard reading keys. tabIndex={0} already supplies the focusable element the keyboard pan/zoom handlers need, so the region can typically be labeled without application — consider a more permissive role (or aria-roledescription on the focusable region) so the inner img/button stay accessible, unless full screen-reader key interception is intentional.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/FilePane/registry/views/ImageView/ImageView.tsx, line 35:
<comment>role="application" disables normal screen-reader navigation (virtual cursor) inside the pane, so the image's alt text and the reset button stop being announced/reachable through standard reading keys. tabIndex={0} already supplies the focusable element the keyboard pan/zoom handlers need, so the region can typically be labeled without application — consider a more permissive role (or aria-roledescription on the focusable region) so the inner img/button stay accessible, unless full screen-reader key interception is intentional.</comment>
<file context>
@@ -32,7 +32,11 @@ export function ImageView({ document, filePath }: ViewProps) {
<div
ref={containerRef}
- className={`relative flex h-full touch-none items-center justify-center overflow-hidden bg-background p-4 ${
+ role="application"
+ aria-label={`Image preview of ${getBaseName(filePath)}. Zoom with plus and minus, pan with arrow keys, press 0 to reset.`}
+ // biome-ignore lint/a11y/noNoninteractiveTabindex: focus is required for the keyboard pan/zoom handlers
</file context>
Contributor
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
Kitenite
added a commit
that referenced
this pull request
Aug 12, 2026
* docs: weekly update for PRs merged 2026-08-05 to 2026-08-12 - workspaces: sessions (project-less workspaces, #6274), pin + drag-and-drop pinning (#6364), list/board views with Merged & Deleted history columns (#6296), per-project naming instructions (#6250), multi-PR links per branch with remove/restore (#5924) - automations: session automations (#6280), list health stats + Up next grouping + read-only team automations (#6291, #6276), detail page Settings/Run History tabs, inline run errors, would-run preview, RRule validation (#6305) - editor: image pane pan/pinch/zoom (#6285) - tasks: multi-repo filters, expanded task states, PR author/review filters, PR checks (#6201) - monorepos: per-project sparse checkout (#6034) - customization: project icon + color picker, v1 color carry-over (#6249) - terminal presets: built-in Superset CLI preset (#6270) - agents: resume interrupted sessions + Resume Args setting (#6253, #6265), Superset hooks toggle + scoping (#6072) - cli: interactive command browser + branded help (#6273); reference updated for session mode, --task, --skip-branch-prefix, --session (verified against packages/cli/src/commands) * docs: tighten wording and drop em dashes Shorten this week's additions per the writing guide: one thought per sentence, plainest word, no em dashes as prose punctuation. Also sweeps the three pre-existing em dashes in recipes. * docs: add screenshots from the 08-09 changelog batch cli command browser, agent resume banner, PR view with checks, and the project icon/color picker. Copied into apps/docs/public/images per the SOP rather than referenced across apps.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
FilePaneregistryImageView) now supports pinch-to-zoom, scroll/drag panning, and double-click reset, instead of a fixed fit-to-pane image.Why / Context
Images opened in a workspace pane were locked to a fitted view with no way to inspect details (screenshots, diagrams, dense UI captures). This adds standard image-viewer navigation gestures.
How It Works
{scale, x, y}transform is applied to the image wrapper; zoom is anchored at the cursor (zoomAtPoint), clamped to 25%–1600%.wheelevent withctrlKeyset, so ctrl/cmd+wheel zooms. The listener is attached natively as non-passive because React'sonWheelcan'tpreventDefault(needed to stop scroll chaining/page zoom).objectUrlearly return would have left the once-mounted wheel listener unattached).Only the v2 pane registry view is changed; the sunsetting v1
FileViewerPaneis untouched.Manual QA Checklist
Testing
bun run lintbun run typecheck(desktop)Summary by cubic
Adds pan, pinch-to-zoom, keyboard zoom/pan, and quick reset to the desktop image file pane so you can inspect screenshots and diagrams in place. Also extracts a reusable
usePanZoomhook for future viewers.New Features
FilePaneImageViewonly.Bug Fixes
Written for commit 4717c1e. Summary will update on new commits.
Summary by CodeRabbit