Files
backnotprop__plannotator/packages/ui/components/CommentPopover.multiTarget.test.tsx
Michael Ramos f8951cd3c6 feat(annotate): shift-click multi-element selection for raw-HTML pinpoint (#1254)
* 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.
2026-08-10 10:19:21 -07:00

211 lines
7.2 KiB
TypeScript

/**
* CommentPopover multi-target seams (all opt-in, default off):
* - target chips render (primary first) with remove + hover handlers
* - refocusToken returns focus to the textarea
* - captureStrayKeys routes a window-level stray printable keydown into the
* textarea so the first keystroke after a shift-click is never lost
* - yieldState applies the fade / click-through classes
* - with none of the props set, none of the new DOM appears (byte-identical
* default composer)
*
* Requires DOM — runs under bun test with the happy-dom preload (DOM_TESTS=1).
*/
import { afterEach, describe, expect, test } from 'bun:test';
import React from 'react';
import { act } from 'react';
import { createRoot, type Root } from 'react-dom/client';
import { CommentPopover, type CommentTargetChip } from './CommentPopover';
const hasDom = typeof document !== 'undefined';
let root: Root | null = null;
let host: HTMLDivElement | null = null;
afterEach(async () => {
if (root) {
await act(async () => {
root!.unmount();
});
root = null;
}
host?.remove();
host = null;
if (hasDom) document.body.innerHTML = '';
});
type PopoverProps = Partial<React.ComponentProps<typeof CommentPopover>>;
async function mountPopover(props: PopoverProps = {}) {
host = document.createElement('div');
document.body.appendChild(host);
root = createRoot(host);
await act(async () => {
root!.render(
<CommentPopover
anchorRect={new DOMRect(100, 100, 50, 20)}
contextText="Selected text"
isGlobal={false}
onSubmit={() => {}}
onClose={() => {}}
{...props}
/>,
);
});
}
async function remount(props: PopoverProps) {
await act(async () => {
root!.render(
<CommentPopover
anchorRect={new DOMRect(100, 100, 50, 20)}
contextText="Selected text"
isGlobal={false}
onSubmit={() => {}}
onClose={() => {}}
{...props}
/>,
);
});
}
function popoverEl(): HTMLElement {
const el = document.querySelector<HTMLElement>('[data-comment-popover]');
if (!el) throw new Error('popover missing');
return el;
}
function textarea(): HTMLTextAreaElement {
const el = document.querySelector<HTMLTextAreaElement>('[data-comment-popover] textarea');
if (!el) throw new Error('textarea missing');
return el;
}
const CHIPS: CommentTargetChip[] = [
{ key: 't1', label: 'Paragraph', excerpt: 'Primary text' },
{ key: 't2', label: 'Button', excerpt: 'Create' },
];
describe.if(hasDom)('CommentPopover multi-target seams', () => {
test('default composer renders none of the multi-target DOM', async () => {
await mountPopover();
expect(document.querySelector('[data-target-chips]')).toBeNull();
expect(popoverEl().className).not.toContain('pn-composer-yield');
});
test('chips render primary-first with working remove and hover handlers', async () => {
const removed: string[] = [];
const hovered: string[] = [];
await mountPopover({
targetChips: CHIPS,
onRemoveTargetChip: (key) => removed.push(key),
onHoverTargetChip: (key) => hovered.push(key),
});
const chips = Array.from(document.querySelectorAll<HTMLElement>('[data-target-chip]'));
expect(chips.length).toBe(2);
expect(chips[0]!.getAttribute('data-target-chip-primary')).toBe('true');
expect(chips[0]!.textContent).toContain('Paragraph');
expect(chips[1]!.textContent).toContain('Create');
await act(async () => {
chips[1]!.dispatchEvent(new MouseEvent('mouseenter', { bubbles: false }));
});
// React attaches mouseenter via its own delegation of mouseout/mouseover;
// fall back to mouseover which React maps for onMouseEnter in tests.
if (hovered.length === 0) {
await act(async () => {
chips[1]!.dispatchEvent(new MouseEvent('mouseover', { bubbles: true }));
});
}
expect(hovered).toContain('t2');
const removeButton = document.querySelector<HTMLButtonElement>('[data-target-chip-remove="t2"]');
if (!removeButton) throw new Error('remove button missing');
await act(async () => {
removeButton.click();
});
expect(removed).toEqual(['t2']);
});
test('refocusToken bump returns focus to the textarea', async () => {
await mountPopover({ targetChips: CHIPS, refocusToken: 0 });
textarea().blur();
document.body.focus();
expect(document.activeElement).not.toBe(textarea());
await remount({ targetChips: CHIPS, refocusToken: 1 });
expect(document.activeElement).toBe(textarea());
});
test('captureStrayKeys routes a body-targeted printable keydown into the textarea', async () => {
await mountPopover({ targetChips: CHIPS, captureStrayKeys: true });
const el = textarea();
el.blur();
const stray = new KeyboardEvent('keydown', {
key: 'h',
bubbles: true,
cancelable: true,
});
await act(async () => {
document.body.dispatchEvent(stray);
});
expect(stray.defaultPrevented).toBe(true);
expect(el.value).toBe('h');
expect(document.activeElement).toBe(el);
// A keydown already headed somewhere useful is left alone.
const focused = new KeyboardEvent('keydown', {
key: 'i',
bubbles: true,
cancelable: true,
});
await act(async () => {
el.dispatchEvent(focused);
});
expect(focused.defaultPrevented).toBe(false);
});
test('stray keys insert at the remembered caret, not end-of-text', async () => {
await mountPopover({ targetChips: CHIPS, captureStrayKeys: true });
const el = textarea();
// Type 'held' through React, then park the caret between 'he' and 'ld'.
const setter = Object.getOwnPropertyDescriptor(Object.getPrototypeOf(el), 'value')?.set;
await act(async () => {
setter?.call(el, 'held');
el.dispatchEvent(new Event('input', { bubbles: true }));
});
el.selectionStart = el.selectionEnd = 2;
el.blur();
const stray = new KeyboardEvent('keydown', { key: 'X', bubbles: true, cancelable: true });
await act(async () => {
document.body.dispatchEvent(stray);
});
expect(stray.defaultPrevented).toBe(true);
expect(el.value).toBe('heXld');
await act(async () => {
await new Promise((r) => requestAnimationFrame(() => r(null)));
});
expect(el.selectionStart).toBe(3);
});
test('yieldState drives fade and click-through classes with a reduced-motion-aware style', async () => {
await mountPopover({ targetChips: CHIPS, yieldState: 'none' });
expect(popoverEl().className).toContain('pn-composer-yieldable');
expect(popoverEl().className).not.toContain('pn-composer-yield-near');
await remount({ targetChips: CHIPS, yieldState: 'near' });
expect(popoverEl().className).toContain('pn-composer-yield-near');
await remount({ targetChips: CHIPS, yieldState: 'over' });
expect(popoverEl().className).toContain('pn-composer-yield-over');
const style = Array.from(document.querySelectorAll('style'))
.map((s) => s.textContent ?? '')
.find((s) => s.includes('pn-composer-yieldable'));
expect(style).toBeDefined();
expect(style).toContain('pointer-events: none');
expect(style).toContain('prefers-reduced-motion: reduce');
expect(style).toContain('180ms');
});
});