Skip to content

ENG-797: Handle multiple canvases loaded at the same time #928

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
sid597 wants to merge 4 commits into
base: main
Choose a base branch
from
+58 −2
Open

ENG-797: Handle multiple canvases loaded at the same time #928

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
f057c8d
[ENG-797] Handle multiple canvases loaded at the same time
sid597 Mar 30, 2026
4adf7ac
[ENG-797] Add pageUid to action listener deps
sid597 Mar 30, 2026
447b95a
[ENG-797] Fix cleanup guard and narrow closest selector
sid597 Mar 30, 2026
1b84834
[ENG-797] Restore discourseContext.relations in Export and lastAction…
sid597 Apr 10, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 52 additions & 1 deletion apps/roam/src/components/canvas/Tldraw.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,33 @@ export const discourseContext: DiscourseContextType = {
lastActions: [],
Copy link
Copy Markdown
Collaborator Author

@sid597 sid597 Apr 1, 2026

Choose a reason for hiding this comment

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

needs to stay, because we use this to log on error

Copy link
Copy Markdown
Collaborator Author

@sid597 sid597 Apr 1, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

We use this to send log of last actions taken by user in case of error. Should not remove this.

};

let activeCanvasPageUid: string | null = null;
let activeCanvasEditor: Editor | null = null;

const setActiveCanvas = ({
Copy link
Copy Markdown
Collaborator Author

@sid597 sid597 Mar 30, 2026

Choose a reason for hiding this comment

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

Surface 3 handled, we track the active canvas and ensure only that canvas is focused, so only this canvas responds to the clipboard events

pageUid,
editor,
}: {
pageUid: string;
editor: Editor | null;
}) => {
if (activeCanvasPageUid === pageUid && activeCanvasEditor === editor) {
if (editor && !editor.getInstanceState().isFocused) editor.focus();
return;
}

if (activeCanvasEditor && activeCanvasEditor !== editor) {
activeCanvasEditor.blur();
}

activeCanvasPageUid = pageUid;
activeCanvasEditor = editor;

if (editor && !editor.getInstanceState().isFocused) {
editor.focus();
}
};

export const DEFAULT_WIDTH = 160;
export const DEFAULT_HEIGHT = 64;
export const MAX_WIDTH = "400px";
Expand Down Expand Up @@ -725,6 +752,17 @@ const TldrawCanvasShared = ({
inSidebar: !!containerRef.current?.closest(".rm-sidebar-outline"),
});
}, [pageUid]);

useEffect(() => {
return () => {
const editor = appRef.current;
if (activeCanvasPageUid === pageUid && activeCanvasEditor === editor) {
activeCanvasEditor?.blur();
activeCanvasPageUid = null;
activeCanvasEditor = null;
}
};
}, [pageUid]);
const { store, needsUpgrade, performUpgrade, error, isLoading } =
useStoreAdapter(storeAdapterArgs);
const migratedCloudStoreRef = useRef<string | null>(null);
Expand Down Expand Up @@ -788,10 +826,14 @@ const TldrawCanvasShared = ({
uid?: string;
val?: string;
shapeId?: TLShapeId;
targetCanvasPageUid?: string;
onRefresh: () => void;
}>,
) => {
if (!/canvas/i.test(e.detail.action)) return;
const targetCanvasPageUid =
e.detail.targetCanvasPageUid ?? activeCanvasPageUid;
if (targetCanvasPageUid !== pageUid) return;
Comment on lines +834 to +836
Copy link
Copy Markdown
Contributor

@graphite-app graphite-app bot Apr 10, 2026

Choose a reason for hiding this comment

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

Actions will be lost when activeCanvasPageUid is null. If a canvas unmounts and clears the active canvas state (lines 759-762), any actions dispatched before another canvas becomes active will be silently ignored by all canvases.

This can happen when:

  1. The last active canvas is closed/unmounted
  2. An action is dispatched (e.g., from query builder)
  3. The action's targetCanvasPageUid is undefined
  4. activeCanvasPageUid is null (from cleanup)
  5. The check null !== pageUid evaluates to true for all canvases
  6. All canvases return early and ignore the action

Fix by handling the null case explicitly:

const targetCanvasPageUid = e.detail.targetCanvasPageUid;
if (targetCanvasPageUid !== undefined && targetCanvasPageUid !== pageUid) return;
if (targetCanvasPageUid === undefined && activeCanvasPageUid !== null && activeCanvasPageUid !== pageUid) return;
Suggested change
const targetCanvasPageUid =
e.detail.targetCanvasPageUid ?? activeCanvasPageUid;
if (targetCanvasPageUid !== pageUid) return;
const targetCanvasPageUid = e.detail.targetCanvasPageUid;
if (targetCanvasPageUid !== undefined && targetCanvasPageUid !== pageUid) return;
if (targetCanvasPageUid === undefined && activeCanvasPageUid !== null && activeCanvasPageUid !== pageUid) return;

Spotted by Graphite

Fix in Graphite

Is this helpful? React 👍 or 👎 to let us know.

Comment on lines +834 to +836
Copy link
Copy Markdown
Contributor

@devin-ai-integration devin-ai-integration bot Apr 10, 2026

Choose a reason for hiding this comment

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

🟡 Canvas actions silently dropped when active canvas unmounts while another canvas remains open

When two canvases are open (e.g., main window + sidebar) and the active one unmounts (user navigates away from the main page), the cleanup at apps/roam/src/components/canvas/Tldraw.tsx:759-762 sets activeCanvasPageUid = null. The surviving sidebar canvas was never registered as active (because the onMount check at apps/roam/src/components/canvas/Tldraw.tsx:1052 skips setActiveCanvas when another canvas is already active). Subsequent "canvas" action events dispatched from outside any canvas container (where targetCanvasPageUid is undefined) resolve to undefined ?? null = null at line 835, and null !== pageUid causes the remaining canvas to skip the event. The action is silently dropped until the user clicks on the surviving canvas. This is a behavioral regression from the pre-PR code where any canvas with a live editor would handle the action.

Suggested change
const targetCanvasPageUid =
e.detail.targetCanvasPageUid ?? activeCanvasPageUid;
if (targetCanvasPageUid !== pageUid) return;
const targetCanvasPageUid =
e.detail.targetCanvasPageUid ?? activeCanvasPageUid;
if (targetCanvasPageUid != null && targetCanvasPageUid !== pageUid) return;
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

const app = appRef.current;
if (!app) return;
const { x, y } = app.getViewportScreenCenter();
Expand Down Expand Up @@ -829,7 +871,7 @@ const TldrawCanvasShared = ({
actionListener,
);
};
}, [appRef, allNodes]);
}, [appRef, allNodes, pageUid]);

// Catch a custom event we used patch-package to add
useEffect(() => {
Expand Down Expand Up @@ -917,6 +959,10 @@ const TldrawCanvasShared = ({
tabIndex={-1}
onDragOver={handleDragOver}
onDrop={handleDrop}
onPointerDownCapture={() => {
if (!appRef.current) return;
setActiveCanvas({ pageUid, editor: appRef.current });
}}
>
{isCloudflareSync && (
<div
Expand Down Expand Up @@ -987,6 +1033,7 @@ const TldrawCanvasShared = ({
<TldrawEditor
// baseUrl="https://samepage.network/assets/tldraw/"
// instanceId={initialState.instanceId}
autoFocus={false}
initialState="select"
shapeUtils={[...defaultShapeUtils, ...customShapeUtils]}
tools={[...defaultTools, ...defaultShapeTools, ...customTools]}
Expand All @@ -1002,6 +1049,10 @@ const TldrawCanvasShared = ({

appRef.current = app;

if (!activeCanvasPageUid || activeCanvasPageUid === pageUid) {
setActiveCanvas({ pageUid, editor: app });
}

void syncCanvasNodeTitlesOnLoad(
app,
allNodes.map((n) => n.type),
Expand Down
7 changes: 6 additions & 1 deletion apps/roam/src/components/results-view/ResultsTable.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,11 @@ const ResultRow = ({
return (
<Button
{...buttonProps}
onClick={() => {
onClick={(event) => {
Copy link
Copy Markdown
Collaborator Author

@sid597 sid597 Mar 30, 2026

Choose a reason for hiding this comment

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

Pass this to the correct canvas

const targetCanvasPageUid =
event.currentTarget.closest<HTMLElement>(
".roamjs-tldraw-canvas-container[data-page-uid]",
)?.dataset.pageUid || undefined;
document.dispatchEvent(
new CustomEvent("roamjs:query-builder:action", {
detail: {
Expand All @@ -238,6 +242,7 @@ const ResultRow = ({
val: r["text"],
onRefresh,
queryUid: parentUid,
targetCanvasPageUid,
},
}),
);
Expand Down
Loading