diff --git a/skills/htmlit/SKILL.md b/skills/htmlit/SKILL.md index 6d1b9f6..da71116 100644 --- a/skills/htmlit/SKILL.md +++ b/skills/htmlit/SKILL.md @@ -177,21 +177,12 @@ links the whole thread). **Only comments and freeform messages arrive in `prompts` - highlights do not.** - `prompts[].prompt` is what the user typed. -- `prompts[].tag` is `"text"` (a commented text selection), an element tag like - `"h1"`/`"img"`/`"g"` (a non-text target such as an image or a Mermaid node, - where text can't be selected), or `"message"` (freeform chat). +- `prompts[].tag` is `"text"` (a commented text selection), an element tag like `"h1"`/`"img"`/`"g"` (a non-text target such as an image or a Mermaid element, where text can't be selected), or `"message"` (freeform chat). - `prompts[].range` (when present) pins the exact commented span: `container` is a CSS selector and `start`/`end` are character offsets into that element's text, with the commented `text`. It is `null` for element/diagram targets and freeform messages, where `selector` (if any) locates the target. -- `prompts[].commentId` (non-empty for a text-span comment **or a diagram - node/edge comment**) is the id of the "you asked here" anchor left on that - target - a purple underline on text, or a purple outline box on the diagram - element. **When you answer such a comment, set - `data-htmlit-answer-for=""` on your `data-htmlit-answer` block** - (see below) - the anchor then links to your answer, and clicking it in the page - scrolls to and flashes that answer. (For a freeform `message` it is empty; just - answer normally.) +- `prompts[].commentId` (non-empty for a text-span comment **or a diagram element comment**) is the id of the "you asked here" anchor left on that target - a purple underline on text, or a purple outline box on the diagram element. **When you answer such a comment, set `data-htmlit-answer-for=""` on your `data-htmlit-answer` block** (see below) - the anchor then links to your answer, and clicking it in the page scrolls to and flashes that answer. (For a freeform `message` it is empty; just answer normally.) - `prompts[].selector` is a CSS selector for the target (the range's container for text, or the element itself). `type` is `feedback`, `ended`, or (with `--once`) `timeout`. @@ -307,13 +298,7 @@ The injected client auto-renders, on first load and after every morph: numbers) with a **Copy** button to grab the syntax in place. A diagram inside a hidden tab or a closed `
` renders **automatically the moment it is revealed**, so it is safe to put diagrams in tabs/accordions - they never - collapse. Manual - arrangement is remembered per diagram - source, so it survives a theme switch or a live morph. (A Mermaid node/edge is - **clicked** to leave a comment - its text can't be text-selected - while a drag - still pans/moves it, so commenting and rearrange coexist without a mode. The SVG - shows the normal arrow cursor at rest and switches to a grabbing cursor only - while you actually drag.) + collapse. Manual arrangement is remembered per diagram source, so it survives a theme switch or a live morph. (A Mermaid flowchart node/edge or sequence participant/message/note is **clicked** to leave a comment - its text can't be text-selected - while a drag still pans/moves it, so commenting and rearrange coexist without a mode. The SVG shows the normal arrow cursor at rest and switches to a grabbing cursor only while you actually drag.) - **Code** - `
...
` is highlighted by highlight.js and rendered with a **language name** and a **Copy** button on top and a pinned **line-number gutter** (the gutter stays put while long lines scroll @@ -376,14 +361,7 @@ small floating menu offers two actions: are saved with the review but are a live-review marker only - stripped from your snapshot and from the HTML/PDF exports. -A **Mermaid** node/edge (whose text isn't selectable, since dragging pans the -diagram) is targeted with a **click**, which offers **Ask agent** only (a text -note doesn't apply to a diagram shape). Sending leaves a persistent purple -**outline box** on that node/edge - the diagram equivalent of a text comment -anchor - that re-resolves across Mermaid re-renders and, once you answer, becomes -the clickable jump to your answer. The CSS selector is never shown to the user, -but comments deliver it (and a `commentId`) to you in `prompts[]`; highlights and -anchors stay local. +A **Mermaid** flowchart node/edge or sequence participant/message/note (whose text isn't selectable, since dragging pans the diagram) is targeted with a **click**, which offers **Ask agent** only (a text note doesn't apply to a diagram shape). Sending leaves a persistent purple **outline box** on that element - the diagram equivalent of a text comment anchor - that re-resolves across Mermaid re-renders and, once you answer, becomes the clickable jump to your answer. The CSS selector is never shown to the user, but comments deliver it (and a `commentId`) to you in `prompts[]`; highlights and anchors stay local. ## Visual guidance diff --git a/skills/htmlit/client/annotate.js b/skills/htmlit/client/annotate.js index f16eb2f..e4a84de 100644 --- a/skills/htmlit/client/annotate.js +++ b/skills/htmlit/client/annotate.js @@ -10,6 +10,8 @@ var pending = null; var selMenuOpen = false; var hlSeq = 0; +export const DIAGRAM_TARGET_ATTR = "data-htmlit-diagram-key"; + export function getPending() { return pending; } export function hasPendingOrMenu() { return !!(pending || selMenuOpen); } @@ -18,7 +20,7 @@ export function annotateEl(el) { buildElPending(el); openSelMenu(); } -// A click (not a drag) on a diagram node/edge. If it already carries a comment +// A click (not a drag) on a diagram element. If it already carries a comment // anchor: jump to the answer when there is one, else open the anchor's menu // (comment again / remove). Otherwise start a fresh comment on it. export function diagramClick(el) { @@ -113,15 +115,19 @@ export function inMermaid(node) { var el = node && (node.nodeType === 1 ? node : node.parentElement); return !!(el && el.closest && el.closest(".mermaid")); } -// A stable key for a clicked diagram node/edge: the diagram's source text plus the -// element's id with the (per-render) svg-id prefix stripped, so it re-resolves even -// after Mermaid re-renders the SVG (theme switch, morph) with a fresh svg id. +// A stable key for a clicked diagram element: prefer an explicit target key for +// Mermaid shapes without ids, otherwise strip the per-render SVG prefix from its id. export function diagramKeyOf(el) { var frame = el && el.closest && el.closest(".mermaid"); if (!frame || !frame.dataset || !frame.dataset.htmlitSrc) return null; var svg = frame.querySelector("svg"); + var keyed = el && el.closest ? el.closest("[" + DIAGRAM_TARGET_ATTR + "]") : null; + if (svg && keyed && keyed.closest("svg") === svg) { + var targetKey = keyed.getAttribute(DIAGRAM_TARGET_ATTR); + if (targetKey) return { src: frame.dataset.htmlitSrc, nodeKey: targetKey }; + } var idEl = el.id ? el : (el.closest ? el.closest("[id]") : null); - if (!svg || !idEl || (idEl.closest && !idEl.closest(".mermaid"))) return null; + if (!svg || !idEl || idEl === svg || (idEl.closest && !idEl.closest(".mermaid"))) return null; var prefix = svg.id ? svg.id + "-" : ""; var id = idEl.id || ""; var key = prefix && id.indexOf(prefix) === 0 ? id.slice(prefix.length) : id; @@ -136,6 +142,10 @@ export function resolveDiagramTarget(t) { if (!f.dataset || f.dataset.htmlitSrc !== t.src) continue; var svg = f.querySelector("svg"); if (!svg) return null; + var keyed = svg.querySelectorAll("[" + DIAGRAM_TARGET_ATTR + "]"); + for (var k = 0; k < keyed.length; k++) { + if (keyed[k].getAttribute(DIAGRAM_TARGET_ATTR) === t.nodeKey) return keyed[k]; + } var prefix = svg.id ? svg.id + "-" : ""; var els = svg.querySelectorAll("[id]"); for (var j = 0; j < els.length; j++) { @@ -305,7 +315,7 @@ export function addCommentAnchor(range, prompt, id) { appState.highlights.push({ id: id || newAnchorId(), kind: HighlightKind.COMMENT, prompt: prompt || "", comments: [prompt || ""], range: { container: range.container, start: range.start, end: range.end, text: range.text } }); } -// A comment anchor on a diagram node/edge: no text range, just a stable target. +// A comment anchor on a diagram element: no text range, just a stable target. export function addCommentAnchorEl(target, text, prompt, id) { if (!target || !target.src) return; var existing = findAnchorByTarget(target); @@ -403,7 +413,7 @@ export function commentItemFromPending(prompt) { var ex = findAnchorBySpan(rng); return { selector: pending.range.container, tag: "text", text: pending.text, prompt: prompt, note: false, range: rng, target: null, commentId: (ex && ex.id) || newAnchorId() }; } - // diagram node/edge (a fresh click, or re-commenting an existing diagram anchor) + // diagram element (a fresh click, or re-commenting an existing diagram anchor) var diag = pending.diagram || pending.target || null; if (diag && diag.src) { var exd = findAnchorByTarget(diag); diff --git a/skills/htmlit/client/diagram.js b/skills/htmlit/client/diagram.js index 4534b42..155dc67 100644 --- a/skills/htmlit/client/diagram.js +++ b/skills/htmlit/client/diagram.js @@ -5,11 +5,47 @@ import { theme } from "./theme.js"; import { renderHighlights, scheduleReposition } from "./overlays.js"; import { positionRail } from "./rail.js"; import { makeCopyButton, makeLinenos, lineCount, highlightMermaidSource } from "./codeblock.js"; -import { clearPending, diagramClick } from "./annotate.js"; +import { clearPending, DIAGRAM_TARGET_ATTR, diagramClick } from "./annotate.js"; export const diagramLayouts = {}; // diagram source text -> saved arrangement export const diagramRegistry = []; // { svg, fit } per live diagram, for export/print +function directChildMatches(el, selector) { + for (var i = 0; i < el.children.length; i++) if (el.children[i].matches(selector)) return true; + return false; +} + +function setupSequenceTargets(svg) { + var actorGroups = Array.prototype.filter.call(svg.querySelectorAll("g"), function (group) { + return directChildMatches(group, "rect.actor, text.actor.actor-box"); + }); + actorGroups.forEach(function (group, index) { + group.setAttribute(DIAGRAM_TARGET_ATTR, "sequence-actor-" + index); + }); + + var noteGroups = Array.prototype.filter.call(svg.querySelectorAll("g"), function (group) { + return directChildMatches(group, "rect.note, text.noteText"); + }); + noteGroups.forEach(function (group, index) { + group.setAttribute(DIAGRAM_TARGET_ATTR, "sequence-note-" + index); + }); + + var messages = Array.prototype.slice.call(svg.querySelectorAll("text.messageText")); + var messageLines = Array.prototype.slice.call(svg.querySelectorAll(".messageLine0, .messageLine1")); + messages.forEach(function (message, index) { + message.setAttribute(DIAGRAM_TARGET_ATTR, "sequence-message-" + index); + }); + + return function (el) { + if (!el || !el.closest) return null; + var keyed = el.closest("[" + DIAGRAM_TARGET_ATTR + "]"); + if (keyed && keyed.closest("svg") === svg) return keyed; + var line = el.closest(".messageLine0, .messageLine1"); + var lineIndex = line ? messageLines.indexOf(line) : -1; + return lineIndex >= 0 ? (messages[lineIndex] || null) : null; + }; +} + /** * @typedef {{x: number, y: number}} Point * @typedef {{left: number, top: number, right: number, bottom: number}} Rect @@ -190,7 +226,7 @@ export function enhanceMermaid() { try { setupDiagram(svg); } catch (e) {} }); // The diagram SVG is now in the DOM, so any "you asked here" boxes on its - // nodes/edges can be (re)resolved and drawn. + // elements can be (re)resolved and drawn. if (typeof renderHighlights === "function") renderHighlights(); if (typeof positionRail === "function") positionRail(); } @@ -200,6 +236,7 @@ export function setupDiagram(svg) { var box = svg.parentNode; var sourceKey = (box && box.dataset && box.dataset.htmlitSrc) || svg.id || ""; var space = svg.querySelector("g.root") || svg; + var sequenceTarget = setupSequenceTargets(svg); // Guarantee the frame is the bar's containing block AND clips it, as inline // styles - so the zoom bar can never escape the frame if the injected // stylesheet hasn't applied yet during a live morph (a reload used to "fix" it). @@ -566,6 +603,7 @@ if (box && box.classList && box.classList.contains("mermaid")) { var u = toUser(e.clientX, e.clientY); var nodeEl = closest("g.node"), labelEl = closest("g.edgeLabels g.edgeLabel"), pathEl = closest("g.edgePaths path"); var node = nodeEl && nodeByEl(nodeEl); + var annotationTarget = sequenceTarget(t); // A label is not independently movable - grabbing it bends its edge, so the // label always stays attached to the line. var edge = (pathEl && edgeByPath(pathEl)) || (labelEl && edgeByLabel(labelEl)) || null; @@ -574,7 +612,7 @@ if (box && box.classList && box.classList.contains("mermaid")) { // prefer it over the edge path (whose bounding box spans both nodes and would // "highlight the whole square"). Fall back to the path when there is no label. else if (edge && edge.src && edge.tgt && edge.src !== edge.tgt) drag = { type: "edge", edge: edge, hit: labelEl || edge.label || pathEl || edge.el }; - else drag = { type: "pan", sx: e.clientX, sy: e.clientY }; + else drag = { type: "pan", hit: annotationTarget, sx: e.clientX, sy: e.clientY }; drag.moved = false; drag.cx0 = e.clientX; drag.cy0 = e.clientY; try { svg.setPointerCapture(e.pointerId); } catch (_) {} }); @@ -615,10 +653,11 @@ if (box && box.classList && box.classList.contains("mermaid")) { try { svg.releasePointerCapture(e.pointerId); } catch (_) {} var d = drag; drag = null; svg.style.cursor = "default"; if (d.moved) { save(); return; } - // A click (no drag) acts on the pressed node/edge - text isn't selectable in a + // A click (no drag) acts on the pressed diagram element - text isn't selectable in a // diagram. diagramClick() routes it: jump to an answered anchor, open an // existing anchor's menu, or start a fresh comment. A background click dismisses. - if (d.type === "node" || d.type === "edge") diagramClick(d.hit || (d.node && d.node.el) || (d.edge && d.edge.el)); + var hit = d.hit || (d.node && d.node.el) || (d.edge && d.edge.el); + if (hit) diagramClick(hit); else clearPending(); } svg.addEventListener("pointerup", endDrag); diff --git a/tests/client/fixtures/sequence.html b/tests/client/fixtures/sequence.html new file mode 100644 index 0000000..fb767c3 --- /dev/null +++ b/tests/client/fixtures/sequence.html @@ -0,0 +1,16 @@ + + + + + htmlit sequence annotation fixture + + +

Sequence annotation fixture

+
sequenceDiagram
+  participant A as "Caller"
+  participant B as "Worker"
+  A->>B: "Resolve request"
+  Note over A,B: "The first attempt can race"
+  B-->>A: "Return result"
+ + diff --git a/tests/client/sequence-annotation.test.mjs b/tests/client/sequence-annotation.test.mjs new file mode 100644 index 0000000..e231758 --- /dev/null +++ b/tests/client/sequence-annotation.test.mjs @@ -0,0 +1,126 @@ +/** + * Browser regression test for comments on Mermaid sequence diagrams. + * + * Sequence actors, messages, and notes do not use the flowchart g.node / + * g.edgePaths structure, and most of their SVG elements have no id. A click + * therefore used to fall through as a background pan, leaving no annotation + * menu or stable target for the comment anchor. + */ + +import assert from "node:assert/strict"; +import { dirname, join } from "node:path"; +import { fileURLToPath } from "node:url"; +import { test } from "node:test"; + +import { startReview } from "./harness.mjs"; + +const HERE = dirname(fileURLToPath(import.meta.url)); +const FIXTURE = join(HERE, "fixtures", "sequence.html"); + +async function clickSequenceText(page, selector, text) { + const point = await page.evaluate( + ({ selector, text }) => { + const target = [...document.querySelectorAll(`.mermaid ${selector}`)] + .find((el) => (el.textContent || "").includes(text)); + if (!target) return null; + target.scrollIntoView({ block: "center" }); + const rect = target.getBoundingClientRect(); + return { x: rect.left + rect.width / 2, y: rect.top + rect.height / 2 }; + }, + { selector, text }, + ); + assert.ok(point, `expected ${selector} containing ${text}`); + await page.mouse.click(point.x, point.y); +} + +async function menuDisplay(page) { + return page.evaluate(() => { + const chrome = document.getElementById("htmlit-chrome"); + const menu = chrome.shadowRoot.querySelector(".selmenu"); + return getComputedStyle(menu).display; + }); +} + +test("sequence diagram elements can be commented and re-resolve after reload", async (t) => { + let browser; + try { + const { default: puppeteer } = await import("puppeteer"); + browser = await puppeteer.launch({ args: ["--no-sandbox", "--disable-setuid-sandbox"] }); + } catch (err) { + return t.skip(`no browser available: ${err.message}`); + } + + let review; + let page; + try { + review = await startReview(FIXTURE); + page = await browser.newPage(); + await page.setViewport({ width: 1200, height: 900 }); + await page.goto(review.url, { waitUntil: "domcontentloaded" }); + await page.waitForFunction( + () => { + const svg = document.querySelector(".mermaid svg"); + return svg && svg.__htmlitDrag && svg.querySelector(".messageText"); + }, + { timeout: 20000 }, + ); + + for (const [selector, text] of [ + ["text.actor.actor-box", "Caller"], + ["text.noteText", "first attempt can race"], + ]) { + await clickSequenceText(page, selector, text); + assert.equal(await menuDisplay(page), "flex", `clicking ${text} should open the annotation menu`); + await page.keyboard.press("Escape"); + } + + await clickSequenceText(page, ".messageText", "Resolve request"); + assert.equal(await menuDisplay(page), "flex", "clicking a sequence message should open the annotation menu"); + + const highlightSaved = page.waitForResponse( + (response) => response.url().endsWith(`/api/${review.key}/highlights`) && response.request().method() === "POST", + ); + const targetKey = await page.evaluate(() => { + const root = document.getElementById("htmlit-chrome").shadowRoot; + root.querySelector('[data-act="hlcomment"]').click(); + const input = root.querySelector(".pop .ce"); + input.textContent = "Why can this race?"; + input.dispatchEvent(new InputEvent("input", { bubbles: true, inputType: "insertText", data: "Why can this race?" })); + root.querySelector('[data-act="popadd"]').click(); + const message = [...document.querySelectorAll(".mermaid .messageText")] + .find((el) => (el.textContent || "").includes("Resolve request")); + return message && message.getAttribute("data-htmlit-diagram-key"); + }); + await highlightSaved; + + assert.match(targetKey || "", /^sequence-message-\d+$/, "the sequence message should receive a stable target key"); + + await page.evaluate(() => { + document.getElementById("htmlit-chrome").shadowRoot.querySelector('[data-act="send"]').click(); + }); + const feedback = await fetch(`${review.base}/api/poll?key=${review.key}&timeout=3`).then((response) => response.json()); + const prompt = (feedback.prompts || []).find((item) => item.prompt === "Why can this race?"); + assert.ok(prompt, "the sequence comment should be delivered to the agent"); + assert.equal(prompt.tag, "text"); + assert.ok(prompt.commentId, "the sequence comment should carry an anchor id"); + + await page.reload({ waitUntil: "domcontentloaded" }); + await page.waitForFunction( + (key) => { + const message = [...document.querySelectorAll(".mermaid .messageText")] + .find((el) => (el.textContent || "").includes("Resolve request")); + const chrome = document.getElementById("htmlit-chrome"); + return message && + message.getAttribute("data-htmlit-diagram-key") === key && + chrome && + chrome.shadowRoot.querySelectorAll(".hls .cmbox").length === 1; + }, + { timeout: 20000 }, + targetKey, + ); + } finally { + if (page) await page.close(); + if (review) await review.stop(); + await browser.close(); + } +});