Fix Zoom & pan issues on mobile#4414
Conversation
📝 WalkthroughWalkthrough
ChangesGesture-Based Photo Zoom and Pan
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 1✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
resources/js/components/gallery/photoModule/PhotoBox.vue (1)
143-146: 💤 Low valueModule-level touch tracking variables may retain stale state.
The non-reactive tracking variables (
touchStartX,touchStartY,lastPinchDist,lastTapTime) are declared at module scope and will persist across component unmount/remount cycles. While this likely works fine in practice, encapsulating them in a reactive object or resetting them inonUnmountedwould provide cleaner lifecycle management.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1b7ca20e-4260-45d8-8665-3dd39d1c1865
📒 Files selected for processing (1)
resources/js/components/gallery/photoModule/PhotoBox.vue
| function clampPan(scale: number, x: number, y: number): { x: number; y: number } { | ||
| const el = swipe.value; | ||
| if (!el) return { x, y }; | ||
| // Maximum shift before the image edge passes the container edge | ||
| const maxX = (el.clientWidth * (scale - 1)) / 2; | ||
| const maxY = (el.clientHeight * (scale - 1)) / 2; | ||
| return { x: Math.max(-maxX, Math.min(maxX, x)), y: Math.max(-maxY, Math.min(maxY, y)) }; | ||
| } |
There was a problem hiding this comment.
Pan clamping calculation assumes image fills the container.
The clampPan function calculates pan boundaries using the container dimensions (el.clientWidth, el.clientHeight), but images may be smaller than the container due to max-w-full and max-h-full constraints. This produces incorrect pan limits when the image doesn't fill the viewport.
The correct calculation should use the actual rendered image dimensions. You can query the image element's offsetWidth and offsetHeight after layout, or compute the effective size from the natural dimensions and container constraints.
💡 Suggested approach
function clampPan(scale: number, x: number, y: number): { x: number; y: number } {
- const el = swipe.value;
- if (!el) return { x, y };
- // Maximum shift before the image edge passes the container edge
- const maxX = (el.clientWidth * (scale - 1)) / 2;
- const maxY = (el.clientHeight * (scale - 1)) / 2;
+ const container = swipe.value;
+ const img = container?.querySelector<HTMLImageElement>("`#image`");
+ if (!container || !img) return { x, y };
+ // Use actual rendered image dimensions
+ const imgW = img.offsetWidth;
+ const imgH = img.offsetHeight;
+ const maxX = Math.max(0, (imgW * scale - container.clientWidth) / 2);
+ const maxY = Math.max(0, (imgH * scale - container.clientHeight) / 2);
return { x: Math.max(-maxX, Math.min(maxX, x)), y: Math.max(-maxY, Math.min(maxY, y)) };
}Note: This approach queries the image element each time. For better performance, consider caching the image dimensions in a ref and updating them when the photo changes or on window resize.
Codecov Report✅ All modified and coverable lines are covered by tests. 🚀 New features to boost your workflow:
|
1 participant
Summary by CodeRabbit
Release Notes