Skip to content

slides: mount editor on mobile + Pointer Events migration#261

Merged
hackerwins merged 8 commits into
mainfrom
slides/mobile-edit-pointer-events
May 18, 2026
Merged

slides: mount editor on mobile + Pointer Events migration#261
hackerwins merged 8 commits into
mainfrom
slides/mobile-edit-pointer-events

Conversation

@hackerwins

@hackerwins hackerwins commented May 18, 2026

Copy link
Copy Markdown
Collaborator

Summary

Phase B Task 1a + 1b of Slides Mobile Light Edit. Splits cleanly into two cohesive halves:

Task 1a — Pointer Events migration (slides package only)

  • Mechanical rename of mouse* event listeners to pointer* across the slides editor module (editor.ts, thumbnail-panel.ts, context-menu.ts, layout-picker.ts). 5 test files updated to dispatch PointerEvent instead of MouseEvent.
  • PointerEvent is a strict superset of MouseEvent (it inherits from it in TS); browsers synthesize pointer events from both mouse and touch input. Desktop behavior is unchanged.
  • Prerequisite for touch drag on iOS Safari, which never synthesizes mousemove during a touch drag (only mousedown/mouseup for taps). Pen tablets and stylus input get supported as a side-effect.

Task 1b — Mount full SlidesEditor on mobile

  • MobileSlidesView gains a mode: 'view' | 'edit' prop. 'view' keeps Phase A's read-only SlideRenderer with swipe nav. 'edit' (default) builds canvas + overlay and mounts the desktop SlidesEditor.
  • New touchHandleTolerance?: number option on SlidesEditor. handleHitTest now expands hit rectangles by that many pixels on each side AND switches to closest-center disambiguation when expanded rects overlap (fingers on small selections). Desktop default is 0; mobile passes 22 (≈ 44px Apple HIG min).
  • Two iOS-specific CSS guards on canvas-host: touch-action: none (canvas owns horizontal pointer — no browser swipe/pinch competition) and -webkit-touch-callout: none + user-select: none (kills long-press preview that contextmenu blocking doesn't cover).
  • Mutations flow through the existing SlidesStore, so Yorkie sync and undo/redo are inherited from desktop with zero new mutation surface.

Permission-gated 'view' fallback for shared-link viewers is Task 4 (separate PR). Bottom-sheet text formatting + slide-ops FAB are Tasks 2-3.

Test plan

  • pnpm verify:fast green (792 slides tests + 2 new hit-test tolerance/overlap tests).
  • Desktop regression smoke (puppeteer 1400×900): select via dispatched PointerEvent, drag a shape 100px, verify nw handle moved and screenshot confirms element followed.
  • iPhone 16 Pro simulator (Safari): user-driven matrix — tap-select ✅, drag-move ✅, drag-resize ✅, drag-rotate ✅, long-press callout suppressed ✅, double-tap text-edit ✅, blank-tap clear ✅. Korean IME ❌ — deferred to Task 2 (text-edit-focused PR) since it needs docs-side IME work.
  • Real device (physical iPhone) pass for Korean IME and pinch behavior — deferred to Task 2.
  • Android Chrome touch pass — deferred to Task 1b real-device follow-up.

Self-review fixes already applied

Pre-push review (via superpowers:requesting-code-review) caught and fixed:

  • Critical: mobile edit-mode cleanup now calls editor.detach() to match slides-view.tsx:479 — was leaking document-level pointer/key listeners per mount cycle.
  • Important: onCurrentSlideChange callback uses functional setState to avoid stale-closure compare against currentSlideId.

Deferred with rationale (logged in lessons):

  • Global [data-handle] style selector is a pre-existing desktop pattern (slides-view.tsx:301-303); follow-up scopes both.
  • jsdom PointerEvent.pointerId defaults to 0 vs real-browser 1+ — only matters if a future PR adds setPointerCapture.

Design + planning

  • Design doc: docs/design/slides/slides-mobile-edit.md
  • Todo: docs/tasks/active/20260517-slides-mobile-edit-todo.md (Tasks 0/1a/1b done; 2-4 follow as separate PRs)
  • Lessons: docs/tasks/active/20260517-slides-mobile-edit-lessons.md

Summary by CodeRabbit

  • New Features

    • Added mobile editor mode to replace read-only viewing with interactive editing on touch devices.
  • Improvements

    • Migrated to Pointer Events for improved touch interaction and drag reliability on mobile.
    • Enhanced handle hit-testing with tolerance adjustment for better touch accuracy.
  • Documentation

    • Added design and implementation documentation for mobile editing features.

Review Change Stack

Spike on the iPhone 16 Pro simulator (Task 0) confirmed the
SlidesEditor mounts cleanly on the mobile shell but that iOS
Safari does not synthesize mousemove during a touch drag —
selection works on tap, but drag/resize/rotate never receive
move events. Long-press callout also isn't a contextmenu event,
so the spike's onContextMenu suppression was a no-op.

Resolution chosen at the gate: split Task 1 into 1a (migrate
the slides editor from Mouse Events to Pointer Events — strict
superset, no state-machine change, desktop unaffected) and 1b
(the mobile-mount work plus -webkit-touch-callout: none CSS).
Pen tablet / stylus input becomes supported as a side-effect.

Lands the design doc, multi-PR todo, and lessons file with the
matrix that drove the decision. No code changes yet.
Replaces every mousedown/mousemove/mouseup/mouseenter/mouseleave
listener in the slides editor module with the pointer-event
equivalent. PointerEvent is a strict superset of MouseEvent (it
inherits from it in TS), and browsers synthesize pointer events
from both mouse and touch — so this rename costs nothing on
desktop but is the prerequisite for touch drag on iOS Safari,
which never synthesizes mousemove during a touch drag (only
mousedown/mouseup for taps). Pen tablets and stylus input get
supported as a side-effect.

Scope is the slides editor module only (editor.ts, thumbnail-
panel.ts, context-menu.ts, layout-picker.ts). The text-box-editor
delegates click handling to the docs TextEditor, which still uses
mouse events; tests that target it keep MouseEvent dispatches.
The presenter module (read-only playback) is also out of scope.

Phase B spike findings that drove this:
docs/tasks/active/20260517-slides-mobile-edit-lessons.md
The mobile shell needs fingertips to land on 8px handles reliably.
Add a `tolerance` parameter to `handleHitTest` (default 0 — desktop
unchanged) that expands each handle's hit rectangle by that many
pixels on every side. `SlidesEditor` accepts the matching
`touchHandleTolerance` option and forwards it to every hit-test
call.

22px expansion around 9 handles on a small selection makes their
hit rectangles overlap, so a flat first-match-wins iteration picked
by DOM order and felt non-deterministic from a fingertip. Switch
to closest-center: among all handles whose expanded rect contains
the point, pick the one whose center is nearest. Two new unit tests
cover the overlap path and confirm desktop tolerance=0 behavior
when only one rect matches stays equivalent.
Adds a `mode: 'view' | 'edit'` prop to MobileSlidesView, defaulting
to 'edit'. The 'view' branch keeps Phase A's read-only SlideRenderer
path with swipe nav. The 'edit' branch builds a canvas + overlay
and mounts the desktop SlidesEditor against them with
touchHandleTolerance=22 forwarded — touch drag/resize/rotate work
out of the box via the Pointer Events migration in the previous
commit (one-back).

Two iOS-specific guards on the canvas-host: touch-action:none
gives the editor sole ownership of horizontal pointer (no
competition from browser swipe nav, double-tap zoom, pinch);
-webkit-touch-callout + user-select kill the long-press callout
preview that contextmenu blocking does NOT cover. slides-detail
passes mode="edit" explicitly. Permission-gated 'view' fallback
for shared-link viewers is the Task 4 wiring.

Phase B Task 1b — see
docs/tasks/active/20260517-slides-mobile-edit-todo.md.
Three findings from the sim smoke pass: (1) flat 22px hit tolerance
around 9 close-packed handles needed closest-center disambiguation
to feel deterministic — fix in the previous slides commit. (2) iOS
Hangul IME composition in the docs text-box mounted from slides
behaves differently from desktop; deferred to Task 2 (text-edit
PR) since that's where docs-side IME work belongs. (3) The
_spike-login.html cookie installer is the repeatable trick for
dropping the iPhone sim into an authenticated session — local-only,
never committed, recreate per spike.
Self-review caught two issues in the new edit-mode mount:

1. **Listener leak (Critical):** the cleanup dropped editorRef but
   never called editor.detach(), so every mount cycle leaked the
   editor's document-level pointer/key listeners, any in-flight RAF,
   and an active text-box. Desktop slides-view.tsx:479 already
   tears down the same way — mobile now matches.

2. **Stale-closure setState (Important):** the onCurrentSlideChange
   callback compared the editor's reported slide id against
   currentSlideId captured in closure at mount time, which never
   refreshed because the effect intentionally omits currentSlideId
   from its dep list (so footer-arrow taps don't tear down the
   editor). Switch to the prev-state reducer form so we always
   compare against the latest React state.
Code review surfaced two non-blocking items kept out of this PR
but worth recording for the next person:

- The `[data-handle] { pointer-events: auto !important }` style is
  injected as a global selector by BOTH desktop (slides-view.tsx)
  and mobile (mobile-slides-view.tsx). The mobile mount inherits
  the desktop pattern verbatim; isMobile branches so they never
  fight. A follow-up should scope both. Logged in lessons.
- jsdom defaults PointerEvent's pointerId to 0; real browsers use
  1+. Not a present bug because the editor doesn't read pointerId,
  but a trap for any future PR that adds setPointerCapture. Logged.

Also drops a confusing tie-break paragraph in the closest-center
overlap test — the test uses biased points that make the tie
analysis irrelevant.
@coderabbitai

coderabbitai Bot commented May 18, 2026

Copy link
Copy Markdown
📝 Walkthrough

Walkthrough

This PR implements Phase B of mobile-editing support by establishing Pointer Events touch handling, improving touch-target hit-testing, and mounting the desktop SlidesEditor on mobile in an optional edit mode, with comprehensive design documentation and task sequencing for multi-PR rollout.

Changes

Mobile Light Edit (Phase B)

Layer / File(s) Summary
Phase B Design & Planning Documentation
docs/design/README.md, docs/design/slides/slides-mobile-edit.md, docs/tasks/README.md, docs/tasks/active/20260517-slides-mobile-edit-*.md
Design document describes architecture shift from renderer-only to editor-backed mobile view with Pointer Events touch handling, text-edit bottom-sheet, FAB slide-ops, and undo/redo. Task docs detail multi-PR sequencing (Tasks 0–4), gate decisions splitting Pointer Events migration (1a) from editor mount (1b), implementation observations (hit-test tolerance overlap resolution, IME deferral, style-injection patterns), and per-PR checklists.
Pointer Events Migration & Hit-Test Tolerance
packages/slides/src/view/editor/editor.ts, packages/slides/src/view/editor/hit-test.ts
Editor adds touchHandleTolerance option and migrates drag-interaction listeners (shape/connector insert, lasso, element/adjustment/rotation/resizing) from mousemove/mouseup to pointermove/pointerup. handleHitTest now accepts tolerance parameter, expands hit rectangles on all sides, and selects the candidate handle with closest center instead of first DOM match; JSDoc updated accordingly.
Pointer Events in UI Components
packages/slides/src/view/editor/context-menu.ts, packages/slides/src/view/editor/layout-picker.ts, packages/slides/src/view/editor/thumbnail-panel.ts
Context menu, layout picker, and thumbnail panel consistently switched from mouseenter/mouseleave/mousedown to pointerenter/pointerleave/pointerdown for hover feedback, outside-click dismissal, and thumbnail selection to support touch input.
Mobile Editor Mount & Mode Integration
packages/frontend/src/app/slides/mobile-slides-view.tsx, packages/frontend/src/app/slides/slides-detail.tsx
MobileSlidesView adds optional mode: 'view' | 'edit' prop (default 'edit'). In edit mode, mounts SlidesEditor via initializeEditor with touchHandleTolerance, injects CSS to re-enable pointer events on handles, syncs slide changes bidirectionally, disables browser touch gestures and iOS callouts. View mode preserves existing SlideRenderer read-only behavior. SlidesLayout explicitly passes mode="edit".
Test Updates for Pointer Events & Hit-Test
packages/slides/test/view/editor/*.test.ts
All editor, context-menu, layout-picker, thumbnail-panel, and hit-test test suites updated: dispatchMouseDown helper now emits pointerdown; drag sequences dispatch pointermove/pointerup instead of mousemove/mouseup; hit-test tests added for tolerance expansion, closest-center selection, and overlapping handle scenarios; test expectations adjusted for pointer-event event types across all interaction flows.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • wafflebase/wafflebase#255: Extends the existing mobile MobileSlidesView read-only implementation by adding edit mode and mounting SlidesEditor with pointer-event touch handling.
  • wafflebase/wafflebase#239: Modifies SlidesEditor insert/drag logic in editor.ts (this PR migrates to pointer events and adds touch hit-tolerance; related PR refactors insert-mode sizing/Escape cancel).
  • wafflebase/wafflebase#198: Updates showLayoutPicker outside-click and hover listeners (this PR switches to pointer events; related PR changes close semantics and trigger handling).

Poem

🐰 A mobile canvas dreams of touch,
Pointer events migrate and such!
Hit-test tolerance, closest or near,
SlidesEditor mounts—edit mode's here!
From Mouse to Pointer, drag flows true,
Phase B lands, and iOS works too! ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately and concisely summarizes the two main changes: Pointer Events migration and mounting the editor on mobile (Task 1a and 1b of Phase B).
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch slides/mobile-edit-pointer-events

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions

github-actions Bot commented May 18, 2026

Copy link
Copy Markdown
Contributor

Verification: verify:self

Result: ✅ PASS in 215.6s

Lane Status Duration
sheets:build ✅ pass 13.7s
docs:build ✅ pass 12.8s
slides:build ✅ pass 14.0s
verify:fast ✅ pass 126.4s
frontend:build ✅ pass 20.0s
verify:frontend:chunks ✅ pass 0.3s
backend:build ✅ pass 5.0s
cli:build ✅ pass 2.1s
verify:entropy ✅ pass 21.2s

Verification: verify:integration

Result: ✅ PASS

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@docs/design/slides/slides-mobile-edit.md`:
- Around line 79-88: The fenced code blocks that contain the ASCII diagrams (the
block showing "before (Phase A) ... after (Phase B)" and the later similar
block) are missing language identifiers; update each opening triple-backtick to
include a language tag (e.g., ```text) so markdownlint MD040 is satisfied and
the docs remain lint-clean.

In `@docs/tasks/active/20260517-slides-mobile-edit-todo.md`:
- Around line 145-147: The grep in the checklist only scans packages/slides/src
and can miss test files; update the command used in the doc (the grep invocation
that searches for new MouseEvent or dispatchEvent.*mouse) to search repo-wide or
include both packages/slides/src and packages/slides/test (for example by
removing the directory argument to search the repo or adding --include for both
paths) so test-event checks catch real test files.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 2126e311-7680-4628-a388-e94f94fa0467

📥 Commits

Reviewing files that changed from the base of the PR and between 3519485 and ef9c41f.

📒 Files selected for processing (18)
  • docs/design/README.md
  • docs/design/slides/slides-mobile-edit.md
  • docs/tasks/README.md
  • docs/tasks/active/20260517-slides-mobile-edit-lessons.md
  • docs/tasks/active/20260517-slides-mobile-edit-todo.md
  • packages/frontend/src/app/slides/mobile-slides-view.tsx
  • packages/frontend/src/app/slides/slides-detail.tsx
  • packages/slides/src/view/editor/context-menu.ts
  • packages/slides/src/view/editor/editor.ts
  • packages/slides/src/view/editor/hit-test.ts
  • packages/slides/src/view/editor/layout-picker.ts
  • packages/slides/src/view/editor/thumbnail-panel.ts
  • packages/slides/test/view/editor/context-menu.test.ts
  • packages/slides/test/view/editor/editor.test.ts
  • packages/slides/test/view/editor/hit-test.test.ts
  • packages/slides/test/view/editor/layout-picker.test.ts
  • packages/slides/test/view/editor/text-box-editor.test.ts
  • packages/slides/test/view/editor/thumbnail-panel.test.ts

Comment thread docs/design/slides/slides-mobile-edit.md Outdated
Comment thread docs/tasks/active/20260517-slides-mobile-edit-todo.md
@codecov

codecov Bot commented May 18, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

Two minor markdownlint / accuracy nits:

- Tag the two ascii-diagram fences in slides-mobile-edit.md with
  ```text so MD040 stops warning.
- Widen the Task 1a checklist's grep to scan packages/slides (not
  just packages/slides/src) so the test-event audit catches files
  under packages/slides/test — which is the directory the
  migration actually had to touch.
@hackerwins
hackerwins merged commit e550169 into main May 18, 2026
4 checks passed
@hackerwins
hackerwins deleted the slides/mobile-edit-pointer-events branch May 18, 2026 00:29
hackerwins added a commit that referenced this pull request May 18, 2026
These seven slides + PPTX tasks shipped (PRs #238, #240, #241, #255,
#259, #260, #261) but their checklist boxes were never ticked during
execution, so the archive script left them in active/. Sweep the
checkboxes to reflect what actually landed, add lessons files for the
two that were missing (keyboard-shortcuts, presentation-mode), and
convert the keyboard-shortcuts onLinkRequest popover bullet from a
checkbox to a "Deferred (follow-up)" note so the gate stops blocking on
work that is intentionally out of scope.

Co-authored-by: Claude Opus 4.7 (1M context) <[email protected]>
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.

1 participant