feat(Viewer): add zoom/pan and rotate to image viewer - #1756
Conversation
6f33707 to
e9e75ef
Compare
|
You need to sign-off your commits otherwise we can not merge them at all |
e9e75ef to
1fda3da
Compare
|
Fixed the JSDoc lint errors ( |
| @load="handleLoadEnd(false)" | ||
| @error="handleLoadEnd(true)"> | ||
| <ViewerHandlerMedia v-slot="{ handleLoadEnd }"> | ||
| <div class="viewer-image-container" @dblclick.capture="onDoubleClick"> |
There was a problem hiding this comment.
question: Why does it need the capture mode (and preventDefault + stopPropagation in the handler)?
There was a problem hiding this comment.
The .capture modifier is needed because panzoom registers its own dblclick listener on the same element — without capture, both handlers fire and the image zooms twice (once by panzoom's built-in handler, once by ours). Intercepting in the capture phase lets us handle it first and call stopPropagation to prevent panzoom's handler from running.
preventDefault suppresses the browser's default double-click text selection behavior. stopPropagation also prevents the event from bubbling up to the parent Viewer, which closes on double-click.
There was a problem hiding this comment.
I see. Indeed, panzoom doesn't allow customizing the double-click behavior directly via the panzoom options...
One nitpick: preventDefault and stopPropagation can also be set via Vue like capture mode is, to not mix Vue and JS ways for the same event:
@dblclick.capture.stop.prevent="onDoubleClick"
There was a problem hiding this comment.
issue(blocking): currently it is possible to move the image even on scale: 1, moving it almost completely away:
| Initial | Moved |
|---|---|
 |
 |
It should only be movable on the axis, if the image on this axis eexceeds its container limits.
There was a problem hiding this comment.
It is fixed now for scale: 1, but zoomed it is possible to move the image completely out of the viewport.
Install panzoom library (9.4.4, 6 kB) and apply it to the image viewer wrapper in ViewerHandlerImages.vue: - Scroll wheel to zoom in/out (1× – 8×) - Drag to pan when zoomed in - Double-click to zoom in 3× at cursor, double-click again to reset - Auto-center image when zoom returns to 1× - Pan blocked at minimum zoom to prevent accidental shifts - Cursor changes: zoom-in at 1×, grab/grabbing when zoomed Fixes nextcloud#1535 Signed-off-by: Vladimir Poluliashenko <vladopol@gmail.com> Assisted-by: ClaudeCode:claude-sonnet-4-6
Signed-off-by: Vladimir Poluliashenko <vladopol@gmail.com> Assisted-by: ClaudeCode:claude-sonnet-4-6
…lement Signed-off-by: Vladimir Poluliashenko <vladopol@gmail.com> Assisted-by: ClaudeCode:claude-sonnet-4-6
- Use useTemplateRef API (Vue 3.5+) for panzoom wrapper ref - Use plain variable for panzoom instance (no reactive state needed) - Remove unnecessary null guard in initPanzoom (wrapperRef is always defined when called from onImageLoad) - Add comment explaining dblclick.capture usage Signed-off-by: Vladimir Poluliashenko <vladopol@gmail.com> Assisted-by: ClaudeCode:claude-sonnet-4-6
4ec6e63 to
6d3ea60
Compare
Replace panzoom's built-in bounds option (ineffective when the panzoom element fills its parent 100%) with manual clamping via getBoundingClientRect. clampToBounds() is called on panend and after each zoom to smoothly return the image within the viewport if needed. Signed-off-by: Vladimir Poluliashenko <vladopol@gmail.com> Assisted-by: ClaudeCode:claude-sonnet-4-6
|
There is still the same problem for me: Recording.2026-06-18.165650.mp4 |
- Clamp on every pan event (not just panend) for immediate stopping - Disable smoothScroll (inertia) so the image halts on release without overshooting the boundary Signed-off-by: Vladimir Poluliashenko <vladopol@gmail.com> Assisted-by: ClaudeCode:claude-sonnet-4-6
|
Pan just gets stuck when the original image (not the container) is reached. Recording.2026-06-18.173040.mp4 |
|
Also, zooming out with double-click may result in the image being completely disappeared depending on the double-click position... |
|
Hello there, We hope that the review process is going smooth and is helpful for you. We want to ensure your pull request is reviewed to your satisfaction. If you have a moment, our community management team would very much appreciate your feedback on your experience with this PR review process. Your feedback is valuable to us as we continuously strive to improve our community developer experience. Please take a moment to complete our short survey by clicking on the following link: https://cloud.nextcloud.com/apps/forms/s/i9Ago4EQRZ7TWxjfmeEpPkf6 Thank you for contributing to Nextcloud and we hope to hear from you soon! (If you believe you should not receive this message, you can add yourself to the blocklist.) |
|
After zooming out on the second double click, the image is stuck in some uneven position, and neither pan nor zoom works again. 
|
smoothZoomAbs(x, y, 0) scaled the image to zero, making it disappear. Using ZOOM_MIN (1) correctly returns the image to its original size. Signed-off-by: Vladimir Poluliashenko <vladopol@gmail.com> Assisted-by: ClaudeCode:claude-sonnet-4-6
de808a3 to
88db0c4
Compare
Add rotate left (−90°) and rotate right (+90°) buttons to the image viewer actions menu alongside the existing "Open in web browser" action. Rotation state and controls are fully self-contained in ViewerHandlerImages: - rotateLeft/rotateRight adjust rotation in 90° steps using unbounded angle increments so the CSS transition always animates in the correct direction - Rotation resets to 0 when switching images - ViewerHandlerImages exposes an actions array; ViewerApp renders it generically via component :is — no image-specific code in ViewerApp Fixes nextcloud#1757 Signed-off-by: Vladimir Poluliashenko <vladopol@gmail.com> Assisted-by: ClaudeCode:claude-sonnet-4-6
vladopol
changed the title
| instance.on('pan', () => { | ||
| clampToBounds() | ||
| }) |
There was a problem hiding this comment.
This triggers infinite loop after 1-2 user drag interactions only for me?
function clampToBounds() looks like it tries to replicate native panzoom options bounds: true and boundsPadding: 1, why wouldn't use them directly?
|
Hi @vladopol, After several rounds of reviews, the implemented feature doesn't work correctly in its basic functionality. The issues could have been found with quick manual testing. This suggests the changes were AI-generated and submitted without being verified first. This violates our AI policy in author accountability and code quality requirement. Therefore, the PR will be closed according to the maintainer's discretion. Thanks for your interest in contributing to our project. Feel free to open PR again when the feature is ready and tested. Or we may implement it later ourselves. |
|
Reworked and resubmitted as #1812. The review findings were valid. Details, the fix, and how each of your scenarios is now covered by automated and manual tests are in the new PR. Thanks for the thorough review and the feedback comments — they're really valuable. |
4 participants
Summary
Adds zoom/pan and rotate controls to the image viewer in Talk Desktop.
Zoom and pan
Installs
panzoom(v9.4.4, 6 kB) — same library already used in spreed for screen share zoom (spreed#14028).zoom-inat 1×,grab/grabbingwhen zoomedAddresses the macOS limitation where pinch-to-zoom had no effect and Cmd+/- zoomed the entire application UI instead of the image.
Closes #1535
Rotate
Adds rotate left (−90°) and rotate right (+90°) buttons to the actions menu.
ViewerHandlerImagesViewerHandlerImagesexposes anactionsarray;ViewerApprenders it generically viacomponent :is— no image-specific code leaks into the generic componentCloses #1757
Architecture
Handler components can expose an
actionsarray to contribute entries to the viewer's action menu without modifyingViewerApp. Each action is{ key, label, icon, onClick }. This keepsViewerAppgeneric and all feature-specific logic inside the handler.Test plan
Assisted-by: ClaudeCode:claude-sonnet-4-6