Skip to content

[ENG-1108] Canvas tool lock #851

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
trangdoan982 wants to merge 2 commits into
base: main
Choose a base branch
from
Open

[ENG-1108] Canvas tool lock #851

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4a9d243
wip
trangdoan982 Mar 3, 2026
81fc1d3
lint
trangdoan982 Mar 3, 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
4 changes: 3 additions & 1 deletion apps/roam/src/components/canvas/DiscourseNodeUtil.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ import { getSetting } from "~/utils/extensionSettings";
import DiscourseContextOverlay from "~/components/DiscourseContextOverlay";
import { getDiscourseNodeColors } from "~/utils/getDiscourseNodeColors";
import { render as renderToast } from "roamjs-components/components/Toast";
import { setCurrentToolToSelectIfUnlocked } from "./toolLock";

// TODO REPLACE WITH TLDRAW DEFAULTS
// https://github.com/tldraw/tldraw/pull/1580/files
Expand Down Expand Up @@ -106,6 +107,7 @@ export const createNodeShapeTools = (
return class DiscourseNodeTool extends StateNode {
static id = n.type;
static initial = "idle";
static isLockable = true;
shapeType = n.type;

override onEnter = () => {
Expand All @@ -126,7 +128,7 @@ export const createNodeShapeTools = (
props: { fontFamily: "sans", size: "s" },
});
this.editor.setEditingShape(shapeId);
this.editor.setCurrentTool("select");
setCurrentToolToSelectIfUnlocked(this.editor);
};
};
});
Expand Down
7 changes: 5 additions & 2 deletions apps/roam/src/components/canvas/DiscourseRelationShape/DiscourseRelationTool.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import {
} from "./DiscourseRelationUtil";
import { discourseContext } from "~/components/canvas/Tldraw";
import { dispatchToastEvent } from "~/components/canvas/ToastListener";
import { setCurrentToolToSelectIfUnlocked } from "~/components/canvas/toolLock";

export type AddReferencedNodeType = Record<string, ReferenceFormatType[]>;
type ReferenceFormatType = {
Expand All @@ -28,6 +29,7 @@ export const createAllReferencedNodeTools = (
class ReferencedNodeTool extends StateNode {
static override initial = "idle";
static override id = action;
static override isLockable = true;
static override children = (): TLStateNodeConstructor[] => [
this.Idle,
this.Pointing,
Expand Down Expand Up @@ -278,7 +280,7 @@ export const createAllReferencedNodeTools = (
};

override onCancel = () => {
this.editor.setCurrentTool("select");
setCurrentToolToSelectIfUnlocked(this.editor);
};

override onKeyUp: TLEventHandlers["onKeyUp"] = (info) => {
Expand Down Expand Up @@ -315,6 +317,7 @@ export const createAllRelationShapeTools = (
class RelationShapeTool extends StateNode {
static override initial = "idle";
static override id = name;
static override isLockable = true;
static override children = (): TLStateNodeConstructor[] => [
this.Idle,
this.Pointing,
Expand Down Expand Up @@ -574,7 +577,7 @@ export const createAllRelationShapeTools = (
};

override onCancel = () => {
this.editor.setCurrentTool("select");
setCurrentToolToSelectIfUnlocked(this.editor);
};

override onKeyUp: TLEventHandlers["onKeyUp"] = (info) => {
Expand Down
8 changes: 6 additions & 2 deletions apps/roam/src/components/canvas/DiscourseRelationShape/DiscourseRelationUtil.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,10 @@ const getCanvasPageUidFromDOM = (): string | null => {
// With multiple canvases, find the one that contains the currently focused element
// or has been recently interacted with (has :focus-within)
for (const container of containers) {
if (container.matches(":focus-within") || container.contains(document.activeElement)) {
if (
container.matches(":focus-within") ||
container.contains(document.activeElement)
) {
return container.getAttribute("data-page-uid");
}
}
Expand All @@ -108,6 +111,7 @@ import { AddReferencedNodeType } from "./DiscourseRelationTool";
import { dispatchToastEvent } from "~/components/canvas/ToastListener";
import internalError from "~/utils/internalError";
import { USE_REIFIED_RELATIONS } from "~/data/userSettings";
import { setCurrentToolToSelectIfUnlocked } from "~/components/canvas/toolLock";

const COLOR_ARRAY = Array.from(DefaultColorStyle.values)
.filter((c) => !["red", "green", "grey"].includes(c))
Expand Down Expand Up @@ -1084,7 +1088,7 @@ export class BaseDiscourseRelationUtil extends ShapeUtil<DiscourseRelationShape>
static override props = arrowShapeProps;

cancelAndWarn = (title: string) => {
this.editor.setCurrentTool("select");
setCurrentToolToSelectIfUnlocked(this.editor);
dispatchToastEvent({
id: `tldraw-cancel-and-warn-${title}`,
title,
Expand Down
5 changes: 4 additions & 1 deletion apps/roam/src/components/canvas/DiscourseToolPanel.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import { TOOL_ARROW_ICON_SVG, NODE_COLOR_ICON_SVG } from "~/icons";
import { getDiscourseNodeColors } from "~/utils/getDiscourseNodeColors";
import { DEFAULT_WIDTH, DEFAULT_HEIGHT } from "./Tldraw";
import { DEFAULT_STYLE_PROPS, FONT_SIZES } from "./DiscourseNodeUtil";
import { lockToolIfNeeded, setCurrentToolToSelectIfUnlocked } from "./toolLock";

export type DiscourseGraphPanelProps = {
nodes: DiscourseNode[];
Expand Down Expand Up @@ -167,6 +168,7 @@ const DiscourseGraphPanel = ({
const itemIndex = target.dataset.drag_item_index!;
const item = panelItems[+itemIndex];
if (item) {
lockToolIfNeeded(editor);
editor.setCurrentTool(item.id);
}
dragState.set({
Expand All @@ -190,9 +192,10 @@ const DiscourseGraphPanel = ({
props: { fontFamily: "sans", size: "s" },
});
editor.setEditingShape(shapeId);
editor.setCurrentTool("select");
setCurrentToolToSelectIfUnlocked(editor);
} else {
// For relations, just activate the tool
lockToolIfNeeded(editor);
editor.setCurrentTool(current.item.id);
}

Expand Down
1 change: 1 addition & 0 deletions apps/roam/src/components/canvas/Tldraw.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -730,6 +730,7 @@ const TldrawCanvasShared = ({
const discourseGraphTool = class DiscourseGraphTool extends StateNode {
static override id = "discourse-tool";
static override initial = "idle";
static override isLockable = true;
};
const discourseNodeTools = createNodeShapeTools(allNodes);
const discourseRelationTools = createAllRelationShapeTools(allRelationNames);
Expand Down
13 changes: 13 additions & 0 deletions apps/roam/src/components/canvas/toolLock.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
import { Editor } from "tldraw";

export const setCurrentToolToSelectIfUnlocked = (editor: Editor): void => {
if (!editor.getInstanceState().isToolLocked) {
editor.setCurrentTool("select");
}
};

export const lockToolIfNeeded = (editor: Editor): void => {
if (!editor.getInstanceState().isToolLocked) {
editor.updateInstanceState({ isToolLocked: true });
}
Comment on lines +9 to +12
Copy link
Contributor

@devin-ai-integration devin-ai-integration bot Mar 3, 2026

Choose a reason for hiding this comment

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

🔴 lockToolIfNeeded unconditionally forces isToolLocked = true, overriding user preference

Every click on a tool in the DiscourseToolPanel calls lockToolIfNeeded(editor) before editor.setCurrentTool(...). This function unconditionally sets isToolLocked = true in the editor's global instance state whenever it's currently false.

Root Cause and Impact

The lockToolIfNeeded function at apps/roam/src/components/canvas/toolLock.ts:9-12 always forces isToolLocked to true:

export const lockToolIfNeeded = (editor: Editor): void => {
  if (!editor.getInstanceState().isToolLocked) {
    editor.updateInstanceState({ isToolLocked: true });
  }
};

This is called from DiscourseToolPanel.tsx:171 (panel click) and DiscourseToolPanel.tsx:198 (drag-drop relation). After this call, the global isToolLocked state is permanently set to true. This means:

  1. User has tool lock OFF (the default).
  2. User clicks any tool in the panel → lockToolIfNeeded silently forces isToolLocked = true.
  3. Now all tools (including those activated via keyboard shortcuts at apps/roam/src/components/canvas/uiOverrides.tsx:355,374,397) behave as locked, because the global state has been mutated.
  4. The user's preference is permanently overridden with no way to know it happened, and they must manually toggle the lock off via the tldraw UI.

Impact: The user's tool lock preference is silently and permanently overridden after any panel interaction. This changes the behavior of all subsequent tool usage across the entire canvas.

Prompt for agents 
The lockToolIfNeeded function in apps/roam/src/components/canvas/toolLock.ts unconditionally forces isToolLocked to true, which permanently overrides the user's preference. There are two possible fixes depending on the intended behavior:

1. If the intent is that panel clicks should NOT modify the lock state (just respect whatever the user has set): Remove lockToolIfNeeded entirely and remove its calls at apps/roam/src/components/canvas/DiscourseToolPanel.tsx lines 171 and 198. The setCurrentToolToSelectIfUnlocked already handles the unlocked case correctly.

2. If the intent is that panel clicks should temporarily lock the tool (so one click = one use): Save and restore the previous isToolLocked state. For example, set isToolLocked to true before setCurrentTool, and after the shape is created (in setCurrentToolToSelectIfUnlocked), restore the original value. This would require threading the original state through or storing it somewhere accessible.
Open in Devin Review

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

};