Skip to content

Responsive 450 - #520

Open
Subomiadekunle wants to merge 7 commits into
oss-slu:mainfrom
Subomiadekunle:responsive-450
Open

Subomiadekunle wants to merge 7 commits into
oss-slu:mainfrom
Subomiadekunle:responsive-450

Conversation

@Subomiadekunle

Copy link
Copy Markdown
Collaborator

Closes #450 (part of #445). Builds on #519 (and the PRs it includes). Please review that first; this PR’s diff includes its commits until it’s merged.

What’s new
When the editor changes size (a resized window, a rotated phone or tablet):

Objects stay aligned. Images and annotations keep their scene positions and are drawn in one SVG that scales as a whole, so labels, lines and highlights stay on the same spots of the image.
The view stays put. When zoomed, the same part of the scene stays in the middle of the view instead of jumping elsewhere.
A drag in progress ends where it is. Its screen-to-scene scale belonged to the old size, so continuing would have moved the object the wrong distance.
Handles stay touch-sized for the new canvas size.

Narrow layouts

Less padding around the editor on screens up to 768px wide, so the canvas gets more of the width (303px instead of 271px on a 375px phone).
The “Ctrl + scroll…” hint is hidden on touch screens.

Testing

6 new tests in templates/tests/sceneAnnotationEditor.test.js: the scene drawn once, aligned and unchanged at narrow and wide sizes; handle sizes updating; the zoomed view keeping its center; a drag ending on resize; a drag continuing when only the height changes; the narrow-screen styles.
npm test -- --runInBand: all 22 suites pass. Lint and git diff --check: clean.

@mudabs mudabs left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @Subomiadekunle for the responsive resizing updates and the added coverage for narrow layouts, zoomed views, touch targets, and interrupted drags. The implementation is well tested, but please address this issue before approval:

  • Preserve the viewport when a line or arrow is being drawn during a resize.
    • In templates/js/scenes.js, handleCanvasResize() captures the SVG around lines 682–688 before calling activeDrag.finish().
    • Finishing a line or arrow drag calls addAnnotation(), which rerenders the workspace and replaces the SVG.
    • The resize handler then uses the detached SVG reference with the updated viewCenter.
    • In the browser, the detached SVG may report a zero-sized bounding rectangle, causing the scroll correction to calculate an incorrect position and make the canvas jump.
    • Please save the current view center before finishing the drag, then reacquire the current SVG after activeDrag.finish() before applying the scroll correction.
  • Add a regression test in templates/tests/sceneAnnotationEditor.test.js.
    • Start drawing a line or arrow.
    • Resize the canvas before releasing the drag.
    • Confirm the annotation is finalized and the same scene region remains visible.
      The local validation is currently clean:
  • npm run lint
  • git diff --check
  • npm test -- --runInBand
  • 22 test suites passed
  • 854 tests passed, 5 skipped

@sonarqubecloud

Copy link
Copy Markdown

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Small Rock] Support Responsive Resizing

2 participants