mirror of
https://github.com/backnotprop/plannotator.git
synced 2026-09-14 14:17:26 +08:00
Michael Ramos
f8951cd3c6
* feat(annotate): shift-click multi-element selection for raw-HTML pinpoint drafts One comment covering multiple targets: shift-clicking elements while a pinpoint draft composer is open toggles them in/out of the SAME draft. - bridge: pendingMultiTargets registry with per-target pinned outline boxes, DOM-identity + anchor-equality toggle dedup, primary promotion on removal, draft cancel on last removal, shift-hover preview, rAF pointer relay for the composer yield, remove-target/flash-target parent messages, capped at 16 additional targets at the source - pins: registerPin now allows several elements per annotation id (deduped by (id, element)/(id, anchor)); badges number by first-seen id so all targets of one annotation share one number; find-and-mark restores additionalAnchors as same-numbered pins (anchor-only, fail-closed) - parent hook: multi-target-added/removed/pointer messages validated and capped at the trust boundary (key<=64, label<=64 truncated, text via the 10k surrogate-safe cap, anchors via parseHtmlElementAnchor, array cap 16); draftTargets state, chip removal with deterministic promotion mirrored on both sides, composerFocusToken for focus return - types: additive Annotation.htmlAdditionalTargets (label/text/anchor per extra target; anchor optional so fail-closed targets still export) - CommentPopover (all seams optional, default off): horizontally scrollable target chips with remove buttons and hover-to-flash, refocusToken, captureStrayKeys first-keystroke guard, yieldState fade/click-through with 180ms transition and prefers-reduced-motion fallback - HtmlViewer: composer-yield state machine (composerYield.ts) fed by parent mousemoves plus bridge-relayed pointer positions, with 48px/96px hysteresis - export: multi-target comments append an 'Also applies to N more elements' block (label + excerpt per target); single-target output byte-identical - share URLs: additional targets are dropped exactly like htmlAnchor (the compact tuple format never carried anchors); drafts carry them verbatim * test(annotate): cover shift-click multi-select across bridge, DTO, composer, export, drafts, sharing - srcdoc.test.ts (bridge DOM): shift-click add + toggle-off with echoed removals and per-target pinned boxes; create-mark commits all targets under one id with one badge number (second annotation numbers 2); primary promotion and last-removal cancel; parent remove-target mirrors without echo; flash-target; 16-target cap at the source; find-and-mark restores additionalAnchors as same-numbered pins with stale anchors failing closed - htmlPinpointProtocol.test.tsx: multi-target-added/removed/pointer DTO validation (key/label/text/anchor caps, hostile payloads), selection targetKey/targetLabel validation; mounted-composer flows — primary chip, shift-adds into ONE submitted comment carrying htmlAnchor + 2 additional targets, single-target submit shape unchanged, promotion onto the comment, last-removal closes the composer, chip removal, 16-cap at the trust boundary, drag selections never arm multi-select - CommentPopover.multiTarget.test.tsx: chips render primary-first with remove/hover handlers, refocusToken focus return, captureStrayKeys stray keydown routing (and non-interference when focused), yieldState classes + reduced-motion-aware 180ms style; default composer renders none of it - composerYield.test.ts: distance + hysteresis state machine (48px over-exit, 80/96px near enter/exit) - parser.test.ts: multi-target export block (labels, excerpt clipping, fail-closed targets) and byte-identical single-target output - useAnnotationDraft.seam.test.tsx: multi-target annotations round-trip the draft transport verbatim (save body + restoreDraft) - sharing.multiTarget.test.ts: share tuples drop anchors AND additional targets while the comment itself still shares * fix(annotate): keep pinpoint drafts alive when the pinned element scrolls out of view Found by the real-browser signoff harness: reaching a second element to shift-click often scrolls the pinned primary out of the viewport BEFORE any additional target exists, and the scroll-out teardown then cleared the bridge's pin state mid-compose — the shift-click landed on a dead draft and started a new one instead of adding to it (and even a single-target pinpoint draft silently lost its visual pin on commit after scrolling). Pinpoint drafts (pendingPinViaPinpoint) now survive scroll-out; drag selections keep the existing close-on-scroll-out behavior unchanged. * fix(annotate): address adversarial review of multi-select (arm handshake, label sanitization, iframe shift relay, removal resync) D1 (blocker): the bridge accepted shift-toggles for ANY pinpoint draft while the parent only mirrors targets when the comment composer owns it — in quickLabel mode the user could pin elements the saved annotation would never carry. Multi-select is now ARMED EXPLICITLY: the parent posts arm-multi-select (keyed to the primary, so a stale arm can never arm a new draft) only from the composer flow, and the bridge refuses the toggle — shift-click behaves as a plain click — until armed. D2: target labels derive from page-controlled attributes (aria-label), so newlines could smuggle real markdown structure (fake headings) into agent-read feedback. parseTargetLabel now collapses all whitespace at the trust boundary, and the exporter collapses again (defense in depth for persisted pre-fix drafts). D3: the composer yield armed Shift only from parent-window keydown/mousemove, but window blur (focus entering the iframe) cleared it and modifier keydowns don't reach the parent from the sandbox — from the second shift-click on, the composer never yielded. The bridge pointer relay now carries the observed shiftKey (validated strict boolean) and arms/disarms yield directly. D4: a forged multi-target-removed desynced parent (promotes) from bridge (keeps original). applyTargetRemoval now ALWAYS echoes remove-target — idempotent for legit bridge-side removals, forcing convergence after forgery. D5: the 'Also applies to N more elements' block gains a leading blank line so markdown lazy continuation cannot fold it into the preceding blockquote. D6: the stray-key guard registers in capture phase (a bubbling global shortcut can no longer both fire and have its character appended), inserts at the textarea's remembered caret instead of end-of-text, and refocusToken now preserves the caret rather than jumping to the end. D7: the expanded dialog no longer carries the dead yield class/style — its data-comment-popover wrapper spans the viewport, making proximity meaningless; the dialog deliberately does not yield. Also reverts the incidental bun.lock version-catch-up churn. Tests: unarmed/stale-arm refusal, quickLabel non-arming and non-mirroring, newline-label collapse at both layers, bridge-shift-driven yield, forged removal echo + bridge-side idempotent resync, caret-preserving stray keys, blockquote separation. Signoff harness re-run green (14/14) on rules-ui-signoff.html including the arm handshake. * fix(annotate): reset multi-select arm on every new pinpoint draft (D1-R) The re-review caught that multiSelectArmed was never cleared when annotateElement started a fresh draft — only clearPendingPin reset it. So a comment-mode draft (armed) followed by a mode switch the parent doesn't mirror (quick label posts no arm) and a new pinpoint click left the stale arm live: the bridge accepted shift-clicks and pinned elements the saved annotation would never carry. Reset the flag at the top of annotateElement alongside clearMultiTargets. Regression test reproduces the exact sequence (armed draft -> new unarmed draft -> shift-click must not add a target); mutation-verified that removing only this reset fails it. * docs(annotate): correct the first-keystroke guard comment reasoning The comment claimed capture phase prevents a global shortcut from also firing; preventDefault does not stop the dispatcher (it ignores defaultPrevented by design). Restate the actual invariant: the guard is safe only because no bare printable single-key binding exists on this surface, and flag that as a constraint for future bindings. Comment only.
45 lines
1.8 KiB
TypeScript
45 lines
1.8 KiB
TypeScript
/**
|
|
* Share-URL contract for multi-target HTML annotations: element anchors are
|
|
* deliberately dropped from share payloads (they are meaningless in another
|
|
* viewer's DOM), and htmlAdditionalTargets follow the exact same rule — the
|
|
* compact tuple format never carries them. The COMMENT itself (text, quoted
|
|
* primary text, author, images) still shares.
|
|
*/
|
|
import { describe, expect, test } from "bun:test";
|
|
import { AnnotationType, type Annotation } from "../types";
|
|
import { fromShareable, toShareable } from "./sharing";
|
|
|
|
const MULTI: Annotation = {
|
|
id: "ann-1",
|
|
blockId: "",
|
|
startOffset: 0,
|
|
endOffset: 0,
|
|
type: AnnotationType.COMMENT,
|
|
text: "Unify these",
|
|
originalText: "Primary chip",
|
|
createdA: 1,
|
|
author: "reviewer",
|
|
htmlAnchor: { selector: "p.primary", tagName: "p", text: "Primary chip" },
|
|
htmlAdditionalTargets: [
|
|
{ label: "Button", text: "Create", anchor: { selector: "span.btn", tagName: "span", text: "Create" } },
|
|
],
|
|
};
|
|
|
|
describe("sharing — multi-target annotations", () => {
|
|
test("toShareable serializes the comment without anchors or additional targets", () => {
|
|
const shareable = toShareable([MULTI]);
|
|
expect(shareable).toEqual([["C", "Primary chip", "Unify these", "reviewer", undefined]]);
|
|
expect(JSON.stringify(shareable)).not.toContain("htmlAdditionalTargets");
|
|
expect(JSON.stringify(shareable)).not.toContain("selector");
|
|
});
|
|
|
|
test("round trip keeps the comment but has no target array", () => {
|
|
const restored = fromShareable(toShareable([MULTI]));
|
|
expect(restored.length).toBe(1);
|
|
expect(restored[0]!.text).toBe("Unify these");
|
|
expect(restored[0]!.originalText).toBe("Primary chip");
|
|
expect(restored[0]!.htmlAnchor).toBeUndefined();
|
|
expect(restored[0]!.htmlAdditionalTargets).toBeUndefined();
|
|
});
|
|
});
|