From 0a127efb1bf56182a555a94ba140d477c73d9b87 Mon Sep 17 00:00:00 2001 From: Trang Doan Date: Mon, 14 Sep 2026 11:50:08 -0400 Subject: [PATCH 01/17] ENG-2228 Add link support to tldraw text shapes in Obsidian Text shapes could not carry a link, so a canvas narrative could not open the source its text stands for. tldraw gates its Edit link action on the duck-type `'url' in shape.props`, and TLTextShape has no such prop -- so this is a schema change, not UI wiring. Adding the prop lights up the stock menu item, and a retroactive migration backfills `url: ""` on pre-existing text records, which is required both for validation and for those shapes to become link-eligible. The ticket names page links as the primary target, so the prop uses a wider protocol allowlist than tldraw's T.linkUrl (which permits only http/https/ mailto) and tldraw's EditLinkDialog is forked, since it rejects non-http protocols on input before the schema sees them. The fork keeps geo shapes on the strict validator, as their url prop is still stock. Clicking an obsidian:// link resolves it in-app rather than via the OS protocol handler. Both shape-util registration sites now share baseShapeUtils: the schema and the editor are built from separate arrays and nothing asserts they agree. Co-Authored-By: Claude Opus 5 --- apps/obsidian/package.json | 6 +- .../src/components/canvas/TextLinkDialog.tsx | 207 ++++++++++++++++++ .../components/canvas/TldrawViewComponent.tsx | 20 +- .../canvas/shapes/TextShapeWithLinkUtil.tsx | 100 +++++++++ .../canvas/shapes/baseShapeUtils.ts | 15 ++ .../canvas/shapes/textLinkMigrations.ts | 35 +++ .../utils/__tests__/textShapeLink.test.ts | 153 +++++++++++++ .../canvas/utils/externalContentHandlers.ts | 17 +- .../components/canvas/utils/textShapeLink.ts | 56 +++++ .../src/components/canvas/utils/tldraw.ts | 7 +- apps/obsidian/styles.css | 9 + apps/obsidian/vitest.config.mts | 17 ++ pnpm-lock.yaml | 42 ++++ 13 files changed, 671 insertions(+), 13 deletions(-) create mode 100644 apps/obsidian/src/components/canvas/TextLinkDialog.tsx create mode 100644 apps/obsidian/src/components/canvas/shapes/TextShapeWithLinkUtil.tsx create mode 100644 apps/obsidian/src/components/canvas/shapes/baseShapeUtils.ts create mode 100644 apps/obsidian/src/components/canvas/shapes/textLinkMigrations.ts create mode 100644 apps/obsidian/src/components/canvas/utils/__tests__/textShapeLink.test.ts create mode 100644 apps/obsidian/src/components/canvas/utils/textShapeLink.ts create mode 100644 apps/obsidian/vitest.config.mts diff --git a/apps/obsidian/package.json b/apps/obsidian/package.json index f6b6c8dcda..474f3dfa4d 100644 --- a/apps/obsidian/package.json +++ b/apps/obsidian/package.json @@ -10,7 +10,10 @@ "lint": "eslint .", "lint:fix": "eslint . --fix", "publish": "tsx scripts/publish.ts", - "check-types": "tsc --noEmit --skipLibCheck" + "check-types": "tsc --noEmit --skipLibCheck", + "test": "pnpm test:unit", + "test:unit": "vitest run --config vitest.config.mts", + "test:watch": "vitest --config vitest.config.mts" }, "keywords": [], "author": "", @@ -35,6 +38,7 @@ "tsx": "^4.19.2", "typescript": "5.5.4", "uuidv7": "1.1.0", + "vitest": "catalog:", "zod": "^3.24.1" }, "dependencies": { diff --git a/apps/obsidian/src/components/canvas/TextLinkDialog.tsx b/apps/obsidian/src/components/canvas/TextLinkDialog.tsx new file mode 100644 index 0000000000..10b7dfb56e --- /dev/null +++ b/apps/obsidian/src/components/canvas/TextLinkDialog.tsx @@ -0,0 +1,207 @@ +import { useCallback, useEffect, useRef, useState } from "react"; +import { + T, + TLBaseShape, + TldrawUiButton, + TldrawUiButtonLabel, + TldrawUiDialogBody, + TldrawUiDialogCloseButton, + TldrawUiDialogFooter, + TldrawUiDialogHeader, + TldrawUiDialogTitle, + TldrawUiInput, + track, + useEditor, +} from "tldraw"; +import { isAllowedTextLinkUrl } from "~/components/canvas/utils/textShapeLink"; + +type ShapeWithUrl = TLBaseShape; + +// Declared as a function property rather than reusing TLUiDialogProps, whose +// method-shorthand onClose trips @typescript-eslint/unbound-method. +type TextLinkDialogProps = { onClose: () => void }; + +type UrlValidity = { isValid: boolean; hasProtocol: boolean }; + +// Only text shapes carry the widened url validator. Geo and friends keep +// tldraw's T.linkUrl, so accepting obsidian:// for them would throw on save. +const validateUrlForShape = (url: string, shapeType: string): UrlValidity => { + const isAllowed = + shapeType === "text" + ? (value: string) => isAllowedTextLinkUrl(value) + : (value: string) => T.linkUrl.isValid(value); + + if (isAllowed(url)) return { isValid: true, hasProtocol: true }; + if (isAllowed(`https://${url}`)) return { isValid: true, hasProtocol: false }; + return { isValid: false, hasProtocol: false }; +}; + +// A fork of tldraw's EditLinkDialog, which is not exported. The only behavioural +// change is validateUrlForShape; everything else mirrors the original so the +// dialog stays visually identical. +export const TextLinkDialog = track(({ onClose }: TextLinkDialogProps) => { + const editor = useEditor(); + const selectedShape = editor.getOnlySelectedShape(); + + if ( + !( + selectedShape && + "url" in selectedShape.props && + typeof selectedShape.props.url === "string" + ) + ) { + return null; + } + + return ( + + ); +}); + +const TextLinkDialogInner = track( + ({ + onClose, + selectedShape, + }: TextLinkDialogProps & { selectedShape: ShapeWithUrl }) => { + const editor = useEditor(); + const rInput = useRef(null); + const shapeType = selectedShape.type; + + useEffect(() => { + editor.timers.requestAnimationFrame(() => rInput.current?.focus()); + }, [editor]); + + const rInitialValue = useRef(selectedShape.props.url); + + const [urlInputState, setUrlInputState] = useState(() => { + const result = validateUrlForShape(selectedShape.props.url, shapeType); + const initialValue = result.isValid + ? result.hasProtocol + ? selectedShape.props.url + : `https://${selectedShape.props.url}` + : "https://"; + return { actual: initialValue, safe: initialValue, valid: true }; + }); + + const handleChange = useCallback( + (rawValue: string) => { + // Auto-correct a doubled https:// from a bad paste. + const fixedRawValue = rawValue.replace( + /https?:\/\/(https?:\/\/)/, + (_match, arg1: string) => arg1, + ); + const result = validateUrlForShape(fixedRawValue, shapeType); + const safeValue = result.isValid + ? result.hasProtocol + ? fixedRawValue + : `https://${fixedRawValue}` + : "https://"; + + setUrlInputState({ + actual: fixedRawValue, + safe: safeValue, + valid: result.isValid, + }); + }, + [shapeType], + ); + + const handleClear = useCallback(() => { + const onlySelectedShape = editor.getOnlySelectedShape(); + if (!onlySelectedShape) return; + editor.updateShapes([ + { + id: onlySelectedShape.id, + type: onlySelectedShape.type, + props: { url: "" }, + }, + ]); + onClose(); + }, [editor, onClose]); + + const handleComplete = useCallback(() => { + const onlySelectedShape = editor.getOnlySelectedShape(); + if (!onlySelectedShape) return; + + if ( + "url" in onlySelectedShape.props && + onlySelectedShape.props.url !== urlInputState.safe + ) { + editor.updateShapes([ + { + id: onlySelectedShape.id, + type: onlySelectedShape.type, + props: { url: urlInputState.safe }, + }, + ]); + } + onClose(); + }, [editor, onClose, urlInputState]); + + const handleCancel = useCallback(() => { + onClose(); + }, [onClose]); + + const isRemoving = rInitialValue.current && !urlInputState.valid; + + return ( + <> + + Edit link + + + +
+ +
+ {urlInputState.valid + ? "Enter a URL, or an obsidian:// link to a page." + : "Invalid URL"} +
+
+ + + + Cancel + + {isRemoving ? ( + + Clear + + ) : ( + + Save + + )} + + + ); + }, +); diff --git a/apps/obsidian/src/components/canvas/TldrawViewComponent.tsx b/apps/obsidian/src/components/canvas/TldrawViewComponent.tsx index 8350aa5e9c..a22d94ad22 100644 --- a/apps/obsidian/src/components/canvas/TldrawViewComponent.tsx +++ b/apps/obsidian/src/components/canvas/TldrawViewComponent.tsx @@ -1,6 +1,5 @@ import { useCallback, useEffect, useRef, useState } from "react"; import { - defaultShapeUtils, DefaultToolbar, DefaultToolbarContent, ErrorBoundary, @@ -52,6 +51,8 @@ import { DragHandleOverlay } from "./overlays/DragHandleOverlay"; import { NodeCardContextMenu } from "./NodeCardContextMenu"; import { WHITE_LOGO_SVG } from "~/icons"; import { CustomContextMenu } from "./CustomContextMenu"; +import { baseShapeUtils } from "~/components/canvas/shapes/baseShapeUtils"; +import { TextLinkDialog } from "~/components/canvas/TextLinkDialog"; import { openFileInSidebar, openFileInNewTab, @@ -106,7 +107,7 @@ export const TldrawPreviewComponent = ({ }); const customShapeUtils = [ - ...defaultShapeUtils, + ...baseShapeUtils, createDiscourseNodeUtil({ app: plugin.app, canvasFile: file, @@ -464,6 +465,21 @@ export const TldrawPreviewComponent = ({ }, }} overrides={{ + // Swap only edit-link's dialog; the menu item, icon, placement + // and eligibility gate stay stock. + actions: (editor, actions, helpers) => { + const editLink = actions["edit-link"]; + if (editLink) { + actions["edit-link"] = { + ...editLink, + onSelect: () => { + editor.markHistoryStoppingPoint("edit-link"); + helpers.addDialog({ component: TextLinkDialog }); + }, + }; + } + return actions; + }, tools: (editor, tools) => { tools["discourse-node"] = { id: "discourse-node", diff --git a/apps/obsidian/src/components/canvas/shapes/TextShapeWithLinkUtil.tsx b/apps/obsidian/src/components/canvas/shapes/TextShapeWithLinkUtil.tsx new file mode 100644 index 0000000000..aecb1a9631 --- /dev/null +++ b/apps/obsidian/src/components/canvas/shapes/TextShapeWithLinkUtil.tsx @@ -0,0 +1,100 @@ +import React, { useCallback } from "react"; +import { + TextShapeUtil, + TLTextShape, + stopEventPropagation, + textShapeProps, +} from "tldraw"; +import { usePlugin } from "~/components/PluginContext"; +import { textLinkUrl } from "~/components/canvas/utils/textShapeLink"; +import { + parseObsidianOpenUrl, + resolveObsidianUrlToFile, +} from "~/components/canvas/utils/externalContentHandlers"; +import { openFileInSidebar } from "~/components/canvas/utils/openFileUtils"; +import { showToast } from "~/components/canvas/utils/toastUtils"; + +export type TextShapeWithLinkProps = TLTextShape["props"] & { url: string }; + +export const getTextShapeUrl = (shape: TLTextShape): string => + (shape.props as Partial).url ?? ""; + +// 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; +}): React.ReactElement => { + const plugin = usePlugin(); + + // Opening obsidian:// in-app avoids an OS protocol round trip that would + // defocus and re-enter Obsidian. + const handleClick = useCallback( + (event: React.MouseEvent) => { + const parsed = parseObsidianOpenUrl(url); + if (!parsed) return; + event.preventDefault(); + + const file = resolveObsidianUrlToFile(plugin, parsed); + if (!file) { + showToast({ + severity: "warning", + title: "Cannot open link", + description: "The linked file is not in this vault", + }); + return; + } + void openFileInSidebar(plugin.app, file); + }, + [plugin, url], + ); + + return ( + +
+ + ); +}; + +const textShapeWithLinkProps = { ...textShapeProps, url: textLinkUrl }; + +// 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; + + // `static props` alone gives no default, so every createShape({type:"text"}) + // would fail validation on the missing url. + override getDefaultProps(): TextShapeWithLinkProps { + return { ...super.getDefaultProps(), url: "" }; + } + + override component( + shape: TLTextShape, + ): ReturnType { + const url = getTextShapeUrl(shape); + return ( + <> + {super.component(shape)} + {url && ( + + )} + + ); + } +} diff --git a/apps/obsidian/src/components/canvas/shapes/baseShapeUtils.ts b/apps/obsidian/src/components/canvas/shapes/baseShapeUtils.ts new file mode 100644 index 0000000000..0fc7924f39 --- /dev/null +++ b/apps/obsidian/src/components/canvas/shapes/baseShapeUtils.ts @@ -0,0 +1,15 @@ +import { + defaultShapeUtils, + TextShapeUtil, + TLAnyShapeUtilConstructor, +} from "tldraw"; +import { TextShapeWithLinkUtil } from "~/components/canvas/shapes/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: +// the schema and the editor are built from separate arrays and nothing asserts +// they agree. +export const baseShapeUtils: TLAnyShapeUtilConstructor[] = + defaultShapeUtils.map((util) => + util === TextShapeUtil ? TextShapeWithLinkUtil : util, + ); diff --git a/apps/obsidian/src/components/canvas/shapes/textLinkMigrations.ts b/apps/obsidian/src/components/canvas/shapes/textLinkMigrations.ts new file mode 100644 index 0000000000..64462a76e3 --- /dev/null +++ b/apps/obsidian/src/components/canvas/shapes/textLinkMigrations.ts @@ -0,0 +1,35 @@ +import { createMigrationSequence, createMigrationIds } from "tldraw"; +import { + backfillTextShapeUrl, + isTextShapeRecord, +} from "~/components/canvas/utils/textShapeLink"; + +const SEQUENCE_ID_BASE = "com.discourse-graph.obsidian.text-link"; + +const versions = createMigrationIds(`${SEQUENCE_ID_BASE}`, { + addTextShapeUrl: 1, +}); + +type TextLinkMigrationRecord = { + props: Record; +}; + +// Without this backfill a pre-existing text shape has no `url` key, which both +// fails validation on load and leaves the shape ineligible for Edit link. +// `retroactive` defaults to true, which is what reaches files predating it. +export const textLinkMigrations = createMigrationSequence({ + sequenceId: `${SEQUENCE_ID_BASE}`, + sequence: [ + { + id: versions["addTextShapeUrl"], + scope: "record", + filter: (record) => isTextShapeRecord(record), + up: (record) => { + backfillTextShapeUrl(record); + }, + down: (record) => { + delete (record as unknown as TextLinkMigrationRecord).props.url; + }, + }, + ], +}); diff --git a/apps/obsidian/src/components/canvas/utils/__tests__/textShapeLink.test.ts b/apps/obsidian/src/components/canvas/utils/__tests__/textShapeLink.test.ts new file mode 100644 index 0000000000..a9d6d5dd5c --- /dev/null +++ b/apps/obsidian/src/components/canvas/utils/__tests__/textShapeLink.test.ts @@ -0,0 +1,153 @@ +import { describe, expect, it } from "vitest"; +import { createTLSchema, defaultShapeSchemas } from "tldraw"; +import { + backfillTextShapeUrl, + hasLinkUrlProp, + isAllowedTextLinkUrl, + isTextShapeRecord, + textLinkUrl, +} from "~/components/canvas/utils/textShapeLink"; + +const OBSIDIAN_URL = "obsidian://open?vault=MyVault&file=Notes%2FPage.md"; + +const makeTextShape = ( + props: Record = {}, +): Record & { props: 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: "start", + w: 100, + richText: { type: "doc", content: [] }, + 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("textLinkUrl", () => { + it("accepts web, mailto and obsidian links", () => { + for (const url of [ + "", + "https://example.com", + "http://example.com/a?b=c", + "mailto:someone@example.com", + OBSIDIAN_URL, + ]) { + expect(isAllowedTextLinkUrl(url)).toBe(true); + expect(() => textLinkUrl.validate(url)).not.toThrow(); + } + }); + + it("rejects protocols that are not on the allowlist", () => { + // The link renders as , so javascript: would be an XSS vector. + for (const url of [ + "javascript:alert(1)", + "file:///etc/passwd", + "data:text/html,