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..faef5a67ee --- /dev/null +++ b/apps/roam/src/components/canvas/TextShapeWithLinkUtil.tsx @@ -0,0 +1,79 @@ +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..d9ef16aa07 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( diff --git a/apps/roam/src/components/canvas/baseShapeUtils.ts b/apps/roam/src/components/canvas/baseShapeUtils.ts new file mode 100644 index 0000000000..521054ffa8 --- /dev/null +++ b/apps/roam/src/components/canvas/baseShapeUtils.ts @@ -0,0 +1,19 @@ +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 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/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__/syncWorkerRoomSchema.test.ts b/apps/roam/src/utils/__tests__/syncWorkerRoomSchema.test.ts new file mode 100644 index 0000000000..25fe47304e --- /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": {}, + }, +}); + +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 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/__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 new file mode 100644 index 0000000000..88b4646fec --- /dev/null +++ b/apps/roam/src/utils/textShapeLink.ts @@ -0,0 +1,32 @@ +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 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: {