Skip to content

Touch editing 447 - #517

Open
Subomiadekunle wants to merge 2 commits into
oss-slu:mainfrom
Subomiadekunle:touch-editing-447
Open

Subomiadekunle wants to merge 2 commits into
oss-slu:mainfrom
Subomiadekunle:touch-editing-447

Conversation

@Subomiadekunle

Copy link
Copy Markdown
Collaborator

Closes #447 (part of #445). Builds on #446. Please review that first; this PR’s diff includes its commit until it’s merged.

What’s new

No page scrolling while editing: a finger drag on a handle, the selected image or annotation, or anywhere while a drawing tool is active edits the scene instead of scrolling. Swiping elsewhere on the canvas still scrolls the page.
Finger-sized handles: on touch screens, resize and point handles (and line hit areas) scale to about 32px across, based on how large the canvas is drawn.
Taps stay taps: a touch must move 8px before it counts as a drag (a mouse still needs 2px), so tapping a selected object doesn’t nudge it.
No long-press callout menu or double-tap zoom on the canvas.

Testing

9 new tests in templates/tests/sceneAnnotationEditor.test.js: tap to select; tap wobble doesn’t move objects; finger move and resize; creating labels, lines and highlights by touch; moving and resizing annotations; a second finger is ignored; the no-scroll markers and styles; handle sizing.
npm test: all suites pass. npm run lint: clean.

Manual check
Chrome device mode (touch emulation) on iPad Air and iPhone 14 Pro, in portrait and landscape: selected, created, moved and resized images and annotations by touch, with no page scrolling during edits. Swiping an empty part of the canvas still scrolls.

@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 touch editing updates. Since the mouse pointer work from PR #516 is now merged, this review is focused on the touch-specific changes in this PR.

Please address the following items:

  • Update the branch with current main
    • Keep the already merged pointer-event changes from PR #516.
    • Preserve the touch-specific logic and tests from this PR.
    • This will make the remaining diff easier to review and merge.
  • Correct the CSS structure in templates/style.css
    • .scene-canvas is opened around lines 2966 and 2969.
    • The first block is not closed before the second one begins.
    • The extra closing brace near line 3007 leaves the touch selectors and @media block nested inside the outer rule.
    • Combine the declarations into one .scene-canvas block and remove the extra brace.
    • This ensures the touch-action, selected-element, handle, and touch target sizing rules apply to the intended elements.
    • The current tests check the CSS text with a regular expression, so they do not detect the browser parsing and selector-scoping problem.
      After updating the branch and correcting the stylesheet, please rerun:
  • npm run lint
  • git diff --check
  • npm test -- --runInBand
    Once these items are addressed, the touch-specific changes should be ready for another review.

@mudabs mudabs mentioned this pull request Nov 3, 2026
@Subomiadekunle Subomiadekunle mentioned this pull request Nov 3, 2026

@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 touch editing updates and the added coverage for tapping, dragging, resizing, drawing, and multi-touch behavior. The tests and lint checks pass, but please address this stylesheet issue before approval:

  • Correct the CSS structure in templates/style.css.
    • .scene-canvas is opened around lines 2966 and 2969.
    • The first block is not closed before the second begins.
    • The extra closing brace near line 3007 incorrectly scopes the touch selectors and @media block.
    • Combine the declarations into one .scene-canvas block and remove the extra brace.
    • This is necessary so touch-action, selected-element behavior, resize handles, and touch target sizing apply correctly in the browser.
    • The current CSS test uses a regular expression, so it does not detect the browser parsing problem.
      After correcting the stylesheet, please rerun:
  • npm run lint
  • git diff --check
  • npm test -- --runInBand
    Local tests currently pass with 18 suites, 777 tests passed, and 5 skipped. Once the CSS structure is corrected, this should be ready for another review.

@Subomiadekunle

Copy link
Copy Markdown
Collaborator Author

Thanks @mudabs, both items are addressed:

Rebased onto current main, dropping the #446 commit since #516 is merged. The test file conflict was only the new #443/#444 blocks and the touch block both appended at the end, so I kept both.
Fixed templates/style.css: removed the leftover unclosed .scene-canvas { opener and the extra closing brace, so the touch-action, handle, and @media (pointer: coarse) rules are top-level again.

npm run lint, git diff --check, and npm test -- --runInBand all pass (22 suites, 867 passed, 5 skipped).

@sonarqubecloud

sonarqubecloud Bot commented Nov 9, 2026

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 Touch Editing

2 participants