fix(Images): do not capture pointer, wheel and touch input when zoom is disabled - #3337
Open
jospoortvliet wants to merge 2 commits into
Open
fix(Images): do not capture pointer, wheel and touch input when zoom is disabled#3337jospoortvliet wants to merge 2 commits into
jospoortvliet wants to merge 2 commits into
Conversation
pointerDown() set dragging = true unconditionally, unlike
updateZoom(), pointerMove() and onDblclick() which all early-return
on !canZoom. Combined with the .dragging { cursor: move } style,
this showed a move cursor on embedded previews implying they could
be panned, even though pointerMove() only pans when zoomRatio > 1,
which can never happen while canZoom is false (the Files widget's
setting). This does not enable zoom or panning; it only removes a
false affordance.
Assisted-by: Claude Sonnet 5:claude-sonnet-5
Signed-off-by: Jos Poortvliet <jospoortvliet@gmail.com>
Follow-up to 6c37fea. Same root cause, two more spots: the component unconditionally claimed a user input, then bailed out in the handler after the input was already consumed. - wheel: @wheel.stop.prevent called preventDefault()/stopPropagation() before updateZoom() ever checked canZoom, so the embedded preview swallowed page scroll even though it can't zoom. Moved the modifiers into updateZoom() itself, after the canZoom guard, so the browser's default scroll runs unless zoom is actually enabled. - touch-action: the img/video rule set touch-action: none unconditionally, which blocks swipe-to-scroll on touch devices the same way the wheel handler blocked mouse scroll. Scoped it to a new &.canZoom class bound to the existing canZoom prop, so it only applies where panning/pinch-zoom is actually available. Also fixed the mixed tabs/spaces left in pointerDown()'s guard from the previous commit. Verified canZoom is true only for the active file in the Viewer modal (src/views/Viewer.vue:145) and false for the embedded/single-file path (:17) and comparison view (:111), so modal zoom/pan/scroll-lock is unchanged; only embedded previews (e.g. the Smart Picker file-link widget in Text) gain back normal page scroll. Assisted-by: Claude Sonnet 5:claude-sonnet-5 Tested in Talk & Text, viewer and embedded. You can now scroll over images ;-) Signed-off-by: Jos Poortvliet <jospoortvliet@gmail.com>
Contributor
|
/compile |
Member
Author
|
So I have to recompile and commit with the assets? Just double-checking, can do but I thought the CI does this part? |
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
2 participants
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
When you share an image with the smart picker in Talk, you can't scroll over it. It also eats drag attempts, showing a pan cursor but not panning anything. This PR removes these. First commit removes the pan, the second takes care of scroll/zoom events.
The pan removal thing is a very simple fix, just adding a !canZoom to the relevant functions. The second is slightly more involved, moving some stuff around.
To be clear, it'd be nicer to be able to add actual zoom/pan, but I'm not sure how feasible that is in that space, in a way that works everywhere. Plus, you might say - instead of zoom, we should open the full viewer on click. Also makes sense if you ask me... So, hum, something to discuss with the design team first.
Below analysis from Claude:
🤖 -----------
Cause
In
Images.vue,pointerDown()setsdragging = trueunconditionally — unlikeupdateZoom(),pointerMove(), andonDblclick(), which all early-return when!canZoom. Combined with the.dragging { cursor: move }style, this shows the pan cursor even thoughpointerMove()only pans whenzoomRatio > 1, which can never happen whilecanZoomis false (what the Files widget passes).Change
Early-return at the top of
pointerDown()when!this.canZoom. The rest of that method only maintains the pinch-zoom pointer cache, which is dead code when zoom is disabled.---------- 😶🌫️
Scroll capture is a little bigger (in second commit, which also fixes my tabs/spaces mixup):
Removed
.stop.preventfrom@wheelon both<img>and<video>, movedevent.stopPropagation()/event.preventDefault()insideupdateZoom()right after thecanZoomguard.And touch scroll capture:
Removed the bare
touch-action: nonefrom the baseimg,videorule, added acanZoomclass to both elements (bound to the existingcanZoomprop) and scoped touch-action: none under&.canZoom.Testing
npm run lintpasses.Screenshot doesn't really make a ton of sense, I mean, it won't show a drag icon if you try to drag and you can scroll...
🤖 AI (if applicable)
Assisted-by: Claude Sonnet 5:claude-sonnet-5