From 22dd4d65068d31fed27114d9a493ce1acda2bf9e Mon Sep 17 00:00:00 2001 From: Michael Ramos Date: Thu, 17 Sep 2026 14:38:13 -0700 Subject: [PATCH 01/10] refactor(ui): give svgContentSize its own dependency-free module The document's fence block needs the rendered diagram's intrinsic size to size its inline box, and nothing else of the canvas. Importing it from DiagramCanvas dragged the zoom/pan surface, its viewport hook and lucide into every closure that touched a diagram block. DiagramCanvas re-exports it, so components/diagram/DiagramCanvas and the barrel still resolve it. --- .../ui/components/diagram/DiagramCanvas.tsx | 39 ++------------- .../ui/components/diagram/svgContentSize.ts | 47 +++++++++++++++++++ 2 files changed, 52 insertions(+), 34 deletions(-) create mode 100644 packages/ui/components/diagram/svgContentSize.ts diff --git a/packages/ui/components/diagram/DiagramCanvas.tsx b/packages/ui/components/diagram/DiagramCanvas.tsx index f677e2dd2..cb78b151a 100644 --- a/packages/ui/components/diagram/DiagramCanvas.tsx +++ b/packages/ui/components/diagram/DiagramCanvas.tsx @@ -14,6 +14,7 @@ import { cn } from '../../lib/utils'; import { diagramHitSource } from '../../utils/diagram-render'; import { isMac, isModKeyHeld } from '../../utils/platform'; import { Button } from '../ui/button'; +import { svgContentSize } from './svgContentSize'; import { DRAG_THRESHOLD_PX, TOUCH_DRAG_THRESHOLD_PX, @@ -57,40 +58,10 @@ import { * viewport so rings reproject on every change. */ -/** A numeric svg length: `206`, `206pt`, `206px`. */ -function svgLength(value: string | null): number { - if (value === null) return Number.NaN; - return Number.parseFloat(value); -} - -/** The svg's intrinsic size, from its viewBox (Mermaid and Graphviz always - * write one), else its `width` / `height` attributes (a `pt` or `px` suffix - * is accepted, as Graphviz writes them). */ -export function svgContentSize(svg: SVGSVGElement): ContentSize | null { - const viewBox = svg.getAttribute('viewBox'); - if (viewBox !== null) { - const parts = viewBox - .trim() - .split(/[\s,]+/u) - .map(Number); - const width = parts[2]; - const height = parts[3]; - if ( - parts.length === 4 && - width !== undefined && - height !== undefined && - Number.isFinite(width) && - Number.isFinite(height) && - width > 0 && - height > 0 - ) { - return { width, height }; - } - } - const width = svgLength(svg.getAttribute('width')); - const height = svgLength(svg.getAttribute('height')); - return Number.isFinite(width) && Number.isFinite(height) && width > 0 && height > 0 ? { width, height } : null; -} +// `svgContentSize` is the canvas's, but the document's fence block needs it +// without the canvas: it lives in its own dependency-free module and is +// re-exported here so every published path keeps resolving. +export { svgContentSize } from './svgContentSize'; /** One arrow-key press pans this far (the diagram moves WITH the arrow, as * a scroll would); Shift multiplies it by five. */ diff --git a/packages/ui/components/diagram/svgContentSize.ts b/packages/ui/components/diagram/svgContentSize.ts new file mode 100644 index 000000000..2f7d17870 --- /dev/null +++ b/packages/ui/components/diagram/svgContentSize.ts @@ -0,0 +1,47 @@ +import type { ContentSize } from './useDiagramViewport'; + +/** + * The rendered diagram's intrinsic size — a pure read of two svg attributes. + * + * It lives apart from `DiagramCanvas` (which re-exports it, so every + * published path still resolves) because the document's fence block needs + * it to size its inline box and nothing else of the canvas: importing it + * from the canvas dragged the whole zoom/pan surface, its viewport hook and + * lucide into any closure that touched a diagram block. The type import is + * erased, so this file's runtime dependencies are none. + */ + +/** A numeric svg length: `206`, `206pt`, `206px`. */ +function svgLength(value: string | null): number { + if (value === null) return Number.NaN; + return Number.parseFloat(value); +} + +/** The svg's intrinsic size, from its viewBox (Mermaid and Graphviz always + * write one), else its `width` / `height` attributes (a `pt` or `px` suffix + * is accepted, as Graphviz writes them). */ +export function svgContentSize(svg: SVGSVGElement): ContentSize | null { + const viewBox = svg.getAttribute('viewBox'); + if (viewBox !== null) { + const parts = viewBox + .trim() + .split(/[\s,]+/u) + .map(Number); + const width = parts[2]; + const height = parts[3]; + if ( + parts.length === 4 && + width !== undefined && + height !== undefined && + Number.isFinite(width) && + Number.isFinite(height) && + width > 0 && + height > 0 + ) { + return { width, height }; + } + } + const width = svgLength(svg.getAttribute('width')); + const height = svgLength(svg.getAttribute('height')); + return Number.isFinite(width) && Number.isFinite(height) && width > 0 && height > 0 ? { width, height } : null; +} From e260d451c19f1610f341e5b0d9153c31ecdc7cba Mon Sep 17 00:00:00 2001 From: Michael Ramos Date: Thu, 17 Sep 2026 14:38:17 -0700 Subject: [PATCH 02/10] perf(ui): load the diagram Source pane only when it opens The pane is CodeMirror, and a viewer that can never open one -- every fence in a Plannotator document today, since no host passes onSave -- carried the whole editor anyway. React.lazy behind the existing hasPane && sourceOpen condition; the Suspense fallback is a box with the pane's own class list rather than null, so the split it opens into is already the right size and never collapses back onto the canvas. A host that imports DiagramViewer directly still gets a working pane; it arrives one chunk later. --- .../ui/components/diagram/DiagramViewer.tsx | 25 +++++++++++++------ 1 file changed, 17 insertions(+), 8 deletions(-) diff --git a/packages/ui/components/diagram/DiagramViewer.tsx b/packages/ui/components/diagram/DiagramViewer.tsx index c2cd4c9f7..151118dd1 100644 --- a/packages/ui/components/diagram/DiagramViewer.tsx +++ b/packages/ui/components/diagram/DiagramViewer.tsx @@ -1,4 +1,4 @@ -import { useCallback, useEffect, useMemo, useState, type ReactNode } from 'react'; +import { lazy, Suspense, useCallback, useEffect, useMemo, useState, type ReactNode } from 'react'; import type { DiagramKind } from '@plannotator/core/diagram-anchor'; import { cn } from '../../lib/utils'; import { diagramFamilyOf } from '../../utils/diagram-anchor'; @@ -6,7 +6,6 @@ import { diagramFinder, type DiagramTheme } from '../../utils/diagram-render'; import { DiagramCanvas, type DiagramCanvasHandle, type DiagramEscapeOutcome } from './DiagramCanvas'; import { DiagramComposer } from './DiagramComposer'; import { DiagramOverlay } from './DiagramOverlay'; -import { DiagramSourcePane } from './DiagramSourcePane'; import { useDiagramComments, type DiagramComment, type DiagramCreateComment } from './useDiagramComments'; import { useDiagramRender, type DiagramRenderState } from './useDiagramRender'; import { useDiagramSourceDraft, type SaveResult } from './useDiagramSourceDraft'; @@ -21,6 +20,19 @@ import { useDiagramSourceDraft, type SaveResult } from './useDiagramSourceDraft' * (`components/DiagramBlock`) and again at full size in the popout; a host * with its own document store renders it wherever a diagram lives. */ +/** + * The Source pane is CodeMirror, and a viewer that can never open one (every + * fence in a Plannotator document today: no host passes `onSave`) must not + * carry it. It loads on the first open of the pane and never before, so the + * editor stays out of the document-read closure of a chunked host. + */ +const DiagramSourcePane = lazy(async () => ({ default: (await import('./DiagramSourcePane')).DiagramSourcePane })); + +/** The pane's box while its chunk loads: same class list, so the split it + * opens into is already the right size and nothing jumps when it lands. + * Never `null` here — that would collapse the row back onto the canvas. */ +const PANE_CLASS = 'order-last min-h-0 shrink-0 basis-2/5 border-t border-border md:order-first md:w-80 md:basis-auto md:border-r md:border-t-0'; + export interface DiagramViewerProps { readonly kind: DiagramKind; /** The diagram text. With `onSave` this is the saved baseline the pane's @@ -206,12 +218,9 @@ export function DiagramViewer({ column and the pane stays stacked UNDER the canvas, which `order-last` keeps while `md:order-first` puts it left from `md`. */} {hasPane && sourceOpen && ( - + )}
{showFallback ? ( From 8edbf0ab31132d2e365f864336463e2fb8c64662 Mon Sep 17 00:00:00 2001 From: Michael Ramos Date: Thu, 17 Sep 2026 14:38:21 -0700 Subject: [PATCH 03/10] perf(ui): load the diagram popout only when it opens MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The full-size popout is only ever reached by pressing Expand. React.lazy with a null fallback: the overlay has simply not opened yet, so nothing in the document flow moves. The block's pending state (the source fence under "Rendering diagram…", data-mermaid-pending and all) moves to its own dependency-free module, because it is about to become the Suspense fallback the document shows while the block's own chunk loads -- one pending state for both waits. --- packages/ui/components/DiagramBlock.tsx | 53 +++++++--------- .../ui/components/diagram/DiagramPending.tsx | 60 +++++++++++++++++++ 2 files changed, 80 insertions(+), 33 deletions(-) create mode 100644 packages/ui/components/diagram/DiagramPending.tsx diff --git a/packages/ui/components/DiagramBlock.tsx b/packages/ui/components/DiagramBlock.tsx index d08561525..828d2eacb 100644 --- a/packages/ui/components/DiagramBlock.tsx +++ b/packages/ui/components/DiagramBlock.tsx @@ -1,4 +1,4 @@ -import React, { useCallback, useContext, useEffect, useMemo, useRef, useState, useSyncExternalStore } from 'react'; +import React, { lazy, Suspense, useCallback, useContext, useEffect, useMemo, useRef, useState, useSyncExternalStore } from 'react'; import { diagramTargetText, type DiagramKind } from '@plannotator/core/diagram-anchor'; import type { AnnotationRestoreReport } from '../hooks/useAnnotationHighlighter'; import { AnnotationType, type Annotation, type Block } from '../types'; @@ -6,9 +6,9 @@ import type { DiagramTheme } from '../utils/diagram-render'; import { getIdentity } from '../utils/identity'; import { createRuntimeRetryEpoch } from '../utils/runtimeRetry'; import { DiagramAnchorClaims, DiagramAnchorClaimsContext } from './diagram/anchorClaims'; -import { svgContentSize } from './diagram/DiagramCanvas'; -import { DiagramPopout } from './diagram/DiagramPopout'; +import { DiagramPending, DiagramInlineSource } from './diagram/DiagramPending'; import { DiagramViewer } from './diagram/DiagramViewer'; +import { svgContentSize } from './diagram/svgContentSize'; import type { DiagramComment, DiagramCreateComment } from './diagram/useDiagramComments'; import type { DiagramRenderState } from './diagram/useDiagramRender'; import { useTheme } from './ThemeProvider'; @@ -35,6 +35,11 @@ const RETRY_EPOCHS: Record = { mermaid: 'Mermaid', graphviz: 'Graphviz' }; +/** The full-size popout is only ever reached by pressing Expand, so it loads + * then and not with the document. Its fallback is null: the overlay simply + * has not opened yet, and nothing in the document flow moves. */ +const DiagramPopout = lazy(async () => ({ default: (await import('./diagram/DiagramPopout')).DiagramPopout })); + /** The inline box height from the diagram's aspect at a nominal width, so * a wide flowchart is not letterboxed in a tall box and a tall state * diagram is not squeezed into a short one; clamped so neither extreme @@ -260,21 +265,7 @@ export const DiagramBlock: React.FC = // then the render itself): the source stays readable under a quiet // status line. A re-render for a theme change keeps the previous SVG, // so this shows only before the first diagram lands. - return ( - <> -
-
- - - ); + return ; }, [block, kind, label], ); @@ -337,7 +328,7 @@ export const DiagramBlock: React.FC = - +
) : (
= )}
{isExpanded && svgReady && typeof document !== 'undefined' && ( - setIsExpanded(false)} - title={`${label} diagram`} - renderId={`${kind}-${block.id}-popout`} - dataAttributes={{ 'data-block-id': block.id }} - /> + + setIsExpanded(false)} + title={`${label} diagram`} + renderId={`${kind}-${block.id}-popout`} + dataAttributes={{ 'data-block-id': block.id }} + /> + )} ); }; - -const InlineSource: React.FC<{ block: Block; kind: DiagramKind }> = ({ block, kind }) => ( - 
-    {block.content}
-
-); diff --git a/packages/ui/components/diagram/DiagramPending.tsx b/packages/ui/components/diagram/DiagramPending.tsx new file mode 100644 index 000000000..7d4aa3a24 --- /dev/null +++ b/packages/ui/components/diagram/DiagramPending.tsx @@ -0,0 +1,60 @@ +import React from 'react'; +import type { DiagramKind } from '@plannotator/core/diagram-anchor'; +import type { Block } from '../../types'; + +/** + * The diagram fence before there is a diagram: the source under a quiet + * "Rendering diagram…" status. + * + * It lives in its own file, importing nothing but React and two types, + * because it is the ONE pending state two callers must agree on. The block + * shows it while the engine chunk and the first render are in flight + * (`DiagramBlock`'s `renderFallback`), and the document shows it as the + * Suspense fallback while the diagram block's own chunk loads + * (`Viewer`) — a chunked host reaches the second one first. Same markup in + * both places, so a document with a diagram paints the source fence once and + * never flashes or jumps between the two. + * + * Keep it dependency-free: anything imported here lands in the closure of + * every document read, diagram or not. + */ + +/** The fence's own text, as the document would show it unrendered. */ +export const DiagramInlineSource: React.FC<{ block: Block; kind: DiagramKind }> = ({ block, kind }) => ( + 
+    {block.content}
+
+); + +/** The status line plus the source. `data-mermaid-pending` is the hook the + * Mermaid tests wait on and is kept exactly as it was. */ +export const DiagramPending: React.FC<{ block: Block; kind: DiagramKind }> = ({ block, kind }) => ( + <> +
+
+ + +); + +/** + * The same pending state inside the boxes the block itself renders it in + * (the block wrapper, the inline host, the viewer's fallback slot), so the + * Suspense fallback occupies the same space the block will. + */ +export const DiagramBlockPending: React.FC<{ block: Block; kind: DiagramKind }> = ({ block, kind }) => ( +
+
+
+ +
+
+
+); From e017aafeb868a77f46aef5f41c0a91a6c656deb3 Mon Sep 17 00:00:00 2001 From: Michael Ramos Date: Thu, 17 Sep 2026 14:38:26 -0700 Subject: [PATCH 04/10] perf(ui): load the diagram engine only for documents that have a diagram Viewer imported MermaidBlock and GraphvizBlock statically, so every host that reads a document shipped the renderer slot, the canvas, the comment overlay, the finders and the projection whether or not the document had a fence. Both blocks are now React.lazy, one chunk each over a shared DiagramBlock chunk. The Suspense fallback is the block's own pending state in the block's own boxes (DiagramBlockPending), so the source fence paints with the document and the two waits -- the block chunk, then the engine -- read as one. --- packages/ui/components/Viewer.tsx | 61 +++++++++++++++++++------------ 1 file changed, 38 insertions(+), 23 deletions(-) diff --git a/packages/ui/components/Viewer.tsx b/packages/ui/components/Viewer.tsx index 302d9b675..211f98979 100644 --- a/packages/ui/components/Viewer.tsx +++ b/packages/ui/components/Viewer.tsx @@ -1,5 +1,5 @@ import { generateId } from '../utils/generateId'; -import React, { useRef, useState, useEffect, useMemo, forwardRef, useImperativeHandle, useCallback } from 'react'; +import React, { useRef, useState, useEffect, useMemo, forwardRef, useImperativeHandle, useCallback, lazy, Suspense } from 'react'; import { createPortal } from 'react-dom'; import { AnnotationType, type Block, type Annotation, type EditorMode, type InputMethod, type ImageAttachment, type ActionsLabelMode } from '../types'; import { applyHighlight, codeBlockClassName, onCodeHighlightSwap } from '../utils/codeHighlight'; @@ -18,6 +18,20 @@ import { useValidatedCodePaths } from '../hooks/useValidatedCodePaths'; import { AnnotationToolbar } from './AnnotationToolbar'; import { FloatingQuickLabelPicker } from './FloatingQuickLabelPicker'; +/** + * The diagram engine — the renderer slot, the canvas, the comment overlay, + * the popout, and (through the viewer) CodeMirror — is loaded by the first + * diagram fence in the document and by nothing else. A markdown document + * with no diagram never reaches for it; a chunked host that statically + * imports this Viewer pays none of it on a plain document read. + * + * The Suspense fallback is the SAME pending state the block itself shows + * while its engine loads (`DiagramPending`), inside the same boxes, so the + * source fence paints once and the two waits read as one. + */ +const MermaidBlock = lazy(async () => ({ default: (await import('./MermaidBlock')).MermaidBlock })); +const GraphvizBlock = lazy(async () => ({ default: (await import('./GraphvizBlock')).GraphvizBlock })); + // Debug error boundary to catch silent toolbar crashes class ToolbarErrorBoundary extends React.Component< { children: React.ReactNode }, @@ -41,8 +55,7 @@ import { TaterSpriteSitting } from './TaterSpriteSitting'; import { AttachmentsButton } from './AttachmentsButton'; import { MessagesIcon } from './icons/MessagesIcon'; import { DiagramAnchorClaims, DiagramAnchorClaimsContext } from './diagram/anchorClaims'; -import { GraphvizBlock } from './GraphvizBlock'; -import { MermaidBlock } from './MermaidBlock'; +import { DiagramBlockPending } from './diagram/DiagramPending'; import { isGraphvizLanguage, isMermaidLanguage } from './diagramLanguages'; import { getIdentity } from '../utils/identity'; import { type QuickLabel } from '../utils/quickLabels'; @@ -1168,27 +1181,29 @@ export const Viewer = forwardRef(({ ); })() ) : group.block.type === 'code' && isMermaidLanguage(group.block.language) ? ( - + }> + + ) : group.block.type === 'code' && isGraphvizLanguage(group.block.language) ? ( - + }> + + ) : group.block.type === 'table' ? ( Date: Thu, 17 Sep 2026 14:38:31 -0700 Subject: [PATCH 05/10] test(ui): pin the document-read closure of the Viewer entry Bundles Viewer with the chunking bundler the portal build uses and walks the entry chunk's static imports only (rollup reports imports and dynamicImports separately, so a lazy edge is a chunk boundary by construction). Asserts the entry closure reaches no CodeMirror, Source pane, popout, canvas or projection, that each of those is still reachable off-entry, and that the fence's pending state stays in the entry chunk. Single-file builds inline everything and can never show this regression, which is why it needs its own check. --- .../components/Viewer.diagramClosure.test.ts | 120 ++++++++++++++++++ 1 file changed, 120 insertions(+) create mode 100644 packages/ui/components/Viewer.diagramClosure.test.ts diff --git a/packages/ui/components/Viewer.diagramClosure.test.ts b/packages/ui/components/Viewer.diagramClosure.test.ts new file mode 100644 index 000000000..4c46d5c13 --- /dev/null +++ b/packages/ui/components/Viewer.diagramClosure.test.ts @@ -0,0 +1,120 @@ +/** + * What a host pays to read a document. + * + * `Viewer` is the entry every host that renders a Plannotator document + * imports statically, so everything reachable from it through STATIC + * imports sits in that host's document-read chunk — diagram or no diagram. + * The diagram engine is not small: the canvas with its zoom/pan surface and + * its projection, the comment overlay, the full-size popout, and through the + * viewer's Source pane the whole of CodeMirror. + * + * The regression this catches is the one 0.41.0 shipped: `Viewer` imported + * the two diagram blocks statically, `DiagramBlock` imported `DiagramPopout` + * and `DiagramViewer` statically, and `DiagramViewer` imported + * `DiagramSourcePane` statically — so a chunked host carried CodeMirror and + * the whole viewer on every markdown document, about 100 KB gzip of it, and + * no host passes `onSave` for a fence today so none of it could ever run. + * Plannotator's own single-file builds inline everything and never notice, + * which is exactly why a test has to. + * + * Method: bundle `Viewer` with the chunking bundler the portal build uses + * and walk the ENTRY chunk's STATIC imports only (rollup reports `imports` + * and `dynamicImports` separately, so a lazy edge is a chunk boundary by + * construction, not by parsing). Then assert the engine is still REACHABLE + * off-entry: a lazy edge that loads nothing would pass the first half and + * break every diagram. + */ +import { beforeAll, describe, expect, test } from 'bun:test'; +import { resolve } from 'node:path'; + +const uiRoot = resolve(import.meta.dir, '..'); + +/** One marker per edge of the chain, so a partial fix cannot pass. */ +const FORBIDDEN: ReadonlyArray = [ + ['@codemirror/', 'the Source pane editor'], + ['cm-editor', "CodeMirror's own class name (it landed even if the specifier was rewritten)"], + ['DiagramSourcePane', 'the Source pane'], + ['DiagramPopout', 'the full-size popout'], + ['data-diagram-canvas', 'the diagram canvas'], + ['getScreenCTM', 'the diagram projection'], +]; + +let entryClosure = ''; +let offEntry = ''; + +beforeAll(async () => { + const { build } = await import('vite'); + const result = (await build({ + root: uiRoot, + configFile: false, + logLevel: 'silent', + build: { + write: false, + target: 'esnext', + // Unminified: the markers above are read back out of the chunk text. + minify: false, + lib: { entry: resolve(uiRoot, 'components/Viewer.tsx'), formats: ['es'], fileName: 'viewer' }, + rollupOptions: { + // React is the host's; leaving it out keeps the graph to our code. + external: ['react', 'react-dom', 'react/jsx-runtime', 'react-dom/client'], + output: { inlineDynamicImports: false }, + }, + }, + })) as unknown as { output: ReadonlyArray> }; + + const outputs = (Array.isArray(result) ? result[0] : result).output; + const chunks = new Map(); + for (const output of outputs) { + if (output['type'] !== 'chunk') continue; + chunks.set(output['fileName'] as string, { + code: output['code'] as string, + imports: output['imports'] as readonly string[], + }); + } + const entry = outputs.find((output) => output['type'] === 'chunk' && output['isEntry'] === true); + expect(entry).toBeDefined(); + + const reached = new Set(); + const queue = [entry!['fileName'] as string]; + while (queue.length > 0) { + const current = queue.pop()!; + if (reached.has(current)) continue; + reached.add(current); + for (const imported of chunks.get(current)?.imports ?? []) { + if (chunks.has(imported)) queue.push(imported); + } + } + + entryClosure = [...reached].map((name) => chunks.get(name)?.code ?? '').join('\n'); + offEntry = [...chunks.entries()] + .filter(([name]) => !reached.has(name)) + .map(([, chunk]) => chunk.code) + .join('\n'); +}, 120_000); + +describe('document-read closure of the Viewer entry', () => { + test('walked a real entry chunk', () => { + // Guards the check itself: a walk that found nothing would satisfy every + // "does not contain" assertion below while proving nothing. + expect(entryClosure.length).toBeGreaterThan(100_000); + expect(entryClosure).toContain('data-block-id'); + }); + + test.each(FORBIDDEN)('does not statically reach %s (%s)', (marker) => { + expect(entryClosure).not.toContain(marker); + }); + + test('still reaches the diagram engine through dynamic chunks', () => { + for (const [marker] of FORBIDDEN) { + expect(offEntry).toContain(marker); + } + }); + + test('keeps the diagram fence pending state in the entry chunk', () => { + // The Suspense fallback is the block's own pending state, so the source + // fence under "Rendering diagram…" paints with the document and waiting + // for the diagram chunk is not a blank gap or a second layout. + expect(entryClosure).toContain('data-mermaid-pending'); + expect(entryClosure).toContain('Rendering diagram'); + }); +}); From 94cef727f1c37ce8a92e4c507503a24e8da42d47 Mon Sep 17 00:00:00 2001 From: Michael Ramos Date: Thu, 17 Sep 2026 14:53:24 -0700 Subject: [PATCH 06/10] fix(ui): a press on the diagram canvas's own controls never comments on the part behind it The zoom strip, the composer and the popout header are painted over the canvas and are not in the svg, so the elementsFromPoint walk (node, then edge, then cluster) stepped straight past them to whatever part sat underneath: pressing Zoom out over a node opened the composer on that node, and a press on the strip could start a pan. A pointer event whose composed path contains a control surface now resolves no target, opens no composer and starts no pan. Controls are marked with data-diagram-control; buttons, toolbars, inputs, the composer and the source pane count without marking. Released over a control after a press that began on the canvas is handled too: targetUnder answering null would otherwise read as 'comment on the whole diagram'. --- .../ui/components/diagram/DiagramCanvas.tsx | 25 +++++++++++ .../ui/components/diagram/DiagramPopout.tsx | 4 +- .../components/diagram/DiagramViewer.test.tsx | 37 +++++++++++++++++ .../ui/components/diagram/diagramControls.ts | 41 +++++++++++++++++++ 4 files changed, 106 insertions(+), 1 deletion(-) create mode 100644 packages/ui/components/diagram/diagramControls.ts diff --git a/packages/ui/components/diagram/DiagramCanvas.tsx b/packages/ui/components/diagram/DiagramCanvas.tsx index cb78b151a..b99fbd0e5 100644 --- a/packages/ui/components/diagram/DiagramCanvas.tsx +++ b/packages/ui/components/diagram/DiagramCanvas.tsx @@ -14,6 +14,7 @@ import { cn } from '../../lib/utils'; import { diagramHitSource } from '../../utils/diagram-render'; import { isMac, isModKeyHeld } from '../../utils/platform'; import { Button } from '../ui/button'; +import { isDiagramControlEvent } from './diagramControls'; import { svgContentSize } from './svgContentSize'; import { DRAG_THRESHOLD_PX, @@ -51,6 +52,13 @@ import { * `` that the render slot's sanitizer strips, so the canvas owns * every click and there is no armed switch. * + * Chrome painted OVER the canvas (the zoom strip, the composer, the source + * pane, the popout's header) is never a diagram target: a pointer event + * whose composed path contains a control resolves nothing, opens no + * composer and starts no pan (`diagramControls.ts`). Without that the + * `elementsFromPoint` walk stepped past the control to the part behind it, + * so pressing Zoom out over a node opened the composer on that node. + * * The render slot hands over a sanitized svg NODE, not markup: the wrapper * mounts it with `replaceChildren` once per render, so no html string ever * crosses into the app DOM here. The overlay (a sibling of the wrapper, @@ -171,6 +179,10 @@ export function DiagramCanvas({ (event: ReactPointerEvent): Element | null => { const wrapper = wrapperRef.current; if (wrapper === null) return null; + // The zoom strip and the composer are painted OVER the canvas and are + // not in the svg, so the walk below would step past them to whatever + // part sits underneath. A press on chrome addresses no part. + if (isDiagramControlEvent(event)) return null; // Everything under the pointer, topmost first. Without a layout // engine (happy-dom) the event's own target is all there is. const doc = wrapper.ownerDocument; @@ -230,6 +242,11 @@ export function DiagramCanvas({ const onPointerDown = useCallback((event: ReactPointerEvent) => { if (event.button !== 0) return; + // Pressing a control is not the start of a pan either. + if (isDiagramControlEvent(event)) { + pressRef.current = null; + return; + } pressRef.current = { id: event.pointerId, x: event.clientX, @@ -273,6 +290,13 @@ export function DiagramCanvas({ const press = pressRef.current; if (press === null || press.id !== event.pointerId) return; pressRef.current = null; + // Released over a control (the press began on the canvas): not a click + // on the diagram, and `targetUnder` answering null would otherwise + // read as "comment on the whole diagram". + if (isDiagramControlEvent(event)) { + if (press.panning) setPanning(false); + return; + } if (press.panning) { setPanning(false); const target = event.currentTarget; @@ -366,6 +390,7 @@ export function DiagramCanvas({ // comments). On a narrow screen the strip takes the left edge so it // never stacks under a host's own bottom-right controls. data-print-hide="" + data-diagram-control="" data-diagram-zoom-strip="" className="absolute bottom-3 right-3 z-10 flex items-center gap-0.5 rounded-md border border-border bg-card/85 p-0.5 backdrop-blur max-md:bottom-4 max-md:left-4 max-md:right-auto" role="group" diff --git a/packages/ui/components/diagram/DiagramPopout.tsx b/packages/ui/components/diagram/DiagramPopout.tsx index b5d331d4e..15ac2b749 100644 --- a/packages/ui/components/diagram/DiagramPopout.tsx +++ b/packages/ui/components/diagram/DiagramPopout.tsx @@ -42,7 +42,9 @@ export function DiagramPopout({ className="h-[calc(100vh-2rem)] w-[calc(100vw-2rem)] max-w-none" dataAttributes={{ 'data-diagram-popout': '', ...dataAttributes }} > -
+ {/* The popout's own chrome: a press here addresses no diagram part + (see diagram/diagramControls). */} +
{title} {hasPane && ( diff --git a/packages/ui/components/diagram/DiagramViewer.test.tsx b/packages/ui/components/diagram/DiagramViewer.test.tsx index cad0adeeb..a9472b1e2 100644 --- a/packages/ui/components/diagram/DiagramViewer.test.tsx +++ b/packages/ui/components/diagram/DiagramViewer.test.tsx @@ -356,6 +356,43 @@ describe.if(hasDom)('hover, click, compose', () => { } }); + test('a press on the zoom strip over a node opens nothing; 1 px outside it the node opens the composer', async () => { + // Owner report: the controls are painted over the canvas and are not in + // the svg, so the `elementsFromPoint` walk stepped past them to the part + // behind — pressing Zoom out over a node opened the composer on it. + await mount(viewer({ onCreateComment: noop })); + await waitFor(() => expect(host!.querySelector('[id$="-flowchart-D-1"]')).not.toBeNull()); + // Fit is the control to press here: it is already the current viewport + // after mount, so a press that (wrongly) reaches the diagram cannot also + // churn the transform and make this test about something else. + const fitButton = q('[data-diagram-zoom-strip] [aria-label="Fit diagram"]'); + const nodeShape = nodeD().querySelector('polygon, rect, path') ?? nodeD(); + const canvas = q('[data-diagram-canvas]'); + const doc = host!.ownerDocument as Document & { elementsFromPoint?: (x: number, y: number) => Element[] }; + const original = doc.elementsFromPoint; + // What Chromium reports for the two points: inside the strip the button + // is topmost with the node still under it; 1 px outside, only the node. + const STRIP_X = 300; + doc.elementsFromPoint = (x: number) => (x === STRIP_X ? [fitButton, nodeShape, canvas] : [nodeShape, canvas]); + try { + await act(async () => { + pointer('pointerdown', fitButton, { x: STRIP_X, y: 200 }); + pointer('pointerup', fitButton, { x: STRIP_X, y: 200 }); + }); + await settle(); + expect(host!.querySelectorAll('[data-diagram-composer]').length).toBe(0); + + await act(async () => { + pointer('pointerdown', nodeShape, { x: STRIP_X - 1, y: 200 }); + pointer('pointerup', nodeShape, { x: STRIP_X - 1, y: 200 }); + }); + await waitFor(() => expect(host!.querySelector('[data-diagram-composer]')).not.toBeNull()); + expect(q('[data-diagram-composer]').textContent).toContain('node D'); + } finally { + doc.elementsFromPoint = original; + } + }); + test('a click that resolves no part comments on the WHOLE diagram, so a click never does nothing', async () => { const created: Array<{ anchor: DiagramAnchor }> = []; const { rerender } = await mount(viewer({ sourceLineOffset: 10, onCreateComment: (anchor) => { created.push({ anchor }); } })); diff --git a/packages/ui/components/diagram/diagramControls.ts b/packages/ui/components/diagram/diagramControls.ts new file mode 100644 index 000000000..aebc19ee8 --- /dev/null +++ b/packages/ui/components/diagram/diagramControls.ts @@ -0,0 +1,41 @@ +/** + * The canvas's own chrome, and why a pointer event on it must stop there. + * + * `DiagramCanvas` resolves what a click means over EVERYTHING under the + * pointer (`elementsFromPoint`, node → edge → cluster), because the edge hit + * layer sits above the nodes. The controls painted over the canvas — the + * zoom strip, the comment composer, the popout's own chrome — are not in the + * svg, so that walk skipped straight past them to whatever part happened to + * be underneath: pressing Zoom out over a node opened the composer on that + * node, and a press on the strip could start a pan. + * + * So a pointer event whose composed path contains a control surface never + * resolves a target, never opens the composer, and never starts a pan. It is + * the path rather than the point: a control knows it is a control, while a + * rectangle test would have to be kept in step with the layout. + * + * Mark new chrome with `data-diagram-control`. Buttons and toolbars count + * without marking, since anything the pointer can press is chrome by + * definition. + */ +export const DIAGRAM_CONTROL_SELECTOR = + '[data-diagram-control],[data-diagram-composer],[data-diagram-source-pane],[data-diagram-popout-chrome],button,[role="toolbar"],[role="button"],input,textarea,select,a[href]'; + +function isElement(value: unknown): value is Element { + return typeof (value as Element | null)?.matches === 'function'; +} + +/** Whether this pointer event was aimed at the canvas's chrome rather than + * at the diagram. */ +export function isDiagramControlEvent(event: { target: EventTarget | null; nativeEvent?: Event }): boolean { + const native = event.nativeEvent ?? (event as unknown as Event); + const path = typeof native?.composedPath === 'function' ? native.composedPath() : []; + for (const entry of path) { + if (isElement(entry) && entry.matches(DIAGRAM_CONTROL_SELECTOR)) return true; + } + // `composedPath()` is empty once dispatch has finished, and jsdom-class + // DOMs may not implement it at all: the target's own ancestry is the same + // answer for everything but a shadow root. + const target = event.target; + return isElement(target) && target.closest(DIAGRAM_CONTROL_SELECTOR) !== null; +} From d1f481f333c14e9bdcb5234f40944f91ed440b8d Mon Sep 17 00:00:00 2001 From: Michael Ramos Date: Thu, 17 Sep 2026 14:56:54 -0700 Subject: [PATCH 07/10] chore(ui): @plannotator/ui 0.41.1 Lazy diagram engine and the canvas-control fix. Core stays at 0.25.4: nothing under packages/core moved, so 0.41.1 publishes alone. Every export named in the 0.41.0 notes still resolves from the same path; the diagram barrel additionally exports the new pending components and the control predicate. --- bun.lock | 2 +- packages/ui/HANDOFF.md | 43 ++++++++++++++++++++++++- packages/ui/components/diagram/index.ts | 5 ++- packages/ui/package.json | 2 +- 4 files changed, 48 insertions(+), 4 deletions(-) diff --git a/bun.lock b/bun.lock index 7b002dc30..4654a0c34 100644 --- a/bun.lock +++ b/bun.lock @@ -290,7 +290,7 @@ }, "packages/ui": { "name": "@plannotator/ui", - "version": "0.41.0", + "version": "0.41.1", "dependencies": { "@base-ui/react": "^1.7.0", "@codemirror/autocomplete": "^6.20.3", diff --git a/packages/ui/HANDOFF.md b/packages/ui/HANDOFF.md index 32be0e915..16278c507 100644 --- a/packages/ui/HANDOFF.md +++ b/packages/ui/HANDOFF.md @@ -800,13 +800,54 @@ Three class-level deltas, none of which breaks an id- or class-based selector: ` **A click never does nothing: the `diagram` kind.** A click that resolves no part opens the composer on the WHOLE diagram (with a draft already open it closes the draft instead): `{ kind: 'diagram', family, label: , sourceLine: }`, no `id`. This covers gitGraph, pie, and any family the codec does not address. Its ring is the svg's content bounds and its badge sits top-left; it is never unanchored while the diagram renders (`findTarget` returns the svg root). The export line reads `Diagram (), lines a–b`. +### 0.41.1 — the engine is lazy, and the controls are not part of the diagram + +Two fixes, no API change: every export named in this section still resolves +from the same path, and a host upgrading from 0.41.0 changes nothing. + +**Lazy.** 0.41.0 reached the whole engine through STATIC imports from +`components/Viewer`: `Viewer` → `MermaidBlock` / `GraphvizBlock` → +`DiagramBlock` → `DiagramPopout` and `DiagramViewer` → `DiagramSourcePane` → +CodeMirror. Any host that statically imports `Viewer` therefore shipped the +canvas, the overlay, the finders, the popout and the editor on EVERY document +read, including a markdown document with no diagram and — since no host passes +`onSave` for a fence — an editor that could never open. Three edges are now +`React.lazy`: the two block wrappers in `Viewer` (one chunk each over a shared +`DiagramBlock` chunk), `DiagramPopout` in `DiagramBlock` (loaded when Expand +is pressed, fallback `null`), and `DiagramSourcePane` in `DiagramViewer` +(loaded when the pane first opens, fallback a box with the pane's own class +list so the split never collapses). A host that imports `DiagramViewer` +directly still gets a working viewer; its pane simply arrives one chunk later. +`svgContentSize` moved to its own dependency-free module and is re-exported +from `DiagramCanvas` and the barrel, so both published paths are unchanged. +The Suspense fallback for a fence is the block's own pending state +(`components/diagram/DiagramPending`, new, exported for hosts that render +their own fence chrome), so the source fence under "Rendering diagram…" paints +once and neither wait flashes. Measured on `Viewer`'s own document-read +closure (minified, gzip): 980.3 KB → 829.4 KB, -150.9 KB. Single-file builds +inline everything and are unchanged, which is why +`components/Viewer.diagramClosure.test.ts` bundles the entry and walks its +static imports. + +**Controls.** The canvas resolves a click over everything under the pointer +(`elementsFromPoint`, node → edge → cluster) because the edge hit layer sits +above the nodes. The chrome painted over the canvas is not in the svg, so that +walk stepped past it to the part behind: pressing Zoom out over a node opened +the composer on that node, and a press on the strip could start a pan. A +pointer event whose composed path contains a control surface now resolves no +target, opens no composer and starts no pan. Mark host chrome inside the +canvas with `data-diagram-control`; `button`, `[role=toolbar]`, inputs, the +composer and the source pane count without marking +(`components/diagram/diagramControls`). + **Migration for a host that carried the copies.** `useDiagramRender(kind, documentId, source, theme, { retryToken })` now takes the `{ colorTheme, mode }` theme and reports `error.runtimeUnavailable`; `useDiagramAnnotations` becomes the host's projection of its rows onto `comments` plus its mutation behind `onCreateComment` (the viewer half is `useDiagramComments`); `useDiagramDraft`'s `preview`/`dirty`/`stale`/`reload` semantics live in `useDiagramSourceDraft` behind `onSave` (the PATCH, `If-Match`, the query cache and the fence slice stay host-side; answer `stale` on a 412); `DiagramComposer` takes `disabledReason`/`error` instead of a `CommentingPolicy`; the canvas's `onEscape` returns `'consumed' | 'pass'` so a popout can walk the Escape ladder; arrow keys pan (`KEY_PAN_PX`, Shift ×5) in addition to `+` `-` `0`. Icons come from `lucide-react` (already a dependency). --- ## Publishing & versioning -- **The current pair is `@plannotator/ui` `0.41.0` on `@plannotator/core` `0.25.4`. Publish `core` 0.25.4 first, then `ui` 0.41.0** (both by hand from `main` after merge; CI never publishes these packages). 0.41.0 is the diagram engine (see "Diagram engine (0.41.0)"): one renderer slot and one canvas behind `MermaidBlock` / `GraphvizBlock`, the `components/diagram` surface, the Graphviz runtime slot with `@viz-js/viz` pinned `3.30.0`, and `Annotation.diagramAnchor`; core 0.25.4 adds the `diagram-anchor` subpath ui imports, so a ui 0.41.0 on a published core 0.25.3 would fail to compile in a consumer. +- **The current pair is `@plannotator/ui` `0.41.1` on `@plannotator/core` `0.25.4`.** Core is UNCHANGED from 0.41.0, so 0.41.1 publishes alone (`ui` only; core 0.25.4 must already be published). 0.41.1 is two fixes over 0.41.0 with no API change — the diagram engine is loaded lazily by the first diagram fence instead of riding every document read, and a press on the canvas's own controls no longer comments on the part behind them; see "0.41.1 — the engine is lazy, and the controls are not part of the diagram". The 0.41.0 notes below still describe the engine itself. +- **The pair 0.41.0 shipped as was `@plannotator/ui` `0.41.0` on `@plannotator/core` `0.25.4`. Publish `core` 0.25.4 first, then `ui` 0.41.0** (both by hand from `main` after merge; CI never publishes these packages). 0.41.0 is the diagram engine (see "Diagram engine (0.41.0)"): one renderer slot and one canvas behind `MermaidBlock` / `GraphvizBlock`, the `components/diagram` surface, the Graphviz runtime slot with `@viz-js/viz` pinned `3.30.0`, and `Annotation.diagramAnchor`; core 0.25.4 adds the `diagram-anchor` subpath ui imports, so a ui 0.41.0 on a published core 0.25.3 would fail to compile in a consumer. - The previous pair was `@plannotator/ui` `0.40.0` on `@plannotator/core` `0.25.3` (publish order the same). Three things shipped in 0.40.0: (1) **Mermaid 12.0.0**, pinned exactly (was `^11.17.2`): ELK layout by default for flowchart/state/class/ER/requirement, Safari 17.4+ / ES2024 floor, SVG ids byte-identical to 11 but `g.edgePaths` children now in declaration order, and the plan editor no longer imports `utils/mermaid-eager` (the lazy path is the default for everyone; hosts that want startup registration import the eager entry themselves) — see "Mermaid 12 (0.40.0)"; (2) **theme-aware Mermaid diagrams**: New additive exports `utils/mermaidTheme` (`buildMermaidThemeVariables`, `readThemeTokens`, `applyMermaidTheme`, `mermaidThemeKey`, `buildMermaidConfig`, `ensureContrast`, `isDarkBackground`, `MERMAID_THEME_TOKEN_NAMES`) and `utils/cssColor` (parser + OKLab/contrast toolkit). `MermaidBlock` now calls `useTheme()` and `applyMermaidTheme` before each render; `MERMAID_CONFIG`, `loadMermaidRuntime`, `mermaid-eager` and `securityLevel: 'strict'` are unchanged. A host whose document carries no theme tokens renders diagrams byte-identically to 0.39.0; a host that mounts `ThemeProvider` with `theme.css` gets diagrams in its palette and mode with no configuration. No new peer dependencies; core unchanged. See "Theme-aware Mermaid diagrams (0.40.0)".; (3) **element context through the host seam (#1521, #1549), which is what moves `core` to 0.25.3:** `@plannotator/core/html-anchor` gains `parseHtmlElementContext`, `MAX_ELEMENT_CONTEXT_BYTES` and `MAX_PAGE_URL_LENGTH`; `PersistedHtmlAnchor.elementContext?` and `HtmlAnnotationTarget.context?` now round-trip through `buildPersistedHtmlAnchor` and `projectHostThreads`. **Core changes here, so bump and publish `core` first** and update UI's exact core dependency before packing ui — a ui build that imports these from an older published core fails to compile in a consumer, the 0.38.0 failure mode. `@plannotator/ui/components/html-viewer` re-exports the validator, so 0.39.0's import site is unchanged, and rows without context stay byte-identical on the wire. `utils/parser` gains `includeOutline` on `elementContextExportBlock` / `exportAnnotationEntry`, and `exportAnnotationEntry`'s `includeRoute` now defaults to true per field. See "Element context through the host seam". - The previous pair was `@plannotator/ui` `0.39.0` on `@plannotator/core` `0.25.2` (core unchanged; nothing under `packages/core` moved). UI 0.39.0 adds **element context** to raw-HTML and live-app pinpoint annotations (#1517, #1520): a new optional `Annotation.elementContext` (`HtmlElementContext` in `@plannotator/ui/types`) and `HtmlAnnotationTarget.context`, captured by the bridge at click time (tag, id, author classes, ancestor `path`, `role`, accessible `name`, an allowlisted `attrs` set with href/src scrubbed of query and fragment, rendered `text`, an adaptive collapsed HTML `outline`, child count, viewport `rect`, nearest `landmark` and `heading`, a `component` hint, and in live-app sessions `page`), hard-capped at 2 KiB serialized per primary and 1 KiB per extra target, and re-validated at the parent trust boundary by the new `parseHtmlElementContext` export of `@plannotator/ui/components/html-viewer`. New helpers on `@plannotator/ui/utils/parser`: `elementContextExportBlock(ann, { includeRoute })` (the fenced skeleton plus selector/path/role/name/attrs/text/box/near lines the full export now prints under a context-bearing comment) and `exportAnnotationEntry(ann, { includeRoute })` (one annotation as a standalone feedback entry, a pure helper for hosts; `AnnotationPanel`'s card chrome is unchanged from 0.38.2). The field is purely descriptive: `HtmlElementAnchor` and restore are untouched, no `BRIDGE_PROTOCOL_VERSION` bump, share links drop it like anchors, annotations without it export byte-identically, and the repaint path posts only anchors to the bridge. **Host persistence gap in 0.39.0 itself, closed in the next publish (#1521, #1549)**: as shipped, 0.39.0's `@plannotator/core/html-anchor` (`buildPersistedHtmlAnchor`, `projectHostThreads`) does not carry `elementContext`, so a host pinned to 0.39.0 that persists through those helpers drops it on save and must persist and project the field itself. The next publish carries it end to end — see "Element context through the host seam". Peer ranges are unchanged from 0.38.2: `react` / `react-dom` `^19.2.3`, `tailwindcss` as before, and `@codemirror/state ^6.7.2` beside `@codemirror/view ^6.43.10`. Decision-control change in the same window (#1516): the header primary reads `Send Feedback` / `Post Comments` with no inline count (`DecisionPrimary.count` removed; internal, not host-supported surface). - Before that, `@plannotator/ui` `0.38.2` on `@plannotator/core` `0.25.2`. UI 0.38.2 keeps the type word in a titled alert's accessible name through a visually hidden `sr-only` span before the title instead of an `aria-label` on the title row (naming a generic `div` is prohibited by ARIA and WebKit drops it, so VoiceOver on Safari read only the bold title in 0.38.1), and loosens the React peer back to `^19.2.3` (0.38.1 declared `^19.2.8` only because the dependency batch moved it; nothing in the package needs a newer API). **Do not consume ui 0.38.0**: it imports `@plannotator/core/token-hover` (the hover-card trigger settings, #1462) but pins core 0.25.1, which never exported that subpath, so it fails to compile in any consumer; 0.38.1 is the same UI pinning core 0.25.2, which publishes `./token-hover`, the rotated `guide-viewer-manifest` pin, and the `config-types` hover fields (core 0.25.2 is the first core publish since 0.25.1 even though those changes landed over several releases; the package smoke now diffs the UI's core imports against the registry so an unpublished core subpath fails preflight instead of the consumer). UI 0.38.1 also aligns `@codemirror/state` to `^6.7.2` beside `@codemirror/view ^6.43.10`, so a consumer can no longer resolve two state copies. UI 0.38.0 also renders a GitHub alert's bold-only first body line as its title on the icon row (an emoji on that line becomes the icon; `` is stripped and resolved through the new `alertIconRenderer` seam, null by default; grammar in `utils/alertTitle`, importable by a host editor so it writes the bytes the reader parses; a fenced code block inside an alert body still renders as text, deferred because nesting a `CodeBlock` inside a block interacts with the positional annotation anchors and needs its own design). UI 0.38.0 carries the whole unified decision-control stack: the internal primitives (`DecisionControl`, `utils/decisionSpec`, `hooks/useDismissablePopover` — not host-supported surface, see the unsupported list; `useDismissablePopover` also replaced the hand-rolled dismissal inside `ActionMenu`/`ApproveDropdown`, both likewise unsupported) plus one blessed-barrel addition: `decisionControlShortcuts` on `@plannotator/ui/shortcuts` (pure scope data, fetch-free, same contract as the other scopes). The removal of `ToolbarButtons`' platform-mode `muted` prop is internal — `ToolbarButtons` is not host-supported surface. UI 0.37.0 added the Viewer-owned document-header seam (a new public API, hence the minor bump; 0.36.1 was reserved for it but never published) while retaining the `hideQuickLabel` and `StickyHeaderLane` seams from the 0.35.x and 0.36.0 releases; core 0.25.1 publishes the `annotation-threads` subpath already used by `AnnotationPanel` and `utils/parser`, and UI pins that corrected core exactly. diff --git a/packages/ui/components/diagram/index.ts b/packages/ui/components/diagram/index.ts index c2af4d13b..51cb55a15 100644 --- a/packages/ui/components/diagram/index.ts +++ b/packages/ui/components/diagram/index.ts @@ -8,7 +8,10 @@ */ export { DiagramViewer, type DiagramViewerProps } from './DiagramViewer'; export { DiagramPopout } from './DiagramPopout'; -export { DiagramCanvas, svgContentSize, KEY_PAN_PX, type DiagramCanvasHandle, type DiagramEscapeOutcome } from './DiagramCanvas'; +export { DiagramPending, DiagramInlineSource, DiagramBlockPending } from './DiagramPending'; +export { isDiagramControlEvent, DIAGRAM_CONTROL_SELECTOR } from './diagramControls'; +export { svgContentSize } from './svgContentSize'; +export { DiagramCanvas, KEY_PAN_PX, type DiagramCanvasHandle, type DiagramEscapeOutcome } from './DiagramCanvas'; export { DiagramOverlay } from './DiagramOverlay'; export { DiagramComposer } from './DiagramComposer'; export { DiagramSourcePane } from './DiagramSourcePane'; diff --git a/packages/ui/package.json b/packages/ui/package.json index b7e0c2924..0a8ee45e9 100644 --- a/packages/ui/package.json +++ b/packages/ui/package.json @@ -1,6 +1,6 @@ { "name": "@plannotator/ui", - "version": "0.41.0", + "version": "0.41.1", "type": "module", "exports": { "./components/*": "./components/*.tsx", From 249eed78efd6cc93164a6da826323d393a8c87c6 Mon Sep 17 00:00:00 2001 From: Michael Ramos Date: Thu, 17 Sep 2026 15:28:07 -0700 Subject: [PATCH 08/10] test(ui): pin the diagram comment restored before its diagram mounts Making MermaidBlock / GraphvizBlock lazy opens a window in which the document has painted and the draft has restored but no diagram exists in the DOM. A comment on a diagram part has no text to fall back on, so anything that drops or mis-reports it in that window is data loss. The test mounts Viewer (real lazy edges) beside AnnotationPanel with the row already seeded, holds the engine open on a gated runtime loader, and asserts the panel lists the row, nothing reports it unanchored while the block is still pending, and the badge and mark appear once the engine arrives -- no second restore pass, no reload. Negative control: making Viewer's "no diagram in this document" report key on what has MOUNTED rather than on the parse fails it on the unanchored assertion. --- .github/workflows/test.yml | 1 + .../Viewer.diagramLazyRestore.test.tsx | 209 ++++++++++++++++++ 2 files changed, 210 insertions(+) create mode 100644 packages/ui/components/Viewer.diagramLazyRestore.test.tsx diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 861754ef5..a726b2af4 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -114,6 +114,7 @@ jobs: packages/ui/utils/diagramAnchor.test.ts packages/ui/utils/diagramAnchorGraphviz.test.ts packages/ui/hooks/useAnnotationHighlighter.diagramSkip.test.tsx + packages/ui/components/Viewer.diagramLazyRestore.test.tsx packages/ui/components/CommentPopover.skillReferences.test.tsx packages/ui/components/SkillReferenceMenu.placement.test.tsx packages/ui/components/sidebar/FileBrowser.test.ts diff --git a/packages/ui/components/Viewer.diagramLazyRestore.test.tsx b/packages/ui/components/Viewer.diagramLazyRestore.test.tsx new file mode 100644 index 000000000..501864f51 --- /dev/null +++ b/packages/ui/components/Viewer.diagramLazyRestore.test.tsx @@ -0,0 +1,209 @@ +/** + * A restored diagram comment must survive the window in which no diagram has + * mounted yet. + * + * Since 0.41.1 `Viewer` reaches `MermaidBlock` / `GraphvizBlock` through + * `React.lazy`, so on a chunked host the document paints, the draft restores, + * and only then does the diagram engine arrive. Between those two moments the + * annotation names a diagram that does not exist in the DOM. Three things + * must hold across that window, and each of them is a way to lose the comment: + * + * - the row stays LISTED (the panel is the only place a comment on a part of + * a diagram can be read at all, so dropping it here is data loss); + * - it is not reported unanchored — `Viewer`'s "this document has no diagram" + * report keys on the PARSE, not on what has mounted, or every lazy load + * would flash the "Unanchored" chip on a comment that restores fine; + * - once the engine arrives, the block claims the row and paints its badge, + * with no second restore pass and no reload. + * + * The engine's arrival is held open here by gating the runtime loader, which + * is the same pending state the lazy chunk produces one step earlier. + * + * DOM-gated (DOM_TESTS=1). + */ +import { afterAll, afterEach, beforeAll, describe, expect, test } from 'bun:test'; +import { readFileSync } from 'node:fs'; +import { join } from 'node:path'; +import React, { act } from 'react'; +import { createRoot, type Root } from 'react-dom/client'; +import type { AnnotationRestoreReport } from '../hooks/useAnnotationHighlighter'; +import { installInertDiagramSvgParser } from '../test-setup/diagramSvg'; +import { AnnotationType, type Annotation } from '../types'; +import { parseMarkdownToBlocks } from '../utils/parser'; + +const hasDom = typeof document !== 'undefined'; + +// Viewer pulls in web-highlighter, whose UMD bundle reads `window` at +// module-eval time; import lazily (same pattern as Viewer.crossBlockRestore). +const viewerMod = hasDom ? await import('./Viewer') : null; +const Viewer = viewerMod?.Viewer as typeof import('./Viewer')['Viewer']; +const panelMod = hasDom ? await import('./AnnotationPanel') : null; +const AnnotationPanel = panelMod?.AnnotationPanel as typeof import('./AnnotationPanel')['AnnotationPanel']; +const mermaidMod = hasDom ? await import('./MermaidBlock') : null; +const setLoader = mermaidMod?.__setMermaidRuntimeLoaderForTests as + typeof import('./MermaidBlock')['__setMermaidRuntimeLoaderForTests']; + +const FIXTURES = join(import.meta.dir, '..', 'test-setup', 'fixtures', 'diagrams'); +const CAPTURE_ID = 'diagram-fixture'; +const SVG = readFileSync(join(FIXTURES, '06-flowchart-review-decision.svg'), 'utf8'); +const GEOMETRY = JSON.parse(readFileSync(join(FIXTURES, '06-flowchart-review-decision.geometry.json'), 'utf8')) as { + elements: Record; +}; +const CAPTURED = Object.entries(GEOMETRY.elements).map(([id, entry]) => [id.slice(CAPTURE_ID.length), entry] as const); + +const MARKDOWN = [ + '# Plan', + '', + 'Some prose before the diagram.', + '', + '```mermaid', + 'flowchart LR', + ' U([Reviewer]) --> D{Approve?}', + ' D -->|Yes| M[(Merge)]', + ' D -->|No| R[Revise]', + '```', + '', + 'Some prose after.', +].join('\n'); + +const BLOCKS = hasDom ? parseMarkdownToBlocks(MARKDOWN) : []; +const FENCE_ID = hasDom ? (BLOCKS.find((b) => b.type === 'code')?.id ?? '') : ''; + +/** The row a draft restore hands back: a comment on node D of the fence. */ +const RESTORED: Annotation = { + id: 'a1', + blockId: FENCE_ID, + startOffset: 0, + endOffset: 0, + type: AnnotationType.COMMENT, + text: 'rename this decision', + originalText: 'Approve?', + createdA: 1, + diagramAnchor: { v: 1, family: 'flowchart', kind: 'node', id: 'D', label: 'Approve?', sourceLine: [7, 7] }, +}; + +let root: Root | null = null; +let host: HTMLElement | null = null; +let restoreParser: (() => void) | null = null; +const svgProto = (hasDom ? ((globalThis as { SVGGraphicsElement?: typeof SVGElement }).SVGGraphicsElement ?? SVGElement).prototype : {}) as unknown as Record; +const elementProto = (hasDom ? Element.prototype : {}) as unknown as Record; +const saved = { getBBox: svgProto['getBBox'], getScreenCTM: svgProto['getScreenCTM'] }; +const noop = (): void => {}; + +beforeAll(() => { + if (!hasDom) return; + restoreParser = installInertDiagramSvgParser(); + elementProto['setPointerCapture'] ??= noop; + elementProto['releasePointerCapture'] ??= noop; + elementProto['hasPointerCapture'] ??= () => false; + svgProto['getBBox'] = function (this: Element) { + if (this.tagName.toLowerCase() === 'svg') return { x: 0, y: 0, width: 452, height: 182 }; + const found = CAPTURED.find(([suffix]) => this.id.endsWith(suffix)); + if (found === undefined) throw new Error(`no captured geometry for ${this.id}`); + return { ...found[1].bbox }; + }; + svgProto['getScreenCTM'] = function (this: Element) { + if (this.tagName.toLowerCase() === 'svg') return { a: 1, b: 0, c: 0, d: 1, e: 0, f: 0 }; + return CAPTURED.find(([suffix]) => this.id.endsWith(suffix))?.[1].ctm ?? null; + }; +}); + +afterAll(() => { + if (!hasDom) return; + svgProto['getBBox'] = saved.getBBox; + svgProto['getScreenCTM'] = saved.getScreenCTM; + setLoader(undefined); + restoreParser?.(); +}); + +afterEach(async () => { + if (root !== null) { + const finished = root; + await act(async () => { finished.unmount(); }); + root = null; + } + host?.remove(); + host = null; + if (hasDom) document.body.innerHTML = ''; +}); + +async function settle(ms = 25): Promise { + await act(async () => { await new Promise((resolve) => setTimeout(resolve, ms)); }); +} + +async function waitFor(check: () => void, tries = 60): Promise { + let lastError: unknown; + for (let i = 0; i < tries; i += 1) { + try { check(); return; } catch (error) { lastError = error; await settle(); } + } + throw lastError; +} + +/** The engine, held until the test lets it arrive. */ +function gatedRuntime(): { arrive: () => void; loader: () => Promise } { + let release: (() => void) | null = null; + const gate = new Promise((resolve) => { release = resolve; }); + const runtime = { + initialize: noop, + render: (id: string) => Promise.resolve({ svg: SVG.replaceAll(CAPTURE_ID, id) }), + }; + return { + arrive: () => release?.(), + loader: async () => { await gate; return runtime; }, + }; +} + +describe.if(hasDom)('Viewer: a diagram comment restored before the diagram mounts', () => { + test('stays listed and unchipped while the engine is still loading, then gets its badge', async () => { + const engine = gatedRuntime(); + setLoader(engine.loader as never); + const reports: AnnotationRestoreReport[] = []; + + host = document.createElement('div'); + document.body.appendChild(host); + await act(async () => { + root = createRoot(host!); + root.render( + <> + {}} + onSelectAnnotation={() => {}} + selectedAnnotationId={null} + mode="comment" + taterMode={false} + disableCodePathValidation + onRestoreReport={(report) => reports.push(report)} + /> + {}} + onDelete={() => {}} + /> + , + ); + }); + // The lazy block chunk has resolved; its engine has not. + await waitFor(() => expect(host!.querySelector('[data-diagram-pending]')).not.toBeNull()); + await settle(60); + + expect(host!.querySelector('[data-diagram-svg] > svg')).toBeNull(); + // Listed: the panel is the comment's only home until the diagram arrives. + expect(host!.querySelector(`[data-annotation-panel] [data-annotation-id="${RESTORED.id}"]`)).not.toBeNull(); + // Not chipped: nobody may call it unanchored while no diagram has mounted. + expect(reports.flatMap((r) => r.unanchored)).not.toContain(RESTORED.id); + + engine.arrive(); + await waitFor(() => expect(host!.querySelector(`[data-diagram-badge="${RESTORED.id}"]`)).not.toBeNull()); + await waitFor(() => expect(reports.some((r) => r.attempted.includes(RESTORED.id))).toBe(true)); + + expect(host!.querySelector(`[data-annotation-panel] [data-annotation-id="${RESTORED.id}"]`)).not.toBeNull(); + expect(reports.flatMap((r) => r.unanchored)).not.toContain(RESTORED.id); + expect(host!.querySelectorAll('[data-diagram-mark]')).toHaveLength(1); + }); +}); From 85112405f844a283df8c83e162bfa36d41d4be69 Mon Sep 17 00:00:00 2001 From: Michael Ramos Date: Thu, 17 Sep 2026 15:28:15 -0700 Subject: [PATCH 09/10] test(ui): pin the highlighter skip for a quoteless, block-less diagram row A comment on a whole diagram (or a label-less part) carries an empty originalText, and one posted by an agent that named no fence carries blockId ''. That is exactly the shape the text restore pass treats as unrestorable: findTextInDOM('') matches nothing and the blockId names nothing. The diagramAnchor skip has to come first, or such a comment returns from a reload wearing the "Unanchored" chip while the diagram overlay is showing it perfectly well. Negative control: dropping the skip reports both diagram rows as attempted. --- ...AnnotationHighlighter.diagramSkip.test.tsx | 52 +++++++++++++++++-- 1 file changed, 49 insertions(+), 3 deletions(-) diff --git a/packages/ui/hooks/useAnnotationHighlighter.diagramSkip.test.tsx b/packages/ui/hooks/useAnnotationHighlighter.diagramSkip.test.tsx index 8f4908eff..0eb5e2edb 100644 --- a/packages/ui/hooks/useAnnotationHighlighter.diagramSkip.test.tsx +++ b/packages/ui/hooks/useAnnotationHighlighter.diagramSkip.test.tsx @@ -51,11 +51,29 @@ const IN_DIAGRAM_ONLY: Annotation = { ...TEXT, id: 't2', originalText: 'OnlyInsi const ANNOTATIONS = [DIAGRAM, TEXT, IN_DIAGRAM_ONLY]; -function Harness({ onReport, applyRef }: { onReport: (report: Report) => void; applyRef: { current: (() => void) | null } }) { +/** + * The shape an anchor with nothing to quote produces: a comment on the whole + * diagram, or on a label-less part, posted by an agent that named no block. + * `originalText` is empty and `blockId` names no block of the document, which + * is exactly what the text pass treats as unrestorable. + */ +const QUOTELESS: Annotation = { + id: 'd2', + blockId: '', + startOffset: 0, + endOffset: 0, + type: AnnotationType.COMMENT, + text: 'the whole picture is stale', + originalText: '', + createdA: 3, + diagramAnchor: { v: 1, family: 'flowchart', kind: 'diagram', label: '', sourceLine: null }, +}; + +function Harness({ onReport, applyRef, annotations = ANNOTATIONS }: { onReport: (report: Report) => void; applyRef: { current: (() => void) | null }; annotations?: Annotation[] }) { const containerRef = useRef(null); const hook = useAnnotationHighlighter({ containerRef, - annotations: ANNOTATIONS, + annotations, selectedAnnotationId: null, mode: 'comment', onAddAnnotation: () => {}, @@ -63,7 +81,7 @@ function Harness({ onReport, applyRef }: { onReport: (report: Report) => void; a }); // The restore pass is imperative (Viewer runs it once the blocks are on // screen); the harness runs it the same way. - applyRef.current = () => hook.applyAnnotations(ANNOTATIONS); + applyRef.current = () => hook.applyAnnotations(annotations); return (

Should we Approve? the plan

@@ -110,4 +128,32 @@ describe('useAnnotationHighlighter: diagram anchors', () => { await act(async () => root.unmount()); host.remove(); }); + + // A diagram row with no quote and no block is the one shape the text pass + // would otherwise reject outright: `findTextInDOM('')` matches nothing and + // the blockId names nothing. It must still be skipped rather than reported, + // or a comment on a whole diagram comes back from a reload wearing the + // "Unanchored" chip while the diagram overlay is showing it perfectly well. + test.skipIf(!hasDom)('a diagram row with no quote and no blockId is skipped, not called unrestorable', async () => { + const host = document.createElement('div'); + document.body.appendChild(host); + const root = createRoot(host); + const reports: Report[] = []; + const applyRef: { current: (() => void) | null } = { current: null }; + const annotations = [QUOTELESS, TEXT]; + await act(async () => { + root.render( reports.push(r)} applyRef={applyRef} annotations={annotations} />); + }); + await act(async () => { + applyRef.current?.(); + await new Promise((r) => setTimeout(r, 30)); + }); + const last = reports[reports.length - 1]; + expect(last).toBeDefined(); + expect(last!.attempted).not.toContain('d2'); + expect(last!.unanchored).not.toContain('d2'); + expect(host.querySelector('[data-bind-id="d2"], [data-highlight-id="d2"]')).toBeNull(); + await act(async () => root.unmount()); + host.remove(); + }); }); From fc1e1b8a5f62c45fdfb98fc5974dfe0d75b29b6a Mon Sep 17 00:00:00 2001 From: Michael Ramos Date: Thu, 17 Sep 2026 15:28:15 -0700 Subject: [PATCH 10/10] docs(ui): record the lazy-mount restore window in the 0.41.1 notes A report of diagram comments lost across a reload did not hold -- the probe behind it never answered the "Draft Recovered" modal and then counted an un-restored session -- but the window it pointed at is real and is widened by the lazy block wrappers. Name the three properties that keep it safe and the tests that now pin each of them. --- packages/ui/HANDOFF.md | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/packages/ui/HANDOFF.md b/packages/ui/HANDOFF.md index 16278c507..23e897da0 100644 --- a/packages/ui/HANDOFF.md +++ b/packages/ui/HANDOFF.md @@ -840,6 +840,29 @@ canvas with `data-diagram-control`; `button`, `[role=toolbar]`, inputs, the composer and the source pane count without marking (`components/diagram/diagramControls`). +**A diagram comment restored before its diagram mounts.** Making the two +block wrappers lazy opens a window in which the document has painted and a +draft has restored but no diagram exists in the DOM yet. That window was +investigated after a report of diagram comments being lost across a reload; +the report did not hold (the probe behind it never answered the "Draft +Recovered" modal and then counted an un-restored session), and the three +properties that make the window safe were already in place. They are now +pinned, because every one of them is a way to lose a comment that has no text +to fall back on: + +- the highlighter skips a row carrying `diagramAnchor` outright — it is + neither painted, attempted nor reported unanchored — including the shape + with no quote and no `blockId` that a whole-diagram or label-less anchor + produces (`hooks/useAnnotationHighlighter.diagramSkip.test.tsx`); +- `Viewer`'s "this document has no diagram, so nobody can resolve this row" + report keys on the PARSE, never on what has mounted, so a lazy load does + not flash the "Unanchored" chip on a comment that restores fine; +- the row stays listed either way, and the block claims it and paints its + badge whenever it mounts — no second restore pass, no reload + (`components/Viewer.diagramLazyRestore.test.tsx`, which holds the engine + open on a gated runtime loader and asserts the panel row, the absent chip, + then the badge). + **Migration for a host that carried the copies.** `useDiagramRender(kind, documentId, source, theme, { retryToken })` now takes the `{ colorTheme, mode }` theme and reports `error.runtimeUnavailable`; `useDiagramAnnotations` becomes the host's projection of its rows onto `comments` plus its mutation behind `onCreateComment` (the viewer half is `useDiagramComments`); `useDiagramDraft`'s `preview`/`dirty`/`stale`/`reload` semantics live in `useDiagramSourceDraft` behind `onSave` (the PATCH, `If-Match`, the query cache and the fence slice stay host-side; answer `stale` on a 412); `DiagramComposer` takes `disabledReason`/`error` instead of a `CommentingPolicy`; the canvas's `onEscape` returns `'consumed' | 'pass'` so a popout can walk the Escape ladder; arrow keys pan (`KEY_PAN_PX`, Shift ×5) in addition to `+` `-` `0`. Icons come from `lucide-react` (already a dependency). ---