Repository navigation
Zoom pan 449 - #519
Zoom pan 449#519Subomiadekunle wants to merge 5 commits into
Conversation
There was a problem hiding this comment.
Thank you @Subomiadekunle for the pointer, touch, stylus, and zoom/pan updates. The tests and lint checks pass, but please address the following before approval:
- Update the branch with current main
- Keep the merged pointer-event changes from PR #516.
- Preserve the touch and stylus changes while retaining the zoom and pan work from this PR.
- Fix templates/style.css
- Remove the duplicate .scene-canvas opening around lines 2966 to 2969.
- Remove the extra closing brace near line 3008.
- Keep the touch selectors and @media block at the correct level so touch, stylus, pan, and pinch-zoom behavior work reliably.
- Clean up templates/js/scenes.js
- Remove the trailing whitespace around line 2456 so git diff --check passes.
- Re-run validation
- npm run lint
- git diff --check
- npx jest --runInBand templates/tests/sceneAnnotationEditor.test.js templates/tests/sceneCanvas.test.js templates/tests/sceneImport.test.js
- npm test -- --runInBand
Once the branch is updated and these items are corrected, this should be ready for another review.
|
Thanks @mudabs! Addressed: Updated with current main. Merged main (including #516 and #512) with no conflicts. The pointer-event changes from #516 are kept, along with the touch, stylus, zoom and pan work. Fixed templates/style.css. Removed the duplicate .scene-canvas opening and the extra closing brace, so the touch selectors and the @media (pointer: coarse) block are back at the top level. I confirmed in Chrome that the canvas’s touch-action now applies (pan-x pan-y); before the fix it was being ignored. Cleaned up templates/js/scenes.js. The same CSS mistake is in #517 and #518, since #519 builds on them. Would you prefer I fix it on those branches too, or should I close #517 and #518 so you can review and merge #519, which includes all three? |
mudabs
left a comment
There was a problem hiding this comment.
Thank you @Subomiadekunle for addressing the earlier CSS and whitespace feedback and for the zoom, pan, touch, stylus, and pointer-event updates. The latest head passes validation, but please address this remaining issue before approval:
- Restore keyboard focus to #scene-canvas after selection.
- In templates/js/scenes.js around lines 2630–2648, selecting or deselecting an image or annotation rerenders the canvas but does not call dom.canvas.focus().
- The keyboard handler is attached to the canvas around lines 2665–2686.
- Since the SVG child elements are not focusable, keyboard zoom controls and annotation actions such as arrow-key movement and Delete may not work after a mouse or touch selection.
- Focus the canvas after the selection update and add a regression test that selects an object, then verifies keyboard zoom or annotation editing works.
Local validation passed:
- npm run lint
- git diff --check
- npm test -- --runInBand
- 22 suites passed, 848 tests passed, 5 skipped
Once focus is restored after selection, this should be ready for another review.
0b3e408 to
abd457e
Compare
|
|
Thanks @mudabs! After rebasing onto current main (via the updated #518 branch), the canvas click handler now focuses #scene-canvas (dom.canvas.focus({ preventScroll: true })) whenever a pointer press selects or deselects an object, so the arrow keys, Delete and the +/−/0 zoom keys work right away. I added a regression test, run with mouse, touch and pen, that selects an image and nudges it, selects and deletes a label, then deselects and zooms with +. The rebase also replaced the earlier merge commit, and the CSS stays as one closed .scene-canvas block. npm run lint, git diff --check upstream/main...HEAD and npm test -- --runInBand all pass. |



Closes #449 (part of #445). Builds on #446, #447 and #448. Please review those first; this PR’s diff includes their commits until they’re merged.
What’s new
Zoom from 25% to 800%: the − / + buttons with the zoom level shown, Ctrl + scroll wheel (or a trackpad pinch) around the pointer, a two-finger pinch on touch screens, and the +, − and 0 keys on the focused canvas.
Pan with the scrollbars, scroll wheel or a one-finger swipe as before, and now by dragging empty space with a mouse or pen.
Reset view returns to 100%, scrolled to the top left. Opening another scene also starts at the default view, and switching between fit-to-width and actual size keeps the zoom.
Alignment
Zoom only changes how large the whole drawing is shown. Images, highlights, lines and labels are drawn together in one SVG and keep their stored scene coordinates. Edits convert screen movement through the drawn size, so they land under the pointer at any zoom and pan.
Zoom and pan stay distinct from moving objects
A press either edits (a handle, the selected object, or a drawing tool) or pans, never both. A finger already moving an object isn’t turned into a pinch. Zooming and panning never mark the scene as changed.
Testing
9 new tests in templates/tests/sceneAnnotationEditor.test.js: buttons and limits; reset; images, highlights, lines and labels unchanged and aligned while zoomed; moving and drawing in scene units while zoomed and panned; Ctrl+wheel zooming around the pointer; keyboard zoom; panning by dragging empty space without selecting or moving anything; a selected object moving instead of panning; a two-finger pinch; fit-to-width; resetting for another scene.
npm test: all suites pass. npm run lint: clean.
Manual check
In a scene with an image, highlight, line and label: zoomed with the buttons and Ctrl+scroll, panned by dragging, moved an image while zoomed, and reset the view. mode. Pinch was covered by the automated tests.