feat(canvas): M6 — in-place live editing - #833
Merged
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 2 Skipped Deployments
|
github-actions
Bot
added
documentation
|
React Doctor found no new issues. 🎉 Reviewed by React Doctor for commit |
There was a problem hiding this comment.
Pull request overview
Implements M6 “in-place live editing” for spatial-canvas cards: a single active card can mount a full live editor (note/task/event), with dblclick-to-activate and click-away/Escape/tool-select to deactivate, plus a renderer-side Yjs doc registry to safely support same-note multi-mount within one window.
Changes:
- Add active-card state machine (hit-test/reducer/pinning) and wire it into the canvas overlay to mount exactly one active editor at a time.
- Introduce a ref-counted, noteId-keyed shared Yjs doc registry and route
useYjsCollaborationthrough it; gate ContentArea side effects to a single owner. - Add embedded note/task/event editors for active cards, extract
<CalendarEventForm>, fix Excalidraw unmount “restore wipe”, and add E2E/unit coverage for the new flows.
Reviewed changes
Copilot reviewed 25 out of 25 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| docs/superpowers/specs/2026-07-21-spatial-canvas-m6-live-editing-design.md | M6 design doc for active editing + shared-doc approach. |
| docs/superpowers/plans/2026-07-21-spatial-canvas-m6-live-editing.md | Implementation plan detailing tasks and test strategy. |
| apps/desktop/tests/e2e/canvas-editing.e2e.ts | E2E coverage for activate/deactivate, note/task/event editing, restore regression, virtualization. |
| apps/desktop/src/renderer/src/sync/yjs-doc-registry.ts | Ref-counted registry to share one Yjs entry per noteId within a renderer window. |
| apps/desktop/src/renderer/src/sync/yjs-doc-registry.test.ts | Unit tests for refcounting and side-effect ownership promotion. |
| apps/desktop/src/renderer/src/sync/use-yjs-collaboration.ts | Refactor hook to acquire/release shared doc entries and expose side-effect ownership. |
| apps/desktop/src/renderer/src/sync/use-yjs-collaboration.registry.test.ts | Minimal test ensuring registry wiring exports exist. |
| apps/desktop/src/renderer/src/pages/canvas/embedded-note-editor.tsx | Embedded note editor for active cards with non-collab markdown-save fallback. |
| apps/desktop/src/renderer/src/pages/canvas/embedded-note-editor.test.tsx | Unit tests for debounce + save-registry integration. |
| apps/desktop/src/renderer/src/pages/canvas/canvas-task-editor.tsx | Slim in-place task editor reusing existing task field components + shared update mapper. |
| apps/desktop/src/renderer/src/pages/canvas/canvas-task-editor.test.tsx | Unit tests for task field wiring + debounced description persistence. |
| apps/desktop/src/renderer/src/pages/canvas/canvas-event-editor.tsx | In-place event editor that loads a draft and saves via calendar service. |
| apps/desktop/src/renderer/src/pages/canvas/canvas-event-editor.test.tsx | Unit tests for draft conversion and save/dismiss wiring. |
| apps/desktop/src/renderer/src/pages/canvas/canvas-editor.tsx | Guard serialize() to prevent persisting empty scene on Excalidraw teardown. |
| apps/desktop/src/renderer/src/pages/canvas/canvas-card.tsx | Add data-canvas-redirect marker to keep ↗ distinct from dblclick editing. |
| apps/desktop/src/renderer/src/pages/canvas/canvas-card-overlay.tsx | Active-card wiring (dblclick activate, click-away/tool deactivate, pin active for virtualization). |
| apps/desktop/src/renderer/src/pages/canvas/canvas-card-overlay.test.tsx | Update tests to assert activation/deactivation behavior (not redirect). |
| apps/desktop/src/renderer/src/pages/canvas/canvas-card-active.tsx | Active-card container: keyboard containment + editor switch (note/task/event). |
| apps/desktop/src/renderer/src/pages/canvas/canvas-card-active.test.tsx | Unit tests for active container attrs, focus, and Escape handling. |
| apps/desktop/src/renderer/src/pages/canvas/canvas-active.ts | Pure active-state utilities (hitTest/reducer/tool gate/pin). |
| apps/desktop/src/renderer/src/pages/canvas/canvas-active.test.ts | Unit tests for active-state utilities. |
| apps/desktop/src/renderer/src/components/note/content-area/types.ts | Add runSideEffects? prop to support single-owner side effects. |
| apps/desktop/src/renderer/src/components/note/content-area/ContentArea.tsx | Gate task auto-conversion behind runSideEffects + wire ownership from Yjs hook. |
| apps/desktop/src/renderer/src/components/calendar/calendar-event-popover.tsx | Replace inline form body with extracted <CalendarEventForm>. |
| apps/desktop/src/renderer/src/components/calendar/calendar-event-form.tsx | New extracted controlled calendar event form component. |
Comments suppressed due to low confidence (1)
apps/desktop/src/renderer/src/sync/use-yjs-collaboration.ts:194
- When collaboration is disabled (no noteId / enabled=false / noteId mismatch), the hook still returns the previous note’s isRemoteUpdateRef via activeState.isRemoteUpdateRef. That ref can point at a destroyed doc and can incorrectly flag remote updates for the current (disabled) consumer. Return a dummy ref whenever the hook is in the DISABLED_STATE (and ideally derive the disabled boolean once to avoid divergence).
const state =
!noteId ||
!enabled ||
activeState.noteId !== noteId ||
(!activeState.fallback && !activeState.provider?.isSynced)
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+55
to
+72
| useEffect(() => { | ||
| let cancelled = false | ||
| calendarService | ||
| .getEvent(eventId) | ||
| .then((event) => { | ||
| if (cancelled || !event) return | ||
| setDraft(toDraft(event)) | ||
| }) | ||
| .catch((error: unknown) => { | ||
| log.error('Failed to load calendar event', { | ||
| eventId, | ||
| error: extractErrorMessage(error) | ||
| }) | ||
| }) | ||
| return () => { | ||
| cancelled = true | ||
| } | ||
| }, [eventId]) |
…ivate, click-away/Escape/tool deactivate)
…ide effects to one owner
h4yfans
force-pushed
the
spatial-canvas-m6-live-editing
branch
from
July 22, 2026 11:42
da11641 to
200c176
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 26 out of 26 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (2)
apps/desktop/src/renderer/src/sync/use-yjs-collaboration.ts:194
- When collaboration is disabled (no noteId / enabled=false / noteId mismatch), the hook still returns
activeState.isRemoteUpdateRef, which can point at a previous note’s doc. That can leak a stale remote-update flag into callers and makes the return value inconsistent with thestate(which correctly falls back to DISABLED_STATE).
const state =
!noteId ||
!enabled ||
activeState.noteId !== noteId ||
(!activeState.fallback && !activeState.provider?.isSynced)
apps/desktop/src/renderer/src/pages/canvas/canvas-event-editor.tsx:68
- If
calendarService.getEvent(eventId)rejects, the component logs and keepsdraftas null, leaving the card stuck in the “Loading…” state indefinitely (no retry, no error, no way to dismiss besides click-away).
useEffect(() => {
let cancelled = false
calendarService
.getEvent(eventId)
.then((event) => {
if (cancelled || !event) return
setDraft(toDraft(event))
})
.catch((error: unknown) => {
log.error('Failed to load calendar event', {
eventId,
error: extractErrorMessage(error)
})
})
Comment on lines
+51
to
+56
| try { | ||
| await notesService.update({ id: noteId, content: pending }) | ||
| lastSavedContentRef.current = pending | ||
| } catch (err) { | ||
| log.error('Failed to save note', { noteId, error: err }) | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
M6 — In-place live editing (spatial canvas)
Last core milestone before default-on (M7). Double-clicking an idle card promotes it to a single active state that mounts a full live editor; click-away/Escape returns it to the idle preview and unmounts the editor. Renderer-only, over the existing M2 hybrid card system (Excalidraw rectangle +
pointer-events:noneDOM overlay).Source of truth:
docs/superpowers/specs/2026-07-21-spatial-canvas-m6-live-editing-design.md(+ master spec2026-07-17-spatial-canvas-design.md). Plan:docs/superpowers/plans/2026-07-21-spatial-canvas-m6-live-editing.md.What's in it
canvas-active.ts(angle-aware hit-test, reducer, tool-gate, active-pin); double-click→activate, click-away (no swallow) / Escape (swallowed) / tool-select → deactivate; keydown/keyup contained so Cmd/Ctrl+Z reaches the editor, not Excalidraw.<EmbeddedNoteEditor>reuses<ContentArea>, bound to the note's Y.Doc via a new ref-counted, noteId-keyed shared-doc registry (yjs-doc-registry.ts+use-yjs-collaboration.tsrefactor) so a note open in a tab and active on a card share one doc (no echo/dupe). Single-consumer behaviour is byte-identical to before; ownership handoff is reactive.updateTaskmapper.<CalendarEventForm>from the event popover (popover becomes a thin wrapper; no caller change); card owns the draft, saves viacalendarService.updateEvent.Notable finds
serialize()now returns null when the wrapper is disconnected (skips the destructive write; debounce/beforeunload/quit-flush unaffected). Covered by a new restore-regression E2E.note.tsx, gated so there's no double-write). The shared registry is validated by unit tests; E2E [T019] Create migration runner #19 is an honest unauthenticated no-duplicate test.Backward compatibility
spatialCanvasflag stays default-OFF (untouched). No sync / contract / crypto / DB / migration change. Renderer editing over existing entities.Testing
canvas-editing: 8/8 (activate/deactivate, ↗-vs-edit [T020] Create database module exports #20, note [T018] Create Drizzle client with better-sqlite3 driver #18, restore-regression, honest [T019] Create migration runner #19, task [T022] Create path utilities #22, event [T022] Create path utilities #22, perf/virtualization).Deferred to M7 (rollout) — need sign-off
docs:impact --strictreportsmissing-docs; docs are an M7 deliverable per master spec §13. Pushed withMEMRY_DOCS_IMPACT_SKIP=1(intentionally non-docs).Follow-ups (non-blocking)
getEventrejects (Escape still deactivates).toCreatePayload/createDraftFromItemare hand-copied into the event editor — extract to shareddate-utilsin a future pass.