Skip to content

Canvas unification — Phase 2a: tool-keyed interaction dispatcher - #2

Merged
vibhavkatre merged 1 commit into
mainfrom
feat/unified-canvas-interaction-dispatcher
Jul 16, 2026
Merged

Canvas unification — Phase 2a: tool-keyed interaction dispatcher#2
vibhavkatre merged 1 commit into
mainfrom
feat/unified-canvas-interaction-dispatcher

Conversation

@vibhavkatre

Copy link
Copy Markdown
Collaborator

What & why

Phase 2a of the canvas unification. Converts the surface-interaction seam from a single-slot ref into a layer-keyed registry resolved by the active tool.

Today each type's interaction composable registers its pointer handlers into one shared modeInteraction ref. That's fine when only one layer mounts (single-type docs), but the unified canvas mounts several layers at once — they'd clobber each other (last-writer-wins). This makes registration keyed by layer and adds a pure resolver that picks the right handler for the active tool.

Legacy is unchanged: a single-type document registers exactly one surface layer, so the resolver returns it regardless of tool — identical behaviour.

Changes

  • useModeInteraction.js: registry ref { layerKey: handlers }; registerModeInteraction(ref, layerKey, handlers); new pure resolveModeHandlers(registry, activeTool) (0 → null, 1 → that one, many → whiteboard-tools→whiteboard else the other).
  • whiteboard + flowchart composables register under keys and detach on unmount (flowchart gains a teardown it was missing).
  • DiagramCanvas.vue resolves handlers by active tool in delegatesSurface() / delegateSurfaceEvent().

Testing

  • Build ✅ · 100 unit tests ✅ (adds 4 for resolveModeHandlers).
  • Live-app regression ✅ — pen-drawing on a whiteboard still commits a stroke through the refactored seam (1 stroke, 11 points, no page errors); screenshot confirms. Flowchart registers via the identical single-key path (covered by the resolver unit tests; handlers unchanged).
  • Not user-visible on its own — enabling multiple registrants (unified rendering) is the next phase.

🤖 Generated with Claude Code

…patcher

Refactor the surface-interaction seam from a single-slot ref to a layer-keyed
registry resolved by the active tool. Prep for the unified canvas, where several
type layers mount at once and would otherwise clobber one shared handler ref
(last-writer-wins). Legacy single-type documents register exactly ONE surface
layer, so the resolver returns it unchanged — behaviour is identical.

- useModeInteraction: the provided value is now a registry ref { layerKey:
  handlers }. registerModeInteraction(ref, layerKey, handlers) installs/detaches
  by key. New pure resolveModeHandlers(registry, activeTool): 0 registrants →
  null; 1 → that one (legacy); several → whiteboard tools to the whiteboard
  layer, else the other registrant.
- whiteboard + flowchart interaction composables register under 'whiteboard' /
  'flowchart' keys and detach on unmount (flowchart gains the teardown it lacked).
- DiagramCanvas resolves handlers for the active tool in delegatesSurface() and
  delegateSurfaceEvent() instead of reading a single ref.

Verified: build + 100 unit tests (adds 4 for resolveModeHandlers) + live-app
regression — pen-drawing on a whiteboard still commits a stroke through the
refactored seam (1 stroke, 11 points, no errors). Not user-visible on its own;
the unified rendering that lights up multiple registrants is the next step.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@greptile-apps

greptile-apps Bot commented Jul 16, 2026

Copy link
Copy Markdown

Confidence Score: 4/5

Safe to merge; the registry logic is correct and the single-registrant (legacy) path is unchanged.

The double activeModeHandlers() call on every pointermove is the only notable issue — wasteful but not incorrect. Core routing logic and teardown are sound.

DiagramCanvas.vue — the duplicated registry resolution in delegateSurfaceEvent.

Reviews (1): Last reviewed commit: "Frappe Draw: canvas unification phase 2a..." | Re-trigger Greptile

Comment on lines 371 to +374
function delegateSurfaceEvent(handlerName, event) {
if (!delegatesSurface()) return false
const handler = modeInteraction.value[handlerName]
const handlers = activeModeHandlers()
const handler = handlers && handlers[handlerName]

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 delegateSurfaceEvent calls delegatesSurface() (which already runs activeModeHandlers()) and then calls activeModeHandlers() a second time. On pointermove this doubles the registry resolution on every frame.

Suggested change
function delegateSurfaceEvent(handlerName, event) {
if (!delegatesSurface()) return false
const handler = modeInteraction.value[handlerName]
const handlers = activeModeHandlers()
const handler = handlers && handlers[handlerName]
function delegateSurfaceEvent(handlerName, event) {
const handlers = activeModeHandlers()
if (!modeStrategy.value.handlesSurfaceInteraction || handlers == null) return false
const handler = handlers[handlerName]

@vibhavkatre
vibhavkatre merged commit 49804d1 into main Jul 16, 2026
2 checks passed
@vibhavkatre
vibhavkatre deleted the feat/unified-canvas-interaction-dispatcher branch July 16, 2026 17:56
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.

2 participants