ENG-2027: See individual Canvas frame in a block (dg-canvas frame embed) by mattakamatsu · Pull Request #1222 · DiscourseGraphs/discourse-graph

mattakamatsu · 2026-07-12T04:40:59Z

Closes ENG-2027. See the issue for the product framing; this is the implementation + self-review notes.

What

One block syntax, {{dg-canvas:}}, now drives both the whole-canvas embed and a frame-anchored one:

{{dg-canvas: [[Canvas]]}} → the original whole-canvas embed, unchanged.
{{dg-canvas: [[Canvas]] "Frame" shape:ID}} → live embed zoomed to that frame; tracks it as it moves/resizes, ⌖ re-center button, tldraw chrome hidden by default (⌘. restores it).
Frame referenced by shape id (stable across renames) with name fallback. Any broken / unknown / malformed frame reference silently degrades to the whole canvas.
Insert via slash command → pick canvas → pick frame.

The {{dg-frame}} attribute from the first draft is gone — merged into {{dg-canvas}}.

Shared-code bugs fixed along the way

Both pre-existing, surfaced by exercising the embed on real canvases:

Remote-sync ValidationError that blanked canvas pages. The pull-watch handler applied the raw persisted store via applyDiff, which validates but never migrates — so any record predating MigrateNodeTypeToDiscourseNode (a legacy node-uid shape) threw inside mergeRemoteChanges and tore the page down, for plain {{dg-canvas}} too. Extracted to canvasRemoteMerge.ts: migrate the incoming snapshot first (same machinery as loadSnapshot), drop still-invalid records individually, report failures instead of throwing. Unit-tested against real tldraw stores.
CSS that hid all page blocks when an embed mounted. The block-hiding :has() rule only excluded .dg-canvas-embed; any new embed wrapper blanked the host page.

Self-review (10-angle pass, findings addressed in the fix commits)

fix: a } in a frame name no longer parses to null and blanks the block (lazy tail + serializer sanitize).
fix: cancel the zoom-on-mount requestAnimationFrame on unmount; re-center refreshes the not-found state.
perf: resolve frames from the raw persisted store instead of building a throwaway migrated editor (also dedupes with the picker and stops a legacy-store throw).
chore: remove the unused instanceKey session-key plumbing left dead by the merge.
style: use Blueprint surfaces for the embed chrome instead of a hardcoded rgba(...) (AGENTS.md).
chore: explicit return types on the new exports.

For reviewers: refreshConfigTree.ts gained a null-guard (a phantom null child was aborting extension load). It's unrelated to this feature and only hardens one of the ~16 callers of that helper — happy to split it into its own PR if preferred. Canvas concurrency remains last-write-wins (a remote edit in the same 350 ms window as your own save can be dropped until the next sync), unchanged in spirit from today.

Verification

check-types, lint (0 errors), and test (62) green.
Live-verified in plugin-testing-akamatsulab2: all four routing cases (plain / valid-frame / unknown-frame / malformed-tail) route as expected with no errors, the frame embed zooms to its frame, and the ⌖ button + degradation paths work.

🤖 Generated with Claude Code

…erge fixes Feature (per canvas-frame-embed spec v0): a dg-frame block embed that mounts the live canvas anchored to a named tldraw frame, referenced by shape id with name fallback. Frame-anchored embeds re-zoom to the frame on every mount (with a re-center button); frameless embeds remember a per-instance viewport via an instanceKey suffix on the canvas session storage key. Includes a two-step insert dialog (canvas page -> frame) wired to a slash command, and embeds default to tldraw focus mode (cmd+. restores the controls). Fixes found while testing the embed on real canvases: - tldrawStyles.ts hid ALL Roam blocks on any page hosting a tldraw container unless it was inside .dg-canvas-embed, so mounting a dg-frame embed blanked the whole page (silently - pure CSS). The exclusion list now covers every embed wrapper class. - useRoamStore's pull-watch handler applied the raw persisted store via applyDiff, which validates against the live schema without migrating, so any record predating MigrateNodeTypeToDiscourseNode (legacy node-uid shape types) threw an uncaught ValidationError inside mergeRemoteChanges and broke the canvas - for dg-canvas embeds too, triggered by any watched change on the canvas page (including its own first-mount State-block write). The merge logic now lives in canvasRemoteMerge.ts: incoming snapshots run through store.schema.migrateStoreSnapshot (same machinery as loadSnapshot), still-invalid records are dropped individually instead of aborting, failed merges are reported instead of thrown, bare legacy-format props no longer TypeError, and a stale merge timer can no longer outlive the stateId guard and clobber just-saved local edits. - refreshConfigTree now tolerates phantom null children in config-page pulls, which previously aborted the entire extension load. Verified live in plugin-testing-akamatsulab2 (repro pages seeded with a raw legacy-typed shape reproduced the exact ValidationError pre-fix; post-fix the same triggers merge cleanly, migrating records in flight). Unit tests cover parsing, session-key instancing, and the remote merge against real tldraw stores. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

…attribute Per product decision: one block syntax, {{dg-canvas: [[Canvas]]}}, drives both behaviors. A plain reference renders the original whole-canvas embed unchanged (the classic code path, .dg-canvas-embed wrapper). An optional frame argument - "Frame Name" and/or shape:ID - routes to the frame-anchored embed instead, but only when it maps to a real frame on the canvas. Degradation contract (a broken frame reference must never cost the user their canvas): the parser ignores any tail that is not a clean "name"/shape:id argument, and the renderer resolves the frame against the persisted store before mounting - an unmatched name or id silently falls back to the whole-canvas embed rather than showing a not-found notice. - dgFrameEmbed.ts -> dgCanvasEmbed.ts: parse/serialize {{dg-canvas}} with an optional frame argument; a malformed tail parses as whole-canvas. - CanvasFrameEmbed.tsx exports findCanvasFrameRef (editor-less frame existence check via getRoamCanvasSnapshot) and the CanvasFrameEmbed component; renderCanvasEmbed routes plain vs frame and picks the wrapper class. - Removed the dg-frame button observer and renderCanvasFrameEmbed; the frame picker slash command now writes {{dg-canvas: ... "Frame" shape:ID}}. Tests rewritten for the merged syntax including the degradation cases; verified live in plugin-testing-akamatsulab2 that plain / valid-frame / unknown-frame / malformed-tail each route to the expected embed with no errors. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

A `}` inside a serialized frame name aborted the whole-embed regex match, so parseDgCanvasEmbed returned null and renderCanvasEmbed rendered nothing — violating the contract that a bad frame reference always degrades to the whole canvas. Make the tail lazy (`[\s\S]*?` to the `}}` terminator) so a stray `}` is handled by FRAME_ARGS_REGEX instead, and strip curly braces in the serializer as defense-in-depth for picker-written blocks. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

The requestAnimationFrame scheduled at editor mount was never cancelled, so if the embed unmounted within a frame the callback ran zoomToFrame against a disposed tldraw editor and set state on an unmounted component. Track the handle and cancel it in an unmount effect. Also have the re-center (⌖) handler re-resolve the frame so a delete/rename after mount clears or raises the "not found" notice instead of leaving it stale. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

findCanvasFrameRef and the frame picker both called getRoamCanvasSnapshot, which builds and migrates a full throwaway TLStore just to list frame shapes — duplicating the parse the editor mount does moments later, and throwing (then swallowing) on legacy canvases whose node shapes need the discourse migration. Add getPersistedCanvasStore / getCanvasFrameShapes that read frame records straight from block props (frames are default shapes, present in both formats and untouched by migration), and route both callers through them. One source of frame enumeration, no throwaway editor, and legacy canvases list frames. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

instanceKey was threaded through CanvasEmbedOptions, the session storage key, and the persistence registration to give frameless embeds an independent viewport — but merging the frame embed into {{dg-canvas}} left the plain path on the original TldrawCanvas with no embedOptions, so no caller ever set it. Remove the dead parameter, its storage-key branch, and its tests, and correct the CanvasEmbedOptions doc comment that still referenced the removed {{dg-frame}} syntax. (Two same-canvas embeds share one viewport — the original {{dg-canvas}} behavior, intentionally unchanged.) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

The ⌖ re-center button and the frame-not-found notice backed their readability with an inline rgba(255,255,255,0.9) — a new hardcoded shading color (AGENTS.md: don't introduce new shading colors). Use a default (non-minimal) Blueprint Button and a Blueprint Card, whose themed surfaces provide the background and elevation in both light and dark themes. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

AGENTS.md asks for explicit return types. Annotate the new exports that were missing them: renderCanvasFrameEmbedDialog and filterUserRecords. (calculateDiff is relocated code with any-typed internals that resist a precise return type without a rewrite — left as-is.) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

linear-code · 2026-07-12T04:41:01Z

ENG-2027

vercel · 2026-07-12T04:41:04Z

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment

Project	Deployment	Actions	Updated (UTC)
discourse-graph	Skipped		Jul 12, 2026 5:40am

supabase · 2026-07-12T04:41:05Z

This pull request has been ignored for the connected project zytfjzqyijgagqxrzbmz because there are no changes detected in packages/database/supabase directory. You can change this behaviour in Project Integrations Settings ↗︎.

Preview Branches by Supabase.
Learn more about Supabase Branching ↗︎.

Resolves the CanvasEmbed.tsx conflict with the "Edit Block" chrome that landed on main (#—, commit 69f7d22). Both sides rewrote renderCanvasEmbed's tail; the combined version keeps main's handleEditBlock/CanvasEmbedChrome for the whole-canvas path and this branch's frame routing for the frame path: frame ? <CanvasFrameEmbed> : <CanvasEmbedChrome title location> Title/frame parsing stays on parseDgCanvasEmbed (supersedes the removed BLOCK_TEXT_REGEX/extractCanvasTitle); both `frame` and `location` are computed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

…f any Clears the 12 lint warnings CI flagged on canvasRemoteMerge.ts (the file reads as newly added, so the diff-helper code relocated from useRoamStore.ts is re-linted). diffObjects/calculateDiff now use `unknown` with narrowed casts and typed filter predicates; pruneState drops the unused destructured key. Behavior unchanged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

graphite-app · 2026-07-12T05:43:15Z

PR size/scope check

This PR is over our review-size guideline.

Recommended: ~200 lines changed
Acceptable limit: up to 400 lines when well-scoped/self-contained
Preferred file count: fewer than 5 files

Please split this into smaller PRs unless there is a clear reason the changes need to land together.

If keeping it as one PR, please add a brief justification covering:

What single problem this PR solves
Why the files/changes are coupled

devin-ai-integration

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no potential bugs to report.

View in Devin Review to see 1 additional finding.

mattakamatsu · 2026-07-12T05:46:51Z

it also picked up a couple of pre-existing bugs and linting issues; those can be ignored/rejected if desired

sid597 · 2026-07-21T09:03:04Z

@mdroidian is this still on your radar or should I pick this up?

mdroidian

it also picked up a couple of pre-existing bugs and linting issues; those can be ignored/rejected if desired

@mattakamatsu Yeah, generally speaking we want each PR/Ticket to solve a single problem and be less than 500 lines (before tests). For the sake of a quicker review merge, let's drop the cavasRemoteMerge/useRoamStore and the refreshConfigTree fixes.

I also did a quick once over.

mdroidian · 2026-07-21T13:10:54Z

+  };
+
+  return (
+    <div style={{ position: "relative", height: "100%", width: "100%" }}>


use tailwind styles instead of inline

mdroidian · 2026-07-21T13:12:50Z

+
+export type FrameRef = { name?: string; shapeId?: string };
+
+// Decide whether a parsed frame argument actually maps to a frame on the canvas


Let's remind the agent to view the agents.md re: comments

Add comments only when necessary; descriptive names should minimize the need for comments

mdroidian · 2026-07-21T13:13:48Z

+      );
+    if (matches.length > 1) {
+      // eslint-disable-next-line no-console
+      console.warn(


Generally, only use console for debugging
Use a toast if we need to communicate to the user, posthog if want to record the error

mdroidian · 2026-07-21T13:15:13Z

@@ -0,0 +1,198 @@
+import React, { useCallback, useEffect, useMemo, useState } from "react";


Let's tie this into the existing embed dialog rather than creating a new dialog/separate slash command

mdroidian · 2026-07-21T13:15:59Z

+  const renderPageStep = () => {
+    if (canvasPages === null)
+      return (
+        <div className="text-sm text-[#5c7080]">Loading canvas pages...</div>


Does roam recognize text-[#5c7080]?
Regardless, why are we using a custom color here?

mdroidian · 2026-07-21T13:18:38Z

-  /* Hide Roam Blocks only when a full-page canvas is present (not embedded) */
-  .roam-article:has(.roamjs-tldraw-canvas-container:not(.dg-canvas-embed *)) .rm-block-children  {
+  /* Hide Roam Blocks only when a full-page canvas is present (not embedded).
+     Every block-embed wrapper class must be excluded here, or mounting that


The original comment captured the intent just fine.

mdroidian · 2026-07-21T13:20:36Z

+// the worst case is falling back to the whole-canvas embed.
+export const findCanvasFrameRef = ({
+  pageUid,
+  frameName,


Let's not store/search for frameName (which is subject to change), only use frameShapeId which is stable.

frameName should be part of the UI when choosing, but not part of the saved data

mdroidian · 2026-07-21T13:25:23Z

  };
 };

+// The raw persisted record map from block props, without constructing or


Claud, chill on the comments

mdroidian · 2026-07-21T13:27:49Z

          (after?.[":block/props"] || {}) as json,
        ) as Record<string, json>;
        const rjsqb = props["roamjs-query-builder"] as Record<string, unknown>;
+        // Any pending merge was computed from an older props state than this


What's the use case for a "pending merge"? When is this likely to happen?

mdroidian · 2026-07-21T13:29:09Z

@@ -1,13 +1,46 @@
-import getBasicTreeByParentUid from "roamjs-components/queries/getBasicTreeByParentUid";
 import getPageUidByPageTitle from "roamjs-components/queries/getPageUidByPageTitle";


This change should be it's own ticket where the problem is clearly documented/repeated. The fix should also be in roamjs-components, not here.

mattakamatsu and others added 8 commits July 11, 2026 20:28

vercel Bot deployed to Preview July 12, 2026 05:25 View deployment

vercel Bot temporarily deployed to Preview July 12, 2026 05:40 Inactive

mattakamatsu marked this pull request as ready for review July 12, 2026 05:43

mattakamatsu requested a review from sid597 July 12, 2026 05:43

devin-ai-integration Bot reviewed Jul 12, 2026

View reviewed changes

mattakamatsu mentioned this pull request Jul 12, 2026

Render dg-canvas frame embeds as static snapshots by default (v1) #1223

Draft

mdroidian self-requested a review July 21, 2026 13:29

mdroidian requested changes Jul 21, 2026

View reviewed changes


		export type FrameRef = { name?: string; shapeId?: string };

		// Decide whether a parsed frame argument actually maps to a frame on the canvas

		@@ -0,0 +1,198 @@
		import React, { useCallback, useEffect, useMemo, useState } from "react";

		@@ -1,13 +1,46 @@
		import getBasicTreeByParentUid from "roamjs-components/queries/getBasicTreeByParentUid";
		import getPageUidByPageTitle from "roamjs-components/queries/getPageUidByPageTitle";

Uh oh!

Conversation

mattakamatsu commented Jul 12, 2026

What

Shared-code bugs fixed along the way

Self-review (10-angle pass, findings addressed in the fix commits)

Verification

Uh oh!

linear-code Bot commented Jul 12, 2026

Uh oh!

vercel Bot commented Jul 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

supabase Bot commented Jul 12, 2026

Uh oh!

graphite-app Bot commented Jul 12, 2026

PR size/scope check

Uh oh!

devin-ai-integration Bot left a comment

Choose a reason for hiding this comment

✅ Devin Review: No Issues Found

Uh oh!

mattakamatsu commented Jul 12, 2026

Uh oh!

sid597 commented Jul 21, 2026

Uh oh!

mdroidian left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

vercel Bot commented Jul 12, 2026 •

edited

Loading