Skip to content

Stylus editing 448 - #518

Open
Subomiadekunle wants to merge 4 commits into
oss-slu:mainfrom
Subomiadekunle:stylus-editing-448
Open

Subomiadekunle wants to merge 4 commits into
oss-slu:mainfrom
Subomiadekunle:stylus-editing-448

Conversation

@Subomiadekunle

Copy link
Copy Markdown
Collaborator

Closes #448 (part of #445). Builds on #446 and #447. Please review those first; this PR’s diff includes their commits until they’re merged.

What’s new
A pen already works through the pointer events added in #446, so it can select, move, resize, crop and draw like a mouse or finger. This PR closes the gaps where a pen would otherwise behave differently:

Palm rejection: while a pen (or any pointer) is dragging, another press, such as a palm on the screen, doesn’t take over the drag, and its tap doesn’t change the selection.
No long-press menu: a long press with a pen or finger no longer opens the right-click menu on the canvas. A mouse right-click keeps its menu.

Intentional differences

The pen’s eraser end and barrel button don’t edit, the same as a mouse’s right and middle buttons.
A pen uses the mouse’s 2px drag threshold, since pens are precise. Touch keeps its 8px threshold from #447.

Testing

7 new tests in templates/tests/sceneAnnotationEditor.test.js, including one run three times (mouse, touch, pen) that checks all three give the same result for select, move, resize and draw.
npm test: all suites pass. npm run lint: clean.
Manual check
No stylus device was available, so pen input was covered by the automated tests.

@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 stylus editing updates. The added coverage for pointer, touch, and pen interactions is helpful, and the automated tests are passing. Please address the following items:

  • Update the branch with current main
    • Keep the already merged pointer-event changes from PR #516.
    • Preserve the touch changes from PR #517 and the stylus-specific changes from this PR.
  • Correct the CSS block 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 causes the touch selectors and @media block to be scoped incorrectly.
    • Combine the declarations into one .scene-canvas block and remove the extra brace.
    • This is important because the touch-action, selected-element, handle, and stylus interaction rules may not apply to the intended elements.
  • Remove trailing whitespace in templates/js/scenes.js
    • Clean up the whitespace around line 2368 so git diff --check passes.
  • Run the validation again
    • npm run lint
    • git diff --check
    • npm test -- --runInBand
      After updating the branch and correcting these issues, the PR should be ready for another review. Thank you for the thoughtful coverage of the different input types.

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

sonarqubecloud Bot commented Nov 9, 2026

Copy link
Copy Markdown

@Subomiadekunle

Copy link
Copy Markdown
Collaborator Author

Thanks @mudabs, addressed:

Rebased onto the updated #517 branch, which is rebased on current main. The #446 commit is dropped, and the diff still includes the #447 commit until #517 merges.
The .scene-canvas CSS fix comes from #517: one closed block, with the touch rules and @media at the top level.
Removed the trailing whitespace in templates/js/scenes.js.

Lint, git diff --check upstream/main...HEAD, and npm test -- --runInBand all pass (22 suites, 874 passed, 5 skipped).

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

2 participants