mirror of
https://github.com/backnotprop/plannotator.git
synced 2026-09-14 14:17:26 +08:00
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.
211 lines
7.2 KiB
TypeScript
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');
|
|
});
|
|
});
|