From d9cca44c597283c44e62a851d89975ebb32ae0c1 Mon Sep 17 00:00:00 2001 From: Trang Doan Date: Mon, 14 Sep 2026 10:59:17 -0400 Subject: [PATCH 01/10] ENG-2227 Add link support to tldraw text shapes in Roam Text shapes could not carry a link, so a canvas narrative could not point at its detailed source. tldraw gates its built-in Edit link action on `'url' in shape.props`, and the stock text shape has no such prop, so the fix is schema-level rather than UI wiring. Subclass TextShapeUtil to add a `url` prop and render the hyperlink button, and replace the stock util through a shared list so all four stores agree. Existing text shapes are backfilled by migration v6 of the repo's own sequence; without it they fail validation and the canvas will not open. The sync worker validates `text` against stock props, so the client now declares it alongside the custom shape types. Co-Authored-By: Claude Opus 5 --- apps/roam/src/components/Export.tsx | 4 +- .../discourseRelationMigrations.ts | 13 ++ .../canvas/TextShapeWithLinkUtil.tsx | 75 +++++++++++ apps/roam/src/components/canvas/Tldraw.tsx | 4 +- .../canvas/TldrawCanvasCloudflareSync.tsx | 8 +- .../src/components/canvas/baseShapeUtils.ts | 13 ++ .../src/components/canvas/useRoamStore.ts | 4 +- .../src/utils/__tests__/textShapeLink.test.ts | 121 ++++++++++++++++++ apps/roam/src/utils/textShapeLink.ts | 27 ++++ 9 files changed, 260 insertions(+), 9 deletions(-) create mode 100644 apps/roam/src/components/canvas/TextShapeWithLinkUtil.tsx create mode 100644 apps/roam/src/components/canvas/baseShapeUtils.ts create mode 100644 apps/roam/src/utils/__tests__/textShapeLink.test.ts create mode 100644 apps/roam/src/utils/textShapeLink.ts diff --git a/apps/roam/src/components/Export.tsx b/apps/roam/src/components/Export.tsx index cb0567d71e..fe6925d7fc 100644 --- a/apps/roam/src/components/Export.tsx +++ b/apps/roam/src/components/Export.tsx @@ -16,6 +16,7 @@ import { Radio, FormGroup, } from "@blueprintjs/core"; +import { baseShapeUtils } from "~/components/canvas/baseShapeUtils"; import React, { useState, useEffect, useMemo, FormEvent } from "react"; import MenuItemSelect from "roamjs-components/components/MenuItemSelect"; import { saveAs } from "file-saver"; @@ -50,7 +51,6 @@ import { TLParentId, getIndexAbove, TLShape, - defaultShapeUtils, defaultBindingUtils, } from "tldraw"; import { @@ -433,7 +433,7 @@ const ExportDialog: ExportDialogComponent = ({ const tlStore = createTLStore({ migrations, - shapeUtils: [...defaultShapeUtils, ...customShapeUtils], + shapeUtils: [...baseShapeUtils, ...customShapeUtils], bindingUtils: [...defaultBindingUtils, ...customBindingUtils], }); diff --git a/apps/roam/src/components/canvas/DiscourseRelationShape/discourseRelationMigrations.ts b/apps/roam/src/components/canvas/DiscourseRelationShape/discourseRelationMigrations.ts index 9e49a075ac..860ba76fad 100644 --- a/apps/roam/src/components/canvas/DiscourseRelationShape/discourseRelationMigrations.ts +++ b/apps/roam/src/components/canvas/DiscourseRelationShape/discourseRelationMigrations.ts @@ -18,6 +18,7 @@ import { createMigrationIds } from "tldraw"; import { RelationBinding } from "./DiscourseRelationBindings"; import { getRelationColor } from "./DiscourseRelationUtil"; import { DISCOURSE_NODE_SHAPE_TYPE } from "~/components/canvas/DiscourseNodeUtil"; +import { backfillTextShapeUrl, isTextShapeRecord } from "~/utils/textShapeLink"; const SEQUENCE_ID_BASE = "com.roam-research.discourse-graphs"; @@ -52,6 +53,7 @@ export const createMigrations = ({ AddSizeAndFontFamily: 3, RemoveNullAssetFileSize: 4, MigrateNodeTypeToDiscourseNode: 5, + AddTextShapeUrl: 6, }); return createMigrationSequence({ sequenceId: `${SEQUENCE_ID_BASE}`, @@ -203,6 +205,17 @@ export const createMigrations = ({ shape.type = DISCOURSE_NODE_SHAPE_TYPE; }, }, + { + id: versions["AddTextShapeUrl"], + scope: "record", + filter: (r: any) => isTextShapeRecord(r), + up: (shape: any) => { + backfillTextShapeUrl(shape); + }, + down: (shape: any) => { + delete shape.props.url; + }, + }, ], }); }; diff --git a/apps/roam/src/components/canvas/TextShapeWithLinkUtil.tsx b/apps/roam/src/components/canvas/TextShapeWithLinkUtil.tsx new file mode 100644 index 0000000000..208352e750 --- /dev/null +++ b/apps/roam/src/components/canvas/TextShapeWithLinkUtil.tsx @@ -0,0 +1,75 @@ +import React from "react"; +import { + T, + TextShapeUtil, + TLTextShape, + stopEventPropagation, + textShapeProps, +} from "tldraw"; + +export type TextShapeWithLinkProps = TLTextShape["props"] & { url: string }; + +const LINK_ICON = + "data:image/svg+xml,%3Csvg xmlns='http://www.w3.org/2000/svg' width='30' height='30' fill='none'%3E%3Cpath stroke='%23000' stroke-linecap='round' stroke-linejoin='round' stroke-width='2' d='M13 5H7a2 2 0 0 0-2 2v16a2 2 0 0 0 2 2h16a2 2 0 0 0 2-2v-6M19 5h6m0 0v6m0-6L13 17'/%3E%3C/svg%3E"; + +// tldraw does not export HyperlinkButton, so this mirrors its markup to keep +// text links visually identical to geo links. +const HyperlinkButton = ({ + url, + zoomLevel, +}: { + url: string; + zoomLevel: number; +}): JSX.Element => ( + +
+ +); + +export const getTextShapeUrl = (shape: TLTextShape): string => + (shape.props as Partial).url ?? ""; + +const textShapeWithLinkProps = { ...textShapeProps, url: T.linkUrl }; + +// Adds the `url` prop that tldraw's built-in Edit link action gates on +// (`'url' in shape.props`), so text shapes reuse the geo link UI unchanged. +export class TextShapeWithLinkUtil extends TextShapeUtil { + static override props = textShapeWithLinkProps; + + override getDefaultProps(): TextShapeWithLinkProps { + return { ...super.getDefaultProps(), url: "" }; + } + + // The cast bridges two React type copies in the dependency tree, which make + // the base signature's JSX.Element nominally distinct from ours. + override component( + shape: TLTextShape, + ): ReturnType { + const url = getTextShapeUrl(shape); + return ( + <> + {super.component(shape)} + {url && ( + + )} + + ) as ReturnType; + } +} diff --git a/apps/roam/src/components/canvas/Tldraw.tsx b/apps/roam/src/components/canvas/Tldraw.tsx index 505833d02b..21d02b4695 100644 --- a/apps/roam/src/components/canvas/Tldraw.tsx +++ b/apps/roam/src/components/canvas/Tldraw.tsx @@ -5,6 +5,7 @@ import React, { useEffect, useCallback, } from "react"; +import { baseShapeUtils } from "./baseShapeUtils"; import { Icon } from "@blueprintjs/core"; import ExtensionApiContextProvider, { useExtensionAPI, @@ -24,7 +25,6 @@ import { TldrawUi, defaultBindingUtils, defaultShapeTools, - defaultShapeUtils, defaultTools, useEditor, VecModel, @@ -1339,7 +1339,7 @@ const TldrawCanvasShared = ({ // instanceId={initialState.instanceId} autoFocus={false} initialState="select" - shapeUtils={[...defaultShapeUtils, ...customShapeUtils]} + shapeUtils={[...baseShapeUtils, ...customShapeUtils]} tools={[...defaultTools, ...defaultShapeTools, ...customTools]} bindingUtils={[...defaultBindingUtils, ...customBindingUtils]} components={editorComponents} diff --git a/apps/roam/src/components/canvas/TldrawCanvasCloudflareSync.tsx b/apps/roam/src/components/canvas/TldrawCanvasCloudflareSync.tsx index 3bddf46ae8..67dbd8e944 100644 --- a/apps/roam/src/components/canvas/TldrawCanvasCloudflareSync.tsx +++ b/apps/roam/src/components/canvas/TldrawCanvasCloudflareSync.tsx @@ -1,11 +1,11 @@ import { useSync } from "@tldraw/sync"; +import { baseShapeUtils } from "./baseShapeUtils"; import { TLAnyBindingUtilConstructor, TLAnyShapeUtilConstructor, TLAssetStore, TLStoreWithStatus, defaultBindingUtils, - defaultShapeUtils, MigrationSequence, } from "tldraw"; import { useMemo } from "react"; @@ -68,7 +68,7 @@ export const useCloudflareSyncStore = ({ }): CloudflareCanvasStoreAdapterResult => { const assets = useMemo(() => createRoamAssetStore(), []); const shapeUtils = useMemo( - () => [...defaultShapeUtils, ...customShapeUtils], + () => [...baseShapeUtils, ...customShapeUtils], [customShapeUtils], ); const bindingUtils = useMemo( @@ -80,7 +80,9 @@ export const useCloudflareSyncStore = ({ const uri = useMemo(() => { const roomId = getSyncRoomId({ pageUid }); const query = new URLSearchParams(); - for (const shapeType of customShapeTypes) { + // The worker validates `text` against stock tldraw props unless it is + // declared here, which would reject the url prop text links add. + for (const shapeType of [...customShapeTypes, "text"]) { query.append("shapeType", shapeType); } for (const bindingType of customBindingTypes) { diff --git a/apps/roam/src/components/canvas/baseShapeUtils.ts b/apps/roam/src/components/canvas/baseShapeUtils.ts new file mode 100644 index 0000000000..7992764925 --- /dev/null +++ b/apps/roam/src/components/canvas/baseShapeUtils.ts @@ -0,0 +1,13 @@ +import { + defaultShapeUtils, + TextShapeUtil, + TLAnyShapeUtilConstructor, +} from "tldraw"; +import { TextShapeWithLinkUtil } from "./TextShapeWithLinkUtil"; + +// tldraw throws when a shape type is registered twice, so the stock text util +// has to be replaced rather than appended. Every store must use this same list. +export const baseShapeUtils: TLAnyShapeUtilConstructor[] = + defaultShapeUtils.map((util) => + util === TextShapeUtil ? TextShapeWithLinkUtil : util, + ); diff --git a/apps/roam/src/components/canvas/useRoamStore.ts b/apps/roam/src/components/canvas/useRoamStore.ts index 78989afaa6..724c6caa65 100644 --- a/apps/roam/src/components/canvas/useRoamStore.ts +++ b/apps/roam/src/components/canvas/useRoamStore.ts @@ -1,4 +1,5 @@ import { TLRecord } from "@tldraw/tlschema"; +import { baseShapeUtils } from "./baseShapeUtils"; import nanoid from "nanoid"; import { useRef, useMemo, useEffect, useState } from "react"; import getBasicTreeByParentUid from "roamjs-components/queries/getBasicTreeByParentUid"; @@ -14,7 +15,6 @@ import { import { SerializedStore, StoreSnapshot } from "@tldraw/store"; import { defaultBindingUtils, - defaultShapeUtils, getIndices, loadSnapshot, MigrationSequence, @@ -91,7 +91,7 @@ const createCanvasStore = ({ }): TLStore => createTLStore({ migrations, - shapeUtils: [...defaultShapeUtils, ...customShapeUtils], + shapeUtils: [...baseShapeUtils, ...customShapeUtils], bindingUtils: [...defaultBindingUtils, ...customBindingUtils], }); diff --git a/apps/roam/src/utils/__tests__/textShapeLink.test.ts b/apps/roam/src/utils/__tests__/textShapeLink.test.ts new file mode 100644 index 0000000000..f5f70d1b1c --- /dev/null +++ b/apps/roam/src/utils/__tests__/textShapeLink.test.ts @@ -0,0 +1,121 @@ +import { describe, expect, it } from "vitest"; +import { createTLSchema, defaultShapeSchemas } from "tldraw"; +import { + backfillTextShapeUrl, + hasLinkUrlProp, + isTextShapeRecord, +} from "~/utils/textShapeLink"; + +const makeTextShape = ( + props: Record = {}, +): { + id: string; + typeName: string; + type: string; + props: Record; +} & Record => ({ + id: "shape:t1", + typeName: "shape", + type: "text", + x: 0, + y: 0, + rotation: 0, + index: "a1", + parentId: "page:page", + isLocked: false, + opacity: 1, + meta: {}, + props: { + color: "black", + size: "m", + font: "draw", + textAlign: "middle", + w: 100, + text: "hello", + scale: 1, + autoSize: true, + ...props, + }, +}); + +describe("isTextShapeRecord", () => { + it("matches only text shape records", () => { + expect(isTextShapeRecord(makeTextShape())).toBe(true); + expect(isTextShapeRecord({ ...makeTextShape(), type: "geo" })).toBe(false); + expect(isTextShapeRecord({ typeName: "asset", type: "text" })).toBe(false); + expect(isTextShapeRecord(null)).toBe(false); + }); +}); + +describe("backfillTextShapeUrl", () => { + it("adds an empty url to a shape created before link support", () => { + const shape = makeTextShape(); + expect(hasLinkUrlProp(shape)).toBe(false); + backfillTextShapeUrl(shape); + expect(shape.props.url).toBe(""); + }); + + it("makes the shape eligible for the Edit link action", () => { + const shape = makeTextShape(); + backfillTextShapeUrl(shape); + // tldraw's useHasLinkShapeSelected gates on this exact check + expect(hasLinkUrlProp(shape)).toBe(true); + }); + + it("never overwrites a url the user already set", () => { + const shape = makeTextShape({ url: "https://example.com" }); + backfillTextShapeUrl(shape); + backfillTextShapeUrl(shape); + expect(shape.props.url).toBe("https://example.com"); + }); + + it("leaves non-text shapes untouched", () => { + const geo = { ...makeTextShape(), type: "geo" }; + backfillTextShapeUrl(geo); + expect(hasLinkUrlProp(geo)).toBe(false); + }); +}); + +describe("text shape url persistence", () => { + const extendedSchema = createTLSchema({ + shapes: { + ...defaultShapeSchemas, + text: { + ...defaultShapeSchemas.text, + props: { + ...defaultShapeSchemas.text.props, + url: defaultShapeSchemas.geo.props.url, + }, + }, + }, + }); + + const validate = (shape: unknown) => + extendedSchema.validateRecord( + {} as never, + shape as never, + "initialize", + null, + ); + + it("rejects a pre-link text shape that was never migrated", () => { + expect(() => validate(makeTextShape())).toThrow(/url/); + }); + + it("accepts a migrated text shape and round-trips the link", () => { + const shape = makeTextShape(); + backfillTextShapeUrl(shape); + expect(() => validate(shape)).not.toThrow(); + + shape.props.url = "https://example.com"; + const migrated = extendedSchema.migrateStoreSnapshot({ + store: { [shape.id]: structuredClone(shape) } as never, + schema: extendedSchema.serialize(), + }); + expect(migrated.type).toBe("success"); + const migratedShape = ( + migrated as unknown as { value: Record } + ).value[shape.id]; + expect(migratedShape.props.url).toBe("https://example.com"); + }); +}); diff --git a/apps/roam/src/utils/textShapeLink.ts b/apps/roam/src/utils/textShapeLink.ts new file mode 100644 index 0000000000..35cd9bc674 --- /dev/null +++ b/apps/roam/src/utils/textShapeLink.ts @@ -0,0 +1,27 @@ +type UnknownRecord = { + typeName?: unknown; + type?: unknown; + props?: Record; +}; + +export const isTextShapeRecord = (record: unknown): boolean => { + if (typeof record !== "object" || record === null) return false; + const { typeName, type, props } = record as UnknownRecord; + return typeName === "shape" && type === "text" && typeof props === "object"; +}; + +// tldraw gates its Edit link action on `'url' in shape.props`, so a text shape +// missing the key is silently ineligible rather than failing loudly. +export const hasLinkUrlProp = (record: unknown): boolean => { + if (typeof record !== "object" || record === null) return false; + const { props } = record as UnknownRecord; + return typeof props === "object" && props !== null && "url" in props; +}; + +// Never overwrite an existing url: an earlier migration shipped an +// unconditional assignment and had to be corrected (PR #916). +export const backfillTextShapeUrl = (record: unknown): void => { + if (!isTextShapeRecord(record)) return; + const { props } = record as Required; + if (props.url === undefined) props.url = ""; +}; From 9c42b8423b6497b8d4c3358d65416021bf921ed9 Mon Sep 17 00:00:00 2001 From: Trang Doan Date: Mon, 14 Sep 2026 11:16:39 -0400 Subject: [PATCH 02/10] Fix sync room schema and move the text link icon out of the glyphs Declaring "text" to the sync worker as if it were a custom shape type blanked its schema entry to {}, which dropped textShapeMigrations. The room then reported com.tldraw.shape.text version 0 against every client's 2, so getMigrationsSince returned 'Incompatible schema?' and every client was rejected on connect while existing rooms failed to load. The worker now extends the default text schema instead, keeping its migrations, and refuses to blank any default shape type. Covered by syncWorkerRoomSchema.test.ts. A text shape's bounds hug its glyphs, so tldraw's in-bounds link button landed on the last word. Sit it just outside the right edge instead. Also fix a missing separator that merged the two hyperlink button class names into one invalid token, guard isTextShapeRecord against a null props object, and assert that the text util was actually replaced. Co-Authored-By: Claude Opus 5 --- .../canvas/TextShapeWithLinkUtil.tsx | 10 ++- .../canvas/TldrawCanvasCloudflareSync.tsx | 4 +- .../src/components/canvas/baseShapeUtils.ts | 6 ++ .../src/components/canvas/tldrawStyles.ts | 8 ++ .../__tests__/syncWorkerRoomSchema.test.ts | 69 ++++++++++++++ .../__tests__/textShapeLinkStore.test.ts | 89 +++++++++++++++++++ apps/roam/src/utils/textShapeLink.ts | 7 +- .../worker/TldrawDurableObject.ts | 21 ++++- 8 files changed, 205 insertions(+), 9 deletions(-) create mode 100644 apps/roam/src/utils/__tests__/syncWorkerRoomSchema.test.ts create mode 100644 apps/roam/src/utils/__tests__/textShapeLinkStore.test.ts diff --git a/apps/roam/src/components/canvas/TextShapeWithLinkUtil.tsx b/apps/roam/src/components/canvas/TextShapeWithLinkUtil.tsx index 208352e750..faef5a67ee 100644 --- a/apps/roam/src/components/canvas/TextShapeWithLinkUtil.tsx +++ b/apps/roam/src/components/canvas/TextShapeWithLinkUtil.tsx @@ -22,9 +22,13 @@ const HyperlinkButton = ({ zoomLevel: number; }): JSX.Element => ( { const roomId = getSyncRoomId({ pageUid }); const query = new URLSearchParams(); - // The worker validates `text` against stock tldraw props unless it is - // declared here, which would reject the url prop text links add. - for (const shapeType of [...customShapeTypes, "text"]) { + for (const shapeType of customShapeTypes) { query.append("shapeType", shapeType); } for (const bindingType of customBindingTypes) { diff --git a/apps/roam/src/components/canvas/baseShapeUtils.ts b/apps/roam/src/components/canvas/baseShapeUtils.ts index 7992764925..521054ffa8 100644 --- a/apps/roam/src/components/canvas/baseShapeUtils.ts +++ b/apps/roam/src/components/canvas/baseShapeUtils.ts @@ -11,3 +11,9 @@ export const baseShapeUtils: TLAnyShapeUtilConstructor[] = defaultShapeUtils.map((util) => util === TextShapeUtil ? TextShapeWithLinkUtil : util, ); + +// Fail loudly rather than shipping a store whose schema lacks `url` while the +// UI still writes it, which would only surface as validation errors on save. +if (!baseShapeUtils.includes(TextShapeWithLinkUtil)) { + throw new Error("Failed to replace TextShapeUtil in the default shape utils"); +} diff --git a/apps/roam/src/components/canvas/tldrawStyles.ts b/apps/roam/src/components/canvas/tldrawStyles.ts index 03a5265e7d..0a771e78e1 100644 --- a/apps/roam/src/components/canvas/tldrawStyles.ts +++ b/apps/roam/src/components/canvas/tldrawStyles.ts @@ -10,6 +10,14 @@ export default /* css */ ` display: none; } + /* A text shape's bounds hug its glyphs, so tldraw's in-bounds link button + lands on the last word; sit it just outside the right edge instead. */ + .dg-text-link-button { + top: 50%; + right: 0; + transform: translate(100%, -50%); + } + /* Shape Render Fix */ svg.tl-svg-container { overflow: visible; diff --git a/apps/roam/src/utils/__tests__/syncWorkerRoomSchema.test.ts b/apps/roam/src/utils/__tests__/syncWorkerRoomSchema.test.ts new file mode 100644 index 0000000000..592f69b743 --- /dev/null +++ b/apps/roam/src/utils/__tests__/syncWorkerRoomSchema.test.ts @@ -0,0 +1,69 @@ +import { describe, expect, it } from "vitest"; +import { createTLSchema, defaultShapeSchemas } from "tldraw"; +import { baseShapeUtils } from "~/components/canvas/baseShapeUtils"; + +// Mirrors apps/tldraw-sync-worker/worker/TldrawDurableObject.ts. The worker +// blanks declared shape types to {}, which is safe only for types the client +// also registers without migrations. Blanking a default type drops its +// migration sequence, so the room reports version 0 against the client's 2 and +// every client is rejected as too old. +const textUtil = baseShapeUtils.find( + (util) => util.type === "text", +) as unknown as { + props: never; + migrations: never; +}; + +const clientSchema = createTLSchema({ + shapes: { + ...defaultShapeSchemas, + text: { props: textUtil.props, migrations: textUtil.migrations }, + "discourse-node": {}, + } as never, +}); + +const textSequenceVersion = (schema: ReturnType) => + (schema.serialize() as unknown as { sequences: Record }) + .sequences["com.tldraw.shape.text"]; + +describe("sync worker room schema", () => { + it("stays migration-compatible when text keeps its own migrations", () => { + const workerSchema = createTLSchema({ + shapes: { + ...defaultShapeSchemas, + text: { + ...defaultShapeSchemas.text, + props: { + ...defaultShapeSchemas.text.props, + url: defaultShapeSchemas.geo.props.url, + }, + }, + "discourse-node": {}, + } as never, + }); + + expect(textSequenceVersion(workerSchema)).toBe( + textSequenceVersion(clientSchema), + ); + expect( + workerSchema.migrateStoreSnapshot({ + store: {} as never, + schema: clientSchema.serialize(), + }).type, + ).toBe("success"); + }); + + it("breaks if text is blanked to {} the way custom types are", () => { + const brokenSchema = createTLSchema({ + shapes: { ...defaultShapeSchemas, text: {} } as never, + }); + + expect(textSequenceVersion(brokenSchema)).toBe(0); + expect( + brokenSchema.migrateStoreSnapshot({ + store: {} as never, + schema: clientSchema.serialize(), + }).type, + ).toBe("error"); + }); +}); diff --git a/apps/roam/src/utils/__tests__/textShapeLinkStore.test.ts b/apps/roam/src/utils/__tests__/textShapeLinkStore.test.ts new file mode 100644 index 0000000000..a0e8e65440 --- /dev/null +++ b/apps/roam/src/utils/__tests__/textShapeLinkStore.test.ts @@ -0,0 +1,89 @@ +import { describe, expect, it } from "vitest"; +import { createTLSchema, defaultShapeSchemas, TextShapeUtil } from "tldraw"; +import { baseShapeUtils } from "~/components/canvas/baseShapeUtils"; +import { TextShapeWithLinkUtil } from "~/components/canvas/TextShapeWithLinkUtil"; + +// Built the way createTLStore derives a schema from its utils, so this exercises +// the real util rather than a hand-rolled copy of its props. +const textUtil = baseShapeUtils.find((util) => util.type === "text"); + +const schemaFromUtils = createTLSchema({ + shapes: { + ...defaultShapeSchemas, + text: { + props: (textUtil as unknown as { props: never }).props, + migrations: (textUtil as unknown as { migrations: never }).migrations, + }, + }, +}); + +const makeTextShape = (url?: string) => ({ + id: "shape:t1", + typeName: "shape", + type: "text", + x: 0, + y: 0, + rotation: 0, + index: "a1", + parentId: "page:page", + isLocked: false, + opacity: 1, + meta: {}, + props: { + color: "black", + size: "m", + font: "draw", + textAlign: "middle", + w: 100, + text: "hello", + scale: 1, + autoSize: true, + ...(url === undefined ? {} : { url }), + }, +}); + +describe("baseShapeUtils", () => { + it("replaces the stock text util exactly once", () => { + expect(textUtil).toBe(TextShapeWithLinkUtil); + expect(baseShapeUtils).not.toContain(TextShapeUtil); + expect(baseShapeUtils.filter((u) => u.type === "text")).toHaveLength(1); + }); + + it("keeps every other default util", () => { + expect(baseShapeUtils).toHaveLength(12); + }); +}); + +describe("schema derived from the real util", () => { + const validate = (shape: unknown) => + schemaFromUtils.validateRecord( + {} as never, + shape as never, + "initialize", + null, + ); + + it("accepts a text shape carrying a link", () => { + expect(() => validate(makeTextShape("https://example.com"))).not.toThrow(); + }); + + it("accepts an empty url, the value the migration backfills", () => { + expect(() => validate(makeTextShape(""))).not.toThrow(); + }); + + it("rejects a text shape that never got the url prop", () => { + expect(() => validate(makeTextShape())).toThrow(/url/); + }); + + it("rejects a non-http url", () => { + expect(() => validate(makeTextShape("javascript:alert(1)"))).toThrow(/url/); + }); + + it("gives new text shapes a url so they are link-eligible", () => { + const defaults = new TextShapeWithLinkUtil( + {} as never, + ).getDefaultProps() as { url: string }; + expect("url" in defaults).toBe(true); + expect(defaults.url).toBe(""); + }); +}); diff --git a/apps/roam/src/utils/textShapeLink.ts b/apps/roam/src/utils/textShapeLink.ts index 35cd9bc674..88b4646fec 100644 --- a/apps/roam/src/utils/textShapeLink.ts +++ b/apps/roam/src/utils/textShapeLink.ts @@ -7,7 +7,12 @@ type UnknownRecord = { export const isTextShapeRecord = (record: unknown): boolean => { if (typeof record !== "object" || record === null) return false; const { typeName, type, props } = record as UnknownRecord; - return typeName === "shape" && type === "text" && typeof props === "object"; + return ( + typeName === "shape" && + type === "text" && + typeof props === "object" && + props !== null + ); }; // tldraw gates its Edit link action on `'url' in shape.props`, so a text shape diff --git a/apps/tldraw-sync-worker/worker/TldrawDurableObject.ts b/apps/tldraw-sync-worker/worker/TldrawDurableObject.ts index 4b0e2117ac..b4a9884b6d 100644 --- a/apps/tldraw-sync-worker/worker/TldrawDurableObject.ts +++ b/apps/tldraw-sync-worker/worker/TldrawDurableObject.ts @@ -16,17 +16,34 @@ type RoomSchemaConfig = { const STORAGE_SCHEMA_CONFIG_KEY = "schemaConfig"; +// Text shapes carry a link url; reuse geo's validator rather than redeclaring it. +const textShapeSchema = { + ...defaultShapeSchemas.text, + props: { + ...defaultShapeSchemas.text.props, + url: defaultShapeSchemas.geo.props.url, + }, +}; + const createRoomSchema = ({ shapeTypes, bindingTypes }: RoomSchemaConfig) => { + // A default shape type must keep its own props and migrations. Blanking one + // to {} drops its migration sequence, so the room reports version 0 while + // every client reports 2 and all of them are rejected as too old. const customShapeSchemas = Object.fromEntries( - shapeTypes.map((type) => [type, {}]), + shapeTypes + .filter((type) => !(type in defaultShapeSchemas)) + .map((type) => [type, {}]), ); const customBindingSchemas = Object.fromEntries( - bindingTypes.map((type) => [type, {}]), + bindingTypes + .filter((type) => !(type in defaultBindingSchemas)) + .map((type) => [type, {}]), ); return createTLSchema({ shapes: { ...defaultShapeSchemas, + text: textShapeSchema, ...customShapeSchemas, }, bindings: { From c11d41719d3d1e42541451e536108fa5d225c1b2 Mon Sep 17 00:00:00 2001 From: Trang Doan Date: Fri, 18 Sep 2026 16:38:51 -0400 Subject: [PATCH 03/10] Remove unnecessary type assertions in sync worker schema test Co-Authored-By: Claude Opus 5 --- apps/roam/src/utils/__tests__/syncWorkerRoomSchema.test.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/apps/roam/src/utils/__tests__/syncWorkerRoomSchema.test.ts b/apps/roam/src/utils/__tests__/syncWorkerRoomSchema.test.ts index 592f69b743..25fe47304e 100644 --- a/apps/roam/src/utils/__tests__/syncWorkerRoomSchema.test.ts +++ b/apps/roam/src/utils/__tests__/syncWorkerRoomSchema.test.ts @@ -19,7 +19,7 @@ const clientSchema = createTLSchema({ ...defaultShapeSchemas, text: { props: textUtil.props, migrations: textUtil.migrations }, "discourse-node": {}, - } as never, + }, }); const textSequenceVersion = (schema: ReturnType) => @@ -39,7 +39,7 @@ describe("sync worker room schema", () => { }, }, "discourse-node": {}, - } as never, + }, }); expect(textSequenceVersion(workerSchema)).toBe( @@ -55,7 +55,7 @@ describe("sync worker room schema", () => { it("breaks if text is blanked to {} the way custom types are", () => { const brokenSchema = createTLSchema({ - shapes: { ...defaultShapeSchemas, text: {} } as never, + shapes: { ...defaultShapeSchemas, text: {} }, }); expect(textSequenceVersion(brokenSchema)).toBe(0); From fe3c62668c0ef692a3f57dc74ef104d1b3002cab Mon Sep 17 00:00:00 2001 From: Trang Doan Date: Wed, 30 Sep 2026 21:42:05 -0400 Subject: [PATCH 04/10] Revert props-schema approach for text shape links Michael's review of the Obsidian twin (ENG-2228, #1442) asked to store the link in meta.url and reuse tldraw's own link UI instead of changing the text shape schema, migrating, and copying components. The same reasoning applies here more strongly: a Roam graph has collaborators on different extension versions, and each sequence bump locks out whoever has not updated yet. This removes the custom text shape util, the shared util list, migration v6 and the sync worker change. The meta-based implementation follows. Co-Authored-By: Claude Opus 5.5 --- apps/roam/src/components/Export.tsx | 4 +- .../discourseRelationMigrations.ts | 13 -- .../canvas/TextShapeWithLinkUtil.tsx | 79 ------------ apps/roam/src/components/canvas/Tldraw.tsx | 4 +- .../canvas/TldrawCanvasCloudflareSync.tsx | 4 +- .../src/components/canvas/baseShapeUtils.ts | 19 --- .../src/components/canvas/tldrawStyles.ts | 8 -- .../src/components/canvas/useRoamStore.ts | 4 +- .../__tests__/syncWorkerRoomSchema.test.ts | 69 ---------- .../src/utils/__tests__/textShapeLink.test.ts | 121 ------------------ .../__tests__/textShapeLinkStore.test.ts | 89 ------------- apps/roam/src/utils/textShapeLink.ts | 32 ----- .../worker/TldrawDurableObject.ts | 21 +-- 13 files changed, 10 insertions(+), 457 deletions(-) delete mode 100644 apps/roam/src/components/canvas/TextShapeWithLinkUtil.tsx delete mode 100644 apps/roam/src/components/canvas/baseShapeUtils.ts delete mode 100644 apps/roam/src/utils/__tests__/syncWorkerRoomSchema.test.ts delete mode 100644 apps/roam/src/utils/__tests__/textShapeLink.test.ts delete mode 100644 apps/roam/src/utils/__tests__/textShapeLinkStore.test.ts delete mode 100644 apps/roam/src/utils/textShapeLink.ts diff --git a/apps/roam/src/components/Export.tsx b/apps/roam/src/components/Export.tsx index fe6925d7fc..cb0567d71e 100644 --- a/apps/roam/src/components/Export.tsx +++ b/apps/roam/src/components/Export.tsx @@ -16,7 +16,6 @@ import { Radio, FormGroup, } from "@blueprintjs/core"; -import { baseShapeUtils } from "~/components/canvas/baseShapeUtils"; import React, { useState, useEffect, useMemo, FormEvent } from "react"; import MenuItemSelect from "roamjs-components/components/MenuItemSelect"; import { saveAs } from "file-saver"; @@ -51,6 +50,7 @@ import { TLParentId, getIndexAbove, TLShape, + defaultShapeUtils, defaultBindingUtils, } from "tldraw"; import { @@ -433,7 +433,7 @@ const ExportDialog: ExportDialogComponent = ({ const tlStore = createTLStore({ migrations, - shapeUtils: [...baseShapeUtils, ...customShapeUtils], + shapeUtils: [...defaultShapeUtils, ...customShapeUtils], bindingUtils: [...defaultBindingUtils, ...customBindingUtils], }); diff --git a/apps/roam/src/components/canvas/DiscourseRelationShape/discourseRelationMigrations.ts b/apps/roam/src/components/canvas/DiscourseRelationShape/discourseRelationMigrations.ts index 860ba76fad..9e49a075ac 100644 --- a/apps/roam/src/components/canvas/DiscourseRelationShape/discourseRelationMigrations.ts +++ b/apps/roam/src/components/canvas/DiscourseRelationShape/discourseRelationMigrations.ts @@ -18,7 +18,6 @@ import { createMigrationIds } from "tldraw"; import { RelationBinding } from "./DiscourseRelationBindings"; import { getRelationColor } from "./DiscourseRelationUtil"; import { DISCOURSE_NODE_SHAPE_TYPE } from "~/components/canvas/DiscourseNodeUtil"; -import { backfillTextShapeUrl, isTextShapeRecord } from "~/utils/textShapeLink"; const SEQUENCE_ID_BASE = "com.roam-research.discourse-graphs"; @@ -53,7 +52,6 @@ export const createMigrations = ({ AddSizeAndFontFamily: 3, RemoveNullAssetFileSize: 4, MigrateNodeTypeToDiscourseNode: 5, - AddTextShapeUrl: 6, }); return createMigrationSequence({ sequenceId: `${SEQUENCE_ID_BASE}`, @@ -205,17 +203,6 @@ export const createMigrations = ({ shape.type = DISCOURSE_NODE_SHAPE_TYPE; }, }, - { - id: versions["AddTextShapeUrl"], - scope: "record", - filter: (r: any) => isTextShapeRecord(r), - up: (shape: any) => { - backfillTextShapeUrl(shape); - }, - down: (shape: any) => { - delete shape.props.url; - }, - }, ], }); }; diff --git a/apps/roam/src/components/canvas/TextShapeWithLinkUtil.tsx b/apps/roam/src/components/canvas/TextShapeWithLinkUtil.tsx deleted file mode 100644 index faef5a67ee..0000000000 --- a/apps/roam/src/components/canvas/TextShapeWithLinkUtil.tsx +++ /dev/null @@ -1,79 +0,0 @@ -import React from "react"; -import { - T, - TextShapeUtil, - TLTextShape, - stopEventPropagation, - textShapeProps, -} from "tldraw"; - -export type TextShapeWithLinkProps = TLTextShape["props"] & { url: string }; - -const LINK_ICON = - "data:image/svg+xml,%3Csvg xmlns='http://www.w3.org/2000/svg' width='30' height='30' fill='none'%3E%3Cpath stroke='%23000' stroke-linecap='round' stroke-linejoin='round' stroke-width='2' d='M13 5H7a2 2 0 0 0-2 2v16a2 2 0 0 0 2 2h16a2 2 0 0 0 2-2v-6M19 5h6m0 0v6m0-6L13 17'/%3E%3C/svg%3E"; - -// tldraw does not export HyperlinkButton, so this mirrors its markup to keep -// text links visually identical to geo links. -const HyperlinkButton = ({ - url, - zoomLevel, -}: { - url: string; - zoomLevel: number; -}): JSX.Element => ( - -
- -); - -export const getTextShapeUrl = (shape: TLTextShape): string => - (shape.props as Partial).url ?? ""; - -const textShapeWithLinkProps = { ...textShapeProps, url: T.linkUrl }; - -// Adds the `url` prop that tldraw's built-in Edit link action gates on -// (`'url' in shape.props`), so text shapes reuse the geo link UI unchanged. -export class TextShapeWithLinkUtil extends TextShapeUtil { - static override props = textShapeWithLinkProps; - - override getDefaultProps(): TextShapeWithLinkProps { - return { ...super.getDefaultProps(), url: "" }; - } - - // The cast bridges two React type copies in the dependency tree, which make - // the base signature's JSX.Element nominally distinct from ours. - override component( - shape: TLTextShape, - ): ReturnType { - const url = getTextShapeUrl(shape); - return ( - <> - {super.component(shape)} - {url && ( - - )} - - ) as ReturnType; - } -} diff --git a/apps/roam/src/components/canvas/Tldraw.tsx b/apps/roam/src/components/canvas/Tldraw.tsx index 21d02b4695..505833d02b 100644 --- a/apps/roam/src/components/canvas/Tldraw.tsx +++ b/apps/roam/src/components/canvas/Tldraw.tsx @@ -5,7 +5,6 @@ import React, { useEffect, useCallback, } from "react"; -import { baseShapeUtils } from "./baseShapeUtils"; import { Icon } from "@blueprintjs/core"; import ExtensionApiContextProvider, { useExtensionAPI, @@ -25,6 +24,7 @@ import { TldrawUi, defaultBindingUtils, defaultShapeTools, + defaultShapeUtils, defaultTools, useEditor, VecModel, @@ -1339,7 +1339,7 @@ const TldrawCanvasShared = ({ // instanceId={initialState.instanceId} autoFocus={false} initialState="select" - shapeUtils={[...baseShapeUtils, ...customShapeUtils]} + shapeUtils={[...defaultShapeUtils, ...customShapeUtils]} tools={[...defaultTools, ...defaultShapeTools, ...customTools]} bindingUtils={[...defaultBindingUtils, ...customBindingUtils]} components={editorComponents} diff --git a/apps/roam/src/components/canvas/TldrawCanvasCloudflareSync.tsx b/apps/roam/src/components/canvas/TldrawCanvasCloudflareSync.tsx index d9ef16aa07..3bddf46ae8 100644 --- a/apps/roam/src/components/canvas/TldrawCanvasCloudflareSync.tsx +++ b/apps/roam/src/components/canvas/TldrawCanvasCloudflareSync.tsx @@ -1,11 +1,11 @@ import { useSync } from "@tldraw/sync"; -import { baseShapeUtils } from "./baseShapeUtils"; import { TLAnyBindingUtilConstructor, TLAnyShapeUtilConstructor, TLAssetStore, TLStoreWithStatus, defaultBindingUtils, + defaultShapeUtils, MigrationSequence, } from "tldraw"; import { useMemo } from "react"; @@ -68,7 +68,7 @@ export const useCloudflareSyncStore = ({ }): CloudflareCanvasStoreAdapterResult => { const assets = useMemo(() => createRoamAssetStore(), []); const shapeUtils = useMemo( - () => [...baseShapeUtils, ...customShapeUtils], + () => [...defaultShapeUtils, ...customShapeUtils], [customShapeUtils], ); const bindingUtils = useMemo( diff --git a/apps/roam/src/components/canvas/baseShapeUtils.ts b/apps/roam/src/components/canvas/baseShapeUtils.ts deleted file mode 100644 index 521054ffa8..0000000000 --- a/apps/roam/src/components/canvas/baseShapeUtils.ts +++ /dev/null @@ -1,19 +0,0 @@ -import { - defaultShapeUtils, - TextShapeUtil, - TLAnyShapeUtilConstructor, -} from "tldraw"; -import { TextShapeWithLinkUtil } from "./TextShapeWithLinkUtil"; - -// tldraw throws when a shape type is registered twice, so the stock text util -// has to be replaced rather than appended. Every store must use this same list. -export const baseShapeUtils: TLAnyShapeUtilConstructor[] = - defaultShapeUtils.map((util) => - util === TextShapeUtil ? TextShapeWithLinkUtil : util, - ); - -// Fail loudly rather than shipping a store whose schema lacks `url` while the -// UI still writes it, which would only surface as validation errors on save. -if (!baseShapeUtils.includes(TextShapeWithLinkUtil)) { - throw new Error("Failed to replace TextShapeUtil in the default shape utils"); -} diff --git a/apps/roam/src/components/canvas/tldrawStyles.ts b/apps/roam/src/components/canvas/tldrawStyles.ts index 0a771e78e1..03a5265e7d 100644 --- a/apps/roam/src/components/canvas/tldrawStyles.ts +++ b/apps/roam/src/components/canvas/tldrawStyles.ts @@ -10,14 +10,6 @@ export default /* css */ ` display: none; } - /* A text shape's bounds hug its glyphs, so tldraw's in-bounds link button - lands on the last word; sit it just outside the right edge instead. */ - .dg-text-link-button { - top: 50%; - right: 0; - transform: translate(100%, -50%); - } - /* Shape Render Fix */ svg.tl-svg-container { overflow: visible; diff --git a/apps/roam/src/components/canvas/useRoamStore.ts b/apps/roam/src/components/canvas/useRoamStore.ts index 724c6caa65..78989afaa6 100644 --- a/apps/roam/src/components/canvas/useRoamStore.ts +++ b/apps/roam/src/components/canvas/useRoamStore.ts @@ -1,5 +1,4 @@ import { TLRecord } from "@tldraw/tlschema"; -import { baseShapeUtils } from "./baseShapeUtils"; import nanoid from "nanoid"; import { useRef, useMemo, useEffect, useState } from "react"; import getBasicTreeByParentUid from "roamjs-components/queries/getBasicTreeByParentUid"; @@ -15,6 +14,7 @@ import { import { SerializedStore, StoreSnapshot } from "@tldraw/store"; import { defaultBindingUtils, + defaultShapeUtils, getIndices, loadSnapshot, MigrationSequence, @@ -91,7 +91,7 @@ const createCanvasStore = ({ }): TLStore => createTLStore({ migrations, - shapeUtils: [...baseShapeUtils, ...customShapeUtils], + shapeUtils: [...defaultShapeUtils, ...customShapeUtils], bindingUtils: [...defaultBindingUtils, ...customBindingUtils], }); diff --git a/apps/roam/src/utils/__tests__/syncWorkerRoomSchema.test.ts b/apps/roam/src/utils/__tests__/syncWorkerRoomSchema.test.ts deleted file mode 100644 index 25fe47304e..0000000000 --- a/apps/roam/src/utils/__tests__/syncWorkerRoomSchema.test.ts +++ /dev/null @@ -1,69 +0,0 @@ -import { describe, expect, it } from "vitest"; -import { createTLSchema, defaultShapeSchemas } from "tldraw"; -import { baseShapeUtils } from "~/components/canvas/baseShapeUtils"; - -// Mirrors apps/tldraw-sync-worker/worker/TldrawDurableObject.ts. The worker -// blanks declared shape types to {}, which is safe only for types the client -// also registers without migrations. Blanking a default type drops its -// migration sequence, so the room reports version 0 against the client's 2 and -// every client is rejected as too old. -const textUtil = baseShapeUtils.find( - (util) => util.type === "text", -) as unknown as { - props: never; - migrations: never; -}; - -const clientSchema = createTLSchema({ - shapes: { - ...defaultShapeSchemas, - text: { props: textUtil.props, migrations: textUtil.migrations }, - "discourse-node": {}, - }, -}); - -const textSequenceVersion = (schema: ReturnType) => - (schema.serialize() as unknown as { sequences: Record }) - .sequences["com.tldraw.shape.text"]; - -describe("sync worker room schema", () => { - it("stays migration-compatible when text keeps its own migrations", () => { - const workerSchema = createTLSchema({ - shapes: { - ...defaultShapeSchemas, - text: { - ...defaultShapeSchemas.text, - props: { - ...defaultShapeSchemas.text.props, - url: defaultShapeSchemas.geo.props.url, - }, - }, - "discourse-node": {}, - }, - }); - - expect(textSequenceVersion(workerSchema)).toBe( - textSequenceVersion(clientSchema), - ); - expect( - workerSchema.migrateStoreSnapshot({ - store: {} as never, - schema: clientSchema.serialize(), - }).type, - ).toBe("success"); - }); - - it("breaks if text is blanked to {} the way custom types are", () => { - const brokenSchema = createTLSchema({ - shapes: { ...defaultShapeSchemas, text: {} }, - }); - - expect(textSequenceVersion(brokenSchema)).toBe(0); - expect( - brokenSchema.migrateStoreSnapshot({ - store: {} as never, - schema: clientSchema.serialize(), - }).type, - ).toBe("error"); - }); -}); diff --git a/apps/roam/src/utils/__tests__/textShapeLink.test.ts b/apps/roam/src/utils/__tests__/textShapeLink.test.ts deleted file mode 100644 index f5f70d1b1c..0000000000 --- a/apps/roam/src/utils/__tests__/textShapeLink.test.ts +++ /dev/null @@ -1,121 +0,0 @@ -import { describe, expect, it } from "vitest"; -import { createTLSchema, defaultShapeSchemas } from "tldraw"; -import { - backfillTextShapeUrl, - hasLinkUrlProp, - isTextShapeRecord, -} from "~/utils/textShapeLink"; - -const makeTextShape = ( - props: Record = {}, -): { - id: string; - typeName: string; - type: string; - props: Record; -} & Record => ({ - id: "shape:t1", - typeName: "shape", - type: "text", - x: 0, - y: 0, - rotation: 0, - index: "a1", - parentId: "page:page", - isLocked: false, - opacity: 1, - meta: {}, - props: { - color: "black", - size: "m", - font: "draw", - textAlign: "middle", - w: 100, - text: "hello", - scale: 1, - autoSize: true, - ...props, - }, -}); - -describe("isTextShapeRecord", () => { - it("matches only text shape records", () => { - expect(isTextShapeRecord(makeTextShape())).toBe(true); - expect(isTextShapeRecord({ ...makeTextShape(), type: "geo" })).toBe(false); - expect(isTextShapeRecord({ typeName: "asset", type: "text" })).toBe(false); - expect(isTextShapeRecord(null)).toBe(false); - }); -}); - -describe("backfillTextShapeUrl", () => { - it("adds an empty url to a shape created before link support", () => { - const shape = makeTextShape(); - expect(hasLinkUrlProp(shape)).toBe(false); - backfillTextShapeUrl(shape); - expect(shape.props.url).toBe(""); - }); - - it("makes the shape eligible for the Edit link action", () => { - const shape = makeTextShape(); - backfillTextShapeUrl(shape); - // tldraw's useHasLinkShapeSelected gates on this exact check - expect(hasLinkUrlProp(shape)).toBe(true); - }); - - it("never overwrites a url the user already set", () => { - const shape = makeTextShape({ url: "https://example.com" }); - backfillTextShapeUrl(shape); - backfillTextShapeUrl(shape); - expect(shape.props.url).toBe("https://example.com"); - }); - - it("leaves non-text shapes untouched", () => { - const geo = { ...makeTextShape(), type: "geo" }; - backfillTextShapeUrl(geo); - expect(hasLinkUrlProp(geo)).toBe(false); - }); -}); - -describe("text shape url persistence", () => { - const extendedSchema = createTLSchema({ - shapes: { - ...defaultShapeSchemas, - text: { - ...defaultShapeSchemas.text, - props: { - ...defaultShapeSchemas.text.props, - url: defaultShapeSchemas.geo.props.url, - }, - }, - }, - }); - - const validate = (shape: unknown) => - extendedSchema.validateRecord( - {} as never, - shape as never, - "initialize", - null, - ); - - it("rejects a pre-link text shape that was never migrated", () => { - expect(() => validate(makeTextShape())).toThrow(/url/); - }); - - it("accepts a migrated text shape and round-trips the link", () => { - const shape = makeTextShape(); - backfillTextShapeUrl(shape); - expect(() => validate(shape)).not.toThrow(); - - shape.props.url = "https://example.com"; - const migrated = extendedSchema.migrateStoreSnapshot({ - store: { [shape.id]: structuredClone(shape) } as never, - schema: extendedSchema.serialize(), - }); - expect(migrated.type).toBe("success"); - const migratedShape = ( - migrated as unknown as { value: Record } - ).value[shape.id]; - expect(migratedShape.props.url).toBe("https://example.com"); - }); -}); diff --git a/apps/roam/src/utils/__tests__/textShapeLinkStore.test.ts b/apps/roam/src/utils/__tests__/textShapeLinkStore.test.ts deleted file mode 100644 index a0e8e65440..0000000000 --- a/apps/roam/src/utils/__tests__/textShapeLinkStore.test.ts +++ /dev/null @@ -1,89 +0,0 @@ -import { describe, expect, it } from "vitest"; -import { createTLSchema, defaultShapeSchemas, TextShapeUtil } from "tldraw"; -import { baseShapeUtils } from "~/components/canvas/baseShapeUtils"; -import { TextShapeWithLinkUtil } from "~/components/canvas/TextShapeWithLinkUtil"; - -// Built the way createTLStore derives a schema from its utils, so this exercises -// the real util rather than a hand-rolled copy of its props. -const textUtil = baseShapeUtils.find((util) => util.type === "text"); - -const schemaFromUtils = createTLSchema({ - shapes: { - ...defaultShapeSchemas, - text: { - props: (textUtil as unknown as { props: never }).props, - migrations: (textUtil as unknown as { migrations: never }).migrations, - }, - }, -}); - -const makeTextShape = (url?: string) => ({ - id: "shape:t1", - typeName: "shape", - type: "text", - x: 0, - y: 0, - rotation: 0, - index: "a1", - parentId: "page:page", - isLocked: false, - opacity: 1, - meta: {}, - props: { - color: "black", - size: "m", - font: "draw", - textAlign: "middle", - w: 100, - text: "hello", - scale: 1, - autoSize: true, - ...(url === undefined ? {} : { url }), - }, -}); - -describe("baseShapeUtils", () => { - it("replaces the stock text util exactly once", () => { - expect(textUtil).toBe(TextShapeWithLinkUtil); - expect(baseShapeUtils).not.toContain(TextShapeUtil); - expect(baseShapeUtils.filter((u) => u.type === "text")).toHaveLength(1); - }); - - it("keeps every other default util", () => { - expect(baseShapeUtils).toHaveLength(12); - }); -}); - -describe("schema derived from the real util", () => { - const validate = (shape: unknown) => - schemaFromUtils.validateRecord( - {} as never, - shape as never, - "initialize", - null, - ); - - it("accepts a text shape carrying a link", () => { - expect(() => validate(makeTextShape("https://example.com"))).not.toThrow(); - }); - - it("accepts an empty url, the value the migration backfills", () => { - expect(() => validate(makeTextShape(""))).not.toThrow(); - }); - - it("rejects a text shape that never got the url prop", () => { - expect(() => validate(makeTextShape())).toThrow(/url/); - }); - - it("rejects a non-http url", () => { - expect(() => validate(makeTextShape("javascript:alert(1)"))).toThrow(/url/); - }); - - it("gives new text shapes a url so they are link-eligible", () => { - const defaults = new TextShapeWithLinkUtil( - {} as never, - ).getDefaultProps() as { url: string }; - expect("url" in defaults).toBe(true); - expect(defaults.url).toBe(""); - }); -}); diff --git a/apps/roam/src/utils/textShapeLink.ts b/apps/roam/src/utils/textShapeLink.ts deleted file mode 100644 index 88b4646fec..0000000000 --- a/apps/roam/src/utils/textShapeLink.ts +++ /dev/null @@ -1,32 +0,0 @@ -type UnknownRecord = { - typeName?: unknown; - type?: unknown; - props?: Record; -}; - -export const isTextShapeRecord = (record: unknown): boolean => { - if (typeof record !== "object" || record === null) return false; - const { typeName, type, props } = record as UnknownRecord; - return ( - typeName === "shape" && - type === "text" && - typeof props === "object" && - props !== null - ); -}; - -// tldraw gates its Edit link action on `'url' in shape.props`, so a text shape -// missing the key is silently ineligible rather than failing loudly. -export const hasLinkUrlProp = (record: unknown): boolean => { - if (typeof record !== "object" || record === null) return false; - const { props } = record as UnknownRecord; - return typeof props === "object" && props !== null && "url" in props; -}; - -// Never overwrite an existing url: an earlier migration shipped an -// unconditional assignment and had to be corrected (PR #916). -export const backfillTextShapeUrl = (record: unknown): void => { - if (!isTextShapeRecord(record)) return; - const { props } = record as Required; - if (props.url === undefined) props.url = ""; -}; diff --git a/apps/tldraw-sync-worker/worker/TldrawDurableObject.ts b/apps/tldraw-sync-worker/worker/TldrawDurableObject.ts index b4a9884b6d..4b0e2117ac 100644 --- a/apps/tldraw-sync-worker/worker/TldrawDurableObject.ts +++ b/apps/tldraw-sync-worker/worker/TldrawDurableObject.ts @@ -16,34 +16,17 @@ type RoomSchemaConfig = { const STORAGE_SCHEMA_CONFIG_KEY = "schemaConfig"; -// Text shapes carry a link url; reuse geo's validator rather than redeclaring it. -const textShapeSchema = { - ...defaultShapeSchemas.text, - props: { - ...defaultShapeSchemas.text.props, - url: defaultShapeSchemas.geo.props.url, - }, -}; - const createRoomSchema = ({ shapeTypes, bindingTypes }: RoomSchemaConfig) => { - // A default shape type must keep its own props and migrations. Blanking one - // to {} drops its migration sequence, so the room reports version 0 while - // every client reports 2 and all of them are rejected as too old. const customShapeSchemas = Object.fromEntries( - shapeTypes - .filter((type) => !(type in defaultShapeSchemas)) - .map((type) => [type, {}]), + shapeTypes.map((type) => [type, {}]), ); const customBindingSchemas = Object.fromEntries( - bindingTypes - .filter((type) => !(type in defaultBindingSchemas)) - .map((type) => [type, {}]), + bindingTypes.map((type) => [type, {}]), ); return createTLSchema({ shapes: { ...defaultShapeSchemas, - text: textShapeSchema, ...customShapeSchemas, }, bindings: { From 96d528432ccd8438110cf33b3a277c8c1140850f Mon Sep 17 00:00:00 2001 From: Trang Doan Date: Wed, 30 Sep 2026 21:56:22 -0400 Subject: [PATCH 05/10] ENG-2227 Store text shape links in meta and reuse tldraw's link UI Text shapes keep their link in shape.meta.url, so the stock text schema is unchanged: no migration, no sync worker change, and a canvas saved by this build still opens in an older one, which ignores the link and round-trips it. The existing tldraw patch now makes text shapes eligible for the stock Edit link action unconditionally (gating on meta.url would stop a shape ever getting its first link), has EditLinkDialog read and write meta.url for text, and exports HyperlinkButton. An InFrontOfTheCanvas overlay renders that button just outside each linked text shape, since text bounds hug their glyphs. meta has no schema, so the overlay validates with T.linkUrl and renders the parsed URL; relative paths linkUrl would resolve against a dummy origin are rejected. Co-Authored-By: Claude Opus 5.5 --- apps/roam/src/components/canvas/Tldraw.tsx | 11 +- .../canvas/overlays/TextLinkOverlay.tsx | 61 ++++++++++ .../src/utils/__tests__/textShapeLink.test.ts | 101 +++++++++++++++++ apps/roam/src/utils/textShapeLink.ts | 15 +++ patches/tldraw@2.4.6.patch | 104 ++++++++++++++++-- pnpm-lock.yaml | 8 +- 6 files changed, 287 insertions(+), 13 deletions(-) create mode 100644 apps/roam/src/components/canvas/overlays/TextLinkOverlay.tsx create mode 100644 apps/roam/src/utils/__tests__/textShapeLink.test.ts create mode 100644 apps/roam/src/utils/textShapeLink.ts diff --git a/apps/roam/src/components/canvas/Tldraw.tsx b/apps/roam/src/components/canvas/Tldraw.tsx index 235d06ef6e..6921e0b230 100644 --- a/apps/roam/src/components/canvas/Tldraw.tsx +++ b/apps/roam/src/components/canvas/Tldraw.tsx @@ -55,6 +55,7 @@ import { import "tldraw/tldraw.css"; import tldrawStyles from "./tldrawStyles"; import { DragHandleOverlay } from "./overlays/DragHandleOverlay"; +import { TextLinkOverlay } from "./overlays/TextLinkOverlay"; import { hasAcceptedRelationSchema, isDiscourseNodeShape } from "./canvasUtils"; import getDiscourseNodes, { DiscourseNode } from "~/utils/getDiscourseNodes"; import getDiscourseRelations, { @@ -172,6 +173,14 @@ const setActiveCanvas = ({ } }; +// InFrontOfTheCanvas takes one component; module scope keeps its identity stable across renders. +const CanvasOverlays = (): JSX.Element => ( + <> + + + +); + export const DEFAULT_WIDTH = 160; export const DEFAULT_HEIGHT = 64; export const MAX_WIDTH = "400px"; @@ -1017,7 +1026,7 @@ const TldrawCanvasShared = ({ const editorComponents: TLEditorComponents = { ...defaultEditorComponents, OnTheCanvas: ToastListener, - InFrontOfTheCanvas: DragHandleOverlay, + InFrontOfTheCanvas: CanvasOverlays, }; const customUiComponents: TLUiComponents = createUiComponents({ allNodes, diff --git a/apps/roam/src/components/canvas/overlays/TextLinkOverlay.tsx b/apps/roam/src/components/canvas/overlays/TextLinkOverlay.tsx new file mode 100644 index 0000000000..50133dd016 --- /dev/null +++ b/apps/roam/src/components/canvas/overlays/TextLinkOverlay.tsx @@ -0,0 +1,61 @@ +import React from "react"; +import { HyperlinkButton, TLShapeId, useEditor, useValue } from "tldraw"; +import { getTextShapeLinkUrl } from "~/utils/textShapeLink"; + +type TextLink = { id: TLShapeId; url: string; left: number; top: number }; + +// Matches the hit area of tldraw's .tl-hyperlink-button. +const BUTTON_SIZE = 44; + +// Text bounds hug the glyphs, so the icon sits just outside the right edge +// rather than in the shape corner where geo shapes draw it. +export const TextLinkOverlay = (): JSX.Element => { + const editor = useEditor(); + const links = useValue( + "textShapeLinks", + () => { + const viewport = editor.getViewportPageBounds(); + return editor.getCurrentPageShapes().flatMap((shape) => { + const url = getTextShapeLinkUrl(shape); + if (!url) return []; + const bounds = editor.getShapePageBounds(shape.id); + if (!bounds || !viewport.collides(bounds)) return []; + const anchor = editor.pageToViewport({ + x: bounds.maxX, + y: bounds.midY, + }); + return [ + { + id: shape.id, + url, + left: anchor.x, + top: anchor.y - BUTTON_SIZE / 2, + }, + ]; + }); + }, + [editor], + ); + const zoomLevel = useValue("zoomLevel", () => editor.getZoomLevel(), [ + editor, + ]); + + return ( +
+ {links.map(({ id, url, left, top }) => ( +
+ +
+ ))} +
+ ); +}; diff --git a/apps/roam/src/utils/__tests__/textShapeLink.test.ts b/apps/roam/src/utils/__tests__/textShapeLink.test.ts new file mode 100644 index 0000000000..7c06a1cf4b --- /dev/null +++ b/apps/roam/src/utils/__tests__/textShapeLink.test.ts @@ -0,0 +1,101 @@ +import { describe, expect, it } from "vitest"; +import { createTLSchema, TLShape } from "tldraw"; +import { getTextShapeLinkUrl } from "~/utils/textShapeLink"; + +const makeShape = ({ + type = "text", + meta = {}, +}: { + type?: string; + meta?: Record; +} = {}): TLShape => + ({ + id: "shape:t1", + typeName: "shape", + type, + x: 0, + y: 0, + rotation: 0, + index: "a1", + parentId: "page:page", + isLocked: false, + opacity: 1, + meta, + props: { + color: "black", + size: "m", + font: "draw", + textAlign: "middle", + w: 100, + text: "hello", + scale: 1, + autoSize: true, + }, + }) as unknown as TLShape; + +const withUrl = (url: unknown): TLShape => makeShape({ meta: { url } }); + +describe("getTextShapeLinkUrl", () => { + it("returns the parsed href for a valid link", () => { + expect(getTextShapeLinkUrl(withUrl("https://Example.com"))).toBe( + "https://example.com/", + ); + expect(getTextShapeLinkUrl(withUrl("mailto:a@b.co"))).toBe("mailto:a@b.co"); + }); + + it("treats a missing or cleared link as no link", () => { + expect(getTextShapeLinkUrl(makeShape())).toBeUndefined(); + expect(getTextShapeLinkUrl(withUrl(""))).toBeUndefined(); + expect(getTextShapeLinkUrl(withUrl(42))).toBeUndefined(); + }); + + it("ignores links on shapes other than text", () => { + const geo = makeShape({ + type: "geo", + meta: { url: "https://example.com" }, + }); + expect(getTextShapeLinkUrl(geo)).toBeUndefined(); + }); + + // meta has no schema, so a collaborator or API writer can store anything here. + it("rejects protocols that could run script", () => { + expect(getTextShapeLinkUrl(withUrl("javascript:alert(1)"))).toBeUndefined(); + expect( + getTextShapeLinkUrl(withUrl(" javascript:alert(1)")), + ).toBeUndefined(); + expect(getTextShapeLinkUrl(withUrl("data:text/html,x"))).toBeUndefined(); + }); + + it("rejects relative URLs that linkUrl resolves against a dummy origin", () => { + expect(getTextShapeLinkUrl(withUrl("/page"))).toBeUndefined(); + expect(getTextShapeLinkUrl(withUrl("//evil.example/x"))).toBeUndefined(); + }); +}); + +describe("text shape link persistence", () => { + // The stock schema is what older extension builds and the sync worker use. + const stockSchema = createTLSchema(); + + it("validates a text shape carrying meta.url against the stock schema", () => { + expect(() => + stockSchema.validateRecord( + {} as never, + withUrl("https://example.com") as never, + "initialize", + null, + ), + ).not.toThrow(); + }); + + it("round-trips meta.url through a snapshot load with no migration", () => { + const shape = withUrl("https://example.com"); + const migrated = stockSchema.migrateStoreSnapshot({ + store: { [shape.id]: shape }, + schema: stockSchema.serialize(), + }); + expect(migrated.type).toBe("success"); + const loaded = (migrated as unknown as { value: Record }) + .value[shape.id]; + expect(loaded.meta.url).toBe("https://example.com"); + }); +}); diff --git a/apps/roam/src/utils/textShapeLink.ts b/apps/roam/src/utils/textShapeLink.ts new file mode 100644 index 0000000000..a91e5d0dd5 --- /dev/null +++ b/apps/roam/src/utils/textShapeLink.ts @@ -0,0 +1,15 @@ +import { T, TLShape } from "tldraw"; + +// meta is unvalidated, so this is the only guard before an href. Returning the +// parsed form keeps the href identical to what passed validation. +export const getTextShapeLinkUrl = (shape: TLShape): string | undefined => { + if (shape.type !== "text") return undefined; + const { url } = shape.meta; + if (typeof url !== "string" || !T.linkUrl.isValid(url)) return undefined; + try { + // linkUrl resolves relative paths against a dummy origin; only absolute URLs render. + return new URL(url).href; + } catch { + return undefined; + } +}; diff --git a/patches/tldraw@2.4.6.patch b/patches/tldraw@2.4.6.patch index 721bebb47d..e9f96d730b 100644 --- a/patches/tldraw@2.4.6.patch +++ b/patches/tldraw@2.4.6.patch @@ -2,10 +2,10 @@ diff --git a/CHANGELOG.md b/CHANGELOG.md deleted file mode 100644 index 095ed92631ae57d9caac683759c9b38491c92677..0000000000000000000000000000000000000000 diff --git a/dist-cjs/index.d.ts b/dist-cjs/index.d.ts -index ec996d145b83f637e0d43ad0db1f0e3ab87efb70..b033a8df604153626c203f3c808267a8c9592923 100644 +index ec996d145b83f637e0d43ad0db1f0e3ab87efb70..678a8cb45aac93bac4d9c627e6403c9dabf70725 100644 --- a/dist-cjs/index.d.ts +++ b/dist-cjs/index.d.ts -@@ -407,6 +407,18 @@ export declare function DefaultStylePanelContent({ styles }: TLUiStylePanelConte +@@ -407,6 +407,22 @@ export declare function DefaultStylePanelContent({ styles }: TLUiStylePanelConte */ export declare const DefaultToolbar: NamedExoticComponent; @@ -19,12 +19,16 @@ index ec996d145b83f637e0d43ad0db1f0e3ab87efb70..b033a8df604153626c203f3c808267a8 + children?: ReactNode; +} +export declare function MobileStylePanel(): JSX_2.Element; ++export declare function HyperlinkButton({ url, zoomLevel }: { ++ url: string; ++ zoomLevel: number; ++}): JSX_2.Element; +/** patched export for custom toolbar */ + /** @public @react */ export declare function DefaultToolbarContent(): JSX_2.Element; -@@ -2243,6 +2255,7 @@ export declare interface TLUiToolItem shape.type === "text" ? typeof shape.meta.url === "string" ? shape.meta.url : "" : shape.props.url; ++const getLinkUpdate = (shape, url) => shape.type === "text" ? { id: shape.id, type: shape.type, meta: { url } } : { id: shape.id, type: shape.type, props: { url } }; + const EditLinkDialog = track(function EditLinkDialog2({ onClose }) { + const editor = useEditor(); + const selectedShape = editor.getOnlySelectedShape(); +- if (!(selectedShape && "url" in selectedShape.props && typeof selectedShape.props.url === "string")) { ++ if (!(selectedShape && (selectedShape.type === "text" || "url" in selectedShape.props && typeof selectedShape.props.url === "string"))) { + return null; + } + return /* @__PURE__ */ jsx(EditLinkDialogInner, { onClose, selectedShape }); +@@ -39,10 +42,11 @@ const EditLinkDialogInner = track(function EditLinkDialogInner2({ + useEffect(() => { + editor.timers.requestAnimationFrame(() => rInput.current?.focus()); + }, [editor]); +- const rInitialValue = useRef(selectedShape.props.url); ++ const rInitialValue = useRef(getShapeLinkUrl(selectedShape)); + const [urlInputState, setUrlInputState] = useState(() => { +- const urlValidResult = validateUrl(selectedShape.props.url); +- const initialValue = urlValidResult.isValid === true ? urlValidResult.hasProtocol ? selectedShape.props.url : "https://" + selectedShape.props.url : "https://"; ++ const initialUrl = getShapeLinkUrl(selectedShape); ++ const urlValidResult = validateUrl(initialUrl); ++ const initialValue = urlValidResult.isValid === true ? urlValidResult.hasProtocol ? initialUrl : "https://" + initialUrl : "https://"; + return { + actual: initialValue, + safe: initialValue, +@@ -64,23 +68,15 @@ const EditLinkDialogInner = track(function EditLinkDialogInner2({ + const handleClear = useCallback(() => { + const onlySelectedShape = editor.getOnlySelectedShape(); + if (!onlySelectedShape) return; +- editor.updateShapes([ +- { id: onlySelectedShape.id, type: onlySelectedShape.type, props: { url: "" } } +- ]); ++ editor.updateShapes([getLinkUpdate(onlySelectedShape, "")]); + onClose(); + }, [editor, onClose]); + const handleComplete = useCallback(() => { + const onlySelectedShape = editor.getOnlySelectedShape(); + if (!onlySelectedShape) return; +- if (onlySelectedShape && "url" in onlySelectedShape.props) { +- if (onlySelectedShape.props.url !== urlInputState.safe) { +- editor.updateShapes([ +- { +- id: onlySelectedShape.id, +- type: onlySelectedShape.type, +- props: { url: urlInputState.safe } +- } +- ]); ++ if (onlySelectedShape.type === "text" || "url" in onlySelectedShape.props) { ++ if (getShapeLinkUrl(onlySelectedShape) !== urlInputState.safe) { ++ editor.updateShapes([getLinkUpdate(onlySelectedShape, urlInputState.safe)]); + } + } + onClose(); diff --git a/dist-esm/lib/ui/components/primitives/Button/TldrawUiButtonIcon.mjs b/dist-esm/lib/ui/components/primitives/Button/TldrawUiButtonIcon.mjs index fcb8268d733b28ab2d31a91e60818d46dbcfc3a2..812c7e77c5209abbd389f209ffae3dcb66c302c8 100644 --- a/dist-esm/lib/ui/components/primitives/Button/TldrawUiButtonIcon.mjs @@ -138,3 +213,16 @@ index c05d70e3ddd80a9cfdbd0527b93cd6fcd01fed88..2de1239ff423486c73a85df4057948f6 } ) }); } +diff --git a/dist-esm/lib/ui/hooks/menu-hooks.mjs b/dist-esm/lib/ui/hooks/menu-hooks.mjs +index aec3d2936032f118aa70afb33872d3b0f3e9887c..5bcde1a144b5be958821eb110b7940d88b4de5dd 100644 +--- a/dist-esm/lib/ui/hooks/menu-hooks.mjs ++++ b/dist-esm/lib/ui/hooks/menu-hooks.mjs +@@ -124,7 +124,7 @@ function useHasLinkShapeSelected() { + "hasLinkShapeSelected", + () => { + const onlySelectedShape = editor.getOnlySelectedShape(); +- return !!(onlySelectedShape && onlySelectedShape.type !== "embed" && "url" in onlySelectedShape.props && !onlySelectedShape.isLocked); ++ return !!(onlySelectedShape && onlySelectedShape.type !== "embed" && ("url" in onlySelectedShape.props || onlySelectedShape.type === "text") && !onlySelectedShape.isLocked); + }, + [editor] + ); diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index d683776d67..eebdf276ac 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -86,7 +86,7 @@ patchedDependencies: hash: b5bbaa41bf2edae401d2bcd96dfa3c24086b564f297ef2df16a3b3156cd51ecc path: patches/@tldraw__state@2.4.6.patch tldraw@2.4.6: - hash: 56e196052862c9a58a11b43e5e121384cd1d6548416afa0f16e9fbfbf0e4080d + hash: 1dc6e0d5a14876d58a705c3446a937e7811078b17a0918da85f9faa4b096d68a path: patches/tldraw@2.4.6.patch importers: @@ -364,7 +364,7 @@ importers: version: 0.90.0(323501797697e57d5f8d07d52746f463) tldraw: specifier: 2.4.6 - version: 2.4.6(patch_hash=56e196052862c9a58a11b43e5e121384cd1d6548416afa0f16e9fbfbf0e4080d)(@types/react-dom@18.2.17)(@types/react@18.2.21)(react-dom@18.2.0(react@18.2.0))(react@18.2.0) + version: 2.4.6(patch_hash=1dc6e0d5a14876d58a705c3446a937e7811078b17a0918da85f9faa4b096d68a)(@types/react-dom@18.2.17)(@types/react@18.2.21)(react-dom@18.2.0(react@18.2.0))(react@18.2.0) use-sync-external-store: specifier: 1.5.0 version: 1.5.0(react@18.2.0) @@ -16293,7 +16293,7 @@ snapshots: nanoid: 4.0.2 react: 18.2.0 react-dom: 18.2.0(react@18.2.0) - tldraw: 2.4.6(patch_hash=56e196052862c9a58a11b43e5e121384cd1d6548416afa0f16e9fbfbf0e4080d)(@types/react-dom@18.2.17)(@types/react@18.2.21)(react-dom@18.2.0(react@18.2.0))(react@18.2.0) + tldraw: 2.4.6(patch_hash=1dc6e0d5a14876d58a705c3446a937e7811078b17a0918da85f9faa4b096d68a)(@types/react-dom@18.2.17)(@types/react@18.2.21)(react-dom@18.2.0(react@18.2.0))(react@18.2.0) ws: 8.18.3 transitivePeerDependencies: - '@types/react' @@ -23744,7 +23744,7 @@ snapshots: chalk: 5.6.0 clipboardy: 4.0.0 - tldraw@2.4.6(patch_hash=56e196052862c9a58a11b43e5e121384cd1d6548416afa0f16e9fbfbf0e4080d)(@types/react-dom@18.2.17)(@types/react@18.2.21)(react-dom@18.2.0(react@18.2.0))(react@18.2.0): + tldraw@2.4.6(patch_hash=1dc6e0d5a14876d58a705c3446a937e7811078b17a0918da85f9faa4b096d68a)(@types/react-dom@18.2.17)(@types/react@18.2.21)(react-dom@18.2.0(react@18.2.0))(react@18.2.0): dependencies: '@radix-ui/react-alert-dialog': 1.1.15(@types/react-dom@18.2.17)(@types/react@18.2.21)(react-dom@18.2.0(react@18.2.0))(react@18.2.0) '@radix-ui/react-context-menu': 2.2.16(@types/react-dom@18.2.17)(@types/react@18.2.21)(react-dom@18.2.0(react@18.2.0))(react@18.2.0) From 73744befa461e6d8cd69b2116093cfdd3ac9d188 Mon Sep 17 00:00:00 2001 From: Trang Doan Date: Wed, 30 Sep 2026 21:59:29 -0400 Subject: [PATCH 06/10] Co-locate the text link guard with its only caller Co-Authored-By: Claude Opus 5.5 Entire-Checkpoint: 01M3TK0B5ZF8BWDS8PXDES5Y8Y --- .../canvas/overlays/TextLinkOverlay.tsx | 24 +++++++++++++++++-- ...peLink.test.ts => textLinkOverlay.test.ts} | 2 +- apps/roam/src/utils/textShapeLink.ts | 15 ------------ 3 files changed, 23 insertions(+), 18 deletions(-) rename apps/roam/src/utils/__tests__/{textShapeLink.test.ts => textLinkOverlay.test.ts} (97%) delete mode 100644 apps/roam/src/utils/textShapeLink.ts diff --git a/apps/roam/src/components/canvas/overlays/TextLinkOverlay.tsx b/apps/roam/src/components/canvas/overlays/TextLinkOverlay.tsx index 50133dd016..e90b469fd4 100644 --- a/apps/roam/src/components/canvas/overlays/TextLinkOverlay.tsx +++ b/apps/roam/src/components/canvas/overlays/TextLinkOverlay.tsx @@ -1,12 +1,32 @@ import React from "react"; -import { HyperlinkButton, TLShapeId, useEditor, useValue } from "tldraw"; -import { getTextShapeLinkUrl } from "~/utils/textShapeLink"; +import { + HyperlinkButton, + T, + TLShape, + TLShapeId, + useEditor, + useValue, +} from "tldraw"; type TextLink = { id: TLShapeId; url: string; left: number; top: number }; // Matches the hit area of tldraw's .tl-hyperlink-button. const BUTTON_SIZE = 44; +// meta is unvalidated, so this is the only guard before an href. Returning the +// parsed form keeps the href identical to what passed validation. +export const getTextShapeLinkUrl = (shape: TLShape): string | undefined => { + if (shape.type !== "text") return undefined; + const { url } = shape.meta; + if (typeof url !== "string" || !T.linkUrl.isValid(url)) return undefined; + try { + // linkUrl resolves relative paths against a dummy origin; only absolute URLs render. + return new URL(url).href; + } catch { + return undefined; + } +}; + // Text bounds hug the glyphs, so the icon sits just outside the right edge // rather than in the shape corner where geo shapes draw it. export const TextLinkOverlay = (): JSX.Element => { diff --git a/apps/roam/src/utils/__tests__/textShapeLink.test.ts b/apps/roam/src/utils/__tests__/textLinkOverlay.test.ts similarity index 97% rename from apps/roam/src/utils/__tests__/textShapeLink.test.ts rename to apps/roam/src/utils/__tests__/textLinkOverlay.test.ts index 7c06a1cf4b..3e10659a8f 100644 --- a/apps/roam/src/utils/__tests__/textShapeLink.test.ts +++ b/apps/roam/src/utils/__tests__/textLinkOverlay.test.ts @@ -1,6 +1,6 @@ import { describe, expect, it } from "vitest"; import { createTLSchema, TLShape } from "tldraw"; -import { getTextShapeLinkUrl } from "~/utils/textShapeLink"; +import { getTextShapeLinkUrl } from "~/components/canvas/overlays/TextLinkOverlay"; const makeShape = ({ type = "text", diff --git a/apps/roam/src/utils/textShapeLink.ts b/apps/roam/src/utils/textShapeLink.ts deleted file mode 100644 index a91e5d0dd5..0000000000 --- a/apps/roam/src/utils/textShapeLink.ts +++ /dev/null @@ -1,15 +0,0 @@ -import { T, TLShape } from "tldraw"; - -// meta is unvalidated, so this is the only guard before an href. Returning the -// parsed form keeps the href identical to what passed validation. -export const getTextShapeLinkUrl = (shape: TLShape): string | undefined => { - if (shape.type !== "text") return undefined; - const { url } = shape.meta; - if (typeof url !== "string" || !T.linkUrl.isValid(url)) return undefined; - try { - // linkUrl resolves relative paths against a dummy origin; only absolute URLs render. - return new URL(url).href; - } catch { - return undefined; - } -}; From b8fdd1995f1e8ee729ce8330e2f191ea1ffff20b Mon Sep 17 00:00:00 2001 From: Trang Doan Date: Thu, 1 Oct 2026 00:58:17 -0400 Subject: [PATCH 07/10] Hide the text link icon while its shape is selected The overlay renders above the selection foreground, so a 44px button at the right edge swallowed drags on the right resize and rotate handles and opened the link instead. Co-Authored-By: Claude Opus 5.5 Entire-Checkpoint: 01M3TX7QGKXMB58AK5840S5CQ8 --- apps/roam/src/components/canvas/overlays/TextLinkOverlay.tsx | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/apps/roam/src/components/canvas/overlays/TextLinkOverlay.tsx b/apps/roam/src/components/canvas/overlays/TextLinkOverlay.tsx index e90b469fd4..8ddb50e77a 100644 --- a/apps/roam/src/components/canvas/overlays/TextLinkOverlay.tsx +++ b/apps/roam/src/components/canvas/overlays/TextLinkOverlay.tsx @@ -35,7 +35,11 @@ export const TextLinkOverlay = (): JSX.Element => { "textShapeLinks", () => { const viewport = editor.getViewportPageBounds(); + // The overlay sits above the selection handles, so it would swallow + // resize and rotate drags on a selected shape. + const selectedIds = new Set(editor.getSelectedShapeIds()); return editor.getCurrentPageShapes().flatMap((shape) => { + if (selectedIds.has(shape.id)) return []; const url = getTextShapeLinkUrl(shape); if (!url) return []; const bounds = editor.getShapePageBounds(shape.id); From 025180604b095e5d319c33b7c6f572022bcae5f3 Mon Sep 17 00:00:00 2001 From: Trang Doan Date: Thu, 1 Oct 2026 13:49:52 -0400 Subject: [PATCH 08/10] Hide text link icons that overlap the selection handles The overlay renders above every shape and handle, so hiding the icon only for its own shape still let it cover a neighbouring selection's resize, rotate, and relation drag handles. Hide any icon within handle reach of the selection box instead. Co-Authored-By: Claude Opus 5.5 Entire-Checkpoint: 01M3W9CHZ0FXJKXJVVRP73J6NE --- .../canvas/overlays/TextLinkOverlay.tsx | 57 +++++++++++++++---- 1 file changed, 45 insertions(+), 12 deletions(-) diff --git a/apps/roam/src/components/canvas/overlays/TextLinkOverlay.tsx b/apps/roam/src/components/canvas/overlays/TextLinkOverlay.tsx index 8ddb50e77a..04c21074ff 100644 --- a/apps/roam/src/components/canvas/overlays/TextLinkOverlay.tsx +++ b/apps/roam/src/components/canvas/overlays/TextLinkOverlay.tsx @@ -1,5 +1,6 @@ import React from "react"; import { + Editor, HyperlinkButton, T, TLShape, @@ -13,6 +14,10 @@ type TextLink = { id: TLShapeId; url: string; left: number; top: number }; // Matches the hit area of tldraw's .tl-hyperlink-button. const BUTTON_SIZE = 44; +// Furthest any handle sits outside the selection box: tldraw's rotate target on +// a coarse pointer is 31.5px; DragHandleOverlay's relation handles reach 20px. +const HANDLE_REACH = 32; + // meta is unvalidated, so this is the only guard before an href. Returning the // parsed form keeps the href identical to what passed validation. export const getTextShapeLinkUrl = (shape: TLShape): string | undefined => { @@ -27,6 +32,36 @@ export const getTextShapeLinkUrl = (shape: TLShape): string | undefined => { } }; +type ViewportBox = { left: number; top: number; right: number; bottom: number }; + +const getHandleZone = (editor: Editor): ViewportBox | null => { + const selection = editor.getSelectionPageBounds(); + if (!selection) return null; + const topLeft = editor.pageToViewport({ + x: selection.minX, + y: selection.minY, + }); + const bottomRight = editor.pageToViewport({ + x: selection.maxX, + y: selection.maxY, + }); + return { + left: topLeft.x - HANDLE_REACH, + top: topLeft.y - HANDLE_REACH, + right: bottomRight.x + HANDLE_REACH, + bottom: bottomRight.y + HANDLE_REACH, + }; +}; + +const overlapsHandleZone = ( + { left, top }: { left: number; top: number }, + zone: ViewportBox, +): boolean => + left < zone.right && + left + BUTTON_SIZE > zone.left && + top < zone.bottom && + top + BUTTON_SIZE > zone.top; + // Text bounds hug the glyphs, so the icon sits just outside the right edge // rather than in the shape corner where geo shapes draw it. export const TextLinkOverlay = (): JSX.Element => { @@ -35,11 +70,10 @@ export const TextLinkOverlay = (): JSX.Element => { "textShapeLinks", () => { const viewport = editor.getViewportPageBounds(); - // The overlay sits above the selection handles, so it would swallow - // resize and rotate drags on a selected shape. - const selectedIds = new Set(editor.getSelectedShapeIds()); + // The overlay renders above every shape and handle, so an icon near the + // selection would swallow resize, rotate, and relation drags. + const handleZone = getHandleZone(editor); return editor.getCurrentPageShapes().flatMap((shape) => { - if (selectedIds.has(shape.id)) return []; const url = getTextShapeLinkUrl(shape); if (!url) return []; const bounds = editor.getShapePageBounds(shape.id); @@ -48,14 +82,13 @@ export const TextLinkOverlay = (): JSX.Element => { x: bounds.maxX, y: bounds.midY, }); - return [ - { - id: shape.id, - url, - left: anchor.x, - top: anchor.y - BUTTON_SIZE / 2, - }, - ]; + const link = { + id: shape.id, + url, + left: anchor.x, + top: anchor.y - BUTTON_SIZE / 2, + }; + return handleZone && overlapsHandleZone(link, handleZone) ? [] : [link]; }); }, [editor], From f7aedc8e2ed6cc7deeba0ac12380fbf0505f946a Mon Sep 17 00:00:00 2001 From: Trang Doan Date: Thu, 1 Oct 2026 15:00:14 -0400 Subject: [PATCH 09/10] Keep the text link icon visible while the shape is selected Hiding the icon near the selection kept it off the resize, rotate, and relation handles, but the icon disappearing on selection was the larger cost. The icon now always shows and stays clickable, so a drag that starts under it on a short text shape opens the link instead of resizing. Co-Authored-By: Claude Opus 5.5 Entire-Checkpoint: 01M3WDDCGRQEZ0D8949E9E23XB --- .../canvas/overlays/TextLinkOverlay.tsx | 53 +++---------------- 1 file changed, 8 insertions(+), 45 deletions(-) diff --git a/apps/roam/src/components/canvas/overlays/TextLinkOverlay.tsx b/apps/roam/src/components/canvas/overlays/TextLinkOverlay.tsx index 04c21074ff..e90b469fd4 100644 --- a/apps/roam/src/components/canvas/overlays/TextLinkOverlay.tsx +++ b/apps/roam/src/components/canvas/overlays/TextLinkOverlay.tsx @@ -1,6 +1,5 @@ import React from "react"; import { - Editor, HyperlinkButton, T, TLShape, @@ -14,10 +13,6 @@ type TextLink = { id: TLShapeId; url: string; left: number; top: number }; // Matches the hit area of tldraw's .tl-hyperlink-button. const BUTTON_SIZE = 44; -// Furthest any handle sits outside the selection box: tldraw's rotate target on -// a coarse pointer is 31.5px; DragHandleOverlay's relation handles reach 20px. -const HANDLE_REACH = 32; - // meta is unvalidated, so this is the only guard before an href. Returning the // parsed form keeps the href identical to what passed validation. export const getTextShapeLinkUrl = (shape: TLShape): string | undefined => { @@ -32,36 +27,6 @@ export const getTextShapeLinkUrl = (shape: TLShape): string | undefined => { } }; -type ViewportBox = { left: number; top: number; right: number; bottom: number }; - -const getHandleZone = (editor: Editor): ViewportBox | null => { - const selection = editor.getSelectionPageBounds(); - if (!selection) return null; - const topLeft = editor.pageToViewport({ - x: selection.minX, - y: selection.minY, - }); - const bottomRight = editor.pageToViewport({ - x: selection.maxX, - y: selection.maxY, - }); - return { - left: topLeft.x - HANDLE_REACH, - top: topLeft.y - HANDLE_REACH, - right: bottomRight.x + HANDLE_REACH, - bottom: bottomRight.y + HANDLE_REACH, - }; -}; - -const overlapsHandleZone = ( - { left, top }: { left: number; top: number }, - zone: ViewportBox, -): boolean => - left < zone.right && - left + BUTTON_SIZE > zone.left && - top < zone.bottom && - top + BUTTON_SIZE > zone.top; - // Text bounds hug the glyphs, so the icon sits just outside the right edge // rather than in the shape corner where geo shapes draw it. export const TextLinkOverlay = (): JSX.Element => { @@ -70,9 +35,6 @@ export const TextLinkOverlay = (): JSX.Element => { "textShapeLinks", () => { const viewport = editor.getViewportPageBounds(); - // The overlay renders above every shape and handle, so an icon near the - // selection would swallow resize, rotate, and relation drags. - const handleZone = getHandleZone(editor); return editor.getCurrentPageShapes().flatMap((shape) => { const url = getTextShapeLinkUrl(shape); if (!url) return []; @@ -82,13 +44,14 @@ export const TextLinkOverlay = (): JSX.Element => { x: bounds.maxX, y: bounds.midY, }); - const link = { - id: shape.id, - url, - left: anchor.x, - top: anchor.y - BUTTON_SIZE / 2, - }; - return handleZone && overlapsHandleZone(link, handleZone) ? [] : [link]; + return [ + { + id: shape.id, + url, + left: anchor.x, + top: anchor.y - BUTTON_SIZE / 2, + }, + ]; }); }, [editor], From c79dfc1f7dd90f73d623cc2ee0a61ed0019bd721 Mon Sep 17 00:00:00 2001 From: Trang Doan Date: Mon, 5 Oct 2026 11:21:12 -0400 Subject: [PATCH 10/10] Use Tailwind for the text link overlay's static styles Co-Authored-By: Claude Opus 5.5 Entire-Checkpoint: 01M46AF6E7M6V0KQY6S76ZEX9Q --- apps/roam/src/components/canvas/overlays/TextLinkOverlay.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/apps/roam/src/components/canvas/overlays/TextLinkOverlay.tsx b/apps/roam/src/components/canvas/overlays/TextLinkOverlay.tsx index e90b469fd4..d94ac5d334 100644 --- a/apps/roam/src/components/canvas/overlays/TextLinkOverlay.tsx +++ b/apps/roam/src/components/canvas/overlays/TextLinkOverlay.tsx @@ -61,12 +61,12 @@ export const TextLinkOverlay = (): JSX.Element => { ]); return ( -
+
{links.map(({ id, url, left, top }) => (