From e4d357f480a2314e4b1057ece3b0c2ec2d389d5e Mon Sep 17 00:00:00 2001 From: Trang Doan Date: Sun, 27 Sep 2026 16:43:57 -0400 Subject: [PATCH 1/4] ENG-2257 Add an existing discourse relation from relation menu MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Wire "Add existing…" to a picker in the relation type menu that links an accepted relation type to the arrow's node-type pair, then shows it in the menu with a toast. Adds the shared pair-link helper ENG-2258 reuses. Co-Authored-By: Claude Opus 5.5 Entire-Checkpoint: 01M3J9REFKC30VYBVF3H13BG5P --- .../canvas/overlays/DragHandleOverlay.tsx | 1 + .../canvas/overlays/RelationTypeDropdown.tsx | 181 ++++++++++++++---- .../canvas/utils/relationTypeUtils.ts | 62 ++++++ 3 files changed, 204 insertions(+), 40 deletions(-) diff --git a/apps/obsidian/src/components/canvas/overlays/DragHandleOverlay.tsx b/apps/obsidian/src/components/canvas/overlays/DragHandleOverlay.tsx index d1769d326..a118ec608 100644 --- a/apps/obsidian/src/components/canvas/overlays/DragHandleOverlay.tsx +++ b/apps/obsidian/src/components/canvas/overlays/DragHandleOverlay.tsx @@ -444,6 +444,7 @@ export const DragHandleOverlay = ({ plugin, file }: DragHandleOverlayProps) => { diff --git a/apps/obsidian/src/components/canvas/overlays/RelationTypeDropdown.tsx b/apps/obsidian/src/components/canvas/overlays/RelationTypeDropdown.tsx index c2b99dcca..9f4bceca0 100644 --- a/apps/obsidian/src/components/canvas/overlays/RelationTypeDropdown.tsx +++ b/apps/obsidian/src/components/canvas/overlays/RelationTypeDropdown.tsx @@ -15,14 +15,20 @@ import { getArrowInfo, } from "~/components/canvas/utils/relationUtils"; import { + associateRelationTypeWithNodePair, + getAssociableRelationTypesForNodePair, getDiscourseNodeTypeId, getValidRelationTypesForNodePair, } from "~/components/canvas/utils/relationTypeUtils"; +import { showToast } from "~/components/canvas/utils/toastUtils"; +import { getNodeTypeById } from "~/utils/typeUtils"; +import type { DiscourseRelationType } from "~/types"; import { clampMenuCentre } from "~/components/canvas/utils/menuPlacement"; type RelationTypeDropdownProps = { arrowId: TLShapeId; plugin: DiscourseGraphPlugin; + canvasPath: string; onSelect: (relationTypeId: string) => void; onDismiss: () => void; }; @@ -30,12 +36,14 @@ type RelationTypeDropdownProps = { export const RelationTypeDropdown = ({ arrowId, plugin, + canvasPath, onSelect, onDismiss, }: RelationTypeDropdownProps) => { const editor = useEditor(); const dropdownRef = useRef(null); const [isAddMenuOpen, setIsAddMenuOpen] = useState(false); + const [isPickingExisting, setIsPickingExisting] = useState(false); const arrow = useValue( "dropdownArrow", @@ -50,29 +58,45 @@ export const RelationTypeDropdown = ({ } }, [arrow, onDismiss]); - // Get valid relation types based on source/target node types - const validRelationTypes = useMemo(() => { - if (!arrow) return []; + const nodePair = useMemo(() => { + if (!arrow) return null; const bindings = getArrowBindings(editor, arrow); - if (!bindings.start || !bindings.end) return []; + if (!bindings.start || !bindings.end) return null; - const startNode = editor.getShape(bindings.start.toId); - const endNode = editor.getShape(bindings.end.toId); + const sourceNodeTypeId = getDiscourseNodeTypeId( + editor.getShape(bindings.start.toId), + ); + const targetNodeTypeId = getDiscourseNodeTypeId( + editor.getShape(bindings.end.toId), + ); + if (!sourceNodeTypeId || !targetNodeTypeId) return null; - if (!startNode || !endNode) return []; + return { sourceNodeTypeId, targetNodeTypeId }; + }, [arrow, editor]); - const startNodeTypeId = getDiscourseNodeTypeId(startNode); - const endNodeTypeId = getDiscourseNodeTypeId(endNode); - - if (!startNodeTypeId || !endNodeTypeId) return []; - - return getValidRelationTypesForNodePair({ - settings: plugin.settings, - sourceNodeTypeId: startNodeTypeId, - targetNodeTypeId: endNodeTypeId, - }); - }, [arrow, editor, plugin]); + // Associating replaces the discourseRelations array, so its identity refreshes both lists + const { relationTypes, discourseRelations } = plugin.settings; + const validRelationTypes = useMemo( + () => + nodePair + ? getValidRelationTypesForNodePair({ + settings: { relationTypes, discourseRelations }, + ...nodePair, + }) + : [], + [nodePair, relationTypes, discourseRelations], + ); + const associableRelationTypes = useMemo( + () => + nodePair + ? getAssociableRelationTypesForNodePair({ + settings: { relationTypes, discourseRelations }, + ...nodePair, + }) + : [], + [nodePair, relationTypes, discourseRelations], + ); // Position dropdown at arrow midpoint const dropdownPosition = useValue<{ left: number; top: number } | null>( @@ -109,6 +133,7 @@ export const RelationTypeDropdown = ({ } | null>(null); const hasPosition = !!dropdownPosition; const relationTypeCount = validRelationTypes.length; + const associableCount = associableRelationTypes.length; useLayoutEffect(() => { const [menu, flyout] = Array.from( dropdownRef.current?.children ?? [], @@ -120,7 +145,13 @@ export const RelationTypeDropdown = ({ ? flyout.offsetLeft + flyout.offsetWidth - menu.offsetWidth : 0, }); - }, [hasPosition, isAddMenuOpen, relationTypeCount]); + }, [ + hasPosition, + isAddMenuOpen, + isPickingExisting, + relationTypeCount, + associableCount, + ]); // Handle click outside useEffect(() => { @@ -144,10 +175,15 @@ export const RelationTypeDropdown = ({ }; }, [onDismiss]); - // Handle Escape key: close the add menu first, then the dropdown + // Handle Escape key: leave the picker or add menu first, then the dropdown useEffect(() => { const handleKeyDown = (e: KeyboardEvent) => { if (e.key !== "Escape") return; + if (isPickingExisting) { + e.stopPropagation(); + setIsPickingExisting(false); + return; + } if (isAddMenuOpen) { e.stopPropagation(); setIsAddMenuOpen(false); @@ -157,7 +193,7 @@ export const RelationTypeDropdown = ({ }; window.addEventListener("keydown", handleKeyDown, true); return () => window.removeEventListener("keydown", handleKeyDown, true); - }, [isAddMenuOpen, onDismiss]); + }, [isAddMenuOpen, isPickingExisting, onDismiss]); const handleSelect = useCallback( (relationTypeId: string) => { @@ -166,6 +202,38 @@ export const RelationTypeDropdown = ({ [onSelect], ); + const handleAssociate = useCallback( + async (relationType: DiscourseRelationType) => { + if (!nodePair) return; + try { + await associateRelationTypeWithNodePair({ + plugin, + relationTypeId: relationType.id, + ...nodePair, + }); + } catch { + showToast({ + severity: "error", + title: "Couldn't add relation", + targetCanvasId: canvasPath, + }); + return; + } + setIsPickingExisting(false); + const sourceName = + getNodeTypeById(plugin, nodePair.sourceNodeTypeId)?.name ?? "source"; + const targetName = + getNodeTypeById(plugin, nodePair.targetNodeTypeId)?.name ?? "target"; + showToast({ + severity: "success", + title: "Discourse relation added", + description: `${relationType.label} relation added for ${sourceName} and ${targetName}`, + targetCanvasId: canvasPath, + }); + }, + [nodePair, plugin, canvasPath], + ); + if (!dropdownPosition || !arrow) return null; const centre = menuSize @@ -181,11 +249,50 @@ export const RelationTypeDropdown = ({ "flex w-full cursor-pointer items-center justify-start rounded border-none bg-transparent px-2 py-1.5 text-left text-sm font-medium text-gray-700 hover:bg-gray-100"; const addActions = ( <> - + ); const hasRelationTypes = validRelationTypes.length > 0; + const listContent = hasRelationTypes + ? validRelationTypes.map((rt) => ( + + )) + : addActions; + const pickerContent = + associableRelationTypes.length > 0 ? ( + associableRelationTypes.map((rt) => ( + + )) + ) : ( +

+ All relation types are already available for these nodes +

+ ); return (
e.stopPropagation()} >
-
- +
+ {isPickingExisting && ( +
- {hasRelationTypes - ? validRelationTypes.map((rt) => ( - - )) - : addActions} + {isPickingExisting ? pickerContent : listContent}
{/* Outside the scroll container so it isn't clipped, inside dropdownRef so clicks don't dismiss */} {hasRelationTypes && isAddMenuOpen && ( diff --git a/apps/obsidian/src/components/canvas/utils/relationTypeUtils.ts b/apps/obsidian/src/components/canvas/utils/relationTypeUtils.ts index b18d81dcd..a0a11dd2e 100644 --- a/apps/obsidian/src/components/canvas/utils/relationTypeUtils.ts +++ b/apps/obsidian/src/components/canvas/utils/relationTypeUtils.ts @@ -1,7 +1,10 @@ import type { Editor, TLShape, TLShapeId, VecLike } from "tldraw"; import type { DiscourseNodeShape } from "~/components/canvas/shapes/DiscourseNodeShape"; +import type DiscourseGraphPlugin from "~/index"; import type { DiscourseRelation, DiscourseRelationType } from "~/types"; +import generateUid from "~/utils/generateUid"; import { COLOR_PALETTE } from "~/utils/tldrawColors"; +import { isAcceptedSchema } from "~/utils/typeUtils"; export const isDiscourseNodeShape = ( shape: TLShape | null | undefined, @@ -140,6 +143,65 @@ export const getValidRelationTypesForNodePair = ({ return validTypes; }; +/** + * Returns the accepted relation types not yet valid for a node pair in either + * direction, which the "Add existing" picker offers. + */ +export const getAssociableRelationTypesForNodePair = ({ + settings, + sourceNodeTypeId, + targetNodeTypeId, +}: { + settings: RelationTypeSettings; + sourceNodeTypeId: string; + targetNodeTypeId: string; +}): DiscourseRelationType[] => + settings.relationTypes.filter((relationType) => { + if (!isAcceptedSchema(relationType)) return false; + const { direct, reverse } = getRelationDirection({ + discourseRelations: settings.discourseRelations, + relationTypeId: relationType.id, + sourceNodeTypeId, + targetNodeTypeId, + }); + return !direct && !reverse; + }); + +/** + * Makes a relation type valid for a source → target node pair and saves it. + * Restores the previous relations if the save fails. + */ +export const associateRelationTypeWithNodePair = async ({ + plugin, + relationTypeId, + sourceNodeTypeId, + targetNodeTypeId, +}: { + plugin: DiscourseGraphPlugin; + relationTypeId: string; + sourceNodeTypeId: string; + targetNodeTypeId: string; +}): Promise => { + const now = Date.now(); + const relation: DiscourseRelation = { + id: generateUid("rel3"), + sourceId: sourceNodeTypeId, + destinationId: targetNodeTypeId, + relationshipTypeId: relationTypeId, + created: now, + modified: now, + }; + const previousRelations = plugin.settings.discourseRelations; + plugin.settings.discourseRelations = [...previousRelations, relation]; + try { + await plugin.saveSettings(); + } catch (error) { + plugin.settings.discourseRelations = previousRelations; + throw error; + } + return relation; +}; + /** * Checks whether a specific relation type can connect the given source and * target node types (in either direction). From 92a8379041584092db9f67f2c95eb81205a243ad Mon Sep 17 00:00:00 2001 From: Trang Doan Date: Sun, 27 Sep 2026 16:46:46 -0400 Subject: [PATCH 2/4] ENG-2257 Drop unused return value from the pair-link helper Co-Authored-By: Claude Opus 5.5 Entire-Checkpoint: 01M3J9XJKH4F716WY3TM769TG4 --- apps/obsidian/src/components/canvas/utils/relationTypeUtils.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/apps/obsidian/src/components/canvas/utils/relationTypeUtils.ts b/apps/obsidian/src/components/canvas/utils/relationTypeUtils.ts index a0a11dd2e..246b55c15 100644 --- a/apps/obsidian/src/components/canvas/utils/relationTypeUtils.ts +++ b/apps/obsidian/src/components/canvas/utils/relationTypeUtils.ts @@ -181,7 +181,7 @@ export const associateRelationTypeWithNodePair = async ({ relationTypeId: string; sourceNodeTypeId: string; targetNodeTypeId: string; -}): Promise => { +}): Promise => { const now = Date.now(); const relation: DiscourseRelation = { id: generateUid("rel3"), @@ -199,7 +199,6 @@ export const associateRelationTypeWithNodePair = async ({ plugin.settings.discourseRelations = previousRelations; throw error; } - return relation; }; /** From 092601488a5bd7a549e376242b9021016a4dcebf Mon Sep 17 00:00:00 2001 From: Trang Doan Date: Sun, 27 Sep 2026 16:55:21 -0400 Subject: [PATCH 3/4] ENG-2257 Add unit tests for the relation picker helpers Co-Authored-By: Claude Opus 5.5 Entire-Checkpoint: 01M3JADA205HB1WJG30ER2HJVE --- .../utils/__tests__/relationTypeUtils.test.ts | 143 ++++++++++++++++++ 1 file changed, 143 insertions(+) create mode 100644 apps/obsidian/src/components/canvas/utils/__tests__/relationTypeUtils.test.ts diff --git a/apps/obsidian/src/components/canvas/utils/__tests__/relationTypeUtils.test.ts b/apps/obsidian/src/components/canvas/utils/__tests__/relationTypeUtils.test.ts new file mode 100644 index 000000000..fcfc4ea6f --- /dev/null +++ b/apps/obsidian/src/components/canvas/utils/__tests__/relationTypeUtils.test.ts @@ -0,0 +1,143 @@ +import { describe, expect, it, vi } from "vitest"; +import type DiscourseGraphPlugin from "~/index"; +import { + associateRelationTypeWithNodePair, + getAssociableRelationTypesForNodePair, +} from "~/components/canvas/utils/relationTypeUtils"; +import type { DiscourseRelation, DiscourseRelationType } from "~/types"; + +const relationType = ( + id: string, + overrides: Partial = {}, +): DiscourseRelationType => ({ + id, + label: id, + complement: `${id} complement`, + color: "black", + created: 0, + modified: 0, + ...overrides, +}); + +const relation = ( + relationshipTypeId: string, + sourceId: string, + destinationId: string, +): DiscourseRelation => ({ + id: `${relationshipTypeId}-${sourceId}-${destinationId}`, + relationshipTypeId, + sourceId, + destinationId, + created: 0, + modified: 0, +}); + +describe("getAssociableRelationTypesForNodePair", () => { + const relationTypes = [ + relationType("supports"), + relationType("opposes"), + relationType("informs"), + relationType("provisional", { + importedFromRid: "rid:provisional", + status: "provisional", + }), + relationType("importedAccepted", { + importedFromRid: "rid:accepted", + status: "accepted", + }), + ]; + const discourseRelations = [ + relation("supports", "evidence", "claim"), + relation("opposes", "claim", "evidence"), + relation("informs", "evidence", "question"), + ]; + + const associableIds = ( + relations: DiscourseRelation[], + sourceNodeTypeId: string, + targetNodeTypeId: string, + ): string[] => + getAssociableRelationTypesForNodePair({ + settings: { relationTypes, discourseRelations: relations }, + sourceNodeTypeId, + targetNodeTypeId, + }).map(({ id }) => id); + + it("excludes types valid for the pair in either direction", () => { + expect(associableIds(discourseRelations, "evidence", "claim")).toEqual([ + "informs", + "importedAccepted", + ]); + }); + + it("offers every accepted type for a pair with no relations", () => { + expect(associableIds(discourseRelations, "question", "claim")).toEqual([ + "supports", + "opposes", + "informs", + "importedAccepted", + ]); + }); + + it("excludes a type made valid by a provisional relation", () => { + const relations: DiscourseRelation[] = [ + ...discourseRelations, + { + ...relation("informs", "claim", "evidence"), + importedFromRid: "rid:informs", + status: "provisional", + }, + ]; + expect(associableIds(relations, "evidence", "claim")).toEqual([ + "importedAccepted", + ]); + }); +}); + +describe("associateRelationTypeWithNodePair", () => { + const setup = (saveSettings: () => Promise) => { + const existing = [relation("supports", "evidence", "claim")]; + const plugin = { + settings: { discourseRelations: existing }, + saveSettings: vi.fn(saveSettings), + }; + return { + existing, + plugin, + associate: () => + associateRelationTypeWithNodePair({ + plugin: plugin as unknown as DiscourseGraphPlugin, + relationTypeId: "informs", + sourceNodeTypeId: "question", + targetNodeTypeId: "claim", + }), + }; + }; + + it("saves a new local source → target relation in a new array", async () => { + const { existing, plugin, associate } = setup(() => Promise.resolve()); + await associate(); + + const relations = plugin.settings.discourseRelations; + const added = relations.at(-1); + expect(relations).not.toBe(existing); + expect(relations).toHaveLength(2); + expect(added).toMatchObject({ + sourceId: "question", + destinationId: "claim", + relationshipTypeId: "informs", + }); + expect(added?.id).toMatch(/^rel3_/); + expect(added).not.toHaveProperty("status"); + expect(added).not.toHaveProperty("importedFromRid"); + expect(plugin.saveSettings).toHaveBeenCalledTimes(1); + }); + + it("restores the previous relations and rethrows when saving fails", async () => { + const { existing, plugin, associate } = setup(() => + Promise.reject(new Error("disk full")), + ); + await expect(associate()).rejects.toThrow("disk full"); + expect(plugin.settings.discourseRelations).toBe(existing); + }); +}); From fe1a99286dd013042446cf357dd96f7c7dc411aa Mon Sep 17 00:00:00 2001 From: Trang Doan Date: Mon, 28 Sep 2026 10:24:15 -0400 Subject: [PATCH 4/4] ENG-2257 Keep concurrent relation associations intact Roll back a failed save by removing only the added relation, so a concurrent association that saved meanwhile survives. Disable picker rows while a save is pending so a double click can't add the same triple twice. Co-Authored-By: Claude Opus 5.5 Entire-Checkpoint: 01M3M6DWP2S1FKJJRY03MA09AG --- .../canvas/overlays/RelationTypeDropdown.tsx | 9 +++++++-- .../canvas/utils/__tests__/relationTypeUtils.test.ts | 4 ++-- .../src/components/canvas/utils/relationTypeUtils.ts | 12 ++++++++---- 3 files changed, 17 insertions(+), 8 deletions(-) diff --git a/apps/obsidian/src/components/canvas/overlays/RelationTypeDropdown.tsx b/apps/obsidian/src/components/canvas/overlays/RelationTypeDropdown.tsx index 9f4bceca0..476de637f 100644 --- a/apps/obsidian/src/components/canvas/overlays/RelationTypeDropdown.tsx +++ b/apps/obsidian/src/components/canvas/overlays/RelationTypeDropdown.tsx @@ -44,6 +44,7 @@ export const RelationTypeDropdown = ({ const dropdownRef = useRef(null); const [isAddMenuOpen, setIsAddMenuOpen] = useState(false); const [isPickingExisting, setIsPickingExisting] = useState(false); + const [isSaving, setIsSaving] = useState(false); const arrow = useValue( "dropdownArrow", @@ -204,7 +205,8 @@ export const RelationTypeDropdown = ({ const handleAssociate = useCallback( async (relationType: DiscourseRelationType) => { - if (!nodePair) return; + if (!nodePair || isSaving) return; + setIsSaving(true); try { await associateRelationTypeWithNodePair({ plugin, @@ -218,6 +220,8 @@ export const RelationTypeDropdown = ({ targetCanvasId: canvasPath, }); return; + } finally { + setIsSaving(false); } setIsPickingExisting(false); const sourceName = @@ -231,7 +235,7 @@ export const RelationTypeDropdown = ({ targetCanvasId: canvasPath, }); }, - [nodePair, plugin, canvasPath], + [nodePair, isSaving, plugin, canvasPath], ); if (!dropdownPosition || !arrow) return null; @@ -282,6 +286,7 @@ export const RelationTypeDropdown = ({ associableRelationTypes.map((rt) => (