Skip to content

Commit 72e4f1a

Browse files
vanceingallsclaude
andauthored
test(core): previewAdapter contract failing tests (T10 spec for R7) (#1286)
* feat(core): clip-model hf- ids minted at parse, emitted as data-hf-id (R1) * docs(core): document legacy-id round-trip in clip-model readback (R1 review) Addresses Rames' review on #1270: clarifies that a pre-R1 clip authored with id="my-title" round-trips as data-hf-id="my-title" (non-hf-shaped but stable, exact-match) by design — targeting uses exact [data-hf-id="…"] match and does not require the hf- shape; legacy values re-mint only at the R7 write-back. Not a bug. Comment-only. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(core): fix misleading legacy-id migration comment in htmlParser.ts The original comment said legacy data-hf-id values "are re-minted only once the R7 write-back persists freshly-minted ids to source" — which is incorrect. ensureHfIds skips elements that already carry data-hf-id, so legacy values (e.g. data-hf-id="my-title") persist indefinitely and are NOT automatically re-minted. Exact-match targeting still works correctly. Update comment to reflect actual behaviour. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * feat(studio): sourcePatcher data-hf-id targeting (R1, T3) * fix(studio): warn on duplicate match in execDataAttrPattern (R1, T3 review) Addresses Rames' review on #1271: execDataAttrPattern returned the first regex match without checking for a second. A duplicate id/data-hf-id in source (id drift) would silently patch one element and leave the other stale. Now warns when more than one element matches. By the mint contract it should never fire. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(studio): pin hfId-is-authoritative-over-selector contract (R1, T3 review) Adds test: "hfId match is authoritative — selector is not used as a narrowing filter". When hfId matches element A and selector points at element B, findTagByTarget returns A without consulting selector as a narrowing filter. Pins the intended behaviour so a future refactor cannot silently start narrowing by selector. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * feat(core): sourceMutation data-hf-id targeting (R1, T7) * test(core): update htmlParser baselines for R1 hf- id format Elements now get data-hf-id minted by ensureHfIds; parser reads data-hf-id as model id, so HTML id attrs are no longer the model id. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * test(core): data-hf-id survives id/selector patch (R1, T7) Locks the preservation guarantee the write-back design depends on: a Studio edit targeting by id or selector (it never sends hfId) must not strip an existing data-hf-id, or the stable handle is destroyed by the next edit. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(core): escape hfId in selector + warn on duplicate match (R1, T7 review) Addresses review on #1272 (Miguel P3 + Rames): findTargetElement interpolated target.hfId raw into a [data-hf-id="..."] selector. Escape it (CSS attr-value injection guard) and warn when a hfId matches more than one element instead of silently patching an arbitrary one. Adds an injection-guard test. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(core): previewAdapter contract failing tests (T10 spec for R7) --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 1cd4ad6 commit 72e4f1a

2 files changed

Lines changed: 227 additions & 52 deletions

File tree

Lines changed: 199 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -1,75 +1,222 @@
1+
// fallow-ignore-file code-duplication
12
/**
23
* T10 — PreviewAdapter contract (spec for R7).
34
*
4-
* `createPreviewAdapter` does not exist yet. These stubs define the expected
5-
* interface so R7 has a concrete target. Convert from it.todo to real
6-
* assertions in the R7 PR.
5+
* Converted from it.todo stubs. These tests FAIL until Task 3 implements
6+
* createPreviewAdapter in ./previewAdapter.ts.
77
*
8-
* Hit-testing (elementAtPoint) in both linkedom and jsdom returns null for
9-
* all geometry calls — the real tests must inject a position-resolver stub
10-
* or mock elementFromPoint. The contract tested is filtering logic (root
11-
* exclusion, data-hf-id ancestor walk, opacity-at-playhead), not geometry.
8+
* Position resolution: elementFromPoint is always null in jsdom. All
9+
* elementAtPoint tests inject a resolvePoint stub so the contract tested
10+
* is filtering logic (root exclusion, data-hf-id ancestor walk,
11+
* opacity-at-playhead), not geometry.
12+
*
13+
* CSS custom property names used below mirror the Studio constants from
14+
* manualEditsTypes.ts — they will be shared with the PreviewAdapter
15+
* implementation once the draft-marker module moves to core (Task 4).
1216
*/
13-
import { describe, it } from "vitest";
17+
import { describe, it, expect, beforeEach } from "vitest";
18+
import { createPreviewAdapter } from "./previewAdapter.js";
1419

15-
describe("T10 — PreviewAdapter contract (spec for R7)", () => {
16-
describe("elementAtPoint", () => {
17-
it.todo("returns null for the stage root (data-hf-root)");
20+
// ── DOM helpers ────────────────────────────────────────────────────────────
1821

19-
it.todo("returns the nearest ancestor with data-hf-id");
22+
beforeEach(() => {
23+
document.body.innerHTML = "";
24+
});
2025

21-
it.todo("returns null when the hit element has no data-hf-id ancestor");
26+
/** Create + append an element to body; optionally set attrs and inline styles. */
27+
function make(
28+
tag: string,
29+
attrs: Record<string, string> = {},
30+
styles: Record<string, string> = {},
31+
): HTMLElement {
32+
const elem = document.createElement(tag);
33+
for (const [k, v] of Object.entries(attrs)) elem.setAttribute(k, v);
34+
for (const [k, v] of Object.entries(styles)) elem.style.setProperty(k, v);
35+
document.body.appendChild(elem);
36+
return elem;
37+
}
38+
39+
function adapterWith(resolvePoint: (x: number, y: number) => Element | null) {
40+
return createPreviewAdapter(document, { resolvePoint });
41+
}
42+
43+
// ── elementAtPoint ─────────────────────────────────────────────────────────
2244

23-
it.todo("skips elements whose computed opacity is 0 at the given playhead time");
45+
describe("T10 — PreviewAdapter contract (spec for R7)", () => {
46+
describe("elementAtPoint", () => {
47+
it("returns null for the stage root (data-hf-root)", () => {
48+
const root = make("div", { "data-hf-root": "true" });
49+
const adapter = adapterWith(() => root);
50+
expect(adapter.elementAtPoint(0, 0)).toBeNull();
51+
});
52+
53+
it("returns the nearest ancestor with data-hf-id", () => {
54+
const parent = make("div", { "data-hf-id": "hf-abcd" });
55+
const child = document.createElement("span");
56+
parent.appendChild(child);
57+
const adapter = adapterWith(() => child);
58+
expect(adapter.elementAtPoint(0, 0)).toBe(parent);
59+
});
60+
61+
it("returns null when the hit element has no data-hf-id ancestor", () => {
62+
const orphan = make("div");
63+
const adapter = adapterWith(() => orphan);
64+
expect(adapter.elementAtPoint(0, 0)).toBeNull();
65+
});
66+
67+
it("skips elements whose computed opacity is 0 at the given playhead time", () => {
68+
const elem = make("div", { "data-hf-id": "hf-zzzz" }, { opacity: "0" });
69+
const adapter = adapterWith(() => elem);
70+
expect(adapter.elementAtPoint(0, 0, { atTime: 1.0 })).toBeNull();
71+
});
2472
});
2573

26-
describe("applyDraft / revertDraft", () => {
27-
it.todo("applyDraft writes --hf-studio-* CSS props and sets the gesture marker");
28-
29-
it.todo("applyDraft accepts a move payload (dx/dy) and writes the translate draft");
30-
31-
it.todo("applyDraft accepts a resize payload (w/h) and writes the size draft");
74+
// ── applyDraft / revertDraft ───────────────────────────────────────────
3275

33-
it.todo("revertDraft removes draft props and clears the gesture marker");
34-
35-
it.todo("revertDraft restores original translate when an original was recorded");
76+
describe("applyDraft / revertDraft", () => {
77+
it("applyDraft writes --hf-studio-* CSS props and sets the gesture marker", () => {
78+
const target = make("div", { "data-hf-id": "hf-aaaa" });
79+
const adapter = adapterWith(() => null);
80+
adapter.applyDraft({ type: "move", hfId: "hf-aaaa", dx: 10, dy: 20 });
81+
expect(target.style.getPropertyValue("--hf-studio-offset-x")).not.toBe("");
82+
expect(target.hasAttribute("data-hf-studio-manual-edit-gesture")).toBe(true);
83+
});
84+
85+
it("applyDraft accepts a move payload (dx/dy) and writes the translate draft", () => {
86+
const target = make("div", { "data-hf-id": "hf-aaaa" });
87+
const adapter = adapterWith(() => null);
88+
adapter.applyDraft({ type: "move", hfId: "hf-aaaa", dx: 30, dy: 15 });
89+
expect(target.style.getPropertyValue("--hf-studio-offset-x")).toBe("30px");
90+
expect(target.style.getPropertyValue("--hf-studio-offset-y")).toBe("15px");
91+
});
92+
93+
it("applyDraft accepts a resize payload (w/h) and writes the size draft", () => {
94+
const target = make("div", { "data-hf-id": "hf-aaaa" });
95+
const adapter = adapterWith(() => null);
96+
adapter.applyDraft({ type: "resize", hfId: "hf-aaaa", w: 200, h: 100 });
97+
expect(target.style.getPropertyValue("--hf-studio-width")).toBe("200px");
98+
expect(target.style.getPropertyValue("--hf-studio-height")).toBe("100px");
99+
});
100+
101+
it("revertDraft removes draft props and clears the gesture marker", () => {
102+
const target = make("div", { "data-hf-id": "hf-aaaa" });
103+
const adapter = adapterWith(() => null);
104+
adapter.applyDraft({ type: "move", hfId: "hf-aaaa", dx: 10, dy: 20 });
105+
adapter.revertDraft();
106+
expect(target.style.getPropertyValue("--hf-studio-offset-x")).toBe("");
107+
expect(target.style.getPropertyValue("--hf-studio-offset-y")).toBe("");
108+
expect(target.hasAttribute("data-hf-studio-manual-edit-gesture")).toBe(false);
109+
});
110+
111+
it("revertDraft restores original translate when an original was recorded", () => {
112+
const target = make("div", { "data-hf-id": "hf-aaaa" });
113+
target.style.setProperty("translate", "50px 0px");
114+
const adapter = adapterWith(() => null);
115+
adapter.applyDraft({ type: "move", hfId: "hf-aaaa", dx: 10, dy: 0 });
116+
adapter.revertDraft();
117+
expect(target.style.getPropertyValue("translate")).toBe("50px 0px");
118+
});
36119
});
37120

38-
describe("applyDraft edge cases (R7 implementation contract)", () => {
39-
it.todo(
40-
"second applyDraft before revert/commit overwrites first draft — does not accumulate (dx/dy)",
41-
);
42-
43-
it.todo(
44-
"revertDraft is safe to call when no gesture is in progress (idempotent / no-op on empty marker)",
45-
);
46-
47-
it.todo(
48-
"elementAtPoint filtering is stable when playhead changes mid-drag — opacity re-evaluated per call",
49-
);
121+
// ── edge cases ─────────────────────────────────────────────────────────
50122

51-
it.todo(
52-
"stage-root exclusion applies only to the outermost data-hf-root; nested sub-composition roots count as targets",
53-
);
123+
describe("applyDraft edge cases (R7 implementation contract)", () => {
124+
it("second applyDraft before revert/commit overwrites first draft — does not accumulate (dx/dy)", () => {
125+
const target = make("div", { "data-hf-id": "hf-aaaa" });
126+
const adapter = adapterWith(() => null);
127+
adapter.applyDraft({ type: "move", hfId: "hf-aaaa", dx: 10, dy: 20 });
128+
adapter.applyDraft({ type: "move", hfId: "hf-aaaa", dx: 5, dy: 15 });
129+
expect(target.style.getPropertyValue("--hf-studio-offset-x")).toBe("5px");
130+
expect(target.style.getPropertyValue("--hf-studio-offset-y")).toBe("15px");
131+
});
132+
133+
it("revertDraft is safe to call when no gesture is in progress (idempotent / no-op on empty marker)", () => {
134+
const adapter = adapterWith(() => null);
135+
expect(() => adapter.revertDraft()).not.toThrow();
136+
expect(() => adapter.revertDraft()).not.toThrow();
137+
});
138+
139+
it("elementAtPoint filtering is stable when playhead changes mid-drag — opacity re-evaluated per call", () => {
140+
const elem = make("div", { "data-hf-id": "hf-zzzz" });
141+
const adapter = adapterWith(() => elem);
142+
expect(adapter.elementAtPoint(0, 0, { atTime: 0 })).toBe(elem);
143+
// simulates GSAP seeking to a time where the element is hidden
144+
elem.style.setProperty("opacity", "0");
145+
expect(adapter.elementAtPoint(0, 0, { atTime: 1.0 })).toBeNull();
146+
});
147+
148+
it("stage-root exclusion applies only to the outermost data-hf-root; nested sub-composition roots count as targets", () => {
149+
const outerRoot = make("div", { "data-hf-root": "true" });
150+
const innerRoot = document.createElement("div");
151+
innerRoot.setAttribute("data-hf-root", "true");
152+
innerRoot.setAttribute("data-hf-id", "hf-sub1");
153+
outerRoot.appendChild(innerRoot);
154+
155+
const adapterOuter = adapterWith(() => outerRoot);
156+
expect(adapterOuter.elementAtPoint(0, 0)).toBeNull();
157+
158+
const adapterInner = adapterWith(() => innerRoot);
159+
expect(adapterInner.elementAtPoint(0, 0)).toBe(innerRoot);
160+
});
54161
});
55162

56-
describe("commitPreview", () => {
57-
it.todo("returns null when no gesture marker is present");
58-
59-
it.todo("derives a moveElement patch from draft markers on commit");
163+
// ── commitPreview ──────────────────────────────────────────────────────
60164

61-
it.todo("derives a resize patch from draft markers on commit");
62-
63-
it.todo("clears the gesture marker after commit");
165+
describe("commitPreview", () => {
166+
it("returns null when no gesture marker is present", () => {
167+
const adapter = adapterWith(() => null);
168+
expect(adapter.commitPreview()).toBeNull();
169+
});
170+
171+
it("derives a moveElement patch from draft markers on commit", () => {
172+
make("div", { "data-hf-id": "hf-aaaa" });
173+
const adapter = adapterWith(() => null);
174+
adapter.applyDraft({ type: "move", hfId: "hf-aaaa", dx: 30, dy: 15 });
175+
const patch = adapter.commitPreview();
176+
expect(patch).toEqual({ type: "moveElement", hfId: "hf-aaaa", dx: 30, dy: 15 });
177+
});
178+
179+
it("derives a resize patch from draft markers on commit", () => {
180+
make("div", { "data-hf-id": "hf-aaaa" });
181+
const adapter = adapterWith(() => null);
182+
adapter.applyDraft({ type: "resize", hfId: "hf-aaaa", w: 200, h: 100 });
183+
const patch = adapter.commitPreview();
184+
expect(patch).toEqual({ type: "resize", hfId: "hf-aaaa", width: 200, height: 100 });
185+
});
186+
187+
it("clears the gesture marker after commit", () => {
188+
const target = make("div", { "data-hf-id": "hf-aaaa" });
189+
const adapter = adapterWith(() => null);
190+
adapter.applyDraft({ type: "move", hfId: "hf-aaaa", dx: 10, dy: 0 });
191+
adapter.commitPreview();
192+
expect(target.hasAttribute("data-hf-studio-manual-edit-gesture")).toBe(false);
193+
});
64194
});
65195

66-
describe("getElementTimings", () => {
67-
it.todo("reads authored absolute times from data-start / data-end");
196+
// ── getElementTimings ──────────────────────────────────────────────────
68197

69-
it.todo("ignores elements without data-hf-id");
70-
71-
it.todo(
72-
"returns a defined timing entry when data-hf-id is present but data-start / data-end are missing",
73-
);
198+
describe("getElementTimings", () => {
199+
it("reads authored absolute times from data-start / data-end", () => {
200+
make("div", { "data-hf-id": "hf-t1", "data-start": "0.5", "data-end": "2.0" });
201+
const adapter = adapterWith(() => null);
202+
const timings = adapter.getElementTimings();
203+
expect(timings["hf-t1"]).toEqual({ start: 0.5, end: 2.0 });
204+
});
205+
206+
it("ignores elements without data-hf-id", () => {
207+
make("div", { "data-start": "0.5", "data-end": "2.0" }); // no data-hf-id
208+
const adapter = adapterWith(() => null);
209+
const timings = adapter.getElementTimings();
210+
expect(Object.keys(timings)).toHaveLength(0);
211+
});
212+
213+
it("returns a defined timing entry when data-hf-id is present but data-start / data-end are missing", () => {
214+
make("div", { "data-hf-id": "hf-notimed" });
215+
const adapter = adapterWith(() => null);
216+
const timings = adapter.getElementTimings();
217+
expect(timings["hf-notimed"]).toBeDefined();
218+
expect(timings["hf-notimed"].start).toBeUndefined();
219+
expect(timings["hf-notimed"].end).toBeUndefined();
220+
});
74221
});
75222
});
Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
/**
2+
* PreviewAdapter — stub for R7 (Task 3 implements this).
3+
* Exports the typed API contract so tests can import and fail on assertions
4+
* rather than module resolution.
5+
*/
6+
7+
export type DraftPayload =
8+
| { type: "move"; hfId: string; dx: number; dy: number }
9+
| { type: "resize"; hfId: string; w: number; h: number };
10+
11+
export type CommitPatch =
12+
| { type: "moveElement"; hfId: string; dx: number; dy: number }
13+
| { type: "resize"; hfId: string; width: number; height: number };
14+
15+
export interface PreviewAdapter {
16+
elementAtPoint(x: number, y: number, opts?: { atTime?: number }): Element | null;
17+
applyDraft(payload: DraftPayload): void;
18+
revertDraft(): void;
19+
commitPreview(): CommitPatch | null;
20+
getElementTimings(): Record<string, { start?: number; end?: number }>;
21+
}
22+
23+
export function createPreviewAdapter(
24+
_document: Document,
25+
_opts?: { resolvePoint?: (x: number, y: number) => Element | null },
26+
): PreviewAdapter {
27+
throw new Error("not implemented — Task 3");
28+
}

0 commit comments

Comments
 (0)