Marks can be told to re-measure when a surface moves something

studio-qa drove the nested seam and measured a repaint gap: an ARTIFACTS canvas
item moved (220,170) -> (760,430) and its mark stayed at the old rect until an
unrelated window resize snapped it. Anchoring resolved correctly and the stored
selector and offset were right — nothing was wrong except that nothing told the
layer to look again.

An element that MOVES changes no size, no scroll and no viewport, so every signal
the shell can observe for free stays silent. The mark keeps reporting `anchored`
and keeps looking correct, which is why this needed reporting rather than showing
up as an error.

Two ways to say it: `{t:"annotation-refresh"}` from the surface, and refresh() on
the handle for a caller that already holds it. Both re-measure WITHOUT reloading —
the board is not re-read and undo survives, which is the distinction from a surface
or context change where reloading is right.

Re-posting annotation-target with the same frame id stays a no-op, which is the
other half of studio-qa's finding. Overloading it as a nudge would make every
redundant declaration re-anchor and reset undo, so the nudge gets its own verb.
The layer also observes size and scroll on the anchored document; those are free
and explicitly NOT sufficient, which is why the explicit hook exists.

MEASURED BEFORE AND AFTER, both paths: element moved 300x180, mark stale by
exactly -300x-180, and both the message and the handle closed the gap to 0x0.

TWO PROBE DEFECTS, and the second is the instructive one:
- I posted the nudge via `.parent` from the top window, so e.source was the top
  window and the active-surface guard correctly rejected it. The guard working
  looked like the fix failing.
- I then measured `state().records[].anchor` — the anchor as WRITTEN, which
  correctly never changes. The DRAWN position is recomputed each render as
  (current element rect + stored offset). A correctly-following mark looked frozen
  because I was reading a field that does not decide what is on screen. Now
  hit-tested against the rendered SVG.

283 tests.
This commit is contained in:
mvschwarz committed 2026-08-05 06:04:13 -07:00
1 parent 29ebd8b01e
commit a7d3785bec
5 files changed
+133

No files matched your search

+39
View File
@@ -322,6 +322,39 @@ export function attach(shell, { store } = {}) {
observedFrame?.addEventListener("load", frameLoad);
};
cleaners.push(shell.onSurface(() => { observeFrame(); loadSurface().catch(console.error); }));
// RE-MEASURE, NOT RELOAD. render() recomputes every anchor from its selector, so
// a repaint is all a moved element needs. Guarded because a shell that predates
// this seam has no onRefresh.
if (typeof shell.onRefresh === "function") cleaners.push(shell.onRefresh(() => render()));
// The cheap automatic half. These cover RESIZE and SCROLL, which are the cases
// the browser will tell us about for free.
//
// THEY ARE NOT SUFFICIENT AND THAT IS WHY THE EXPLICIT HOOK EXISTS: an element
// that MOVES within an unchanged viewport fires neither. A canvas item dragged
// across a board is exactly that case, and no observer short of polling every
// frame would catch it.
let observer = null;
const watchGeometry = () => {
observer?.disconnect();
observer = null;
const doc = frameDocument();
if (!doc?.documentElement || typeof ResizeObserver === "undefined") return;
try {
observer = new ResizeObserver(() => render());
observer.observe(doc.documentElement);
} catch { observer = null; }
};
const onScroll = () => render();
const watchScroll = () => {
const doc = frameDocument();
try { doc?.addEventListener("scroll", onScroll, { capture: true, passive: true }); } catch {}
};
const unwatchScroll = () => {
const doc = frameDocument();
try { doc?.removeEventListener("scroll", onScroll, { capture: true }); } catch {}
};
cleaners.push(shell.onSurface(() => { unwatchScroll(); watchGeometry(); watchScroll(); }));
watchGeometry(); watchScroll();
observeFrame();
root.classList.toggle("is-marking", Boolean(shell.markup()));
const ready = loadSurface();
@@ -329,6 +362,8 @@ export function attach(shell, { store } = {}) {
return Object.freeze({
detach() {
observer?.disconnect();
unwatchScroll();
cleaners.forEach((clean) => clean?.());
observedFrame?.removeEventListener("load", frameLoad);
window.removeEventListener("keydown", onKey);
@@ -336,6 +371,10 @@ export function attach(shell, { store } = {}) {
root.remove();
},
ready,
// Re-measure every anchor and repaint. Cheap, idempotent, and does NOT reload
// the board or disturb undo history — for a surface (or an agent) that moved
// something the layer cannot observe.
refresh: () => { render(); },
annotate: add,
list: () => clone(state.records),
remove,
+24
View File
@@ -204,6 +204,11 @@
// ============================================================================
const surfaceListeners = new Set();
const markupListeners = new Set();
// "My layout moved — re-measure, but do NOT reload." A separate channel from
// surfaceListeners on purpose: that one means the board CHANGED and reloads
// records, which resets undo history. A surface whose canvas item moved wants
// neither of those, it wants the geometry recomputed.
const refreshListeners = new Set();
// What each surface declared for its half of the header. The shell holds it, so
// a surface that declares once on load keeps its header across a switch away and
// back — it has no reason to speak twice, and reverting to manifest defaults
@@ -330,6 +335,22 @@
if (!e.data || typeof e.data !== "object") return;
if (e.data.t === "open-surface" && byId(e.data.id)) activate(e.data.id);
if (e.data.t === "open-agent") openAgents(e.data.seat);
if (e.data.t === "annotation-refresh") {
// CHEAP AND EXPLICIT. DOM motion inside a surface is invisible from out
// here: an element that MOVES changes neither the frame's size nor its
// scroll, so nothing fires and anchored marks sit at the old geometry until
// something unrelated forces a repaint. Measured by studio-qa: a canvas item
// moved (220,170) -> (760,430) and the mark only snapped on a window resize.
//
// This does not reload the board and does not touch undo history. Re-posting
// annotation-target with the SAME frame id stays a no-op deliberately —
// overloading it as a nudge would make every redundant declaration re-anchor
// and reset undo, so the nudge gets its own verb.
const current = frames.get(activeId);
if (current && e.source === current.contentWindow) {
for (const fn of refreshListeners) { try { fn(); } catch {} }
}
}
if (e.data.t === "annotation-target") {
// Same active-surface guard as the header and the context, for the same
// reason: a backgrounded surface still holds a live channel.
@@ -641,6 +662,9 @@
onMarkup: (fn) => { markupListeners.add(fn); return () => markupListeners.delete(fn); },
// Tell me when the surface changes, so an overlay can clear or re-anchor.
onSurface: (fn) => { surfaceListeners.add(fn); return () => surfaceListeners.delete(fn); },
// Re-measure without reloading. Fired when the active surface says its layout
// moved; a feature drawing over the stage recomputes geometry and repaints.
onRefresh: (fn) => { refreshListeners.add(fn); return () => refreshListeners.delete(fn); },
};
// --- annotations: a SHELL feature, drawn over the stage --------------------
// Annotation belongs to the shell rather than to each surface because it must
+34
View File
@@ -145,6 +145,39 @@ in one place.
read for element identity, so the target silently falls back to the surface
document rather than pretending: the alternative is anchors that can never resolve.
## Re-measuring when the surface moves something (stable)
An anchored mark is redrawn from its element's *current* rect on every render, so
it follows anything the shell can observe: a window resize, a scroll, the surface
frame changing size. **It cannot observe an element that MOVES.**
A canvas item dragged across a board changes neither the viewport nor the scroll
position nor any element's size, so nothing fires and the mark stays at the old
geometry — still reporting `anchored`, still looking correct. Measured: an item
moved 300px right and 180px down and the mark stayed put until an unrelated window
resize snapped it.
Two ways to say "I moved something, re-measure":
```json
{ "t": "annotation-refresh" }
```
and, for a caller that already holds the handle, `window.studioAnnotations.refresh()`.
**Both re-measure without reloading.** The board is not re-read, records are not
touched, and undo history survives — which is the distinction from a surface or
context change, where reloading is the correct behaviour.
**Re-posting `annotation-target` with the same frame id is deliberately a no-op**
and is not the way to nudge. Overloading it would make every redundant declaration
re-anchor and reset undo, so the nudge has its own verb.
The layer also observes size and scroll on the anchored document, which are the
cases the browser reports for free. **Those are a convenience and are explicitly
not sufficient** — the explicit hook exists because motion within an unchanged
viewport is invisible to every observer short of polling each frame.
### The scope key is composed by the SHELL (stable)
`surfaceId` when no context is declared; `surfaceId`, a **NUL** (`U+0000`), then
@@ -216,6 +249,7 @@ default path:
| `{ t: "annotation-context" }` refining the scope | **yes** — verified in a live shell: a surface declaring one gets its own empty board, marks drawn there stay there, and declaring `null` restores the default board intact |
| `{ t: "annotation-target" }` anchoring inside a same-origin nested frame | **yes** — browser-proven against a DECOY: a host surface and its nested document both carrying `#publish`, with the mark hit-tested to confirm it landed on the artifact's button and not the host's |
| cross-origin nested frames degrading to the surface document rather than failing | **yes**, guarded; those surfaces stay spatial-only as they already were |
| `{ t: "annotation-refresh" }` and `refresh()` re-measuring a MOVED element | **yes** — measured before and after: an element moved 300×180px left the mark stale by exactly that offset, and both paths closed the gap to 0×0 without reloading the board |
| the active-surface-only rule on those messages | **yes, but the committed check is STRUCTURAL** — it asserts the `e.source` guard is present in the shell, with a control proving the check can fail. The behaviour was verified by driving a live shell; this repository has no browser dependency to re-run that automatically |
| the scope key composed by the shell and opaque to consumers | **yes** |
| persistence reported at `GET /api/contract` and shown in the UI | **yes** |
+1
View File
@@ -45,6 +45,7 @@ Send from inside your iframe with `parent.postMessage(msg, "*")`:
| `{ "t": "header", … }` | Declares the surface's half of the header. See *The header splits in half* below. |
| `{ "t": "annotation-context", "id": "<opaque>" }` | Refines which board of annotations this surface is showing. Only for a surface holding several documents behind one id — see `annotations.md`. `null` or omitted returns to the default. |
| `{ "t": "annotation-target", "frame": "<iframe element id>" }` | Names a **same-origin nested iframe** whose document annotations should anchor inside. For a surface that hosts its real content in a frame. `null` or omitted returns to the surface's own document. |
| `{ "t": "annotation-refresh" }` | "I moved something — re-measure." Repaints anchored marks without reloading the board or disturbing undo. Needed because an element that *moves* changes no size and no scroll, so nothing else fires. |
This message set is closed: the shell ignores any other `t` value. **New message
types arrive as contract additions, never by surfaces inventing them** — which is
+35
View File
@@ -274,6 +274,41 @@ test("positive control: the two checks above can fail", () => {
"the regression could not be planted, so the doesNotMatch assertion proves nothing");
});
test("the refresh hook re-measures WITHOUT reloading — STRUCTURAL", () => {
// Behaviour was measured in a browser: an element moved 300x180 left the mark
// stale by exactly that offset, and both the message and the handle closed it to
// 0x0. What is pinned here is the property that makes refresh different from a
// surface change — it must NOT call loadSurface, or a nudge would silently reset
// undo history and re-read the board.
const shell = fs.readFileSync(path.join(REPO, "app", "shell.html"), "utf8");
const handler = shell.match(/if \(e\.data\.t === "annotation-refresh"\)\s*\{([\s\S]*?)\n \}/);
assert.ok(handler, "shell.html no longer handles annotation-refresh");
assert.match(handler[1], /e\.source === current\.contentWindow/,
"the refresh handler accepts a nudge from a surface that is not on screen");
assert.match(handler[1], /refreshListeners/,
"the refresh handler does not notify the refresh channel");
assert.doesNotMatch(handler[1], /surfaceListeners/,
"refresh fires the SURFACE channel, which reloads the board and resets undo — that is a reload, not a re-measure");
const layer = fs.readFileSync(path.join(REPO, "app", "annotations.js"), "utf8");
assert.match(layer, /shell\.onRefresh\(\(\) => render\(\)\)/,
"the layer does not subscribe to the refresh channel");
assert.match(layer, /refresh: \(\) => \{ render\(\); \}/,
"the returned handle exposes no refresh()");
});
test("positive control: the refresh separation check can fail", () => {
const shell = fs.readFileSync(path.join(REPO, "app", "shell.html"), "utf8");
// Plant the exact regression the assertion forbids: refresh reloading the board.
const planted = shell.replace(
/(if \(e\.data\.t === "annotation-refresh"\)[\s\S]*?)for \(const fn of refreshListeners\)/,
"$1for (const fn of surfaceListeners)");
const handler = planted.match(/if \(e\.data\.t === "annotation-refresh"\)\s*\{([\s\S]*?)\n \}/);
assert.ok(handler, "the mutation broke the extractor rather than planting the regression");
assert.match(handler[1], /surfaceListeners/,
"the regression could not be planted, so the doesNotMatch assertion above proves nothing");
});
test("positive control: this suite is capable of failing", () => {
assert.throws(() => assert.equal(1, 2));
});