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/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..23e897da0 100644 --- a/packages/ui/HANDOFF.md +++ b/packages/ui/HANDOFF.md @@ -800,13 +800,77 @@ 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`). + +**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). --- ## 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/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/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'); + }); +}); 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); + }); +}); 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' ? ( ` 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, @@ -57,40 +66,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. */ @@ -200,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; @@ -259,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, @@ -302,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; @@ -395,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/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 }) => ( +
+
+
+ +
+
+
+); 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/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 ? ( 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; +} 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/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; +} 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(); + }); }); 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",