diff --git a/packages/core/package-subpaths.json b/packages/core/package-subpaths.json index 2a61955dc0..9ca6bebbe9 100644 --- a/packages/core/package-subpaths.json +++ b/packages/core/package-subpaths.json @@ -38,6 +38,12 @@ "types": "./dist/utils/htmlAttrSafety.d.ts", "environments": ["browser", "bun", "node"] }, + "./rich-text-sanitize": { + "source": "./src/utils/richTextSanitize.ts", + "runtime": "./dist/utils/richTextSanitize.js", + "types": "./dist/utils/richTextSanitize.d.ts", + "environments": ["browser", "bun", "node"] + }, "./composition-contract": { "source": "./src/compositionContract.ts", "runtime": "./dist/compositionContract.js", diff --git a/packages/core/package.json b/packages/core/package.json index e31c894b99..1b2e7d3ba0 100644 --- a/packages/core/package.json +++ b/packages/core/package.json @@ -52,6 +52,12 @@ "import": "./src/utils/htmlAttrSafety.ts", "types": "./src/utils/htmlAttrSafety.ts" }, + "./rich-text-sanitize": { + "bun": "./src/utils/richTextSanitize.ts", + "node": "./dist/utils/richTextSanitize.js", + "import": "./src/utils/richTextSanitize.ts", + "types": "./src/utils/richTextSanitize.ts" + }, "./composition-contract": { "bun": "./src/compositionContract.ts", "node": "./dist/compositionContract.js", @@ -326,6 +332,10 @@ "import": "./dist/utils/htmlAttrSafety.js", "types": "./dist/utils/htmlAttrSafety.d.ts" }, + "./rich-text-sanitize": { + "import": "./dist/utils/richTextSanitize.js", + "types": "./dist/utils/richTextSanitize.d.ts" + }, "./composition-contract": { "import": "./dist/compositionContract.js", "types": "./dist/compositionContract.d.ts" diff --git a/packages/core/src/utils/richTextSanitize.test.ts b/packages/core/src/utils/richTextSanitize.test.ts new file mode 100644 index 0000000000..5563a5205e --- /dev/null +++ b/packages/core/src/utils/richTextSanitize.test.ts @@ -0,0 +1,193 @@ +import { describe, expect, it } from "vitest"; +import { parseHTML } from "linkedom"; +import { isRichTextFormattingTag, sanitizeRichTextChildren } from "./richTextSanitize"; + +// Both parsers, every case. The browser runs this against a live element and +// the server runs it against linkedom, and the whole point of one shared module +// is that the two cannot disagree about what may be written to a file. +const PARSERS: Array<[string, (html: string) => Element]> = [ + [ + "jsdom", + (html) => { + const host = document.createElement("div"); + host.innerHTML = html; + return host; + }, + ], + [ + "linkedom", + (html) => { + const { document: doc } = parseHTML(``); + const host = doc.createElement("div"); + host.innerHTML = html; + return host as unknown as Element; + }, + ], +]; + +function clean(html: string, parse: (html: string) => Element): string { + const host = parse(html); + sanitizeRichTextChildren(host); + return host.innerHTML; +} + +describe.each(PARSERS)("sanitizeRichTextChildren (%s)", (_name, parse) => { + it("keeps a styled span, which is the whole point", () => { + expect(clean('hi', parse)).toBe( + 'hi', + ); + }); + + it("keeps plain text untouched", () => { + expect(clean("just words", parse)).toBe("just words"); + }); + + it("keeps nested formatting and its nesting", () => { + expect(clean('x', parse)).toBe( + 'x', + ); + }); + + it("keeps a line break", () => { + expect(clean("a
b", parse)).toContain("
"); + }); + + it("removes a script and does not leave its source as visible text", () => { + const out = clean("keep", parse); + expect(out).not.toContain("script"); + expect(out).not.toContain("alert"); + expect(out).toContain("keep"); + }); + + it("strips an event handler from a tag it otherwise keeps", () => { + const out = clean('x', parse); + expect(out).not.toContain("onclick"); + expect(out).toContain("color: red"); + }); + + it("strips every attribute that is neither style nor an identity", () => { + const out = clean('x', parse); + expect(out).not.toContain("id="); + expect(out).not.toContain("class="); + expect(out).not.toContain("data-x"); + expect(out).toContain("color: red"); + }); + + // The design panel tracks each text layer by this. Stripping it left the + // panel unable to match a layer to its source after any inline style edit. + it("keeps the attributes a text layer is tracked by", () => { + const out = clean( + 'x', + parse, + ); + expect(out).toContain('data-hf-text-key="child:1"'); + expect(out).toContain('data-hf-id="hf-abc"'); + }); + + it("drops an identity attribute whose value is not a bare token", () => { + const out = clean(`x`, parse); + expect(out).not.toContain("onload"); + expect(out).not.toContain("data-hf-text-key"); + }); + + // These are what the design panel writes onto those same spans. Sanitizing + // them away did not stop a text edit changing layout, it deleted the layout + // the user had already set: colouring one word dropped a sibling's size. + it("keeps the typography the design panel authors on a text layer", () => { + const out = clean( + 'x', + parse, + ); + expect(out).toContain("font-family: Inter"); + expect(out).toContain("font-size: 48px"); + expect(out).toContain("letter-spacing: -1px"); + expect(out).toContain("line-height: 1.2"); + }); + + it("still refuses a value that reaches outside the stylesheet", () => { + const out = clean(`x`, parse); + expect(out).not.toContain("url("); + }); + + it("unwraps a tag that is not formatting, keeping its words in place", () => { + expect(clean("before
middle
after", parse)).toBe("beforemiddleafter"); + }); + + it("unwraps deeply and keeps the formatting found inside", () => { + const out = clean('

deep

', parse); + expect(out).toBe('deep'); + }); + + it("keeps only the allowlisted style properties", () => { + const out = clean('x', parse); + expect(out).toContain("color: red"); + expect(out).not.toContain("position"); + expect(out).not.toContain("z-index"); + }); + + it("keeps every property the allowlist names", () => { + const style = + "color: red; background-color: blue; font-weight: 700; font-style: italic; text-decoration-line: underline"; + const out = clean(`x`, parse); + for (const property of [ + "color", + "background-color", + "font-weight", + "font-style", + "text-decoration-line", + ]) { + expect(out).toContain(property); + } + }); + + it("rejects a value that smuggles a url or a script in", () => { + const out = clean( + 'x', + parse, + ); + expect(out).not.toContain("javascript"); + expect(out).not.toContain("url("); + expect(out).toContain("color: red"); + }); + + it("drops the style attribute entirely when nothing in it survives", () => { + expect(clean('x', parse)).toBe("x"); + }); + + it("keeps a value carrying a function with its own separators", () => { + const out = clean('x', parse); + expect(out).toContain("rgb(1, 2, 3)"); + expect(out).toContain("font-style: italic"); + }); + + it("removes a comment, which is neither text nor formatting", () => { + expect(clean("ab", parse)).toBe("ab"); + }); + + it("leaves an empty element alone", () => { + expect(clean("", parse)).toBe(""); + }); + + it("does not produce unbalanced markup from an unclosed tag", () => { + const out = clean('open', parse); + expect(out).toBe('open'); + }); +}); + +describe("isRichTextFormattingTag", () => { + it("names the tags an inline edit may contain", () => { + for (const tag of ["SPAN", "B", "STRONG", "I", "EM", "U", "BR"]) { + expect(isRichTextFormattingTag(tag)).toBe(true); + } + }); + + it("is case-insensitive, since the two parsers disagree about case", () => { + expect(isRichTextFormattingTag("span")).toBe(true); + }); + + it("says no to anything structural", () => { + for (const tag of ["DIV", "P", "H1", "IMG", "SCRIPT", "A"]) { + expect(isRichTextFormattingTag(tag)).toBe(false); + } + }); +}); diff --git a/packages/core/src/utils/richTextSanitize.ts b/packages/core/src/utils/richTextSanitize.ts new file mode 100644 index 0000000000..360c88e58e --- /dev/null +++ b/packages/core/src/utils/richTextSanitize.ts @@ -0,0 +1,189 @@ +/** + * What inline formatting a composition file is allowed to receive. + * + * Editing text in the Studio preview can style a run of characters, which means + * markup now travels from a contenteditable element into a file on disk. This + * module is the only thing deciding what may make that trip, and it runs on + * both ends of it: in the browser so the preview shows what will be saved, and + * on the server because that is where the file is written and a client is not + * a thing to trust. + * + * One module rather than two implementations. Two would drift, and the drift + * would be a security bug rather than an inconsistency. + * + * It works on an element's subtree in place, which is what both callers already + * have: the browser holds a live element, the server holds a parsed one. Nobody + * has to re-parse untrusted markup into a live document to clean it. + */ + +/** Tags an inline text edit may contain. Everything else is not text styling. */ +const FORMATTING_TAGS = new Set(["SPAN", "B", "STRONG", "I", "EM", "U", "BR"]); + +/** + * Style properties a formatting tag may carry. + * + * This was paint-only, on the reasoning that a property which moves or resizes + * text would let an edit inside one element change the composition's layout, + * and layout is the design panel's job. The reasoning was wrong about who was + * being restricted: the design panel writes exactly these typography + * properties onto exactly these spans, as its text layers. Sanitizing them + * away did not stop text from changing layout, it deleted the layout the user + * had already set β€” colouring one word silently dropped a sibling layer's font + * size. The line that matters is the one below, values that reach outside the + * stylesheet, not which of its own properties the editor is allowed to keep. + */ +const FORMATTING_STYLE_PROPS = new Set([ + "color", + "background-color", + "font-weight", + "font-style", + "text-decoration-line", + "font-family", + "font-size", + "letter-spacing", + "line-height", + // Paints the glyph fill and inherits, so an ancestor that sets it wins over + // any `color` below. The editor mirrors a run's colour into it when that is + // happening, and stripping it here would put the colour back to invisible. + "-webkit-text-fill-color", +]); + +/** + * Attributes a formatting tag may carry. + * + * The identity a text layer is tracked by. Everything else is dropped: a + * contenteditable is a paste target, and an event handler or an id that + * shadows a composition's own is not formatting. + */ +const FORMATTING_ATTRS = new Set(["data-hf-text-key", "data-hf-id"]); + +/** What those attributes are allowed to look like: a bare token, nothing else. */ +const SAFE_ATTR_VALUE = /^[A-Za-z0-9_:-]+$/; + +/** + * Tags dropped whole rather than unwrapped. + * + * Everything else is unwrapped, so an unexpected tag costs the user its + * formatting and not their words. These are the ones whose contents are not + * words: unwrapping a script would turn its source into visible text. + */ +const OPAQUE_TAGS = new Set([ + "SCRIPT", + "STYLE", + "TEMPLATE", + "NOSCRIPT", + "IFRAME", + "OBJECT", + "EMBED", + "SVG", + "MATH", +]); + +/** Anything that reaches out of the stylesheet, in a property that should not. */ +const UNSAFE_VALUE = /url\(|expression\(|javascript:|vbscript:|@import|<\//i; + +const ELEMENT_NODE = 1; +const TEXT_NODE = 3; + +export function isRichTextFormattingTag(tagName: string): boolean { + return FORMATTING_TAGS.has(tagName.toUpperCase()); +} + +/** + * Strip everything but allowed formatting from an element's contents, in place. + * + * The element itself is never touched, only what is inside it. Callers own the + * element, and it is the composition's, not the editor's, to rewrite. + */ +export function sanitizeRichTextChildren(parent: Element): void { + // A snapshot, because the loop moves and removes the very nodes it walks. + for (const child of Array.from(parent.childNodes)) { + if (child.nodeType === TEXT_NODE) continue; + + if (child.nodeType !== ELEMENT_NODE) { + // Comments and processing instructions are neither words nor formatting. + child.parentNode?.removeChild(child); + continue; + } + + const element = child as Element; + const tag = element.tagName.toUpperCase(); + + if (OPAQUE_TAGS.has(tag)) { + element.parentNode?.removeChild(element); + continue; + } + + // Clean the inside before deciding what to do with the outside, so an + // unwrap promotes children that have already been through this. + sanitizeRichTextChildren(element); + + if (!FORMATTING_TAGS.has(tag)) { + unwrap(element); + continue; + } + + stripAttributes(element); + } +} + +/** Replace an element with its own children, keeping their order and place. */ +function unwrap(element: Element): void { + const parent = element.parentNode; + if (!parent) return; + while (element.firstChild) parent.insertBefore(element.firstChild, element); + parent.removeChild(element); +} + +/** Leave a kept tag with a filtered style attribute and its identity, no more. */ +function stripAttributes(element: Element): void { + const style = element.getAttribute("style"); + for (const name of Array.from(element.getAttributeNames())) { + const value = element.getAttribute(name) ?? ""; + if (FORMATTING_ATTRS.has(name.toLowerCase()) && SAFE_ATTR_VALUE.test(value)) continue; + element.removeAttribute(name); + } + if (style === null) return; + const safe = filterStyle(style); + if (safe) element.setAttribute("style", safe); + else element.removeAttribute("style"); +} + +/** Keep only the allowlisted declarations, and only if their values are inert. */ +function filterStyle(style: string): string { + return splitDeclarations(style) + .map((declaration) => { + const colon = declaration.indexOf(":"); + if (colon === -1) return null; + const property = declaration.slice(0, colon).trim().toLowerCase(); + const value = declaration.slice(colon + 1).trim(); + if (!FORMATTING_STYLE_PROPS.has(property)) return null; + if (!value || UNSAFE_VALUE.test(value)) return null; + return `${property}: ${value}`; + }) + .filter((declaration): declaration is string => declaration !== null) + .join("; "); +} + +/** + * Split on the semicolons that separate declarations, not the ones inside a + * value. `color: rgb(1, 2, 3)` is one declaration however many separators its + * value contains. + */ +function splitDeclarations(style: string): string[] { + const declarations: string[] = []; + let current = ""; + let depth = 0; + for (const char of style) { + if (char === "(") depth += 1; + else if (char === ")") depth = Math.max(0, depth - 1); + else if (char === ";" && depth === 0) { + declarations.push(current); + current = ""; + continue; + } + current += char; + } + if (current.trim()) declarations.push(current); + return declarations; +} diff --git a/packages/studio-server/src/helpers/sourceMutation.richText.test.ts b/packages/studio-server/src/helpers/sourceMutation.richText.test.ts new file mode 100644 index 0000000000..0fb0a04e9e --- /dev/null +++ b/packages/studio-server/src/helpers/sourceMutation.richText.test.ts @@ -0,0 +1,150 @@ +import { describe, expect, it } from "vitest"; +import { patchElementInHtml } from "./sourceMutation.js"; + +/** + * The `rich-text` operation is the only one that can write markup into a + * composition, so it is also the only place a patch payload can carry + * something dangerous all the way to a file. These are the tests for that + * boundary, and for the promise that the older text operation did not quietly + * become a markup sink alongside it. + */ + +const DOC = (inner: string) => + `

${inner}

`; + +function patchTitle(inner: string, value: string, type: "rich-text" | "text-content") { + return patchElementInHtml(DOC(inner), { id: "title" }, [{ type, property: "", value }]); +} + +describe("rich-text patch operation", () => { + it("writes allowed formatting into the source", () => { + const { html, matched } = patchTitle( + "hello world", + 'hello world', + "rich-text", + ); + + expect(matched).toBe(true); + // The id is minted here so the bytes Studio records match the bytes on + // disk β€” see stampNewChildIds. + expect(html).toMatch(/o<\/span>/); + }); + + it("keeps the words and drops the script when the payload is hostile", () => { + const { html } = patchTitle("safe", "stillhere", "rich-text"); + + expect(html).not.toContain("script"); + expect(html).not.toContain("alert"); + expect(html).toContain("still"); + expect(html).toMatch(/here<\/b>/); + }); + + it("strips an event handler smuggled onto an allowed tag", () => { + const { html } = patchTitle("safe", 'x', "rich-text"); + + expect(html).not.toContain("onclick"); + expect(html).toContain("x"); + }); + + it("keeps only the allowlisted style properties", () => { + const { html } = patchTitle( + "safe", + 'x', + "rich-text", + ); + + expect(html).toContain("color: red"); + expect(html).not.toContain("position: fixed"); + }); + + it("unwraps a structural tag rather than losing the text inside it", () => { + const { html } = patchTitle("safe", "
kept
", "rich-text"); + + expect(html).toContain("kept"); + expect(html).not.toContain("
kept"); + }); + + it("replaces the previous contents rather than appending to them", () => { + const { html } = patchTitle("old words", "new words", "rich-text"); + + expect(html).toContain("new words"); + expect(html).not.toContain("old words"); + }); + + it("reports unmatched for an element that is not there", () => { + const result = patchElementInHtml(DOC("x"), { id: "absent" }, [ + { type: "rich-text", property: "", value: "y" }, + ]); + + expect(result.matched).toBe(false); + }); + + it("leaves the source alone when the value is null", () => { + const before = DOC("keep me"); + const { html } = patchElementInHtml(before, { id: "title" }, [ + { type: "rich-text", property: "", value: null }, + ]); + + expect(html).toContain("keep me"); + }); +}); + +describe("text-content is still not a markup sink", () => { + it("escapes markup handed to the older operation, exactly as before", () => { + const { html } = patchTitle("safe", 'x', "text-content"); + + expect(html).not.toContain(''); + expect(html).toContain("<span"); + }); +}); + +describe("rich-text round trips what a real composition contains", () => { + it("keeps text that looks like markup as text", () => { + const { html } = patchTitle("safe", "a <b> & c", "rich-text"); + + expect(html).toContain("<b>"); + expect(html).not.toContain(""); + }); + + it("keeps non-ASCII text intact", () => { + const { html } = patchTitle("safe", "hΓ©llo πŸ‘ δΈ–η•Œ", "rich-text"); + + expect(html).toContain("hΓ©llo"); + expect(html).toContain("πŸ‘"); + expect(html).toContain("δΈ–η•Œ"); + }); + + it("keeps a line break", () => { + const { html } = patchTitle("safe", "a
b", "rich-text"); + + expect(html).toMatch(/
/); + }); + + it("keeps the wrapper span a flex element needs", () => { + const { html } = patchTitle( + "safe", + 'a b c', + "rich-text", + ); + + expect(html).toMatch( + /a b<\/span> c<\/span>/, + ); + }); + + it("empties the element when every character was deleted", () => { + const { html } = patchTitle("gone", "", "rich-text"); + + expect(html).toContain('id="title">'); + }); + + it("does not accumulate markup when the same value is written twice", () => { + const value = 'x'; + const once = patchTitle("safe", value, "rich-text").html; + const twice = patchElementInHtml(once, { id: "title" }, [ + { type: "rich-text", property: "", value }, + ]).html; + + expect(twice).toBe(once); + }); +}); diff --git a/packages/studio-server/src/helpers/sourceMutation.test.ts b/packages/studio-server/src/helpers/sourceMutation.test.ts index 476fceac26..3520979fa0 100644 --- a/packages/studio-server/src/helpers/sourceMutation.test.ts +++ b/packages/studio-server/src/helpers/sourceMutation.test.ts @@ -542,3 +542,34 @@ describe("T7 β€” data-hf-id targeting (spec for R1)", () => { expect(html).toContain('data-hf-id="hf-a1b2"'); }); }); + +/** + * A rich-text operation adds elements, so it has to give them their stable ids + * here, in the bytes it writes and returns. + * + * Otherwise the next preview request mints them and writes the file a second + * time, after Studio has recorded the edit. The recorded "after" stops matching + * disk, the content check refuses, and undo reports the file as changed outside + * Studio β€” for every colour applied to a run of characters. + */ +describe("patchElementInHtml stamps the ids a rich-text patch introduces", () => { + it("gives each new span its id in the same write", () => { + const source = '
plain
'; + const { html, matched } = patchElementInHtml(source, { id: "t" }, [ + { type: "rich-text", property: "", value: 'abc' }, + ]); + + expect(matched).toBe(true); + expect(html).toContain("color: red"); + expect((html.match(/data-hf-id=/g) ?? []).length).toBe(2); + }); + + it("leaves an id a rich-text patch carried in alone", () => { + const source = '
plain
'; + const { html } = patchElementInHtml(source, { id: "t" }, [ + { type: "rich-text", property: "", value: 'b' }, + ]); + + expect(html).toContain('data-hf-id="hf-keep"'); + }); +}); diff --git a/packages/studio-server/src/helpers/sourceMutation.ts b/packages/studio-server/src/helpers/sourceMutation.ts index 1d406ddd34..ac7c8562b6 100644 --- a/packages/studio-server/src/helpers/sourceMutation.ts +++ b/packages/studio-server/src/helpers/sourceMutation.ts @@ -2,7 +2,8 @@ import { parseHTML } from "linkedom"; import postcss from "postcss"; import selectorParser from "postcss-selector-parser"; import { isAllowedHtmlAttribute, isSafeAttributeValue } from "@hyperframes/core/html-attr-safety"; -import { ensureHfIds } from "@hyperframes/parsers/hf-ids"; +import { sanitizeRichTextChildren } from "@hyperframes/core/rich-text-sanitize"; +import { EXCLUDED_TAGS, ensureHfIds, mintHfId } from "@hyperframes/parsers/hf-ids"; import { readClipTiming, writeClipTiming } from "@hyperframes/core/composition-contract"; import { parseStyleDecls, patchStyleAttrString } from "./sourceStyleMutation.js"; @@ -136,7 +137,7 @@ export function isHTMLElement(el: Node): el is HTMLElement { } export interface PatchOperation { - type: "inline-style" | "attribute" | "html-attribute" | "text-content"; + type: "inline-style" | "attribute" | "html-attribute" | "text-content" | "rich-text"; property: string; value: string | null; childSelector?: string; @@ -158,6 +159,36 @@ function resolveOperationTarget(parent: HTMLElement, op: PatchOperation): HTMLEl } } +/** + * Give the elements a rich-text patch just introduced their stable ids, here, + * in the bytes about to be written and handed back. + * + * Otherwise the next preview request mints them and writes the file a second + * time, after Studio has already recorded the edit in its history. The recorded + * "after" stops matching disk, the content check refuses, and undo reports the + * file as changed outside Studio β€” for every colour applied to a run of + * characters and every text layer added. The clip split stamps its own clone + * for exactly this reason. + * + * Minted one element at a time with the same function `ensureHfIds` uses, so + * these ids are the ones the next pass would have assigned. Not `ensureHfIds` + * itself: it takes a whole document, and handing it this element's markup would + * put the markup back as one. + */ +function stampNewChildIds(parent: Element): void { + const assigned = new Set(); + const root = parent.ownerDocument?.body ?? parent; + for (const el of root.querySelectorAll("[data-hf-id]")) { + const id = el.getAttribute("data-hf-id"); + if (id) assigned.add(id); + } + for (const el of parent.querySelectorAll("*")) { + if (el.getAttribute("data-hf-id")) continue; + if (EXCLUDED_TAGS.has(el.tagName.toLowerCase())) continue; + el.setAttribute("data-hf-id", mintHfId(el, assigned)); + } +} + // fallow-ignore-next-line complexity export function patchElementInHtml( source: string, @@ -215,6 +246,17 @@ export function patchElementInHtml( textTarget.textContent = op.value; } break; + // The one operation that can write markup, so the one that has to check + // it. Assigned first and sanitised after, rather than sanitising a + // string: parsing is what turns a payload into the tree the allowlist + // can actually judge, and linkedom never runs anything it parses. + case "rich-text": + if (op.value != null) { + opTarget.innerHTML = op.value; + sanitizeRichTextChildren(opTarget); + stampNewChildIds(opTarget); + } + break; } } diff --git a/packages/studio-server/src/routes/files.test.ts b/packages/studio-server/src/routes/files.test.ts index ffd9f61a8c..65ee973f0a 100644 --- a/packages/studio-server/src/routes/files.test.ts +++ b/packages/studio-server/src/routes/files.test.ts @@ -425,6 +425,38 @@ describe("registerFileRoutes", () => { expect(readFileSync(join(projectDir, "index.html"), "utf-8")).toContain("After"); }); + // Without the receipt the client cannot recognise its own edit in the watcher + // broadcast, so it treats it as someone else's write and does a full preview + // reload β€” a visible blank on the stage right after the user typed. + it("leaves a write receipt so the patch's own file-change echo is identifiable", async () => { + const projectDir = createProjectDir(); + writeFileSync(projectDir + "/index.html", '
Before
'); + const app = new Hono(); + registerFileRoutes(app, createAdapter(projectDir)); + + const response = await app.request( + "http://localhost/projects/demo/file-mutations/patch-element/index.html", + { + method: "POST", + headers: { + "Content-Type": "application/json", + "X-Hyperframes-Write-Token": "studio-patch-1", + }, + body: JSON.stringify({ + target: { id: "title" }, + operations: [{ type: "text-content", property: "textContent", value: "After" }], + }), + }, + ); + + expect(response.status).toBe(200); + expect(consumeFileWriteReceipt(join(projectDir, "index.html"))).toEqual({ + path: "index.html", + version: fileContentVersion(readFileSync(join(projectDir, "index.html"), "utf-8")), + writeToken: "studio-patch-1", + }); + }); + it("applies an ordered element patch batch with one file write", async () => { const projectDir = createProjectDir(); const original = diff --git a/packages/studio-server/src/routes/files.ts b/packages/studio-server/src/routes/files.ts index 3ec64bbff1..63b71a4904 100644 --- a/packages/studio-server/src/routes/files.ts +++ b/packages/studio-server/src/routes/files.ts @@ -112,6 +112,7 @@ interface RouteContext { param: (name: string) => string; path: string; query: (name: string) => string | undefined; + header: (name: string) => string | undefined; }; header: (name: string, value: string) => void; json: (data: unknown, status?: number) => Response; @@ -399,6 +400,35 @@ export function commitElementPatchBatches( return { durable: true, files }; } +/** + * Write a mutation result, and leave behind the receipt that claims it. + * + * The file watcher broadcasts every write, including the ones Studio itself just + * asked for. The receipt is what lets the client tell its own echo from an agent + * or an editor writing the file behind its back: without one, the client treats + * its own edit as an external change and does a full preview reload, which blanks + * the stage for a few hundred milliseconds right after the user typed. Every + * mutation route writes through here so no route can forget. + */ +function writeMutationResult( + c: RouteContext, + projectDir: string, + filePath: string, + absPath: string, + html: string, +): { backupPath: string | null; version: string } { + const backup = snapshotBeforeWrite(projectDir, absPath); + if (backup.error) console.warn(`Failed to create backup for ${filePath}: ${backup.error}`); + writeFileSync(absPath, html, "utf-8"); + const version = fileContentVersion(html); + recordFileWriteReceipt(absPath, { + path: filePath, + version, + writeToken: createWriteToken(c.req.header("X-Hyperframes-Write-Token")), + }); + return { backupPath: backupPathForResponse(projectDir, backup.backupPath), version }; +} + /** Write `next` to `absPath` only if it differs from `original`, returning a standardized change response. */ function writeIfChanged( c: RouteContext, @@ -411,15 +441,13 @@ function writeIfChanged( if (next === original) { return c.json({ ok: true, changed: false, content: original, path: filePath }); } - const backup = snapshotBeforeWrite(projectDir, absPath); - if (backup.error) console.warn(`Failed to create backup for ${filePath}: ${backup.error}`); - writeFileSync(absPath, next, "utf-8"); + const { backupPath } = writeMutationResult(c, projectDir, filePath, absPath, next); return c.json({ ok: true, changed: true, content: next, path: filePath, - backupPath: backupPathForResponse(projectDir, backup.backupPath), + backupPath, }); } @@ -1238,10 +1266,13 @@ async function applyGsapMutations( return c.json({ error: "file changed during GSAP mutation", conflict: true }, 409); } if (changed) { - const backup = snapshotBeforeWrite(res.project.dir, res.absPath); - if (backup.error) console.warn(`Failed to create backup for ${res.filePath}: ${backup.error}`); - backupPath = backupPathForResponse(res.project.dir, backup.backupPath); - writeFileSync(res.absPath, newHtml, "utf-8"); + backupPath = writeMutationResult( + c, + res.project.dir, + res.filePath, + res.absPath, + newHtml, + ).backupPath; } const responsePayload: Record = { @@ -2623,10 +2654,13 @@ export function registerFileRoutes(api: Hono, adapter: StudioApiAdapter): void { version, }); } - const backup = snapshotBeforeWrite(ctx.project.dir, ctx.absPath); - if (backup.error) console.warn(`Failed to create backup for ${ctx.filePath}: ${backup.error}`); - writeFileSync(ctx.absPath, result.html, "utf-8"); - const version = fileContentVersion(result.html); + const { version, backupPath } = writeMutationResult( + c, + ctx.project.dir, + ctx.filePath, + ctx.absPath, + result.html, + ); c.header("ETag", version); return c.json({ ok: true, @@ -2635,7 +2669,7 @@ export function registerFileRoutes(api: Hono, adapter: StudioApiAdapter): void { newId: result.newId, path: ctx.filePath, version, - backupPath: backupPathForResponse(ctx.project.dir, backup.backupPath), + backupPath, }); }); @@ -2676,16 +2710,20 @@ export function registerFileRoutes(api: Hono, adapter: StudioApiAdapter): void { path: ctx.filePath, }); } - const backup = snapshotBeforeWrite(ctx.project.dir, ctx.absPath); - if (backup.error) console.warn(`Failed to create backup for ${ctx.filePath}: ${backup.error}`); - writeFileSync(ctx.absPath, patched, "utf-8"); + const { backupPath } = writeMutationResult( + c, + ctx.project.dir, + ctx.filePath, + ctx.absPath, + patched, + ); return c.json({ ok: true, changed: true, matched, content: patched, path: ctx.filePath, - backupPath: backupPathForResponse(ctx.project.dir, backup.backupPath), + backupPath, }); }); @@ -2807,16 +2845,20 @@ export function registerFileRoutes(api: Hono, adapter: StudioApiAdapter): void { result.error === "grouped elements must share a single parent" ? 422 : 400, ); } - const backup = snapshotBeforeWrite(ctx.project.dir, ctx.absPath); - if (backup.error) console.warn(`Failed to create backup for ${ctx.filePath}: ${backup.error}`); - writeFileSync(ctx.absPath, result.html, "utf-8"); + const { backupPath } = writeMutationResult( + c, + ctx.project.dir, + ctx.filePath, + ctx.absPath, + result.html, + ); return c.json({ ok: true, changed: true, groupId: result.groupId, content: result.html, path: ctx.filePath, - backupPath: backupPathForResponse(ctx.project.dir, backup.backupPath), + backupPath, }); }); diff --git a/packages/studio/src/App.tsx b/packages/studio/src/App.tsx index 520b4e8554..538aebf9ec 100644 --- a/packages/studio/src/App.tsx +++ b/packages/studio/src/App.tsx @@ -91,6 +91,7 @@ export function StudioApp() { const captionSync = useCaptionSync(projectId); const timelineElements = usePlayerStore((s) => s.elements); const setSelectedTimelineElementId = usePlayerStore((s) => s.setSelectedElementId); + const setTimelineSelectionSet = usePlayerStore((s) => s.setSelectedElementIds); const timelineDuration = usePlayerStore((s) => s.duration); const isPlaying = usePlayerStore((s) => s.isPlaying); const isMasterView = !activeCompPath || activeCompPath === "index.html"; @@ -277,6 +278,7 @@ export function StudioApp() { previewIframeRef, timelineElements, setSelectedTimelineElementId, + setTimelineSelectionSet, setRightCollapsed: panelLayout.setRightCollapsed, setRightPanelTab: panelLayout.setRightPanelTab, showToast, @@ -416,6 +418,8 @@ export function StudioApp() { rightCollapsed: panelLayout.rightCollapsed, activeCompPathHydrated, domEditSelection: domEditSession.domEditSelection, + domEditGroupSelections: domEditSession.domEditGroupSelections, + applyMarqueeSelection: domEditSession.applyMarqueeSelection, buildDomSelectionFromTarget: domEditSession.buildDomSelectionFromTarget, applyDomSelection: domEditSession.applyDomSelection, setRightPanelTab: panelLayout.setRightPanelTab, diff --git a/packages/studio/src/captions/hooks/useCaptionSync.ts b/packages/studio/src/captions/hooks/useCaptionSync.ts index 5fdbf80138..1b2037a0f4 100644 --- a/packages/studio/src/captions/hooks/useCaptionSync.ts +++ b/packages/studio/src/captions/hooks/useCaptionSync.ts @@ -3,6 +3,7 @@ import { useCaptionStore } from "../store"; import { useMountEffect } from "../../hooks/useMountEffect"; import { trackEvent } from "../../telemetry/client"; import type { CaptionStyle } from "../types"; +import { studioWriteHeaders } from "../../utils/studioFileVersion"; interface CaptionOverrideEntry { wordId?: string; @@ -77,7 +78,7 @@ export function useCaptionSync(projectId: string | null) { fetch(`/api/projects/${pid}/files/${encodeURIComponent("caption-overrides.json")}`, { method: "PUT", - headers: { "Content-Type": "text/plain" }, + headers: { "Content-Type": "text/plain", ...studioWriteHeaders() }, body: JSON.stringify(overrides, null, 2), }).catch((error: unknown) => { // Caption auto-save is a data-loss path; surface failures via telemetry diff --git a/packages/studio/src/components/editor/DomEditOverlay.test.ts b/packages/studio/src/components/editor/DomEditOverlay.test.ts index 736bd7ecb2..0d857d7918 100644 --- a/packages/studio/src/components/editor/DomEditOverlay.test.ts +++ b/packages/studio/src/components/editor/DomEditOverlay.test.ts @@ -14,7 +14,10 @@ import { resolveDomEditRotationGesture, } from "./DomEditOverlay"; import type { DomEditSelection } from "./domEditing"; -import { resolveResizeCenterAnchorOffset } from "./domEditOverlayGestures"; +import { + hoverCacheDescribesPoint, + resolveResizeCenterAnchorOffset, +} from "./domEditOverlayGestures"; // React 19 warns unless the test environment opts into act(). globalThis.IS_REACT_ACT_ENVIRONMENT = true; @@ -628,6 +631,50 @@ describe("resolveDomEditRotationGesture", () => { }); }); +/** + * Shift-click reads the hover cache instead of hit-testing, and the cache is + * filled asynchronously as the pointer moves. Pass over one element on the way to + * another and the cache still names the one you left, so the shift-click added + * THAT element and the click looked like it selected something at random. The + * guard is what makes the cache usable only when it is about the point clicked. + */ +describe("hoverCacheDescribesPoint", () => { + const doc = new Window().document; + + it("rejects a cache left behind by an element the pointer passed over", () => { + const passedOver = doc.createElement("div"); + const clicked = doc.createElement("div"); + doc.body.append(passedOver, clicked); + + expect(hoverCacheDescribesPoint(passedOver, clicked)).toBe(false); + }); + + it("accepts the cache when it names the element at the point", () => { + const clicked = doc.createElement("div"); + doc.body.append(clicked); + + expect(hoverCacheDescribesPoint(clicked, clicked)).toBe(true); + }); + + // The resolver is allowed to hand back a clip ancestor of the raw target, which + // still describes the same click β€” rejecting it would drop the fast path on + // every element that has children. + it("accepts an ancestor of the element at the point", () => { + const clip = doc.createElement("div"); + const child = doc.createElement("span"); + clip.append(child); + doc.body.append(clip); + + expect(hoverCacheDescribesPoint(clip, child)).toBe(true); + }); + + it("rejects a missing cache or an empty point", () => { + const el = doc.createElement("div"); + expect(hoverCacheDescribesPoint(null, el)).toBe(false); + expect(hoverCacheDescribesPoint(el, null)).toBe(false); + }); +}); + // resolveResizeCenterAnchorOffset is the UNROTATED (AABB) fallback used only when // the element's real transformed corners can't be measured. Center-anchored: a // width/height change grows the box from its top-left, drifting the center by half diff --git a/packages/studio/src/components/editor/DomEditOverlay.tsx b/packages/studio/src/components/editor/DomEditOverlay.tsx index 2c6d6add3c..11cd1f0d4c 100644 --- a/packages/studio/src/components/editor/DomEditOverlay.tsx +++ b/packages/studio/src/components/editor/DomEditOverlay.tsx @@ -13,9 +13,10 @@ import { type GestureState, type GroupGestureState, focusDomEditOverlayElement, + resolveShiftClickCandidate, } from "./domEditOverlayGestures"; import { useDomEditOverlayRects } from "./useDomEditOverlayRects"; -import { OffCanvasIndicators, type OffCanvasRect } from "./OffCanvasIndicators"; +import { ChildRectOutlines, OffCanvasIndicators, type OffCanvasRect } from "./OffCanvasIndicators"; import { createDomEditOverlayGestureHandlers } from "./useDomEditOverlayGestures"; import { useDomEditNudge } from "./useDomEditNudge"; import { SnapGuideOverlay, type SnapGuidesState } from "./SnapGuideOverlay"; @@ -29,8 +30,10 @@ import { useDomEditCompositionRect } from "./useDomEditCompositionRect"; import { useMountEffect } from "../../hooks/useMountEffect"; import { startOffCanvasIndicatorRefresh } from "./offCanvasIndicatorRefresh"; import { CanvasContextMenu } from "./CanvasContextMenu"; +import { useInlineTextEditing } from "./useInlineTextEditing"; import type { ZOrderAction, ZOrderPatch } from "./canvasContextMenuZOrder"; import { getPreviewTargetFromPointer } from "../../utils/studioPreviewHelpers"; +import { logSelect } from "../../utils/selectDebug"; // Re-exports for external consumers β€” preserving existing import paths. export { @@ -162,6 +165,9 @@ export const DomEditOverlay = memo(function DomEditOverlay({ groupSelectionsRef.current = groupSelections; const hoverSelectionRef = useRef(hoverSelection); hoverSelectionRef.current = hoverSelection; + + // Double-click an element to edit its text where it sits. + const inlineText = useInlineTextEditing(selectionRef); const onPathOffsetCommitRef = useRef(onPathOffsetCommit); onPathOffsetCommitRef.current = onPathOffsetCommit; const onGroupPathOffsetCommitRef = useRef(onGroupPathOffsetCommit); @@ -318,6 +324,7 @@ export const DomEditOverlay = memo(function DomEditOverlay({ const handleOverlayMouseDown = (event: React.MouseEvent) => { if (!allowCanvasMovement) return; if (suppressNextOverlayMouseDownRef.current) { + logSelect("mousedown-suppressed", { shift: event.shiftKey }); suppressNextOverlayMouseDownRef.current = false; suppressNextBoxMouseDownRef.current = false; suppressNextBoxClickRef.current = false; @@ -326,7 +333,9 @@ export const DomEditOverlay = memo(function DomEditOverlay({ return; } const target = event.target as HTMLElement | null; - if (target?.closest('[data-dom-edit-selection-box="true"]')) return; + const onBox = Boolean(target?.closest('[data-dom-edit-selection-box="true"]')); + logSelect("mousedown", { shift: event.shiftKey, onBox }); + if (onBox) return; // Allow clicks anywhere on the overlay β€” GSAP-translated elements can // extend beyond the composition rect into the gray zone, and users need // to select/deselect them by clicking there. @@ -341,8 +350,20 @@ export const DomEditOverlay = memo(function DomEditOverlay({ const handleOverlayPointerDown = (event: React.PointerEvent) => { if (!allowCanvasMovement || event.button !== 0) return; if (event.shiftKey) { - // Use the already-updated hover selection rather than re-resolving async - const candidate = hoverSelectionRef.current; + const shiftIframe = iframeRef.current; + const candidate = resolveShiftClickCandidate({ + cached: hoverSelectionRef.current, + elementAtPoint: shiftIframe + ? getPreviewTargetFromPointer( + shiftIframe, + event.clientX, + event.clientY, + activeCompositionPathRef.current, + ) + : null, + }); + // Not confident: fall through untouched β€” no preventDefault, no suppression β€” + // so the mousedown path resolves this point instead of guessing here. if (!candidate) return; event.preventDefault(); event.stopPropagation(); @@ -353,6 +374,16 @@ export const DomEditOverlay = memo(function DomEditOverlay({ return; } + // A second press on the same spot opens that element's text. This is the + // press path that actually runs: the pointer handler prevents the default + // on its way through, so the overlay's own mousedown never fires, and the + // browser never pairs the presses into a dblclick either. + if (inlineText.startFromPress(event)) { + event.preventDefault(); + event.stopPropagation(); + return; + } + const target = event.target as HTMLElement | null; if (target?.closest('[data-dom-edit-selection-box="true"]')) return; @@ -376,28 +407,27 @@ export const DomEditOverlay = memo(function DomEditOverlay({ const overlayEl = overlayRef.current; if (overlayEl) { const oRect = overlayEl.getBoundingClientRect(); + // Anywhere empty on the overlay starts one, not just inside the frame. + // An element dragged past the edge sits OUT there in the grey, and a + // rubber band that refuses to start there cannot reach it β€” which left + // the timeline as the only way to select something you can plainly see. + // The hit test collects in overlay space and never clipped to the frame, + // so those elements were always selectable once the band could begin. + event.preventDefault(); + event.stopPropagation(); + suppressNextOverlayMouseDownRef.current = true; + (event.currentTarget as HTMLElement).setPointerCapture(event.pointerId); const cx = event.clientX - oRect.left; const cy = event.clientY - oRect.top; - const inComp = - cx >= compRect.left && - cx <= compRect.left + compRect.width && - cy >= compRect.top && - cy <= compRect.top + compRect.height; - if (inComp) { - event.preventDefault(); - event.stopPropagation(); - suppressNextOverlayMouseDownRef.current = true; - (event.currentTarget as HTMLElement).setPointerCapture(event.pointerId); - marquee.marqueeRef.current = { - startX: cx, - startY: cy, - currentX: cx, - currentY: cy, - pointerId: event.pointerId, - pastThreshold: false, - }; - return; - } + marquee.marqueeRef.current = { + startX: cx, + startY: cy, + currentX: cx, + currentY: cy, + pointerId: event.pointerId, + pastThreshold: false, + }; + return; } } }; @@ -433,7 +463,12 @@ export const DomEditOverlay = memo(function DomEditOverlay({ return (
{ + if (!inlineText.handleKeyDown(event)) return; + event.preventDefault(); + event.stopPropagation(); }} onPointerDown={handleOverlayPointerDown} onMouseDown={handleOverlayMouseDown} @@ -476,6 +519,7 @@ export const DomEditOverlay = memo(function DomEditOverlay({ )} {!hasGroupSelection && selection && overlayRect && compRect.width > 0 && ( )} - {childRects.length > 0 && - compRect.width > 0 && - childRects.map((cr, i) => ( -
- ))} + 0 ? childRects : []} /> + {/* Mounted here rather than with the selection chrome: the chrome does + not render for every selection, and the toolbar belongs to the + editing session, which does. */} + {inlineText.toolbar} { act(() => root.unmount()); }); }); + +// The bug: the overlay above the preview goes pointer-events-none while text is +// being edited, but `pointer-events: none` on a parent does not disable a child +// that sets `auto`. The selection box covers exactly the element being typed +// into, so it kept swallowing every press: the caret could only ever be placed +// once, when the edit opened, and dragging across characters did nothing. +describe("DomEditSelectionChrome while editing text", () => { + const CAPABLE = { + canCrop: true, + canApplyManualOffset: true, + canApplyManualSize: true, + canApplyManualRotation: true, + }; + + function renderChrome(editing: boolean) { + const element = document.createElement("div"); + element.id = "copy"; + document.body.append(element); + const selection = { + element, + id: "copy", + selector: "#copy", + capabilities: CAPABLE, + } as unknown as DomEditSelection; + const host = document.createElement("div"); + document.body.append(host); + const root = createRoot(host); + act(() => { + root.render( + , + ); + }); + return { host, unmount: () => act(() => root.unmount()) }; + } + + it("stops the selection box taking presses, so they reach the caret below", () => { + const { host, unmount } = renderChrome(true); + const box = host.querySelector('[data-dom-edit-selection-box="true"]')!; + expect(box.className).toContain("pointer-events-none"); + expect(box.className).not.toContain("pointer-events-auto"); + unmount(); + }); + + it("keeps the box interactive when no text is being edited", () => { + const { host, unmount } = renderChrome(false); + const box = host.querySelector('[data-dom-edit-selection-box="true"]')!; + expect(box.className).toContain("pointer-events-auto"); + unmount(); + }); + + it("still marks the edited element, so it is clear which one has the caret", () => { + const { host, unmount } = renderChrome(true); + const box = host.querySelector('[data-dom-edit-selection-box="true"]')!; + expect(box.className).toContain("border-studio-accent/80"); + unmount(); + }); + + it("takes away every handle that would sit over the text", () => { + const { host, unmount } = renderChrome(true); + expect(host.querySelectorAll(".pointer-events-auto")).toHaveLength(0); + expect(host.querySelector("[data-dom-edit-crop-frame]")).toBeNull(); + unmount(); + }); + + it("keeps the handles when nothing is being edited", () => { + const { host, unmount } = renderChrome(false); + expect(host.querySelectorAll(".pointer-events-auto").length).toBeGreaterThan(1); + unmount(); + }); +}); diff --git a/packages/studio/src/components/editor/DomEditSelectionChrome.tsx b/packages/studio/src/components/editor/DomEditSelectionChrome.tsx index acbde93f69..8046e8608f 100644 --- a/packages/studio/src/components/editor/DomEditSelectionChrome.tsx +++ b/packages/studio/src/components/editor/DomEditSelectionChrome.tsx @@ -126,6 +126,12 @@ interface DomEditSelectionChromeProps { onStyleCommit?: (property: string, value: string) => Promise | void; onBoxMouseDown: (e: React.MouseEvent) => void; onBoxClick: (event: React.MouseEvent) => void; + /** The canvas' text-editing session: what opens one, and whether one is open. */ + inlineText?: { + editing: boolean; + /** Every press on the box. Returns true when it opened a text edit. */ + startFromPress: (event: React.PointerEvent) => boolean; + }; } // Oriented selection chrome: a rotation wrapper spanning the overlay, rotated by @@ -149,7 +155,16 @@ export function DomEditSelectionChrome({ onStyleCommit, onBoxMouseDown, onBoxClick, + inlineText, }: DomEditSelectionChromeProps) { + // While the text is being edited the chrome is a mark, not a control. The + // overlay above the preview already stands aside for the caret, but + // `pointer-events: none` on a parent does not disable a child that asks for + // them back, and the box is positioned to cover exactly the text being typed + // into: left interactive, it swallows every press, so the caret can never be + // moved and characters can never be selected by dragging. + const editing = inlineText?.editing ?? false; + return ( <>
- {allowCanvasMovement && selection.capabilities.canApplyManualRotation && ( + {allowCanvasMovement && !editing && selection.capabilities.canApplyManualRotation && ( { + // A second press opens the element's text for editing, and must be + // caught here rather than on the canvas: this handler prevents the + // default on the first press, which suppresses the compatibility + // mousedown the canvas would otherwise see, and the pointer capture + // it takes stops the browser pairing the presses into a dblclick. + if (inlineText?.startFromPress(e)) { + e.preventDefault(); + e.stopPropagation(); + return; + } if (!allowCanvasMovement || e.shiftKey) return; if (selection.capabilities.canApplyManualOffset) { gestures.startGesture("drag", e); @@ -221,6 +246,7 @@ export function DomEditSelectionChrome({ is positioned relative to the overlay container using the overlayRect origin, matching the old child-relative offsets. */} {allowCanvasMovement && + !editing && selection.capabilities.canApplyManualSize && RESIZE_HANDLE_DEFS.map((def) => def.handle !== "se" && !selection.capabilities.canApplyManualOffset ? null : ( @@ -245,7 +271,7 @@ export function DomEditSelectionChrome({
{/* Crop owns its element-local oriented frame. Keep it outside the chrome's rotated plane or a rotated selection applies the angle twice. */} - {selection.capabilities.canCrop && groupSelectionCount <= 1 && ( + {selection.capabilities.canCrop && !editing && groupSelectionCount <= 1 && ( { + document.body.innerHTML = ""; +}); + +/** An element standing in for one in the preview, and a fake frame around it. */ +function scene(html: string) { + document.body.innerHTML = `

${html}

`; + const element = document.body.firstElementChild as HTMLElement; + const iframe = document.createElement("iframe"); + document.body.append(iframe); + // The composition is drawn scaled, so the toolbar has to map out of it. + iframe.getBoundingClientRect = () => ({ left: 100, top: 50, width: 400 }) as DOMRect; + Object.defineProperty(iframe, "contentWindow", { value: window }); + const session: InlineTextEditSession = { element, original: html, outline: "" }; + return { element, iframe, session }; +} + +function render(session: InlineTextEditSession | null, iframe: HTMLIFrameElement | null) { + const host = document.createElement("div"); + document.body.append(host); + const root = createRoot(host); + act(() => root.render()); + return { host, root, rerender: () => act(() => root.render(
)) }; +} + +function selectAll(element: HTMLElement) { + const range = document.createRange(); + range.selectNodeContents(element); + const selection = document.getSelection()!; + selection.removeAllRanges(); + selection.addRange(range); + act(() => void document.dispatchEvent(new Event("selectionchange"))); +} + +function toolbarIn(host: HTMLElement): HTMLElement | null { + return host.querySelector('[data-inline-text-toolbar="true"]'); +} + +describe("InlineTextToolbar", () => { + it("stays out of the way until characters are actually selected", () => { + const { session, iframe } = scene("hello world"); + const { host } = render(session, iframe); + + expect(toolbarIn(host)).toBeNull(); + }); + + it("appears once a run of characters is selected", () => { + const { element, session, iframe } = scene("hello world"); + const { host } = render(session, iframe); + + selectAll(element); + + expect(toolbarIn(host)).not.toBeNull(); + }); + + it("goes away when the selection collapses again", () => { + const { element, session, iframe } = scene("hello world"); + const { host } = render(session, iframe); + selectAll(element); + + const selection = document.getSelection()!; + act(() => { + selection.collapseToEnd(); + document.dispatchEvent(new Event("selectionchange")); + }); + + expect(toolbarIn(host)).toBeNull(); + }); + + it("shows nothing at all when no text is being edited", () => { + const { iframe } = scene("hello"); + const { host } = render(null, iframe); + + expect(toolbarIn(host)).toBeNull(); + }); + + it("keeps the press, so clicking a control does not collapse the selection", () => { + const { element, session, iframe } = scene("hello world"); + const { host } = render(session, iframe); + selectAll(element); + + const press = new MouseEvent("mousedown", { bubbles: true, cancelable: true }); + act(() => void toolbarIn(host)!.dispatchEvent(press)); + + expect(press.defaultPrevented).toBe(true); + }); + + // It renders inside the canvas overlay, so a press it lets through is read as + // a click on the composition: the element deselects and the edit commits out + // from under the button that was just pressed. + it("keeps its presses away from the canvas underneath", () => { + const { element, session, iframe } = scene("hello world"); + const seen: string[] = []; + // A stand-in for the canvas overlay: the toolbar renders inside it, and + // these are the handlers that would deselect the element. + const host = document.createElement("div"); + document.body.append(host); + const root = createRoot(host); + act(() => + root.render( +
seen.push("pointerdown")} + onMouseDown={() => seen.push("mousedown")} + onClick={() => seen.push("click")} + > + +
, + ), + ); + selectAll(element); + + const toolbar = toolbarIn(host)!; + act(() => { + for (const type of ["pointerdown", "mousedown", "click"]) { + toolbar.dispatchEvent(new MouseEvent(type, { bubbles: true, cancelable: true })); + } + }); + + expect(seen).toEqual([]); + }); + + // A colour input has a user-agent minimum width, so an invisible one pinned + // only by `inset-0` spills across its neighbours: hovering bold opened the + // colour picker. + it("keeps the invisible colour input inside its own swatch", () => { + const { element, session, iframe } = scene("hello world"); + const { host } = render(session, iframe); + selectAll(element); + + const input = host.querySelector('input[type="color"]')!; + expect(input.className).toContain("w-full"); + expect(input.className).toContain("h-full"); + expect(input.className).toContain("min-w-0"); + }); + + it("styles the selected characters when a control is used", () => { + const { element, session, iframe } = scene("hello world"); + const { host } = render(session, iframe); + selectAll(element); + + act(() => host.querySelector('[aria-label="Bold"]')!.click()); + + expect(element.innerHTML).toBe('hello world'); + }); + + it("reads back the styling it applied, so the control shows the truth", () => { + const { element, session, iframe } = scene('words'); + const { host } = render(session, iframe); + + selectAll(element); + + expect(host.querySelector('[aria-label="Italic"]')?.getAttribute("aria-pressed")).toBe("true"); + expect(host.querySelector('[aria-label="Bold"]')?.getAttribute("aria-pressed")).toBe("false"); + }); + + it("turns a style back off when the control is used again", () => { + const { element, session, iframe } = scene('words'); + const { host } = render(session, iframe); + selectAll(element); + + act(() => host.querySelector('[aria-label="Bold"]')!.click()); + + expect(element.innerHTML).toBe("words"); + }); + + it("places itself over the selection, mapped out of the scaled composition", () => { + const { element, session, iframe } = scene("hello world"); + const { host } = render(session, iframe); + const range = document.createRange(); + range.selectNodeContents(element); + range.getBoundingClientRect = () => + ({ left: 20, top: 40, width: 100, height: 10 }) as unknown as DOMRect; + const selection = document.getSelection()!; + selection.removeAllRanges(); + selection.addRange(range); + act(() => void document.dispatchEvent(new Event("selectionchange"))); + + const toolbar = toolbarIn(host)!; + // Frame at 100,50; scale 400/innerWidth; centre of the range, above it. + const scale = 400 / window.innerWidth; + expect(toolbar.style.left).toBe(`${100 + (20 + 50) * scale}px`); + expect(toolbar.style.top).toBe(`${50 + 40 * scale - 10}px`); + }); + it("shows the selection's colours in the swatch when they differ", () => { + const { element, session, iframe } = scene( + 'Helloworld', + ); + const { host } = render(session, iframe); + + selectAll(element); + + const swatch = toolbarIn(host)!.querySelector("span[aria-hidden]")!; + expect(swatch.style.backgroundImage).toBe("linear-gradient(90deg, red 25.00%, lime 75.00%)"); + // Without this the gradient repeats under the border, painting the end + // colour along the leading edge and the start colour along the trailing one. + expect(swatch.style.backgroundOrigin).toBe("border-box"); + }); + + it("shows a plain swatch when the whole selection is one colour", () => { + const { element, session, iframe } = scene('Hello world'); + const { host } = render(session, iframe); + + selectAll(element); + + const swatch = toolbarIn(host)!.querySelector("span[aria-hidden]")!; + expect(swatch.style.backgroundColor).toBe("red"); + }); +}); + +describe("swatchBackground", () => { + it("blends every colour in the selection, weighted by how much text carries it", () => { + expect( + swatchBackground( + [ + { value: "red", chars: 5 }, + { value: "lime", chars: 15 }, + ], + undefined, + ), + ).toBe("linear-gradient(90deg, red 12.50%, lime 62.50%)"); + }); + + it("stays a plain swatch when the selection is one colour", () => { + expect(swatchBackground([{ value: "red", chars: 5 }], "red")).toBe("red"); + }); + + it("falls back to the agreed colour when the characters carry none", () => { + expect(swatchBackground([], "rgb(1, 2, 3)")).toBe("rgb(1, 2, 3)"); + expect(swatchBackground([], undefined)).toBe("#ffffff"); + }); +}); diff --git a/packages/studio/src/components/editor/InlineTextToolbar.tsx b/packages/studio/src/components/editor/InlineTextToolbar.tsx new file mode 100644 index 0000000000..04f5e9283e --- /dev/null +++ b/packages/studio/src/components/editor/InlineTextToolbar.tsx @@ -0,0 +1,253 @@ +import { useCallback, useEffect, useState } from "react"; +import { applyInlineStyle, readInlineStyle, readInlineStyleSpread } from "./inlineTextStyleRange"; +import type { InlineTextEditSession } from "../../hooks/useInlineTextEdit"; + +/** + * The controls for styling the characters selected inside an open text edit. + * + * It lives in Studio's document rather than the composition's, positioned over + * the selection: putting it in the preview would mean injecting Studio's chrome + * into the user's composition, where it would be captured by a render and + * inherit the composition's own styling. + * + * `position: fixed` and viewport coordinates, so it does not have to know which + * of the canvas' several nested coordinate systems it was mounted into. + */ + +const READ_PROPERTIES = ["color", "font-weight", "font-style", "text-decoration-line"]; + +/** Enough above the text to clear it, without leaving the element behind. */ +const GAP_PX = 10; +const DEFAULT_COLOR = "#ffffff"; + +interface ToolbarPlacement { + left: number; + top: number; + styles: Record; + colours: Array<{ value: string; chars: number }>; +} + +export function InlineTextToolbar({ + session, + iframe, +}: { + session: InlineTextEditSession | null; + iframe: HTMLIFrameElement | null; +}) { + const [placement, setPlacement] = useState(null); + + const refresh = useCallback(() => { + setPlacement(session && iframe ? placeOverSelection(session.element, iframe) : null); + }, [session, iframe]); + + // The selection lives in the preview's document, so the event does too. + useEffect(() => { + const doc = session?.element.ownerDocument; + if (!doc) { + setPlacement(null); + return; + } + doc.addEventListener("selectionchange", refresh); + return () => doc.removeEventListener("selectionchange", refresh); + }, [session, refresh]); + + const apply = useCallback( + (delta: Record) => { + const doc = session?.element.ownerDocument; + const range = doc?.defaultView?.getSelection()?.getRangeAt(0); + if (!range) return; + applyInlineStyle(range, delta); + refresh(); + }, + [session, refresh], + ); + + if (!placement) return null; + const styles = placement.styles; + + return ( +
event.stopPropagation()} + > + + apply({ "font-weight": on ? "700" : null })} + /> + apply({ "font-style": on ? "italic" : null })} + /> + apply({ "text-decoration-line": on ? "underline" : null })} + /> +
+ ); +} + +/** + * The selection's colours blended left to right, each sitting at the middle of + * the share of characters that carry it. A selection with one colour is a plain + * swatch, as before. + */ +export function swatchBackground( + colours: Array<{ value: string; chars: number }>, + agreed: string | undefined, +): string { + if (colours.length === 0) return agreed || DEFAULT_COLOR; + if (colours.length === 1) return colours[0]!.value; + const total = colours.reduce((sum, colour) => sum + colour.chars, 0); + let offset = 0; + const stops = colours.map((colour) => { + const middle = ((offset + colour.chars / 2) / total) * 100; + offset += colour.chars; + return `${colour.value} ${middle.toFixed(2)}%`; + }); + return `linear-gradient(90deg, ${stops.join(", ")})`; +} + +function swallow(event: { preventDefault: () => void; stopPropagation: () => void }): void { + event.preventDefault(); + event.stopPropagation(); +} + +function ToolbarToggle({ + label, + glyph, + on, + onToggle, + bold, + italic, + underline, +}: { + label: string; + glyph: string; + on: boolean; + onToggle: (on: boolean) => void; + bold?: boolean; + italic?: boolean; + underline?: boolean; +}) { + return ( + + ); +} + +/** Where the selection is on screen, or null when there is nothing selected. */ +function placeOverSelection( + element: HTMLElement, + iframe: HTMLIFrameElement, +): ToolbarPlacement | null { + const doc = element.ownerDocument; + const view = doc.defaultView; + const selection = view?.getSelection(); + if (!view || !selection || selection.rangeCount === 0 || selection.isCollapsed) return null; + + const range = selection.getRangeAt(0); + if (!element.contains(range.commonAncestorContainer)) return null; + const rect = range.getBoundingClientRect(); + + // The composition is drawn scaled into the iframe's box, so a point inside it + // is that scale away from a point on Studio's screen. This is the inverse of + // the mapping the canvas uses to turn a press into a caret position. + const box = iframe.getBoundingClientRect(); + const scale = view.innerWidth ? box.width / view.innerWidth : 1; + + return { + left: box.left + (rect.left + rect.width / 2) * scale, + top: box.top + rect.top * scale - GAP_PX, + styles: readInlineStyle(range, READ_PROPERTIES), + colours: readInlineStyleSpread(range, "color"), + }; +} + +function isBold(weight: string | undefined): boolean { + if (!weight) return false; + if (weight === "bold" || weight === "bolder") return true; + return Number.parseInt(weight, 10) >= 600; +} + +/** + * A colour input only accepts `#rrggbb`, and what the page reports is whatever + * the stylesheet said. An unreadable value opens the picker on white rather + * than refusing to open. + */ +function toHexColor(value: string | undefined): string { + if (!value) return DEFAULT_COLOR; + if (/^#[0-9a-f]{6}$/i.test(value)) return value; + const channels = value.match(/\d+(\.\d+)?/g); + if (!channels || channels.length < 3) return DEFAULT_COLOR; + return `#${channels + .slice(0, 3) + .map((channel) => Number(channel).toString(16).padStart(2, "0")) + .join("")}`; +} diff --git a/packages/studio/src/components/editor/OffCanvasIndicators.tsx b/packages/studio/src/components/editor/OffCanvasIndicators.tsx index ead1a53a4f..fe0edcf8a5 100644 --- a/packages/studio/src/components/editor/OffCanvasIndicators.tsx +++ b/packages/studio/src/components/editor/OffCanvasIndicators.tsx @@ -143,3 +143,28 @@ export function OffCanvasIndicators({ ); } + +/** + * The dashed outlines around a selected element's children. + * + * Extracted from the canvas overlay, which is at its 600-line limit, and it + * sits here rather than in its own file because it is the same kind of thing: + * a passive, non-interactive mark the overlay draws over the composition. + */ +export function ChildRectOutlines({ + rects, +}: { + rects: ReadonlyArray<{ left: number; top: number; width: number; height: number }>; +}) { + return ( + <> + {rects.map((rect, index) => ( +
+ ))} + + ); +} diff --git a/packages/studio/src/components/editor/domEditInlineText.test.ts b/packages/studio/src/components/editor/domEditInlineText.test.ts new file mode 100644 index 0000000000..a229e8cd49 --- /dev/null +++ b/packages/studio/src/components/editor/domEditInlineText.test.ts @@ -0,0 +1,73 @@ +import { describe, expect, it, vi } from "vitest"; +import type { DomEditSelection, DomEditTextField } from "./domEditingTypes"; + +const editable = vi.hoisted(() => ({ current: true })); +vi.mock("./domEditingLayers", () => ({ + isTextEditableSelection: () => editable.current, +})); + +const { canEditTextInline } = await import("./domEditInlineText"); + +function field(key: string): DomEditTextField { + return { + key, + label: key, + value: "text", + tagName: "SPAN", + attributes: [], + inlineStyles: {}, + computedStyles: {}, + source: "self", + }; +} + +function selection(partial: Partial = {}): DomEditSelection { + return { + label: "Heading", + tagName: "H1", + isCompositionHost: false, + isInsideLockedComposition: false, + textFields: [field("self")], + ...partial, + } as DomEditSelection; +} + +describe("canEditTextInline", () => { + it("allows an element the panel would let you edit text on", () => { + editable.current = true; + expect(canEditTextInline(selection())).toBe(true); + }); + + // The bar is the panel's bar: nothing becomes editable here that is not + // editable there. + it("refuses an element whose text the panel cannot edit either", () => { + editable.current = false; + expect(canEditTextInline(selection())).toBe(false); + }); + + // Editing the whole element would flatten its children into one string. + it("refuses an element with several text fields", () => { + editable.current = true; + expect(canEditTextInline(selection({ textFields: [field("a"), field("b")] }))).toBe(false); + }); + + it("allows an element with no separate text fields", () => { + editable.current = true; + expect(canEditTextInline(selection({ textFields: [] }))).toBe(true); + }); + + it("refuses the composition host, which is the document rather than copy", () => { + editable.current = true; + expect(canEditTextInline(selection({ isCompositionHost: true }))).toBe(false); + }); + + it("refuses anything inside a locked composition", () => { + editable.current = true; + expect(canEditTextInline(selection({ isInsideLockedComposition: true }))).toBe(false); + }); + + it("refuses nothing at all", () => { + editable.current = true; + expect(canEditTextInline(null)).toBe(false); + }); +}); diff --git a/packages/studio/src/components/editor/domEditInlineText.ts b/packages/studio/src/components/editor/domEditInlineText.ts new file mode 100644 index 0000000000..451c5b1832 --- /dev/null +++ b/packages/studio/src/components/editor/domEditInlineText.ts @@ -0,0 +1,93 @@ +import { isRichTextFormattingTag } from "@hyperframes/core/rich-text-sanitize"; +import type { DomEditSelection } from "./domEditingTypes"; +import { isTextEditableSelection } from "./domEditingLayers"; + +/** + * Whether this element's text can be edited where it sits. + * + * Its own function rather than a condition inside a handler, because this is + * the rule most likely to change: it is the whole answer to "why did nothing + * happen when I double-clicked that". + * + * The bar is deliberately the same as the design panel's, plus one thing the + * panel can do that editing in place cannot. An element with several text + * fields is edited a field at a time there, and making the whole element + * editable would flatten its children into one string, so those keep the panel. + */ +export function canEditTextInline(selection: DomEditSelection | null): boolean { + if (!selection) return false; + if (!isTextEditableSelection(selection)) return false; + // The composition host is the document, not a piece of copy in it. + if (selection.isCompositionHost) return false; + if (selection.isInsideLockedComposition) return false; + if (selection.textFields.length <= 1) return true; + // A styled element reports one field per run of characters, but it is still + // one piece of copy and the caret edits all of it at once. + return canEditElementTextInline(selection.element); +} + +/** + * Whether this element's text can be edited in place, judged from the element + * alone. + * + * The press path cannot use the selection-shaped gate above: building a + * selection is asynchronous, and a press has to decide now whether it is a + * text edit or the start of a drag. This asks the same question of the DOM. + * + * A structural child keeps an element out: those are separate text fields, the + * panel edits them one at a time, and making the whole element editable would + * flatten them into a single string. + * + * A formatting child does not. Styling a run of characters puts a span inside + * the element, so a rule of "no element children" would have let the editor + * lock every element it had ever styled out of itself, permanently, on the + * first colour change. What counts as formatting is the sanitiser's allowlist, + * so the editor and the thing that writes the file agree on it. + */ +export function canEditElementTextInline(element: HTMLElement | null): boolean { + if (!element) return false; + const tag = element.tagName; + if (tag === "BODY" || tag === "HTML") return false; + if (!hasOnlyFormattingChildren(element)) return false; + if (element.isContentEditable) return false; + return (element.textContent ?? "").trim().length > 0; +} + +function hasOnlyFormattingChildren(element: HTMLElement): boolean { + for (const child of Array.from(element.children)) { + if (!isRichTextFormattingTag(child.tagName)) return false; + // Formatting nests, and a structural child hidden inside a span is still + // structural. + if (!hasOnlyFormattingChildren(child as HTMLElement)) return false; + } + return true; +} + +/** Where and when a press landed, for recognising the next one as a pair. */ +export interface PressMark { + x: number; + y: number; + at: number; +} + +/** Long enough to be deliberate, short enough not to catch two separate clicks. */ +const DOUBLE_PRESS_MS = 450; +/** A double press is two presses in the same place, not a tiny drag. */ +const DOUBLE_PRESS_SLOP_PX = 6; + +/** + * Whether this press pairs with the last one into a double press. + * + * Studio cannot use `dblclick` or a click count for this. The selection box + * takes pointer capture on the first press and prevents its default, which + * suppresses the compatibility mouse events and stops the browser pairing the + * two presses at all: no `dblclick` is dispatched, and `detail` stays 1. + */ +export function isDoublePress(previous: PressMark | null, next: PressMark): boolean { + if (!previous) return false; + return ( + next.at - previous.at <= DOUBLE_PRESS_MS && + Math.abs(next.x - previous.x) <= DOUBLE_PRESS_SLOP_PX && + Math.abs(next.y - previous.y) <= DOUBLE_PRESS_SLOP_PX + ); +} diff --git a/packages/studio/src/components/editor/domEditInlineTextElement.test.ts b/packages/studio/src/components/editor/domEditInlineTextElement.test.ts new file mode 100644 index 0000000000..c43ab7c807 --- /dev/null +++ b/packages/studio/src/components/editor/domEditInlineTextElement.test.ts @@ -0,0 +1,59 @@ +// @vitest-environment happy-dom + +import { afterEach, describe, expect, it } from "vitest"; +import { canEditElementTextInline } from "./domEditInlineText"; + +afterEach(() => { + document.body.innerHTML = ""; +}); + +function mount(html: string): HTMLElement { + document.body.innerHTML = html; + return document.body.firstElementChild as HTMLElement; +} + +describe("canEditElementTextInline", () => { + it("opens a plain piece of copy", () => { + expect(canEditElementTextInline(mount("

hello world

"))).toBe(true); + }); + + // The trap this exists for: styling a run of characters puts a span inside + // the element, and a rule of "no element children" would have let the editor + // lock every element it had ever styled out of itself, permanently. + it("still opens an element that has been styled", () => { + const element = mount('

hello world

'); + expect(canEditElementTextInline(element)).toBe(true); + }); + + it("opens an element whose formatting is nested", () => { + const element = mount('

deep

'); + expect(canEditElementTextInline(element)).toBe(true); + }); + + it("keeps out an element with a structural child, which the panel edits field by field", () => { + expect(canEditElementTextInline(mount("

a

b

"))).toBe(false); + }); + + it("keeps out an element hiding something structural inside its formatting", () => { + expect(canEditElementTextInline(mount("

a

"))).toBe(false); + }); + + it("keeps out the document itself", () => { + expect(canEditElementTextInline(document.body)).toBe(false); + expect(canEditElementTextInline(document.documentElement)).toBe(false); + }); + + it("keeps out an element that is already being edited", () => { + const element = mount("

hello

"); + element.setAttribute("contenteditable", "true"); + expect(canEditElementTextInline(element)).toBe(false); + }); + + it("keeps out an element with no words in it", () => { + expect(canEditElementTextInline(mount("

"))).toBe(false); + }); + + it("keeps out nothing at all", () => { + expect(canEditElementTextInline(null)).toBe(false); + }); +}); diff --git a/packages/studio/src/components/editor/domEditOverlayGeometry.test.ts b/packages/studio/src/components/editor/domEditOverlayGeometry.test.ts index 217f3d3430..5f9475c5e0 100644 --- a/packages/studio/src/components/editor/domEditOverlayGeometry.test.ts +++ b/packages/studio/src/components/editor/domEditOverlayGeometry.test.ts @@ -67,6 +67,17 @@ describe("orientedOverlayRect β€” rotation gate (perf fix, V15 18a/18b)", () => number, ]; } + /** `this` applied outside `other`, the way an ancestor composes over a child. */ + multiply(other: { a: number; b: number; c: number; d: number; e: number; f: number }) { + const out = new (this.constructor as new (init?: string) => this)(); + out.a = this.a * other.a + this.c * other.b; + out.b = this.b * other.a + this.d * other.b; + out.c = this.a * other.c + this.c * other.d; + out.d = this.b * other.c + this.d * other.d; + out.e = this.a * other.e + this.c * other.f + this.e; + out.f = this.b * other.e + this.d * other.f + this.f; + return out; + } transformPoint(pt: { x: number; y: number }) { return { x: this.a * pt.x + this.c * pt.y + this.e, @@ -156,6 +167,45 @@ describe("orientedOverlayRect β€” rotation gate (perf fix, V15 18a/18b)", () => expect(rect!.angle).toBeCloseTo(30, 3); }); + /** + * The selection box is drawn at the size the element PAINTS, which is the + * product of every transform between it and the composition root. + * + * A text layer inside a card carrying `scale(1.2)` was drawn at 1/1.2 of the + * text: the top-left was right, because the caller anchors that to the real + * bounding rect, and the right and bottom edges fell short. The same read + * decides whether to draw the box rotated, so an element inside a rotated + * parent got an upright box. + */ + const SCALE_1_2_MATRIX = "matrix(1.2, 0, 0, 1.2, 0, 0)"; + + it("sizes the box by the accumulated transform, not the element's own", () => { + const { overlayEl, iframe, el } = buildHarness(); + // The element carries no transform; its parent scales it by 1.2, so it + // paints at 240x120 and its bounding rect says so. + el.parentElement!.style.transform = SCALE_1_2_MATRIX; + el.style.transform = ROTATE_30DEG_MATRIX; + stubRect(el, { left: 400, top: 450, width: 240, height: 120 }); + + const rect = orientedOverlayRect(overlayEl, iframe, el); + + expect(rect).not.toBeNull(); + // 200x100 local, scaled by the ancestor, then rotated: the oriented box is + // the scaled local box, and the AABB it is anchored to is wider again. + expect(rect!.width).toBeCloseTo(240, 3); + expect(rect!.height).toBeCloseTo(120, 3); + expect(rect!.angle).toBeCloseTo(30, 3); + }); + + it("takes the rotated path when only an ANCESTOR is rotated", () => { + const { overlayEl, iframe, el } = buildHarness(); + el.parentElement!.style.transform = ROTATE_30DEG_MATRIX; + + const rect = orientedOverlayRect(overlayEl, iframe, el); + + expect(rect!.angle).toBeCloseTo(30, 3); + }); + it("preserves an ordinary element's rotation through the group-aware entry point", () => { const { overlayEl, iframe, el } = buildHarness(); el.style.transform = ROTATE_30DEG_MATRIX; diff --git a/packages/studio/src/components/editor/domEditOverlayGeometry.ts b/packages/studio/src/components/editor/domEditOverlayGeometry.ts index 70f2500958..76b312ce97 100644 --- a/packages/studio/src/components/editor/domEditOverlayGeometry.ts +++ b/packages/studio/src/components/editor/domEditOverlayGeometry.ts @@ -117,6 +117,28 @@ interface ElementTransformSnapshot { cs: CSSStyleDeclaration; } +/** + * The transform from the element's own box to the composition's, ACCUMULATED + * over its ancestors rather than read from the element alone. + * + * What the user sees is the product of every transform between the element and + * the composition root, and an element is routinely a child of something + * scaled or rotated. Reading only its own transform drew the selection box at + * the element's untransformed size: a text layer inside a card carrying + * `scale(1.2)` got a box at 1/1.2 of the text, with the top-left correct (the + * caller anchors that to the real bounding rect) and the right and bottom + * edges falling short. The same read decides whether to draw the box rotated, + * so an element inside a rotated parent got an upright box too. + * + * Only the linear part matters here. Each transform's origin contributes + * translation, and the caller discards translation by matching the corners' + * bounding box to the element's real one, so composing the matrices alone is + * enough and there is no per-ancestor origin to unpick. + * + * The walk stops at the composition document's root. The canvas zoom lives on + * the iframe element in Studio's own document and is applied separately by + * `computeOverlayRootScale`; including it here would count it twice. + */ function readElementTransformSnapshot( win: Window, element: HTMLElement, @@ -125,7 +147,13 @@ function readElementTransformSnapshot( if (!DOMMatrixCtor) return null; const cs = win.getComputedStyle(element); try { - const matrix = new DOMMatrixCtor(cs.transform === "none" ? "" : cs.transform); + let matrix = new DOMMatrixCtor(); + for (let node: HTMLElement | null = element; node; node = node.parentElement) { + const transform = node === element ? cs.transform : win.getComputedStyle(node).transform; + if (!transform || transform === "none") continue; + // An ancestor applies outside, so it multiplies on the left. + matrix = new DOMMatrixCtor(transform).multiply(matrix); + } return { matrix, cs }; } catch { return null; diff --git a/packages/studio/src/components/editor/domEditOverlayGestures.ts b/packages/studio/src/components/editor/domEditOverlayGestures.ts index d7fec86a87..34d6519c19 100644 --- a/packages/studio/src/components/editor/domEditOverlayGestures.ts +++ b/packages/studio/src/components/editor/domEditOverlayGestures.ts @@ -10,6 +10,7 @@ import type { GroupOverlayItem, OverlayRect } from "./domEditOverlayGeometry"; import type { SnapContext } from "./snapTargetCollection"; import type { SnapGuidesState } from "./SnapGuideOverlay"; import type { PreviewMouseDownOptions } from "../../hooks/usePreviewInteraction"; +import { logSelect } from "../../utils/selectDebug"; export type GestureKind = "drag" | "resize" | "rotate"; @@ -112,6 +113,47 @@ export function focusDomEditOverlayElement(element: FocusableDomEditOverlay | nu element?.focus({ preventScroll: true }); } +/** + * Whether the hover cache may stand in for a hit-test at this point. + * + * The cache is filled asynchronously as the pointer moves, so it can describe an + * element the pointer has already left. That is harmless for drawing a hover + * outline and wrong for a shift-click, which would add the stale element to the + * selection instead of the one under the pointer. True only when the cached + * element IS the element at the point, or contains it β€” the resolver is allowed + * to hand back a clip ancestor of the raw target, and that still describes the + * same click. + */ +export function hoverCacheDescribesPoint( + cachedElement: Element | null | undefined, + elementAtPoint: Element | null | undefined, +): boolean { + if (!cachedElement || !elementAtPoint) return false; + return cachedElement === elementAtPoint || cachedElement.contains(elementAtPoint); +} + +/** + * The element a shift-click should add, or null to let the slower path resolve it. + * + * Reading the hover cache without checking is safe for a hover outline and wrong + * for a shift-click: the click silently adds whatever the pointer last passed + * over instead of the element under it, which reads as multi-select picking + * things at random. Returning null means "not confident", and the caller must + * then fall through untouched so the mousedown path resolves the point properly. + */ +export function resolveShiftClickCandidate(input: { + cached: T | null; + elementAtPoint: Element | null; +}): T | null { + const describes = hoverCacheDescribesPoint(input.cached?.element, input.elementAtPoint); + logSelect("shift-pointerdown", { + candidate: input.cached ? ((input.cached as { selector?: string }).selector ?? null) : null, + pointTarget: input.elementAtPoint?.id ?? input.elementAtPoint?.tagName ?? null, + cacheIsAboutThisPoint: describes, + }); + return describes ? input.cached : null; +} + /** * Overlay-px translation that keeps the element's CENTER fixed while a corner * resizes: a CSS width/height change grows the layout box from its top-left, so diff --git a/packages/studio/src/components/editor/domEditOverlayStartGesture.ts b/packages/studio/src/components/editor/domEditOverlayStartGesture.ts index 6576a9b5ba..390e05e66e 100644 --- a/packages/studio/src/components/editor/domEditOverlayStartGesture.ts +++ b/packages/studio/src/components/editor/domEditOverlayStartGesture.ts @@ -33,6 +33,7 @@ import { } from "./domEditOverlayGestures"; import { collectSnapContext, buildExcludeElements } from "./snapTargetCollection"; import { logResize, resetResizeMoveLog } from "../../utils/resizeDebug"; +import { logDrag, readDragPositions, resetDragMoveLog } from "../../utils/dragDebug"; export function startGroupDrag( e: React.PointerEvent, @@ -70,6 +71,22 @@ export function startGroupDrag( } members.push(result.member); } + resetDragMoveLog(); + logDrag("group-start", { + // A member whose mapping differs from its neighbours travels a different + // distance for the same pointer delta, which is the group coming apart. + members: Object.fromEntries( + members.map((member) => [ + member.key, + { + map: `${member.screenToOffset.a.toFixed(3)},${member.screenToOffset.d.toFixed(3)}`, + base: `${Math.round(member.baseGsap.x)},${Math.round(member.baseGsap.y)}`, + offset: `${Math.round(member.initialOffset.x)},${Math.round(member.initialOffset.y)}`, + }, + ]), + ), + at: readDragPositions(members), + }); const overlayEl = opts.overlayRef.current; const iframe = opts.iframeRef.current; diff --git a/packages/studio/src/components/editor/domEditing.ts b/packages/studio/src/components/editor/domEditing.ts index 1dd6a1ffa7..7934c87078 100644 --- a/packages/studio/src/components/editor/domEditing.ts +++ b/packages/studio/src/components/editor/domEditing.ts @@ -27,6 +27,7 @@ export { export { buildDefaultDomEditTextField, buildDomEditPatchTarget, + buildDomEditRichTextPatchOperation, buildDomEditStylePatchOperation, buildDomEditTextPatchOperation, collectDomEditLayerItems, diff --git a/packages/studio/src/components/editor/domEditingLayers.ts b/packages/studio/src/components/editor/domEditingLayers.ts index 3102d030f8..178d72f50a 100644 --- a/packages/studio/src/components/editor/domEditingLayers.ts +++ b/packages/studio/src/components/editor/domEditingLayers.ts @@ -519,12 +519,13 @@ export function buildDomEditTextPatchOperation( value: string, childLocator?: DomEditChildLocator, ): PatchOperation { - return { - type: "text-content", - property: "text", - value, - ...childLocator, - }; + return { type: "text-content", property: "text", value, ...childLocator }; +} + +/** Replace an element's contents with markup, for a change no per-child operation + * can express (a text layer added, removed or reordered). Sanitized at both ends. */ +export function buildDomEditRichTextPatchOperation(value: string): PatchOperation { + return { type: "rich-text", property: "", value }; } // ─── Non-editable reason ───────────────────────────────────────────────────── diff --git a/packages/studio/src/components/editor/groupDragMove.ts b/packages/studio/src/components/editor/groupDragMove.ts new file mode 100644 index 0000000000..27281d2a13 --- /dev/null +++ b/packages/studio/src/components/editor/groupDragMove.ts @@ -0,0 +1,102 @@ +import { resolveDomEditGroupOverlayRect } from "./domEditOverlayGeometry"; +import { + resolveEquidistanceGuides, + resolveSnapAdjustment, + snapEngagedForTravel, + SNAP_THRESHOLD_PX, +} from "./snapEngine"; +import { applyManualOffsetDragDraft } from "./manualOffsetDrag"; +import type { GroupGestureState, UseDomEditOverlayGesturesOptions } from "./domEditOverlayGestures"; +import type { GroupOverlayItem } from "./domEditOverlayGeometry"; +import { + findNonRigidMembers, + logDrag, + logDragMove, + readDragPositions, +} from "../../utils/dragDebug"; + +/** + * One frame of a group drag, kept out of onPointerMove β€” which already handles + * four gesture kinds and reads better without this one's snapping arithmetic. + * The previous frame's positions live in the closure so the rigidity check below + * compares against the frame before, not against whatever was last sampled. + */ +export function createGroupDragMover( + opts: UseDomEditOverlayGesturesOptions, + setDraftGroupOverlayItems: (items: GroupOverlayItem[]) => void, +) { + let lastGroupPositions: Record = {}; + + /** Snap the group's delta to nearby edges, publishing the guides drawn for it. */ + // fallow-ignore-next-line complexity + const snapGroupDelta = ( + groupG: GroupGestureState, + e: React.PointerEvent, + proposed: { dx: number; dy: number }, + ) => { + const sc = groupG.snapContext; + if (!sc?.snapEnabled || sc.targets.length === 0) return proposed; + if (!snapEngagedForTravel(proposed.dx, proposed.dy)) return proposed; + const groupBounds = resolveDomEditGroupOverlayRect(groupG.originItems.map((i) => i.rect)); + if (!groupBounds) return proposed; + const allTargets = sc.compositionTarget ? [...sc.targets, sc.compositionTarget] : sc.targets; + const snap = resolveSnapAdjustment({ + movingRect: groupBounds, + proposedDx: proposed.dx, + proposedDy: proposed.dy, + targets: allTargets, + gridEdges: sc.gridEdges ?? undefined, + threshold: SNAP_THRESHOLD_PX, + disabled: e.altKey, + }); + const movingRect = { + ...groupBounds, + left: groupBounds.left + snap.dx, + top: groupBounds.top + snap.dy, + }; + const spacingGuides = e.altKey + ? [] + : resolveEquidistanceGuides({ + movingRect, + targets: allTargets, + threshold: SNAP_THRESHOLD_PX, + }); + opts.snapGuidesRef.current = { guides: snap.guides, spacingGuides }; + return { dx: snap.dx, dy: snap.dy }; + }; + + /** One frame of a group drag: snap the delta, redraw the boxes, move every member. */ + const moveGroupDrag = (groupG: GroupGestureState, e: React.PointerEvent) => { + const { dx, dy } = snapGroupDelta(groupG, e, { + dx: e.clientX - groupG.startX, + dy: e.clientY - groupG.startY, + }); + groupG.lastSnappedDx = dx; + groupG.lastSnappedDy = dy; + + setDraftGroupOverlayItems( + groupG.originItems.map((i) => ({ + ...i, + rect: { ...i.rect, left: i.rect.left + dx, top: i.rect.top + dy }, + })), + ); + const offsets: Record = {}; + for (const m of groupG.members) { + const n = applyManualOffsetDragDraft(m, dx, dy); + offsets[m.key] = `${Math.round(n.x)},${Math.round(n.y)}`; + } + const at = readDragPositions(groupG.members); + const px = Math.round(e.clientX - groupG.startX); + const py = Math.round(e.clientY - groupG.startY); + // A member breaking away IS the fault, so it reports on the frame it happens; + // the throttled line below would step over it. A gap between pointer and + // applied there is snapping pulling the group off the cursor. + const trace = { pointer: `${px},${py}`, applied: `${Math.round(dx)},${Math.round(dy)}`, at }; + const drift = findNonRigidMembers(lastGroupPositions, at); + if (drift.length > 0) logDrag("drift", { ...trace, drift }); + lastGroupPositions = at; + logDragMove({ ...trace, offsets }); + }; + + return moveGroupDrag; +} diff --git a/packages/studio/src/components/editor/groupDropKeepsSelection.test.ts b/packages/studio/src/components/editor/groupDropKeepsSelection.test.ts new file mode 100644 index 0000000000..3fc95b6742 --- /dev/null +++ b/packages/studio/src/components/editor/groupDropKeepsSelection.test.ts @@ -0,0 +1,70 @@ +// @vitest-environment happy-dom +import { describe, expect, it, vi } from "vitest"; +import { createDomEditOverlayGestureHandlers } from "./useDomEditOverlayGestures"; +import type { GroupGestureState } from "./domEditOverlayGestures"; + +/** + * A group drag ended by deselecting the group it had just moved. + * + * Every pointerup trails a click. The gesture ref is cleared before the commit, + * so by the time that click arrives the box no longer looks busy and it reaches + * the canvas as an ordinary click β€” landing in the gap between the members, + * resolving to nothing, and clearing the selection. Captured live as a + * `[hf-select] clear` with `hadGroup: 3` two milliseconds after the drop. + * + * The under-threshold path already ate that click; the committed path has to as + * well, and the flag is set before the two diverge so neither can forget. + */ +describe("dropping a dragged group eats the click that follows", () => { + function harness(travel: { dx: number; dy: number }) { + const suppressNextBoxClickRef = { current: false }; + const groupGestureRef = { + current: { + startX: 0, + startY: 0, + originItems: [], + members: [], + } as unknown as GroupGestureState, + }; + const handlers = createDomEditOverlayGestureHandlers({ + overlayRef: { current: null }, + iframeRef: { current: null }, + boxRef: { current: null }, + selectionRef: { current: null }, + hoverSelectionRef: { current: null }, + overlayRectRef: { current: null }, + groupOverlayItemsRef: { current: [] }, + gestureRef: { current: null }, + groupGestureRef, + blockedMoveRef: { current: null }, + rafPausedRef: { current: false }, + suppressNextBoxClickRef, + setOverlayRect: vi.fn(), + setGroupOverlayItems: vi.fn(), + onBlockedMoveRef: { current: vi.fn() }, + onManualDragStartRef: { current: vi.fn() }, + onPathOffsetCommitRef: { current: vi.fn() }, + onGroupPathOffsetCommitRef: { current: vi.fn() }, + onBoxSizeCommitRef: { current: vi.fn() }, + onRotationCommitRef: { current: vi.fn() }, + onCanvasPointerMoveRef: { current: vi.fn() }, + onCanvasMouseDown: vi.fn(), + snapGuidesRef: { current: null }, + } as never); + + handlers.onPointerUp({ + clientX: travel.dx, + clientY: travel.dy, + currentTarget: { releasePointerCapture: vi.fn() }, + } as never); + return suppressNextBoxClickRef; + } + + it("eats the click after a drag that moved", () => { + expect(harness({ dx: 120, dy: 60 }).current).toBe(true); + }); + + it("still eats it after a press that never travelled", () => { + expect(harness({ dx: 1, dy: 0 }).current).toBe(true); + }); +}); diff --git a/packages/studio/src/components/editor/inlineTextStyleRange.test.ts b/packages/studio/src/components/editor/inlineTextStyleRange.test.ts new file mode 100644 index 0000000000..568689efa8 --- /dev/null +++ b/packages/studio/src/components/editor/inlineTextStyleRange.test.ts @@ -0,0 +1,610 @@ +// @vitest-environment happy-dom + +import { afterEach, describe, expect, it, vi } from "vitest"; +import { applyInlineStyle, readInlineStyle, readInlineStyleSpread } from "./inlineTextStyleRange"; + +afterEach(() => { + vi.restoreAllMocks(); + document.body.innerHTML = ""; +}); + +function mount(html: string): HTMLElement { + document.body.innerHTML = `

${html}

`; + return document.body.firstElementChild as HTMLElement; +} + +/** A range over the host's text, by character offsets across the whole element. */ +/** Every text node and line break in order, with the offset each one starts at. */ +function charSpans(host: HTMLElement): Array<{ node: Node; from: number; length: number }> { + const spans: Array<{ node: Node; from: number; length: number }> = []; + const walker = document.createTreeWalker(host, NodeFilter.SHOW_TEXT | NodeFilter.SHOW_ELEMENT); + let seen = 0; + for (let node = walker.nextNode(); node; node = walker.nextNode()) { + const isBreak = node.nodeType === 1 && (node as Element).tagName === "BR"; + if (node.nodeType === 1 && !isBreak) continue; + const length = isBreak ? 1 : (node.textContent?.length ?? 0); + spans.push({ node, from: seen, length }); + seen += length; + } + return spans; +} + +/** + * A range over the host's text, by character offsets across the whole element. + * + * A line break counts as one character, the same way the module does, but is + * never landed on: a boundary there belongs to the text beside it, which is + * where a real selection would put it too. + */ +function rangeOver(host: HTMLElement, start: number, end: number): Range { + const range = document.createRange(); + const text = charSpans(host).filter((span) => span.node.nodeType === 3); + const at = (offset: number) => text.find((span) => span.from + span.length >= offset); + const from = at(start); + const to = at(end); + if (from) range.setStart(from.node, start - from.from); + if (from && to) range.setEnd(to.node, end - to.from); + return range; +} + +/** Elements left holding half a character by a boundary that fell inside one. */ +function elementsWithHalfACharacter(host: HTMLElement): string[] { + return Array.from(host.querySelectorAll("*")) + .map((node) => node.textContent ?? "") + .filter((text) => { + for (let index = 0; index < text.length; index += 1) { + const code = text.charCodeAt(index); + if (code >= 0xdc00 && code <= 0xdfff) return true; + if (code >= 0xd800 && code <= 0xdbff) { + const next = text.charCodeAt(index + 1); + if (!(next >= 0xdc00 && next <= 0xdfff)) return true; + index += 1; + } + } + return false; + }); +} + +describe("applyInlineStyle", () => { + it("styles exactly the characters selected, and nothing else", () => { + const host = mount("hello world"); + + applyInlineStyle(rangeOver(host, 6, 11), { color: "red" }); + + expect(host.innerHTML).toBe('hello world'); + }); + + it("styles a run in the middle, leaving the text either side alone", () => { + const host = mount("abcdef"); + + applyInlineStyle(rangeOver(host, 2, 4), { color: "red" }); + + expect(host.innerHTML).toBe('abcdef'); + expect(host.textContent).toBe("abcdef"); + }); + + it("does nothing at all when nothing is selected", () => { + const host = mount("abc"); + const range = rangeOver(host, 1, 1); + + applyInlineStyle(range, { color: "red" }); + + expect(host.innerHTML).toBe("abc"); + }); + + // Left alone, every recolour would wrap the last one and the markup would + // grow without bound while only the innermost span had any effect. + it("replaces a colour rather than nesting a second span inside the first", () => { + const host = mount("abc"); + + applyInlineStyle(rangeOver(host, 0, 3), { color: "red" }); + applyInlineStyle(rangeOver(host, 0, 3), { color: "blue" }); + + expect(host.innerHTML).toBe('abc'); + }); + + it("merges with the run beside it when the styling matches", () => { + const host = mount("abcd"); + + applyInlineStyle(rangeOver(host, 0, 2), { color: "red" }); + applyInlineStyle(rangeOver(host, 2, 4), { color: "red" }); + + expect(host.innerHTML).toBe('abcd'); + }); + + it("does not merge runs that only look alike", () => { + const host = mount("abcd"); + + applyInlineStyle(rangeOver(host, 0, 2), { color: "red" }); + applyInlineStyle(rangeOver(host, 2, 4), { color: "blue" }); + + expect(host.innerHTML).toBe( + 'abcd', + ); + }); + + it("leaves no empty span behind when the last style is taken off", () => { + const host = mount("abc"); + + applyInlineStyle(rangeOver(host, 0, 3), { color: "red" }); + applyInlineStyle(rangeOver(host, 0, 3), { color: null }); + + expect(host.innerHTML).toBe("abc"); + }); + + it("keeps a property the new styling does not mention", () => { + const host = mount("abc"); + + applyInlineStyle(rangeOver(host, 0, 3), { color: "red" }); + applyInlineStyle(rangeOver(host, 0, 3), { "font-weight": "700" }); + + expect(host.innerHTML).toContain("color: red"); + expect(host.innerHTML).toContain("font-weight: 700"); + expect(host.querySelectorAll("span")).toHaveLength(1); + }); + + it("styles across the boundary of a run that is already styled", () => { + const host = mount("abcdef"); + applyInlineStyle(rangeOver(host, 0, 3), { color: "red" }); + + applyInlineStyle(rangeOver(host, 1, 5), { "font-weight": "700" }); + + expect(host.textContent).toBe("abcdef"); + expect(host.innerHTML).toContain("font-weight: 700"); + }); + + it("leaves the selection over the characters it just styled", () => { + const host = mount("hello world"); + + applyInlineStyle(rangeOver(host, 0, 5), { color: "red" }); + + expect(document.getSelection()?.toString()).toBe("hello"); + }); + + it("styles more than one property at once", () => { + const host = mount("abc"); + + applyInlineStyle(rangeOver(host, 0, 3), { color: "red", "font-style": "italic" }); + + expect(host.innerHTML).toContain("color: red"); + expect(host.innerHTML).toContain("font-style: italic"); + }); +}); + +// The rebuild is what keeps the markup from growing: whatever shape the +// element was in going in, it comes out as one span per distinct run. +describe("applyInlineStyle rebuilds rather than wraps", () => { + it("flattens markup that was already nested", () => { + const host = mount('abc'); + + applyInlineStyle(rangeOver(host, 0, 3), { "font-style": "italic" }); + + expect(host.querySelectorAll("span")).toHaveLength(1); + expect(host.textContent).toBe("abc"); + }); + + it("leaves no span carrying nothing", () => { + const host = mount('ab'); + + applyInlineStyle(rangeOver(host, 0, 2), { "font-style": "italic" }); + + expect(host.querySelectorAll("span")).toHaveLength(1); + expect(host.textContent).toBe("ab"); + }); + + it("reads a bold tag as styling and writes it back as one span", () => { + const host = mount("abc"); + + applyInlineStyle(rangeOver(host, 0, 3), { color: "red" }); + + expect(host.querySelector("span")?.getAttribute("style")).toContain("font-weight: 700"); + expect(host.querySelector("span")?.getAttribute("style")).toContain("color: red"); + }); + + it("keeps line breaks where they were", () => { + const host = mount("ab
cd"); + + applyInlineStyle(rangeOver(host, 0, 2), { color: "red" }); + + expect(host.querySelectorAll("br")).toHaveLength(1); + expect(host.innerHTML).toBe('ab
cd'); + }); + + // The bug: a chip is `display: flex`, so each span became its own flex item. + // Colouring one word broke the centring and rewrapped the whole line. + it("keeps a flex container's text as one item, so colouring a word cannot reflow it", () => { + const host = mount("Hello this is a test to see how this work"); + host.style.display = "flex"; + + applyInlineStyle(rangeOver(host, 28, 31), { color: "red" }); + + expect(host.children).toHaveLength(1); + expect(host.firstElementChild?.tagName).toBe("SPAN"); + expect(host.firstElementChild?.getAttribute("style")).toBeNull(); + expect(host.querySelector("span span")?.textContent).toBe("how"); + expect(host.textContent).toBe("Hello this is a test to see how this work"); + }); + + it("does the same for a grid container", () => { + const host = mount("abcdef"); + host.style.display = "grid"; + + applyInlineStyle(rangeOver(host, 2, 4), { color: "red" }); + + expect(host.children).toHaveLength(1); + }); + + it("does not wrap an ordinary block, which flows its text already", () => { + const host = mount("abcdef"); + + applyInlineStyle(rangeOver(host, 2, 4), { color: "red" }); + + expect(host.innerHTML).toBe('abcdef'); + }); + + it("reads styling back out of the wrapper it added", () => { + const host = mount("abcdef"); + host.style.display = "flex"; + applyInlineStyle(rangeOver(host, 2, 4), { color: "red" }); + + applyInlineStyle(rangeOver(host, 2, 4), { color: "blue" }); + + expect(host.querySelectorAll("span span")).toHaveLength(1); + expect(host.querySelector("span span")?.getAttribute("style")).toBe("color: blue"); + expect(host.textContent).toBe("abcdef"); + }); + + it("styles a run that sits after a line break", () => { + const host = mount("ab
cd"); + + applyInlineStyle(rangeOver(host, 3, 5), { color: "red" }); + + expect(host.innerHTML).toBe('ab
cd'); + }); +}); + +describe("readInlineStyle", () => { + it("reports the styling of a run that is styled the same throughout", () => { + const host = mount('abc'); + + const styles = readInlineStyle(rangeOver(host, 0, 3), ["color"]); + + expect(styles.color).toBe("rgb(255, 0, 0)"); + }); + + it("reports nothing for a property that is not set anywhere", () => { + const host = mount("abc"); + + const styles = readInlineStyle(rangeOver(host, 0, 3), ["background-color"]); + + expect(styles["background-color"]).toBeUndefined(); + }); +}); + +// Edge cases found by asking what a real composition contains that the happy +// path does not: source formatting, containers that box their children, text +// that is not plain ASCII, and a selection that reaches outside the element. +describe("applyInlineStyle edge cases", () => { + it("keeps a newline that came from the source file as text, not a line break", () => { + // Compositions are written across lines. Turning that whitespace into
+ // would add visible breaks to an element that had none. + const host = mount("\n Hello world\n "); + + applyInlineStyle(rangeOver(host, 7, 12), { color: "red" }); + + expect(host.querySelectorAll("br")).toHaveLength(0); + expect(host.textContent).toBe("\n Hello world\n "); + }); + + it("still writes a real line break back as a line break", () => { + const host = mount("ab
cd"); + + applyInlineStyle(rangeOver(host, 3, 5), { color: "red" }); + + expect(host.querySelectorAll("br")).toHaveLength(1); + expect(host.textContent).toBe("abcd"); + }); + + it("keeps text that looks like markup as text", () => { + const host = mount("a <b> & c"); + + applyInlineStyle(rangeOver(host, 0, 1), { color: "red" }); + + expect(host.textContent).toBe("a & c"); + expect(host.querySelectorAll("b")).toHaveLength(0); + }); + + it("does not cut an emoji in half when the boundary lands inside one", () => { + // A selection offset is counted in UTF-16 units, and an emoji is two of + // them. Splitting one leaves half a character in each span. + const host = mount("abπŸ‘cd"); + + applyInlineStyle(rangeOver(host, 0, 3), { color: "red" }); + + expect(host.textContent).toBe("abπŸ‘cd"); + // textContent would read back whole even if the two halves sat in separate + // spans, so the check that matters is that no element holds half a one. + expect(elementsWithHalfACharacter(host)).toEqual([]); + }); + + it("keeps a whole emoji together when the selection starts inside one", () => { + const host = mount("abπŸ‘cd"); + + applyInlineStyle(rangeOver(host, 3, 6), { color: "red" }); + + expect(host.textContent).toBe("abπŸ‘cd"); + expect(elementsWithHalfACharacter(host)).toEqual([]); + }); + + it("leaves the element alone when the selection reaches outside it", () => { + // Rebuilding on a range whose common ancestor is an ancestor of the element + // would rewrite far more of the document than the user selected. + document.body.innerHTML = '

first

second

'; + const section = document.body.firstElementChild as HTMLElement; + const before = section.innerHTML; + const range = document.createRange(); + range.setStart(section.querySelector("#a")!.firstChild!, 1); + range.setEnd(section.querySelector("#b")!.firstChild!, 2); + + applyInlineStyle(range, { color: "red" }); + + expect(section.innerHTML).toBe(before); + }); + + it("keeps a line-clamped element's text as one item", () => { + // -webkit-box is how line clamping is written, and it boxes its children + // exactly like flex does. + const host = mount("abcdef"); + host.style.display = "-webkit-box"; + + applyInlineStyle(rangeOver(host, 2, 4), { color: "red" }); + + expect(host.children).toHaveLength(1); + }); + + it("styles the whole text when everything is selected", () => { + const host = mount("abcdef"); + + applyInlineStyle(rangeOver(host, 0, 6), { color: "red" }); + + expect(host.innerHTML).toBe('abcdef'); + }); + + it("styles right up to an existing run's edge without merging into it", () => { + const host = mount("abcd"); + applyInlineStyle(rangeOver(host, 0, 2), { color: "red" }); + + applyInlineStyle(rangeOver(host, 2, 4), { "font-style": "italic" }); + + expect(host.querySelectorAll("span")).toHaveLength(2); + expect(host.textContent).toBe("abcd"); + }); + + it("survives being asked to style the same run twice over", () => { + const host = mount("abcdef"); + for (let round = 0; round < 5; round += 1) { + applyInlineStyle(rangeOver(host, 1, 4), { color: "red" }); + } + + expect(host.querySelectorAll("span")).toHaveLength(1); + expect(host.textContent).toBe("abcdef"); + }); +}); + +/** + * An element's children are not always anonymous formatting. The design panel + * keeps them as text layers and tracks each by an attribute on it, so a rebuild + * that emits fresh bare spans throws that identity away β€” and after colouring a + * single word the panel could no longer match a layer to its source, so every + * edit it offered failed with "Couldn't save this text structure change". + */ +describe("applyInlineStyle keeps what the design panel tracks", () => { + it("keeps a layer's key when the styling changes", () => { + const host = mount( + 'Hello' + + 'world', + ); + applyInlineStyle(rangeOver(host, 0, 5), { color: "green" }); + + expect(host.innerHTML).toContain('data-hf-text-key="child:0"'); + expect(host.innerHTML).toContain('data-hf-text-key="child:1"'); + expect(host.innerHTML).toContain("color: green"); + }); + + it("keeps a layer's typography, which the edit never mentioned", () => { + const host = mount( + 'Hello', + ); + applyInlineStyle(rangeOver(host, 0, 5), { color: "green" }); + + expect(host.innerHTML).toContain("font-size: 48px"); + }); + + it("does not put one layer's key on two spans when its text is split", () => { + const host = mount('Hello'); + applyInlineStyle(rangeOver(host, 0, 2), { color: "green" }); + + expect(host.innerHTML.match(/data-hf-text-key="child:0"/g) ?? []).toHaveLength(1); + }); + + it("still merges neighbours that carry no identity to lose", () => { + const host = mount('abcd'); + applyInlineStyle(rangeOver(host, 2, 4), { color: "red" }); + + expect(host.innerHTML).toBe('abcd'); + }); + + // The wrapper the rebuild adds inside a flex container carries nothing. Read + // as the layer it sits around, it hid the real one below it, and the second + // edit of an element threw away every identity the first one had kept. + it("keeps a layer's identity through a second edit inside a flex container", () => { + document.body.innerHTML = + '
' + + 'one
two
'; + const host = document.body.firstElementChild as HTMLElement; + applyInlineStyle(rangeOver(host, 4, 6), { color: "red" }); + const live = document.getSelection()?.getRangeAt(0); + if (live) applyInlineStyle(live, { color: "blue" }); + + expect(host.innerHTML).toContain('data-hf-text-key="child:0"'); + expect(host.innerHTML).toContain("color: blue"); + expect(host.innerHTML).not.toContain("color: red"); + }); + + // Stamped by the writer on every element on the way to disk, so carrying it + // preserves nothing β€” and it made the wrapper this rebuild adds look like a + // layer as soon as the file had been saved once. + it("does not treat the writer's own id as a layer identity", () => { + document.body.innerHTML = + '
' + + 'one
two
'; + const host = document.body.firstElementChild as HTMLElement; + applyInlineStyle(rangeOver(host, 4, 6), { color: "red" }); + + expect(host.innerHTML).toContain('data-hf-text-key="child:0"'); + expect(host.innerHTML).not.toContain("hf-wrap"); + }); + + it("does not merge two tracked layers that end up looking alike", () => { + const host = mount( + 'ab' + + 'cd', + ); + applyInlineStyle(rangeOver(host, 2, 4), { color: "red" }); + + expect(host.innerHTML).toContain('data-hf-text-key="child:0"'); + expect(host.innerHTML).toContain('data-hf-text-key="child:1"'); + }); +}); + +/** + * A control that fires more than once per gesture, which the colour input does: + * a native picker reports every sample while the pointer moves in it, so one + * choice of colour arrives as a stream of them, each applied to whatever is + * selected at the time. + */ +describe("applyInlineStyle survives a control that fires repeatedly", () => { + it("restyles the same characters each time, in text containing a line break", () => { + const host = mount("1.
abcdefg"); + // "cde", on the line after the break. Only the first sample knows where the + // user pointed; every one after it reads the selection back, which is what + // the toolbar does and what the restore has to have got right. + applyInlineStyle(rangeOver(host, 5, 8), { color: "rgb(1, 1, 1)" }); + for (const color of ["rgb(2, 2, 2)", "rgb(3, 3, 3)"]) { + const live = document.getSelection()?.getRangeAt(0); + if (live) applyInlineStyle(live, { color }); + } + + expect(host.innerHTML).toBe('1.
abcdefg'); + }); + + it("puts the selection back over the characters it styled, past a break", () => { + const host = mount("1.
abcdefg"); + applyInlineStyle(rangeOver(host, 5, 8), { color: "red" }); + + expect(document.getSelection()?.toString()).toBe("cde"); + }); +}); + +/** + * A colour that does not paint is the same to the user as one that did not save. + * + * `-webkit-text-fill-color` inherits and paints the glyph fill, so a composition + * that sets it on a text element wins over any `color` the editor puts on a run + * inside it. The run saved correctly and rendered in someone else's colour, + * which reads as the colour picker being broken. + */ +describe("applyInlineStyle when something else is painting the glyphs", () => { + function stubFill(fill: string | null) { + const real = window.getComputedStyle.bind(window); + vi.spyOn(window, "getComputedStyle").mockImplementation(((element: Element) => { + const computed = real(element as HTMLElement); + return new Proxy(computed, { + get: (target, key) => { + if (key === "webkitTextFillColor") return fill ?? undefined; + if (key === "color") return (element as HTMLElement).style.color || "rgb(0, 0, 0)"; + return Reflect.get(target, key); + }, + }); + }) as typeof window.getComputedStyle); + } + + it("mirrors the colour into the fill when an ancestor is overpainting", () => { + const host = mount("Hello world"); + stubFill("rgb(255, 255, 255)"); + applyInlineStyle(rangeOver(host, 6, 11), { color: "rgb(255, 0, 149)" }); + + expect(host.innerHTML).toContain("-webkit-text-fill-color: rgb(255, 0, 149)"); + expect(host.innerHTML).toContain("color: rgb(255, 0, 149)"); + }); + + it("leaves the markup alone when nothing is overpainting", () => { + const host = mount("Hello world"); + stubFill("rgb(255, 0, 149)"); + applyInlineStyle(rangeOver(host, 6, 11), { color: "rgb(255, 0, 149)" }); + + expect(host.innerHTML).not.toContain("-webkit-text-fill-color"); + expect(host.innerHTML).toContain("color: rgb(255, 0, 149)"); + }); + + it("says nothing about a run that carries no colour", () => { + const host = mount("Hello world"); + stubFill("rgb(255, 255, 255)"); + applyInlineStyle(rangeOver(host, 6, 11), { "font-weight": "700" }); + + expect(host.innerHTML).not.toContain("-webkit-text-fill-color"); + }); +}); + +describe("readInlineStyleSpread", () => { + it("reports every colour in the selection, in order, with its share", () => { + const host = mount( + 'Helloworld', + ); + + expect(readInlineStyleSpread(rangeOver(host, 0, 10), "color")).toEqual([ + { value: "red", chars: 5 }, + { value: "lime", chars: 5 }, + ]); + }); + + it("collapses characters that share a colour into one band", () => { + const host = mount('Hello'); + + expect(readInlineStyleSpread(rangeOver(host, 0, 5), "color")).toEqual([ + { value: "red", chars: 5 }, + ]); + }); + + it("reports only what the selection covers", () => { + const host = mount( + 'Helloworld', + ); + + expect(readInlineStyleSpread(rangeOver(host, 6, 10), "color")).toEqual([ + { value: "lime", chars: 4 }, + ]); + }); + + it("ignores whitespace, which shows no colour at all", () => { + // Colour the whole element, then recolour one word: the whitespace around it + // keeps the first colour. It paints no glyph, so counting it puts a band of a + // colour nothing on screen is painted in at the edge of the swatch. + const host = mount( + ' ' + + 'Hello' + + ' world', + ); + + expect(readInlineStyleSpread(rangeOver(host, 0, 12), "color")).toEqual([ + { value: "red", chars: 5 }, + { value: "lime", chars: 5 }, + ]); + }); + + it("is empty when the characters carry no colour of their own", () => { + const host = mount("Hello world"); + + expect(readInlineStyleSpread(rangeOver(host, 0, 5), "color")).toEqual([]); + }); +}); diff --git a/packages/studio/src/components/editor/inlineTextStyleRange.ts b/packages/studio/src/components/editor/inlineTextStyleRange.ts new file mode 100644 index 0000000000..1105f24219 --- /dev/null +++ b/packages/studio/src/components/editor/inlineTextStyleRange.ts @@ -0,0 +1,597 @@ +/** + * Styling a run of characters inside an element being edited in place. + * + * Not `document.execCommand`. That is deprecated, and what it emits varies by + * browser between ``, a class, and an inline style depending on + * `styleWithCSS`. The output of this goes into the user's composition file, so + * it has to be one predictable shape, and the shape is a `` carrying an + * inline style. + * + * Not DOM range surgery either, which is the obvious way and the wrong one. + * Wrapping a range in a span is three lines and then every interesting case is + * a special case: recolouring nests spans that shadow each other, removing a + * style cannot reach the ancestor that set it, and styling across an existing + * run's boundary has to split it. Each fix is a new branch and the branches + * interact. + * + * So the element is read into a flat list of styled runs, the styling is + * applied to a span of characters in that list, and the element is rebuilt + * from it. Replacing, removing, splitting and merging all stop being cases: + * the rebuild emits one span per distinct run and cannot nest or duplicate, + * whatever was there before. Text elements in a composition are a headline or + * a sentence, so reading and rebuilding one is not a cost worth avoiding. + */ + +import { isRichTextFormattingTag } from "@hyperframes/core/rich-text-sanitize"; + +/** One stretch of characters that are all styled the same way. */ +interface StyledRun { + text: string; + style: Record; + /** + * The child element these characters came out of, when they came out of one. + * + * Carried because an element's children are not always anonymous formatting: + * the design panel keeps them as text layers and tracks each by an attribute + * on it. Rebuilding from style alone emitted fresh, bare spans, which threw + * that identity away β€” after colouring a single word the panel could no + * longer match a layer to its source, so every edit it offered failed to + * save. The rebuild puts the identity back on the run that still holds it. + */ + origin: Element | null; + /** + * That identity as a value, so two runs can be compared without comparing + * the nodes they came from. A child with nothing on it but a style has no + * identity to lose, and merges with its neighbour exactly as before. + */ + identity: string; +} + +export type InlineStyleDelta = Record; + +/** + * Tags that mean a style. They are read as styling and written back as spans, + * so there is one representation to reason about instead of two that have to + * agree. Rendering is unchanged; the markup for an edited element is not. + */ +const TAG_STYLES: Record> = { + B: { "font-weight": "700" }, + STRONG: { "font-weight": "700" }, + I: { "font-style": "italic" }, + EM: { "font-style": "italic" }, + U: { "text-decoration-line": "underline" }, +}; + +/** + * Stands in for a `
` while the element is a flat string, so a break counts + * as one character and offsets survive the rebuild. + * + * Not a newline. Compositions are written across lines, so an element's text + * routinely contains real newlines that are only source formatting, and using + * one as the marker turned every one of them into a visible line break the + * first time a word was styled. A NUL never appears: the HTML parser replaces + * it with U+FFFD, so no document can contain one. + */ +const BREAK = "\u0000"; + +/** Apply `style` to the characters the range covers, then rebuild the element. */ +export function applyInlineStyle(range: Range, style: InlineStyleDelta): void { + if (range.collapsed) return; + // Resolved from where the selection starts, not from where it and its end + // happen to meet. A selection dragged past the element's edge meets its end + // at an ancestor, and taking that as the host would rebuild the ancestor: + // every sibling element inside it flattened into text by an edit that was + // meant to colour a word. + const host = editingHost(range.startContainer); + if (!host || !holdsBothEnds(host, range)) return; + + const runs = readRuns(host); + const span = codePointBounds( + runs, + offsetOf(host, range.startContainer, range.startOffset), + offsetOf(host, range.endContainer, range.endOffset), + ); + if (!span) return; + + const next = restyle(runs, span.start, span.end, style); + render(host, next); + // A colour that does not paint is the same to the user as a colour that did + // not save, so check rather than assume. See `mirrorFillColor`. + if (colourIsOverpainted(host)) render(host, next.map(mirrorFillColor)); + selectRange(host, span.start, span.end); +} + +/** + * Whether something above the run is painting the glyphs a different colour. + * + * `-webkit-text-fill-color` inherits and paints the glyph fill, so an ancestor + * that sets it wins over any `color` a descendant sets. A composition doing so + * is not doing anything wrong, but from the editor it reads as the colour + * picker being broken: the run is saved with the colour asked for and renders + * in someone else's. + * + * Asked of the rendered span rather than worked out from the stylesheet. Its + * own `color` is set, so its computed colour IS the one that was asked for, and + * if the fill differs from it then something else is painting it. Both sides + * come from the same computed style, so neither notation nor inheritance has to + * be untangled by hand. + */ +function colourIsOverpainted(host: Element): boolean { + const view = host.ownerDocument.defaultView; + if (!view?.getComputedStyle) return false; + for (const span of host.querySelectorAll("span[style*='color']")) { + if (!span.style.color) continue; + const computed = view.getComputedStyle(span) as CSSStyleDeclaration & { + webkitTextFillColor?: string; + }; + const fill = computed.webkitTextFillColor; + if (!fill || !computed.color) continue; + if (fill !== computed.color) return true; + } + return false; +} + +/** The same run, with its colour also stated as the fill that actually paints. */ +function mirrorFillColor(run: StyledRun): StyledRun { + const colour = run.style.color; + if (!colour) return run; + return { ...run, style: { ...run.style, "-webkit-text-fill-color": colour } }; +} + +/** + * The offsets to style, widened so they never fall inside a character. + * + * A selection offset counts UTF-16 units and an emoji is two of them, so a + * boundary can land between the halves of one. Styling from there puts half the + * character in one span and half in the next, and both render as a question + * mark in a box. + */ +function codePointBounds( + runs: StyledRun[], + start: number | null, + end: number | null, +): { start: number; end: number } | null { + if (start === null || end === null || start >= end) return null; + const text = runs.map((run) => run.text).join(""); + return { + start: isTrailingHalf(text, start) ? start - 1 : start, + end: isTrailingHalf(text, end) ? end + 1 : end, + }; +} + +/** Whether the whole selection lives inside this element. */ +function holdsBothEnds(host: Element, range: Range): boolean { + return host.contains(range.startContainer) && host.contains(range.endContainer); +} + +/** Whether this offset sits on the second half of a character, mid-pair. */ +function isTrailingHalf(text: string, offset: number): boolean { + const code = text.charCodeAt(offset); + return code >= 0xdc00 && code <= 0xdfff; +} + +/** + * What the range is styled with, for a toolbar that has to open showing the + * truth rather than a default. Reports a property only when the whole range + * agrees about it, which is what a control can honestly display. + */ +export function readInlineStyle(range: Range, properties: string[]): Record { + const chars = coveredChars(range); + if (!chars) return {}; + const covered = chars.map((entry) => entry.style); + + const styles: Record = {}; + for (const property of properties) { + const first: string | undefined = covered[0]?.[property]; + if (first === undefined) continue; + if (covered.every((style) => style[property] === first)) styles[property] = first; + } + return styles; +} + +/** + * What one property looks like across the range, in document order, as runs of + * consecutive characters that share a value: `[{ value: "red", chars: 5 }, + * { value: "lime", chars: 6 }]`. + * + * `readInlineStyle` above answers "what is this range" and reports nothing when + * the range disagrees with itself β€” right for a toggle, which can only be on or + * off. A swatch can show more than one value at once, and showing the default + * instead reads as "this text is white" when none of it is. + */ +export function readInlineStyleSpread( + range: Range, + property: string, +): Array<{ value: string; chars: number }> { + const covered = coveredChars(range); + if (!covered) return []; + const spread: Array<{ value: string; chars: number }> = []; + for (const { char, style } of covered) { + // A space paints nothing, so the colour it inherits is not a colour anyone + // can see. Counting it puts a band of the element's own colour in the swatch + // for text that shows none of it β€” the stray edge on a selection that just + // happens to start or end next to a space. + if (!char.trim()) continue; + const value = style[property]; + if (value === undefined) continue; + const last = spread.at(-1); + if (last?.value === value) last.chars++; + else spread.push({ value, chars: 1 }); + } + return spread; +} + +/** Every character the range covers with its style, or null when it covers none. */ +function coveredChars(range: Range): Array<{ char: string; style: Record }> | null { + const host = editingHost(range.startContainer); + if (!host || !holdsBothEnds(host, range)) return null; + const start = offsetOf(host, range.startContainer, range.startOffset); + const end = offsetOf(host, range.endContainer, range.endOffset); + if (start === null || end === null) return null; + const covered = charRuns(readRuns(host)) + .slice(start, Math.max(end, start + 1)) + .map((entry) => ({ char: entry.char, style: entry.style })); + return covered.length > 0 ? covered : null; +} + +/** + * The element the caret is in: the one made editable, never a span inside it. + * + * Reading the nearest element instead would rebuild only the run the caret + * happened to land in, which is how a recolour ends up nested inside the + * colour it was meant to replace. + */ +function editingHost(node: Node): HTMLElement | null { + let element = (node.nodeType === 1 ? node : node.parentElement) as HTMLElement | null; + const editable = element?.closest("[contenteditable]"); + if (editable) return editable; + // No open edit, so climb out of the formatting to the element that owns it. + while (element?.parentElement && isRichTextFormattingTag(element.tagName)) { + element = element.parentElement; + } + return element; +} + +/** Read the element as a flat list of runs, in document order. */ +function readRuns(host: Element): StyledRun[] { + const runs: StyledRun[] = []; + // Text sitting directly in the host belongs to no child, so it has no origin. + walk(host, {}, runs, null); + return runs; +} + +function walk( + node: Node, + inherited: Record, + runs: StyledRun[], + origin: Element | null, +): void { + for (const child of Array.from(node.childNodes)) { + if (child.nodeType === 3) { + const text = child.textContent ?? ""; + if (text) runs.push({ text, style: inherited, origin, identity: identityOf(origin) }); + continue; + } + if (child.nodeType !== 1) continue; + const element = child as HTMLElement; + if (element.tagName === "BR") { + runs.push({ text: BREAK, style: inherited, origin, identity: identityOf(origin) }); + continue; + } + // The outermost child that carries anything is the one the panel knows as + // a layer, so nesting below it keeps pointing at it rather than at its + // inner formatting. An element with nothing on it is not an identity and + // must not shadow one below it β€” the wrapper this rebuild adds inside a + // flex container is exactly that, and taking it as the origin made the + // second edit of an element drop every identity the first one kept. + walk( + element, + { ...inherited, ...TAG_STYLES[element.tagName], ...ownStyle(element) }, + runs, + origin ?? (preservedAttributes(element).size > 0 ? element : null), + ); + } +} + +/** A child's identity as a comparable string, empty when it has none. */ +function identityOf(element: Element | null): string { + if (!element) return ""; + return [...preservedAttributes(element)] + .sort(([a], [b]) => (a < b ? -1 : 1)) + .map(([name, value]) => `${name}=${value}`) + .join("&"); +} + +function ownStyle(element: HTMLElement): Record { + const style: Record = {}; + for (let index = 0; index < element.style.length; index += 1) { + const property = element.style.item(index); + if (property) style[property] = element.style.getPropertyValue(property); + } + return style; +} + +/** One entry per character, which is the easiest thing to slice and compare. */ +function charRuns(runs: StyledRun[]): Array & { char: string }> { + const perChar: Array & { char: string }> = []; + for (const run of runs) { + // By UTF-16 unit, not code point: `restyle` indexes this list with selection + // offsets, which count units, so an emoji has to stay two entries long. + for (let index = 0; index < run.text.length; index += 1) { + perChar.push({ + char: run.text[index] ?? "", + style: run.style, + origin: run.origin, + identity: run.identity, + }); + } + } + return perChar; +} + +/** Apply the delta to `[start, end)` and hand back runs covering the element. */ +function restyle( + runs: StyledRun[], + start: number, + end: number, + delta: InlineStyleDelta, +): StyledRun[] { + const text = runs.map((run) => run.text).join(""); + const perChar = charRuns(runs); + const next: StyledRun[] = []; + // Indexed by UTF-16 unit, not by code point: `perChar`, `start` and `end` all + // count units, and spreading the string would count a surrogate pair once and + // slide every index after an emoji. + for (let index = 0; index < text.length; index += 1) { + appendChar( + next, + text[index] ?? "", + charAfter(perChar[index], index >= start && index < end, delta), + ); + } + return next; +} + +/** What one character is styled with once the delta has been applied to it. */ +function charAfter( + at: Omit | undefined, + inside: boolean, + delta: InlineStyleDelta, +): Omit { + const style = at?.style ?? {}; + return { + style: inside ? withDelta(style, delta) : style, + origin: at?.origin ?? null, + identity: at?.identity ?? "", + }; +} + +/** + * One character onto the run list, merged into the run before it when they + * belong together. + * + * Merged as it is built, so equal neighbours never become two spans. Two that + * the design panel tracks as separate layers stay apart even when they now look + * identical, because merging them deletes one of them. + */ +function appendChar(runs: StyledRun[], char: string, at: Omit): void { + const last = runs[runs.length - 1]; + if (last && last.identity === at.identity && sameStyle(last.style, at.style)) { + last.text += char; + return; + } + runs.push({ text: char, ...at }); +} + +function withDelta(style: Record, delta: InlineStyleDelta): Record { + const next = { ...style }; + for (const [property, value] of Object.entries(delta)) { + if (value === null) delete next[property]; + else next[property] = value; + } + return next; +} + +function sameStyle(a: Record, b: Record): boolean { + return styleKey(a) === styleKey(b); +} + +/** Sorted, so two runs styled the same way compare equal whatever the order. */ +function styleKey(style: Record): string { + return Object.keys(style) + .sort() + .map((property) => `${property}: ${style[property]}`) + .join("; "); +} + +/** Rebuild the element: bare text where there is no styling, one span where there is. */ +function render(host: Element, runs: StyledRun[]): void { + const doc = host.ownerDocument; + const nodes = runNodes(doc, runs); + host.replaceChildren(); + if (nodes.length > 1 && laysOutItsChildren(host)) { + // Wrapped, because in a flex or grid container every child is an item to + // be laid out. Text that was one anonymous item becomes several boxes the + // moment a word inside it is coloured, and the element visibly reflows: + // centring, wrapping and order all change under an edit that was only ever + // meant to change a colour. One wrapper keeps it a single item, and the + // runs inside it stay inline text. + const wrapper = doc.createElement("span"); + wrapper.append(...nodes); + host.append(wrapper); + return; + } + host.append(...nodes); +} + +function runNodes(doc: Document, runs: StyledRun[]): Node[] { + const nodes: Node[] = []; + // One span per origin keeps its attributes: an identity that appeared twice + // would be two layers claiming to be the same one. A run split off from an + // origin is a new layer and is written as one. + const claimed = new Set(); + for (const run of runs) { + const carried = + run.origin && !claimed.has(run.origin) ? preservedAttributes(run.origin) : new Map(); + if (carried.size > 0 && run.origin) claimed.add(run.origin); + nodes.push(...runNode(doc, run, carried)); + } + return nodes; +} + +/** + * One run's nodes: its line breaks as `
`, and its text as bare text when it + * has nothing to carry or a span when it has. The identity goes on the first + * piece only, so a run broken across lines does not claim it twice. + */ +function runNode(doc: Document, run: StyledRun, carried: Map): Node[] { + const key = styleKey(run.style); + const nodes: Node[] = []; + for (const [index, piece] of run.text.split(BREAK).entries()) { + if (index > 0) nodes.push(doc.createElement("br")); + if (!piece) continue; + if (!key && carried.size === 0) { + nodes.push(doc.createTextNode(piece)); + continue; + } + const span = doc.createElement("span"); + for (const [name, value] of carried) span.setAttribute(name, value); + if (key) span.setAttribute("style", key); + span.textContent = piece; + nodes.push(span); + carried.clear(); + } + return nodes; +} + +/** + * The one attribute the writer assigns rather than the author. + * + * Left out on purpose. It is stamped onto every element on the way to disk, so + * carrying it preserves nothing β€” and it made the wrapper this rebuild adds + * inside a flex container look like a layer as soon as the file had been saved + * once, which put it back to shadowing the real layers underneath it. + */ +const DERIVED_ATTR = "data-hf-id"; + +/** + * What a child carries besides its styling: the identity the design panel + * tracks it by. Its style is not copied β€” that is what the run holds, already + * merged with whatever the edit changed. + */ +function preservedAttributes(element: Element): Map { + const kept = new Map(); + for (const name of element.getAttributeNames()) { + if (name === "style" || name === DERIVED_ATTR) continue; + kept.set(name, element.getAttribute(name) ?? ""); + } + return kept; +} + +/** Displays whose children are boxes it positions, rather than text it flows. */ +const LAYS_OUT_CHILDREN = new Set([ + "flex", + "inline-flex", + "grid", + "inline-grid", + // How line clamping is written, and it boxes its children like flex. + "-webkit-box", + "-webkit-inline-box", +]); + +function laysOutItsChildren(host: Element): boolean { + const view = host.ownerDocument.defaultView; + if (!view) return false; + return LAYS_OUT_CHILDREN.has(view.getComputedStyle(host).display); +} + +/** Where a DOM position falls, counted in characters from the element's start. */ +function offsetOf(host: Element, container: Node, containerOffset: number): number | null { + // A position between children, expressed as a child index. + if (container === host) { + return Array.from(host.childNodes) + .slice(0, containerOffset) + .reduce((count, child) => count + subtreeCharLength(child), 0); + } + let count = 0; + const walker = host.ownerDocument.createTreeWalker( + host, + NodeFilter.SHOW_TEXT | NodeFilter.SHOW_ELEMENT, + ); + let node = walker.nextNode(); + while (node) { + if (node === container) return count + containerOffset; + count += charLength(node); + node = walker.nextNode(); + } + return null; +} + +/** How many character positions a node occupies itself: its own text, or one + * for a break. An element contributes nothing; the walk visits its text. */ +function charLength(node: Node): number { + if (node.nodeType === 3) return (node.textContent ?? "").length; + return nodeName(node) === "BR" ? 1 : 0; +} + +/** The same count for a child and everything inside it, for a position given as + * a child index rather than a place in a text node. */ +function subtreeCharLength(node: Node): number { + return (node.textContent ?? "").length || (nodeName(node) === "BR" ? 1 : 0); +} + +function nodeName(node: Node): string { + return node.nodeType === 1 ? (node as Element).tagName : ""; +} + +/** Put the selection back over the characters that were just styled. */ +function selectRange(host: Element, start: number, end: number): void { + const doc = host.ownerDocument; + const selection = doc.defaultView?.getSelection(); + const from = positionAt(host, start); + const to = positionAt(host, end); + if (!selection || !from || !to) return; + const range = doc.createRange(); + range.setStart(from.node, from.offset); + range.setEnd(to.node, to.offset); + selection.removeAllRanges(); + selection.addRange(range); +} + +/** + * The DOM position a character offset lands on, after a rebuild. + * + * Counts a line break as one position, because everything that produced the + * offset did. This walked text nodes only, so in an element containing a `
` + * it landed one character early for every break before the offset — and the + * selection it put back was not the one that had just been styled. + * + * Which was invisible until a control fired more than once. The colour input + * does: a native picker reports every sample while the pointer moves in it, and + * each one restyled a selection that had walked one character further along + * than the last. Choosing a colour for three characters painted a different + * shade onto each character of the whole line. + */ +function positionAt(host: Element, offset: number): { node: Node; offset: number } | null { + let count = 0; + const walker = host.ownerDocument.createTreeWalker( + host, + NodeFilter.SHOW_TEXT | NodeFilter.SHOW_ELEMENT, + ); + let node = walker.nextNode(); + let last: Node | null = null; + while (node) { + if (node.nodeType !== 3) { + if (nodeName(node) === "BR") count += 1; + node = walker.nextNode(); + continue; + } + const length = (node.textContent ?? "").length; + if (count + length >= offset) return { node, offset: offset - count }; + count += length; + last = node; + node = walker.nextNode(); + } + if (last) return { node: last, offset: (last.textContent ?? "").length }; + return { node: host, offset: 0 }; +} diff --git a/packages/studio/src/components/editor/manualEditsDom.ts b/packages/studio/src/components/editor/manualEditsDom.ts index 9ac4588a26..4a041e020f 100644 --- a/packages/studio/src/components/editor/manualEditsDom.ts +++ b/packages/studio/src/components/editor/manualEditsDom.ts @@ -221,6 +221,7 @@ function isIdentityAfterTranslateStrip(m: DOMMatrix): boolean { return m.is2D && m.a === 1 && m.b === 0 && m.c === 0 && m.d === 1; } +// fallow-ignore-next-line complexity function stripGsapTranslateFromTransform(element: HTMLElement): void { if (element.hasAttribute(STUDIO_MANUAL_EDIT_GESTURE_ATTR)) return; const transform = element.style.getPropertyValue("transform"); @@ -256,6 +257,7 @@ function stripGsapTranslateFromTransform(element: HTMLElement): void { // and push the offset straight into GSAP's x/y via gsap.set; the var() offset is // still persisted (buildPathOffsetPatches), and GSAP re-reads it at init on // reload. Returns true when handled as GSAP (caller must skip the CSS path). +// fallow-ignore-next-line complexity function applyStudioPathOffsetViaGsap( element: HTMLElement, offset: { x: number; y: number }, @@ -553,26 +555,26 @@ function queryStudioElements(doc: Document, attr: string): HTMLElement[] { function reapplyPathOffsets(doc: Document): void { for (const el of queryStudioElements(doc, STUDIO_PATH_OFFSET_ATTR)) { - const gsapSkip = gsapAnimatesProperty(el, "x", "y"); + // Unlike size below, the offset channels COMPOSE — applying both doubles the move. + if (gsapAnimatesProperty(el, "x", "y")) continue; const x = el.style.getPropertyValue(STUDIO_OFFSET_X_PROP); const y = el.style.getPropertyValue(STUDIO_OFFSET_Y_PROP); - if (gsapSkip) continue; - if (x || y) { - applyStudioPathOffset( - el, - { - x: Number.parseFloat(x) || 0, - y: Number.parseFloat(y) || 0, - }, - { updateBase: false }, - ); - } + if (!x && !y) continue; + const offset = { x: Number.parseFloat(x) || 0, y: Number.parseFloat(y) || 0 }; + applyStudioPathOffset(el, offset, { updateBase: false }); } } +/** + * Put the studio's committed size back after a seek, GSAP-sized elements included. + * Size does not compose the way the offset above does: both channels write width + * and height, so the later write wins on the same number. Standing aside meant + * nothing held the size while a soft reload reverted the old timeline (GSAP hands + * back each tween's recorded starting width), so the element sat at its stylesheet + * size until the new one rendered — the jump after a resize. + */ function reapplyBoxSizes(doc: Document): void { for (const el of queryStudioElements(doc, STUDIO_BOX_SIZE_ATTR)) { - if (gsapAnimatesProperty(el, "width", "height")) continue; const w = Number.parseFloat(el.style.getPropertyValue(STUDIO_WIDTH_PROP)); const h = Number.parseFloat(el.style.getPropertyValue(STUDIO_HEIGHT_PROP)); if (Number.isFinite(w) && Number.isFinite(h) && w > 0 && h > 0) { diff --git a/packages/studio/src/components/editor/manualOffsetDrag.test.ts b/packages/studio/src/components/editor/manualOffsetDrag.test.ts index 5af32996a0..0db15b1149 100644 --- a/packages/studio/src/components/editor/manualOffsetDrag.test.ts +++ b/packages/studio/src/components/editor/manualOffsetDrag.test.ts @@ -88,6 +88,41 @@ describe("measureManualOffsetDragScreenToOffsetMatrix", () => { expect(element.style.getPropertyValue("translate")).toBe(""); }); + /** + * The element that has never been offset is the common case, and it used to skip + * the measurement and assume the canvas zoom was the whole story. Any transform + * above the element makes that assumption wrong: the mirrored parent here sends a + * rightward drag left, so the overlay followed the pointer while the element went + * the other way, and only on drop did the overlay jump to where the element really + * was. The fixture mirrors x and scales both axes by 1.2, as a `rotationY: 180` + * card at `scale: 1.2` does. + */ + it("measures a mirrored parent even when the element carries no offset yet", () => { + const window = new Window(); + const element = window.document.createElement("div"); + window.document.body.append(element); + + element.getBoundingClientRect = () => { + const offsetX = Number.parseFloat(element.style.getPropertyValue(STUDIO_OFFSET_X_PROP)) || 0; + const offsetY = Number.parseFloat(element.style.getPropertyValue(STUDIO_OFFSET_Y_PROP)) || 0; + return new window.DOMRect(100 - 1.2 * offsetX, 200 + 1.2 * offsetY, 40, 20); + }; + + const measured = measureManualOffsetDragScreenToOffsetMatrix(element, { x: 0, y: 0 }); + if (!measured.ok) throw new Error(measured.reason); + + // Dragging one screen px right must move the element one screen px right, which + // on a mirrored parent means writing a NEGATIVE offset. + const offset = resolveManualOffsetForPointerDelta({ + initialOffset: { x: 0, y: 0 }, + screenToOffset: measured.matrix, + dx: 60, + dy: 60, + }); + expect(offset.x).toBeCloseTo(-50, 6); + expect(offset.y).toBeCloseTo(50, 6); + }); + it("measures movement in parent viewport pixels when the element is inside a scaled iframe", () => { const window = new Window(); const iframe = window.document.createElement("iframe"); @@ -133,7 +168,12 @@ describe("measureManualOffsetDragScreenToOffsetMatrix", () => { expect(nextOffset).toEqual({ x: 100, y: 50 }); }); - it("returns identity matrix for non-path-offset elements with zero initial offset", () => { + // Carrying no path offset used to be taken as permission to assume the response + // instead of measuring it. It is not a signal about the transforms above the + // element, so it no longer changes the answer: an element that does not move is + // unmeasurable either way, and the caller falls back rather than being handed a + // matrix that was never checked. + it("does not treat a missing path offset as a measurable response", () => { const window = new Window(); const element = window.document.createElement("div"); window.document.body.append(element); @@ -141,10 +181,7 @@ describe("measureManualOffsetDragScreenToOffsetMatrix", () => { const measured = measureManualOffsetDragScreenToOffsetMatrix(element, { x: 0, y: 0 }); - expect(measured.ok).toBe(true); - if (measured.ok) { - expectMatrixClose(measured.matrix, { a: 1, b: 0, c: 0, d: 1 }); - } + expect(measured.ok).toBe(false); }); it("rejects path-offset elements whose movement response cannot be measured", () => { @@ -160,6 +197,56 @@ describe("measureManualOffsetDragScreenToOffsetMatrix", () => { }); }); +/** + * A group drag is rigid: every member is handed the SAME pointer delta and must + * travel the same distance on screen, or the group visibly comes apart mid-drag. + * Members do not share a mapping though — each measures its own, because each can + * sit under different ancestor transforms. A member whose movement cannot be + * measured falls back to a guess, and this pins what that guess costs the group. + */ +describe("group drag stays rigid", () => { + function member(key: string, response: number, measurable: boolean) { + const window = new Window(); + const element = window.document.createElement("div"); + window.document.body.append(element); + element.getBoundingClientRect = () => { + const ox = Number.parseFloat(element.style.getPropertyValue(STUDIO_OFFSET_X_PROP)) || 0; + const oy = Number.parseFloat(element.style.getPropertyValue(STUDIO_OFFSET_Y_PROP)) || 0; + const move = measurable ? response : 0; + return new window.DOMRect(100 + move * ox, 200 + move * oy, 40, 20); + }; + const result = createManualOffsetDragMember({ + key, + selection: { element } as never, + element, + rect: { left: 100, top: 200, width: 40, height: 20, editScaleX: 1, editScaleY: 1 }, + }); + if (!result.ok) throw new Error(result.reason); + return { member: result.member, response }; + } + + /** Screen distance this member travels for a pointer delta of `d`. */ + function screenTravel(entry: ReturnType, d: number): number { + const offset = resolveManualOffsetForPointerDelta({ + initialOffset: entry.member.initialOffset, + screenToOffset: entry.member.screenToOffset, + dx: d, + dy: 0, + }); + return offset.x * entry.response; + } + + it("moves every measurable member the same distance for one pointer delta", () => { + // Two members under different ancestor scales: one 1:1, one inside a half-scale + // parent. Different offsets, identical screen travel — that is what rigid means. + const a = member("a", 1, true); + const b = member("b", 0.5, true); + + expect(screenTravel(a, 60)).toBeCloseTo(60, 6); + expect(screenTravel(b, 60)).toBeCloseTo(60, 6); + }); +}); + describe("createManualOffsetDragMember uses raw CSS var offset", () => { it("ignores GSAP transform — initialOffset comes from CSS vars only", () => { const window = new Window(); diff --git a/packages/studio/src/components/editor/manualOffsetDrag.ts b/packages/studio/src/components/editor/manualOffsetDrag.ts index a808c792fb..bb4b7df792 100644 --- a/packages/studio/src/components/editor/manualOffsetDrag.ts +++ b/packages/studio/src/components/editor/manualOffsetDrag.ts @@ -213,9 +213,9 @@ export function applyManualOffsetDragMatrix(matrix: ManualOffsetDragMatrix, poin * The perspective w-divisor (matrix3d m44) of the element's current transform. * For a plain `translateZ(z)` under `perspective(p)`, m44 = (p - z) / p, so the * element renders 1/m44× larger and a translate of `d` composition px moves - * `d / m44` px on screen. Returns 1 for 2D transforms (no foreshortening). Used - * to keep the drag offset → screen-movement mapping correct for depth elements, - * which the flat-scale fast path below would otherwise get wrong by 1/m44. + * `d / m44` px on screen. Returns 1 for 2D transforms (no foreshortening). Only + * the unmeasurable-element fallback needs this — the measured path reads the + * foreshortening off the element's real movement along with everything else. */ function readTransformWDivisor(element: HTMLElement): number { const t = element.ownerDocument.defaultView?.getComputedStyle(element).transform; @@ -225,25 +225,25 @@ function readTransformWDivisor(element: HTMLElement): number { return Number.isFinite(w) && w > 0 ? w : 1; } +/** + * How far the element actually moves on screen per unit of drag offset, measured + * rather than assumed. + * + * The offset is written on the element, but what reaches the screen is that offset + * put through every transform above it. A parent carrying a rotation, a mirror, a + * scale or a perspective changes both the direction and the distance — a card at + * `rotationY: 180` sends a rightward drag left. Guessing this from the canvas zoom + * alone was wrong for every such element: the overlay tracked the pointer while the + * element went somewhere else, and the overlay only jumped to the truth on drop, + * when it re-measured. Moving the element and watching where it lands costs three + * layout reads once per gesture and is right for any transform, including ones no + * closed-form fast path would cover. + */ export function measureManualOffsetDragScreenToOffsetMatrix( element: HTMLElement, initialOffset: { x: number; y: number }, options: { probeSize?: number; scaleX?: number; scaleY?: number } = {}, ): { ok: true; matrix: ManualOffsetDragMatrix } | { ok: false; reason: string } { - if ( - !element.hasAttribute("data-hf-studio-path-offset") && - initialOffset.x === 0 && - initialOffset.y === 0 - ) { - const sx = options.scaleX || 1; - const sy = options.scaleY || 1; - // Fold in the perspective foreshortening: a depth element (z≠0) moves - // 1/m44× faster on screen than its flat scale implies, so the screen→offset - // matrix must scale by m44 or the element outruns the pointer/overlay. - const w = readTransformWDivisor(element); - return { ok: true, matrix: { a: w / sx, b: 0, c: 0, d: w / sy } }; - } - const probeSize = options.probeSize ?? DEFAULT_OFFSET_PROBE_PX; if (!Number.isFinite(probeSize) || probeSize <= 0) { return { ok: false, reason: "Invalid movement probe size." }; @@ -325,6 +325,8 @@ export function resolveManualOffsetForPointerDelta(input: { }; } +// Pre-existing complexity — surfaced by this branch touching the file, not by new logic. +// fallow-ignore-next-line complexity export function createManualOffsetDragMember(input: { key: string; selection: DomEditSelection; @@ -522,6 +524,7 @@ export function restoreManualOffsetDragMembers(members: ManualOffsetDragMember[] } } +/** Teardown after a COMMITTED drag. */ export function endManualOffsetDragMembers(members: ManualOffsetDragMember[]): void { for (const member of members) { endStudioManualEditGesture(member.element, member.gestureToken); @@ -550,6 +553,7 @@ export function endManualOffsetDragMembers(members: ManualOffsetDragMember[]): v } } +/** Release the timelines this gesture paused, re-rendering at the playhead. */ export function resumeGsapTimelines(element: HTMLElement): void { const ids = element.getAttribute("data-hf-drag-paused-timelines"); element.removeAttribute("data-hf-drag-paused-timelines"); diff --git a/packages/studio/src/components/editor/marqueeOutsideCanvas.test.ts b/packages/studio/src/components/editor/marqueeOutsideCanvas.test.ts new file mode 100644 index 0000000000..a65ae2d105 --- /dev/null +++ b/packages/studio/src/components/editor/marqueeOutsideCanvas.test.ts @@ -0,0 +1,31 @@ +import { describe, expect, it } from "vitest"; +import { rectsOverlap } from "../../utils/marqueeGeometry"; + +/** + * An element dragged past the edge of the frame sits out in the grey, and the + * rubber band refused to START there — it only began inside the composition + * rect, so the one gesture that could reach those elements could not be begun + * near them, leaving the timeline as the only way to select something plainly + * visible on screen. + * + * The collection half never had that limit: it compares rects in overlay space + * and never clipped to the frame, so a band drawn out in the grey has always + * been able to find what it covers. This pins that, including the negative + * coordinates an off-canvas element actually has. + */ +describe("marquee reaches elements outside the composition", () => { + const offCanvas = { left: -180, top: 40, width: 90, height: 40 }; + + it("covers an element sitting left of the frame", () => { + expect(rectsOverlap({ left: -220, top: 10, width: 160, height: 120 }, offCanvas)).toBe(true); + }); + + it("covers one sitting above the frame", () => { + const above = { left: 60, top: -140, width: 80, height: 50 }; + expect(rectsOverlap({ left: 20, top: -200, width: 200, height: 120 }, above)).toBe(true); + }); + + it("does not claim one the band misses", () => { + expect(rectsOverlap({ left: 400, top: 400, width: 50, height: 50 }, offCanvas)).toBe(false); + }); +}); diff --git a/packages/studio/src/components/editor/reapplyBoxSizeAfterSeek.test.ts b/packages/studio/src/components/editor/reapplyBoxSizeAfterSeek.test.ts new file mode 100644 index 0000000000..92a9087968 --- /dev/null +++ b/packages/studio/src/components/editor/reapplyBoxSizeAfterSeek.test.ts @@ -0,0 +1,64 @@ +// @vitest-environment jsdom +import { afterEach, describe, expect, it } from "vitest"; +import { reapplyPositionEditsAfterSeek } from "./manualEditsDom"; +import { STUDIO_BOX_SIZE_ATTR, STUDIO_HEIGHT_PROP, STUDIO_WIDTH_PROP } from "./manualEditsTypes"; + +/** + * A resize commit hands the size to a GSAP tween, and a soft reload reverts the + * old timeline before the new one renders — GSAP restores each tween's recorded + * starting width on the way out. Nothing else held the size across that window, + * so the element sat at its stylesheet size for a few hundred milliseconds: the + * jump after a resize. Worse, the next gesture then started from a box that + * disagreed with the studio's own vars and snapped on its first move. + * + * The seek reapply is what closes the window, and it used to stand aside for + * exactly the elements that need it — the ones GSAP sizes. + */ +describe("box size survives a seek while GSAP owns the size", () => { + afterEach(() => { + document.body.innerHTML = ""; + Reflect.deleteProperty(window, "__timelines"); + }); + + function cardSizedByGsap(): HTMLElement { + const el = document.createElement("div"); + el.id = "card"; + el.setAttribute(STUDIO_BOX_SIZE_ATTR, "true"); + el.style.setProperty(STUDIO_WIDTH_PROP, "305px"); + el.style.setProperty(STUDIO_HEIGHT_PROP, "202px"); + document.body.append(el); + // A timeline that animates this element's width/height, as the committed + // resize leaves behind. + Object.assign(window, { + __timelines: { + main: { + getChildren: () => [{ targets: () => [el], vars: { width: 305, height: 202 } }], + }, + }, + }); + return el; + } + + it("re-applies the committed size after the timeline gave it back", () => { + const el = cardSizedByGsap(); + // The revert: GSAP puts the tween's recorded starting size back. + el.style.width = "395px"; + el.style.height = "261px"; + + reapplyPositionEditsAfterSeek(document); + + expect(el.style.width).toBe("305px"); + expect(el.style.height).toBe("202px"); + }); + + it("leaves an element alone once its studio size is cleared", () => { + const el = cardSizedByGsap(); + el.style.removeProperty(STUDIO_WIDTH_PROP); + el.style.removeProperty(STUDIO_HEIGHT_PROP); + el.style.width = "395px"; + + reapplyPositionEditsAfterSeek(document); + + expect(el.style.width).toBe("395px"); + }); +}); diff --git a/packages/studio/src/components/editor/snapEngageTravel.test.ts b/packages/studio/src/components/editor/snapEngageTravel.test.ts new file mode 100644 index 0000000000..74b045bd58 --- /dev/null +++ b/packages/studio/src/components/editor/snapEngageTravel.test.ts @@ -0,0 +1,65 @@ +import { describe, expect, it } from "vitest"; +import { + resolveSnapAdjustment, + snapEngagedForTravel, + SNAP_THRESHOLD_PX, + type SnapTarget, +} from "./snapEngine"; + +/** + * Picking a selection up used to move it. An element resting within the snap + * threshold of a guide is already snappable, so the snap computed on the first + * frame of a drag displaced it by up to the threshold while the pointer had not + * moved at all — captured live as `pointer "0,0"` against `applied "4,-3"`, with + * every member of the group jumping 12,-8 composition px before the drag had + * started. Snapping pulls toward a guide as you drag; it has nothing to say about + * a gesture that has not moved. + */ +describe("snapping waits for the drag to travel", () => { + // Moving box's right edge is at 150; the target's left edge is at 154, so the + // pair is 4px apart — inside the threshold, and snappable the moment it is asked. + const movingRect = { left: 100, top: 50, width: 50, height: 40 }; + const target: SnapTarget = { + left: 154, + top: 50, + right: 254, + bottom: 90, + centerX: 204, + centerY: 70, + id: "neighbour", + }; + + const snapAt = (dx: number, dy: number) => + resolveSnapAdjustment({ + movingRect, + proposedDx: dx, + proposedDy: dy, + targets: [target], + threshold: SNAP_THRESHOLD_PX, + disabled: false, + disabledForTravel: !snapEngagedForTravel(dx, dy), + }); + + it("does not move a selection that has not been dragged yet", () => { + expect(snapAt(0, 0)).toMatchObject({ dx: 0, dy: 0 }); + }); + + it("leaves a sub-threshold twitch alone", () => { + expect(snapAt(1, -1)).toMatchObject({ dx: 1, dy: -1 }); + }); + + it("still snaps once the drag is a real one", () => { + expect(snapEngagedForTravel(0, 0)).toBe(false); + expect(snapEngagedForTravel(10, 0)).toBe(true); + // Without the travel gate the same delta snaps, which is the behaviour to keep. + const engaged = resolveSnapAdjustment({ + movingRect, + proposedDx: 0, + proposedDy: 0, + targets: [target], + threshold: SNAP_THRESHOLD_PX, + disabled: false, + }); + expect(engaged.dx).toBe(4); + }); +}); diff --git a/packages/studio/src/components/editor/snapEngine.ts b/packages/studio/src/components/editor/snapEngine.ts index 6a64a22a29..f80db5f3b4 100644 --- a/packages/studio/src/components/editor/snapEngine.ts +++ b/packages/studio/src/components/editor/snapEngine.ts @@ -3,6 +3,24 @@ // All position values are in overlay-space (screen) pixels. export const SNAP_THRESHOLD_PX = 6; +/** + * Pointer travel a MOVE must reach before snapping is allowed to touch it. + * + * An element resting within the threshold of a guide is already "snappable", so + * a snap computed on the very first frame displaces it by up to the threshold + * while the pointer has moved nothing — pick a selection up and the whole thing + * teleports before you have dragged at all. Snapping is meant to pull toward a + * guide as the user drags, so it does not participate until the drag is real. + * The value matches the distance a drag must cover to count as a drag rather + * than a click, so nothing below it moves anything. + */ +const SNAP_ENGAGE_TRAVEL_PX = 4; + +/** Whether a move of this size has travelled far enough for snapping to apply. */ +export function snapEngagedForTravel(dx: number, dy: number): boolean { + return Math.hypot(dx, dy) >= SNAP_ENGAGE_TRAVEL_PX; +} + const EQUIDISTANCE_TOLERANCE_PX = 1; // --------------------------------------------------------------------------- @@ -359,8 +377,10 @@ export function resolveSnapAdjustment(input: { gridEdges?: { x: SnapEdge[]; y: SnapEdge[] }; threshold: number; disabled: boolean; + /** Set when the gesture has not travelled far enough for snapping yet. */ + disabledForTravel?: boolean; }): SnapResult { - if (input.disabled || input.threshold <= 0) { + if (input.disabled || input.disabledForTravel || input.threshold <= 0) { return DISABLED_RESULT(input.proposedDx, input.proposedDy); } diff --git a/packages/studio/src/components/editor/useDomEditOverlayGestures.ts b/packages/studio/src/components/editor/useDomEditOverlayGestures.ts index 7af8df2e7b..65e8bb4222 100644 --- a/packages/studio/src/components/editor/useDomEditOverlayGestures.ts +++ b/packages/studio/src/components/editor/useDomEditOverlayGestures.ts @@ -30,7 +30,6 @@ import { type GroupOverlayItem, type OverlayRect, orientedOverlayRect, - resolveDomEditGroupOverlayRect, } from "./domEditOverlayGeometry"; import { BLOCKED_MOVE_THRESHOLD_PX, @@ -50,8 +49,15 @@ import { startGroupDrag as _startGroupDrag, } from "./domEditOverlayStartGesture"; import { hugRectForElement } from "./domEditOverlayCrop"; -import { resolveSnapAdjustment, resolveEquidistanceGuides, SNAP_THRESHOLD_PX } from "./snapEngine"; +import { + resolveSnapAdjustment, + resolveEquidistanceGuides, + snapEngagedForTravel, + SNAP_THRESHOLD_PX, +} from "./snapEngine"; import { logResize, logResizeMove, logResizeSettle } from "../../utils/resizeDebug"; +import { logDrag, logDragSettle, readDragPositions } from "../../utils/dragDebug"; +import { createGroupDragMover } from "./groupDragMove"; export function createDomEditOverlayGestureHandlers(opts: UseDomEditOverlayGesturesOptions) { const setDraftOverlayRect = (next: OverlayRect) => { @@ -91,6 +97,8 @@ export function createDomEditOverlayGestureHandlers(opts: UseDomEditOverlayGestu }, ) => _startGesture(kind, e, opts, options); + const moveGroupDrag = createGroupDragMover(opts, setDraftGroupOverlayItems); + // fallow-ignore-next-line complexity const onPointerMove = (e: React.PointerEvent) => { const g = opts.gestureRef.current; @@ -114,55 +122,7 @@ export function createDomEditOverlayGestureHandlers(opts: UseDomEditOverlayGestu } if (groupG) { - let dx = e.clientX - groupG.startX; - let dy = e.clientY - groupG.startY; - - const sc = groupG.snapContext; - if (sc?.snapEnabled && sc.targets.length > 0) { - const groupBounds = resolveDomEditGroupOverlayRect( - groupG.originItems.map((item) => item.rect), - ); - if (groupBounds) { - const allTargets = sc.compositionTarget - ? [...sc.targets, sc.compositionTarget] - : sc.targets; - const snap = resolveSnapAdjustment({ - movingRect: groupBounds, - proposedDx: dx, - proposedDy: dy, - targets: allTargets, - gridEdges: sc.gridEdges ?? undefined, - threshold: SNAP_THRESHOLD_PX, - disabled: e.altKey, - }); - dx = snap.dx; - dy = snap.dy; - const movedRect = { - left: groupBounds.left + dx, - top: groupBounds.top + dy, - width: groupBounds.width, - height: groupBounds.height, - }; - const spacingGuides = e.altKey - ? [] - : resolveEquidistanceGuides({ - movingRect: movedRect, - targets: allTargets, - threshold: SNAP_THRESHOLD_PX, - }); - opts.snapGuidesRef.current = { guides: snap.guides, spacingGuides }; - } - } - groupG.lastSnappedDx = dx; - groupG.lastSnappedDy = dy; - - setDraftGroupOverlayItems( - groupG.originItems.map((item) => ({ - ...item, - rect: { ...item.rect, left: item.rect.left + dx, top: item.rect.top + dy }, - })), - ); - for (const member of groupG.members) applyManualOffsetDragDraft(member, dx, dy); + moveGroupDrag(groupG, e); return; } @@ -215,6 +175,9 @@ export function createDomEditOverlayGestureHandlers(opts: UseDomEditOverlayGestu movingRect, proposedDx: dx, proposedDy: dy, + // Same reason as the group path: a snap on a drag that has not travelled + // yet moves the element while the pointer is still. + disabledForTravel: !snapEngagedForTravel(dx, dy), targets: allTargets, gridEdges: sc.gridEdges ?? undefined, threshold: SNAP_THRESHOLD_PX, @@ -319,9 +282,14 @@ export function createDomEditOverlayGestureHandlers(opts: UseDomEditOverlayGestu opts.rafPausedRef.current = false; const rawDx = e.clientX - groupG.startX; const rawDy = e.clientY - groupG.startY; + // The click that trails every pointerup has to be eaten either way. The + // gesture ref is already cleared above, so by the time it arrives the box + // no longer looks busy, and handleBoxClick hands it to the canvas as an + // ordinary click — which lands between the members, resolves to nothing, + // and deselects the group the drag just moved. + opts.suppressNextBoxClickRef.current = true; if (Math.hypot(rawDx, rawDy) < BLOCKED_MOVE_THRESHOLD_PX) { restoreGroupPathOffsets(groupG); - opts.suppressNextBoxClickRef.current = true; return; } const dx = groupG.lastSnappedDx ?? rawDx; @@ -336,6 +304,17 @@ export function createDomEditOverlayGestureHandlers(opts: UseDomEditOverlayGestu selection: member.selection, next: applyManualOffsetDragCommit(member, dx, dy), })); + logDrag("drop", { + pointer: `${Math.round(rawDx)},${Math.round(rawDy)}`, + applied: `${Math.round(dx)},${Math.round(dy)}`, + committed: Object.fromEntries( + updates.map((update, index) => [ + groupG.members[index]?.key ?? String(index), + `${Math.round(update.next.x)},${Math.round(update.next.y)}`, + ]), + ), + at: readDragPositions(groupG.members), + }); void Promise.resolve(opts.onGroupPathOffsetCommitRef.current(updates)) .catch(() => { for (const member of groupG.members) { @@ -346,7 +325,15 @@ export function createDomEditOverlayGestureHandlers(opts: UseDomEditOverlayGestu restoreStudioPathOffset(member.element, member.initialPathOffset); } }) - .finally(() => endManualOffsetDragMembers(groupG.members)); + .finally(() => { + logDrag("committed", { at: readDragPositions(groupG.members) }); + endManualOffsetDragMembers(groupG.members); + // The gesture teardown resumes the paused timelines and re-seeks the + // player, which re-renders from whatever the preview currently holds. + // If the reloaded source has not landed yet that is the OLD position, + // so this is where a snap-back would show. + logDragSettle("settle", groupG.members); + }); return; } diff --git a/packages/studio/src/components/editor/useInlineTextEditing.tsx b/packages/studio/src/components/editor/useInlineTextEditing.tsx new file mode 100644 index 0000000000..9c137cbb65 --- /dev/null +++ b/packages/studio/src/components/editor/useInlineTextEditing.tsx @@ -0,0 +1,112 @@ +import { useRef, type ReactNode, type RefObject } from "react"; +import { InlineTextToolbar } from "./InlineTextToolbar"; +import { useDomEditActionsContextOptional } from "../../contexts/DomEditContext"; +import { useInlineTextEdit } from "../../hooks/useInlineTextEdit"; +import { usePlayerStore } from "../../player/store/playerStore"; +import { getPreviewTargetFromPointer } from "../../utils/studioPreviewHelpers"; +import { + canEditElementTextInline, + canEditTextInline, + isDoublePress, + type PressMark, +} from "./domEditInlineText"; +import type { DomEditSelection } from "./domEditingTypes"; + +/** + * The canvas' side of editing text where it sits. + * + * Holds the session, decides which presses open one, and knows the two things + * the canvas has to do differently while one is open: stand aside so the caret + * underneath can be reached, and stop taking focus back. + * + * The actions context is read here rather than threaded through the overlay's + * props, the same way the agent surfaces in that overlay read it, and it is + * absent in standalone player mounts, which have no project to edit. + */ +export function useInlineTextEditing(selectionRef: RefObject): { + editing: boolean; + /** Open an edit when this press pairs with the last one. */ + startFromPress: (event: { clientX: number; clientY: number }) => boolean; + /** Handle a key on the canvas. Returns true when it opened an edit. */ + handleKeyDown: (event: { key: string; shiftKey: boolean }) => boolean; + /** + * The styling controls for the current selection, for the caller to render. + * + * Handed back rather than mounted somewhere central because it belongs to the + * session this hook owns, and appears and disappears with it. + */ + toolbar: ReactNode; +} { + const actions = useDomEditActionsContextOptional(); + const inlineText = useInlineTextEdit({ + onCommit: (html) => void actions?.handleDomRichTextCommit(html), + onPause: () => usePlayerStore.getState().setIsPlaying(false), + }); + const lastPressRef = useRef(null); + + /** + * The element under this press, hit-tested now rather than read from state. + * + * Every cached answer in the overlay is React state that has not caught up + * with the press happening now: the hover is documented as an async cache, + * and the selection from the first press has not re-rendered. Reading either + * meant the first double press on any element opened nothing, which is every + * double press that matters. + */ + const elementUnderPress = (event: { clientX: number; clientY: number }) => { + const iframe = actions?.previewIframeRef?.current; + if (!iframe) return null; + // Studio suppresses pointer events inside the composition so the canvas + // overlay can own input, which means a plain elementFromPoint only ever + // finds wrappers. This helper lifts that for the length of the hit test, + // and is the same one the canvas uses to decide what was clicked. + return getPreviewTargetFromPointer( + iframe, + event.clientX, + event.clientY, + selectionRef.current?.compositionPath ?? null, + ); + }; + + /** A point on Studio's canvas, in the scaled composition's coordinates. */ + const compositionPoint = (event: { clientX: number; clientY: number }) => { + const iframe = actions?.previewIframeRef?.current; + const view = iframe?.contentWindow; + if (!iframe || !view?.innerWidth) return undefined; + const box = iframe.getBoundingClientRect(); + const scale = box.width / view.innerWidth || 1; + return { x: (event.clientX - box.left) / scale, y: (event.clientY - box.top) / scale }; + }; + + return { + editing: inlineText.session !== null, + toolbar: ( + + ), + // Enter opens the selected element's text, the way every design tool does, + // and is the dependable way in: a double press has to survive the canvas' + // gesture machinery, while this is one key on a selection that has settled. + handleKeyDown: (event) => { + if (event.key !== "Enter" || event.shiftKey) return false; + const target = selectionRef.current; + if (inlineText.session || !canEditTextInline(target)) return false; + return inlineText.start(target!.element); + }, + startFromPress: (event) => { + const press = { x: event.clientX, y: event.clientY, at: Date.now() }; + const paired = isDoublePress(lastPressRef.current, press); + lastPressRef.current = press; + + if (!paired || inlineText.session) return false; + + const element = elementUnderPress(event); + if (!canEditElementTextInline(element)) return false; + // The caret opens where the press landed, which means mapping the point + // out of Studio's coordinates and into the composition's own. + return inlineText.start(element!, compositionPoint(event)); + }, + }; +} diff --git a/packages/studio/src/contexts/DomEditContext.tsx b/packages/studio/src/contexts/DomEditContext.tsx index f89e27e915..ff810b1e53 100644 --- a/packages/studio/src/contexts/DomEditContext.tsx +++ b/packages/studio/src/contexts/DomEditContext.tsx @@ -24,6 +24,7 @@ export interface DomEditActionsValue extends Pick< | "handleDomRotationCommit" | "handleDomManualEditsReset" | "handleDomTextCommit" + | "handleDomRichTextCommit" | "handleDomTextFieldStyleCommit" | "handleDomAddTextField" | "handleDomRemoveTextField" @@ -149,6 +150,7 @@ export function DomEditProvider({ handleDomManualEditsReset, handleDomTextCommit, + handleDomRichTextCommit, handleDomTextFieldStyleCommit, handleDomAddTextField, handleDomRemoveTextField, @@ -236,6 +238,7 @@ export function DomEditProvider({ handleDomRotationCommit, handleDomManualEditsReset, handleDomTextCommit, + handleDomRichTextCommit, handleDomTextFieldStyleCommit, handleDomAddTextField, handleDomRemoveTextField, @@ -305,6 +308,7 @@ export function DomEditProvider({ handleDomRotationCommit, handleDomManualEditsReset, handleDomTextCommit, + handleDomRichTextCommit, handleDomTextFieldStyleCommit, handleDomAddTextField, handleDomRemoveTextField, diff --git a/packages/studio/src/hooks/domEditPersistFailure.ts b/packages/studio/src/hooks/domEditPersistFailure.ts index 1e7bd7a406..7e04e6f75b 100644 --- a/packages/studio/src/hooks/domEditPersistFailure.ts +++ b/packages/studio/src/hooks/domEditPersistFailure.ts @@ -19,13 +19,6 @@ export class DomEditPersistUnsafeValueError extends Error { } } -export class DomEditPersistUnsupportedTextStructureError extends Error { - constructor() { - super("Couldn't save this text structure change"); - this.name = "DomEditPersistUnsupportedTextStructureError"; - } -} - export type DomEditPersistFailureSelection = Pick< DomEditSelection, "label" | "hfId" | "id" | "selector" | "selectorIndex" | "sourceFile" diff --git a/packages/studio/src/hooks/domSelectionTimelineMirror.ts b/packages/studio/src/hooks/domSelectionTimelineMirror.ts new file mode 100644 index 0000000000..346acd1ae2 --- /dev/null +++ b/packages/studio/src/hooks/domSelectionTimelineMirror.ts @@ -0,0 +1,60 @@ +import type { SelectElementOptions, TimelineElement } from "../player"; +import { findMatchingTimelineElementId, findTimelineIdByAncestor } from "../utils/studioHelpers"; +import type { DomEditSelection } from "../components/editor/domEditing"; +import { logSelect } from "../utils/selectDebug"; + +interface TimelineMirrorDeps { + timelineElements: TimelineElement[]; + setSelectedTimelineElementId: (id: string | null, options?: SelectElementOptions) => void; + setTimelineSelectionSet: (ids: Set) => void; +} + +/** + * Mirror a canvas selection onto the timeline: the whole set first, then the + * primary as its anchor. + * + * The timeline is the source of truth for what is selected and it syncs back — + * whatever it holds replaces the canvas selection a moment later. Announcing only + * the primary therefore drops every other member. Worse, anchoring with + * `preserveSet` on an id the set does not yet contain empties the set outright, + * and an empty set syncs back as "nothing is selected" — which is how adding a + * second element, or moving a group, could wipe the selection instead of keeping + * it. Publishing the members first is what makes the anchor a member, so + * preserving the set is meaningful rather than destructive. + */ +export function announceTimelineSelection( + deps: TimelineMirrorDeps, + group: DomEditSelection[], + primary: DomEditSelection | null, +): void { + const { timelineElements, setSelectedTimelineElementId, setTimelineSelectionSet } = deps; + if (!primary) { + setTimelineSelectionSet(new Set()); + setSelectedTimelineElementId(null); + return; + } + const timelineIdFor = (selection: DomEditSelection) => + findMatchingTimelineElementId(selection, timelineElements) ?? + findTimelineIdByAncestor( + selection.element, + timelineElements, + selection.sourceFile || "index.html", + ); + const members = group.map(timelineIdFor).filter((id): id is string => Boolean(id)); + const anchor = timelineIdFor(primary); + // A member with no timeline row of its own resolves to null and is dropped here, + // so a group can announce fewer ids than it has — or none, which reads back as an + // empty selection and takes the canvas selection with it. + logSelect("announce", { + group: group.length, + published: members.length, + anchor, + anchorPublished: anchor != null && members.includes(anchor), + }); + // Only a real multi-selection publishes members. A single selection keeps the + // older contract on purpose: anchoring with preserveSet holds a live set the + // element already belongs to (a late async primary must not collapse a group) + // and collapses otherwise, which is what a fresh click means. + if (group.length > 1) setTimelineSelectionSet(new Set(members)); + setSelectedTimelineElementId(anchor, { preserveSet: true }); +} diff --git a/packages/studio/src/hooks/gsapRuntimePatch.test.ts b/packages/studio/src/hooks/gsapRuntimePatch.test.ts index 1a17f7e063..e34d293a41 100644 --- a/packages/studio/src/hooks/gsapRuntimePatch.test.ts +++ b/packages/studio/src/hooks/gsapRuntimePatch.test.ts @@ -523,3 +523,46 @@ describe("patchRuntimeTweenInPlace — composition isolation", () => { expect(otherTween.invalidate).not.toHaveBeenCalled(); }); }); + +describe("patchRuntimeTweenInPlace — deferSeek", () => { + /** + * A group drag commits one member at a time. Each in-place patch used to seek, + * and a seek re-renders the WHOLE timeline — so every member still queued behind + * the current one got repainted from its un-patched tween, back to where it sat + * before the drag, and stayed there until its own patch landed. That is the jump. + */ + it("does not seek while a group commit is still writing its other members", () => { + const a = { id: "a" }; + const rendered = { a: 0, b: 0 }; + const tweenA = makeTween({ vars: { x: 0 }, targetIds: ["a"], duration: 0 }, a); + const tweenB = makeTween({ vars: { x: 0 }, targetIds: ["b"], duration: 0 }, a); + const { iframe, seek } = fakeIframe(a, [tweenA, tweenB], { + onSeek: () => { + rendered.a = tweenA.vars.x as number; + rendered.b = tweenB.vars.x as number; + }, + }); + + const first = patchRuntimeTweenInPlace( + iframe, + "#a", + { kind: "set", props: { x: 500 } }, + undefined, + true, + ); + + expect(first).toBe(true); + expect(tweenA.vars.x).toBe(500); + // No repaint yet: "b" keeps the transform the gesture left on it instead of + // being rendered from its own tween, which still holds the pre-drag value. + expect(seek).not.toHaveBeenCalled(); + expect(rendered).toEqual({ a: 0, b: 0 }); + + tweenB.vars.x = 600; + const last = patchRuntimeTweenInPlace(iframe, "#a", { kind: "set", props: { x: 500 } }); + + expect(last).toBe(true); + expect(seek).toHaveBeenCalledTimes(1); + expect(rendered).toEqual({ a: 500, b: 600 }); + }); +}); diff --git a/packages/studio/src/hooks/gsapRuntimePatch.ts b/packages/studio/src/hooks/gsapRuntimePatch.ts index de2c6480e4..9d32650387 100644 --- a/packages/studio/src/hooks/gsapRuntimePatch.ts +++ b/packages/studio/src/hooks/gsapRuntimePatch.ts @@ -277,12 +277,16 @@ function applyChange(tween: RuntimeTween, change: RuntimeTweenChange): boolean { /** * Edit one tween in `window.__timelines` in place + re-seek to the current playhead. * Returns `true` on a confident patch, `false` otherwise (caller soft-reloads). + * + * `deferSeek` skips the re-render, for a caller patching several tweens in a row + * that will render once after the last one. */ export function patchRuntimeTweenInPlace( iframe: HTMLIFrameElement | null, selector: string, change: RuntimeTweenChange, compositionId?: string, + deferSeek = false, ): boolean { if (!iframe) return false; // A base `gsap.set` has no timeline tween to resolve — apply the value straight @@ -312,7 +316,13 @@ export function patchRuntimeTweenInPlace( if (change.kind !== "keyframe-rebuild") { tween.invalidate?.(); } - seekToCurrent(iframe, timeline); + // A seek re-renders the WHOLE timeline, not just the tween we patched. Under a + // multi-element commit that is a visible jump: the members still queued behind + // this one get repainted from their un-patched tweens, back to where they were + // before the gesture, and stay there until their own patch lands. Deferring + // leaves them showing the gesture's own transform, and the caller's last patch + // seeks once for the whole group. + if (!deferSeek) seekToCurrent(iframe, timeline); return true; } catch { return false; diff --git a/packages/studio/src/hooks/gsapScriptCommitHelpers.ts b/packages/studio/src/hooks/gsapScriptCommitHelpers.ts index 8c6e45a020..4bca7d0e2c 100644 --- a/packages/studio/src/hooks/gsapScriptCommitHelpers.ts +++ b/packages/studio/src/hooks/gsapScriptCommitHelpers.ts @@ -3,6 +3,7 @@ import type { DomEditSelection } from "../components/editor/domEditingTypes"; export { PROPERTY_DEFAULTS } from "./gsapShared"; import { idSelector, matchesExactlyOne } from "./gsapShared"; +import { studioWriteHeaders } from "../utils/studioFileVersion"; /** * The selector to author a NEW tween against, minting an id on the element when @@ -119,7 +120,7 @@ export async function assignGsapTargetAutoIdIfNeeded({ `/api/projects/${encodeURIComponent(projectId)}/file-mutations/patch-element/${encodeURIComponent(targetPath)}`, { method: "POST", - headers: { "Content-Type": "application/json" }, + headers: { "Content-Type": "application/json", ...studioWriteHeaders() }, body: JSON.stringify(patchBody), }, ); diff --git a/packages/studio/src/hooks/gsapScriptCommitTypes.ts b/packages/studio/src/hooks/gsapScriptCommitTypes.ts index 929e79b8ec..ed9f4a6ae4 100644 --- a/packages/studio/src/hooks/gsapScriptCommitTypes.ts +++ b/packages/studio/src/hooks/gsapScriptCommitTypes.ts @@ -22,6 +22,16 @@ export interface CommitMutationOptions { coalesceMs?: number; softReload?: boolean; skipReload?: boolean; + /** + * Write the source but leave the preview alone; the caller renders once when it + * is done. For a multi-write action like a group drag, rendering after each + * write shows a source where the members not yet written still hold their old + * values, so they snap back until their own write lands. This also defers the + * in-place runtime patch's seek, which re-renders the whole timeline and repaints + * the queued members the same way. Unlike `skipReload` this changes nothing about + * error handling — a failed write still throws. + */ + deferPreviewSync?: boolean; beforeReload?: () => void; /** * Serialize this commit against others sharing the same key. Used to chain @@ -39,6 +49,14 @@ export interface CommitMutationOptions { * existing soft/full reload path. Structural edits omit this and reload as before. */ instantPatch?: { selector: string; change: RuntimeTweenChange }; + /** + * The same fast path for a batched commit: one patch per element the batch + * wrote, applied in order. All of them must land for the reload to be skipped + * — one that can't be applied leaves the preview half-patched, so the whole + * batch falls back to the reload. Only the last patch re-renders (see + * `deferSeek`), so a ten-element batch repaints once. + */ + instantPatches?: Array<{ selector: string; change: RuntimeTweenChange }>; } export interface CommitMutationCall { diff --git a/packages/studio/src/hooks/keyframeCacheAstLoad.test.ts b/packages/studio/src/hooks/keyframeCacheAstLoad.test.ts new file mode 100644 index 0000000000..d9a1081df1 --- /dev/null +++ b/packages/studio/src/hooks/keyframeCacheAstLoad.test.ts @@ -0,0 +1,78 @@ +import { afterEach, describe, expect, it, vi } from "vitest"; +import { fetchParsedAnimations } from "./keyframeCacheAstLoad"; + +/** + * Parsing a composition is a whole-file read + parse on the server, and a + * multi-element action asks for the same file once per element. Callers that + * overlap in time share one request; a caller that comes after the last one + * settled does not, so a parse issued after a write is never served a + * pre-write answer. + */ +describe("fetchParsedAnimations — in-flight sharing", () => { + afterEach(() => { + vi.unstubAllGlobals(); + }); + + function stubFetch(): { calls: () => number; settle: () => void } { + let calls = 0; + const pending: Array<() => void> = []; + vi.stubGlobal("fetch", () => { + calls++; + return new Promise((resolve) => { + pending.push(() => + resolve({ + ok: true, + json: () => Promise.resolve({ animations: [{ id: "a", targetSelector: "#a" }] }), + } as Response), + ); + }); + }); + return { + calls: () => calls, + settle: () => { + for (const release of pending.splice(0, pending.length)) release(); + }, + }; + } + + it("serves overlapping reads of one file from a single request", async () => { + const fetchStub = stubFetch(); + + const pending = [ + fetchParsedAnimations("p", "index.html"), + fetchParsedAnimations("p", "index.html"), + fetchParsedAnimations("p", "index.html"), + ]; + fetchStub.settle(); + const results = await Promise.all(pending); + + expect(fetchStub.calls()).toBe(1); + expect(results.map((parsed) => parsed?.animations.length)).toEqual([1, 1, 1]); + }); + + it("does not share across files", async () => { + const fetchStub = stubFetch(); + + const pending = [ + fetchParsedAnimations("p", "index.html"), + fetchParsedAnimations("p", "other.html"), + ]; + fetchStub.settle(); + await Promise.all(pending); + + expect(fetchStub.calls()).toBe(2); + }); + + it("re-requests once the previous read has settled", async () => { + const fetchStub = stubFetch(); + + const first = fetchParsedAnimations("p", "index.html"); + fetchStub.settle(); + await first; + const second = fetchParsedAnimations("p", "index.html"); + fetchStub.settle(); + await second; + + expect(fetchStub.calls()).toBe(2); + }); +}); diff --git a/packages/studio/src/hooks/keyframeCacheAstLoad.ts b/packages/studio/src/hooks/keyframeCacheAstLoad.ts index 71e38bbb61..e4abc42525 100644 --- a/packages/studio/src/hooks/keyframeCacheAstLoad.ts +++ b/packages/studio/src/hooks/keyframeCacheAstLoad.ts @@ -42,7 +42,32 @@ function hasAnimations(value: unknown): value is ParsedGsapAnimations { ); } -export async function fetchParsedAnimations( +/** + * Requests for the same file that overlap in time, keyed `projectId|sourceFile`. + * + * Every parse re-reads and re-parses the whole composition server-side, and a + * multi-element action asks for the same file once per element. Sharing the + * in-flight promise makes that one request. Only OVERLAPPING calls share: the + * entry is dropped the moment it settles, so a call made after a write still + * gets a fresh parse. + */ +const inFlightParses = new Map>(); + +export function fetchParsedAnimations( + projectId: string, + sourceFile: string, +): Promise { + const key = `${projectId}|${sourceFile}`; + const inFlight = inFlightParses.get(key); + if (inFlight) return inFlight; + const request = requestParsedAnimations(projectId, sourceFile).finally(() => { + inFlightParses.delete(key); + }); + inFlightParses.set(key, request); + return request; +} + +async function requestParsedAnimations( projectId: string, sourceFile: string, ): Promise { diff --git a/packages/studio/src/hooks/timelineTimingSync.ts b/packages/studio/src/hooks/timelineTimingSync.ts index 919d259a90..aac58e926f 100644 --- a/packages/studio/src/hooks/timelineTimingSync.ts +++ b/packages/studio/src/hooks/timelineTimingSync.ts @@ -7,6 +7,7 @@ import { applySoftReload, applySoftReloadFinalization } from "../utils/gsapSoftR import { furthestClipEndFromDocument } from "../player/lib/timelineElementHelpers"; import type { RecordEditInput } from "../utils/studioFileHistory"; import { patchDocumentRootDuration } from "./timelineEditingGsap"; +import { studioWriteHeaders } from "../utils/studioFileVersion"; class GsapPreviewConvergenceError extends Error {} class GsapOwnershipProtocolError extends GsapPreviewConvergenceError {} @@ -58,6 +59,8 @@ async function rollbackOwnedMutation( `/api/projects/${encodeURIComponent(projectId)}/gsap-mutation-rollback/${encodeURIComponent(targetPath)}`, { method: "POST", + // Deliberately unclaimed: a rollback runs because a mutation did not + // converge, so let the restored file reload the preview. headers: { "Content-Type": "application/json" }, body: JSON.stringify({ expected, restore }), }, @@ -156,7 +159,7 @@ async function postGsapMutation( `/api/projects/${encodeURIComponent(projectId)}/gsap-mutations/${encodeURIComponent(filePath)}`, { method: "POST", - headers: { "Content-Type": "application/json" }, + headers: { "Content-Type": "application/json", ...studioWriteHeaders() }, body: JSON.stringify(mutation), }, ); diff --git a/packages/studio/src/hooks/useDomEditCommits.test.tsx b/packages/studio/src/hooks/useDomEditCommits.test.tsx index 1f0af1c06e..102b9c8c68 100644 --- a/packages/studio/src/hooks/useDomEditCommits.test.tsx +++ b/packages/studio/src/hooks/useDomEditCommits.test.tsx @@ -117,14 +117,6 @@ function stubPatchFetch( return fetchMock; } -function stubUnexpectedPersistFetch() { - const fetchMock = vi.fn(async (): Promise => { - throw new Error("persist should not run"); - }); - vi.stubGlobal("fetch", fetchMock); - return fetchMock; -} - async function flushAsyncWork(): Promise { for (let i = 0; i < 8; i += 1) { await Promise.resolve(); @@ -812,14 +804,26 @@ function renderStyleCommitWithFetch(fetchHandler: FetchHandler) { }; } -async function expectRejectedTextStructureEdit( +/** + * Adding or removing a text layer, which no per-child operation can express. + * + * Both used to be refused outright — the panel offered the buttons and neither + * could ever save — so this asserts the opposite of what it used to: one + * `rich-text` operation carrying the element's new markup, and no complaint. + */ +async function expectPersistedTextStructureEdit( commit: (hook: ReturnType) => Promise, + expectedMarkup: (markup: string) => void, ): Promise { - const fetchMock = stubUnexpectedPersistFetch(); + const fetchMock = stubPatchFetch({ + ok: true, + changed: true, + matched: true, + content: '
First
', + }); const { iframe, element } = createPreviewElement( '
FirstSecond
', ); - const originalInnerHtml = element.innerHTML; const selection = createSelection(element, { textFields: [ textField({ key: "first", value: "First", source: "child" }), @@ -833,13 +837,17 @@ async function expectRejectedTextStructureEdit( await commit(rendered.hook); }); - expect(fetchMock).not.toHaveBeenCalled(); - expect(rendered.showToast).toHaveBeenCalledWith( - expect.stringContaining("text structure change"), - "error", + const patchPost = fetchMock.mock.calls.find((call) => + requestUrl(call[0]).includes("/file-mutations/patch-element/"), ); - expect(element.innerHTML).toBe(originalInnerHtml); - expect(rendered.recordEdit).not.toHaveBeenCalled(); + expect(patchPost).toBeDefined(); + const body = JSON.parse(String(patchPost?.[1]?.body)) as { + operations: Array<{ type: string; value?: string }>; + }; + expect(body.operations).toHaveLength(1); + expect(body.operations[0]?.type).toBe("rich-text"); + expectedMarkup(body.operations[0]?.value ?? ""); + expect(rendered.showToast).not.toHaveBeenCalled(); } finally { rendered.cleanup(); } @@ -1146,12 +1154,26 @@ describe("useDomEditCommits style persist handling", () => { } }); - it("refuses added child text fields without persisting serialized markup", async () => { - await expectRejectedTextStructureEdit((hook) => hook.handleDomAddTextField("first")); + it("persists an added child text field as the element's new markup", async () => { + await expectPersistedTextStructureEdit( + (hook) => hook.handleDomAddTextField("first"), + (markup) => { + expect(markup).toContain("First"); + expect(markup).toContain("Second"); + // The layer that was added, between the two that were there. + expect(markup.match(/ { - await expectRejectedTextStructureEdit((hook) => hook.handleDomRemoveTextField("first")); + it("persists a removed child text field as the element's new markup", async () => { + await expectPersistedTextStructureEdit( + (hook) => hook.handleDomRemoveTextField("first"), + (markup) => { + expect(markup).not.toContain("First"); + expect(markup).toContain("Second"); + }, + ); }); it("keeps single self text commits on the text-content path", async () => { diff --git a/packages/studio/src/hooks/useDomEditCommits.ts b/packages/studio/src/hooks/useDomEditCommits.ts index 7a02be735d..55e4125bae 100644 --- a/packages/studio/src/hooks/useDomEditCommits.ts +++ b/packages/studio/src/hooks/useDomEditCommits.ts @@ -33,6 +33,7 @@ import { readErrorResponseBody, } from "./useDomEditCommitsHelpers"; import { cutoverCommittedOrThrow, type CutoverResult } from "../utils/sdkCutover"; +import { studioWriteHeaders } from "../utils/studioFileVersion"; interface RecordEditInput { label: string; kind: EditHistoryKind; @@ -201,7 +202,7 @@ export function useDomEditCommits({ `/api/projects/${pid}/file-mutations/patch-element/${encodeURIComponent(targetPath)}`, { method: "POST", - headers: { "Content-Type": "application/json" }, + headers: { "Content-Type": "application/json", ...studioWriteHeaders() }, body: JSON.stringify(patchBody), }, ); @@ -377,6 +378,7 @@ export function useDomEditCommits({ handleDomHtmlAttributeCommit, handleDomAttributesCommit, handleDomTextCommit, + handleDomRichTextCommit, commitDomTextFields, handleDomTextFieldStyleCommit, handleDomAddTextField, @@ -440,6 +442,7 @@ export function useDomEditCommits({ handleDomHtmlAttributeCommit, handleDomAttributesCommit, handleDomTextCommit, + handleDomRichTextCommit, commitDomTextFields, handleDomTextFieldStyleCommit, handleDomAddTextField, diff --git a/packages/studio/src/hooks/useDomEditCommitsHelpers.ts b/packages/studio/src/hooks/useDomEditCommitsHelpers.ts index dd82aff581..a440c6e4ef 100644 --- a/packages/studio/src/hooks/useDomEditCommitsHelpers.ts +++ b/packages/studio/src/hooks/useDomEditCommitsHelpers.ts @@ -1,6 +1,7 @@ import { StudioSaveHttpError, trackStudioSaveFailure } from "../utils/studioSaveDiagnostics"; import type { DomEditPatchBatch } from "./domEditCommitTypes"; import { formatFieldsSuffix } from "./gsapScriptCommitHelpers"; +import { studioWriteHeaders } from "../utils/studioFileVersion"; export function formatUnsafeFieldList(fields: Array<{ path: string }>): string { return fields.map((field) => field.path).join(", "); @@ -99,7 +100,7 @@ export async function patchElementBatches(projectId: string, batches: DomEditPat `/api/projects/${encodeURIComponent(projectId)}/file-mutations/patch-element-batches`, { method: "POST", - headers: { "Content-Type": "application/json" }, + headers: { "Content-Type": "application/json", ...studioWriteHeaders() }, body, }, ); diff --git a/packages/studio/src/hooks/useDomEditPreviewSync.ts b/packages/studio/src/hooks/useDomEditPreviewSync.ts index 6a288b6ee6..cb0c334fe1 100644 --- a/packages/studio/src/hooks/useDomEditPreviewSync.ts +++ b/packages/studio/src/hooks/useDomEditPreviewSync.ts @@ -8,13 +8,17 @@ import { findElementForSelection, type DomEditSelection } from "../components/ed import { reapplyPositionEditsAfterSeek } from "../components/editor/manualEdits"; import type { SidebarTab } from "../components/sidebar/LeftSidebar"; import type { PatchTarget } from "../utils/sourcePatcher"; +import { logSelect } from "../utils/selectDebug"; interface UseDomEditPreviewSyncParams { previewIframe: HTMLIFrameElement | null; activeCompPath: string | null; captionEditMode: boolean; domEditSelectionRef: React.MutableRefObject; + domEditGroupSelectionsRef: React.MutableRefObject; domEditSelection: DomEditSelection | null; + /** Re-resolves a whole multi-selection against the current preview document. */ + refreshDomEditGroupSelectionsFromPreview: (selections: DomEditSelection[]) => Promise; applyDomSelection: ( selection: DomEditSelection | null, options?: { revealPanel?: boolean; preserveGroup?: boolean }, @@ -35,8 +39,10 @@ export function useDomEditPreviewSync({ activeCompPath, captionEditMode, domEditSelectionRef, + domEditGroupSelectionsRef, domEditSelection, applyDomSelection, + refreshDomEditGroupSelectionsFromPreview, buildDomSelectionFromTarget, refreshPreviewDocumentVersion, syncPreviewHistoryHotkey, @@ -72,6 +78,21 @@ export function useDomEditPreviewSync({ // Clear so overlay geometry isn't computed on a stale, detached node. // (Drag-release-in-gray-zone is handled separately by // suppressNextBoxClickRef; the dragged element still resolves here.) + // + // One lost member is not the whole selection though. A multi-select that + // loses its primary here used to be wiped entirely, so moving a group and + // having any one of its elements fail to re-resolve deselected all of + // them. Re-resolve the group instead and keep whoever survived; it only + // clears when nobody did. + const group = domEditGroupSelectionsRef.current; + logSelect("preview-sync-lost", { + target: currentSelection.selector ?? currentSelection.id ?? null, + group: group.length, + }); + if (group.length > 1) { + await refreshDomEditGroupSelectionsFromPreview(group); + return; + } applyDomSelection(null, { revealPanel: false }); return; } @@ -103,8 +124,10 @@ export function useDomEditPreviewSync({ applyDomSelection, buildDomSelectionFromTarget, captionEditMode, + domEditGroupSelectionsRef, domEditSelectionRef, previewIframe, + refreshDomEditGroupSelectionsFromPreview, refreshPreviewDocumentVersion, syncPreviewHistoryHotkey, applyStudioManualEditsToPreviewRef, diff --git a/packages/studio/src/hooks/useDomEditSession.test.tsx b/packages/studio/src/hooks/useDomEditSession.test.tsx index 3c5a3e0ffd..299ea28899 100644 --- a/packages/studio/src/hooks/useDomEditSession.test.tsx +++ b/packages/studio/src/hooks/useDomEditSession.test.tsx @@ -230,6 +230,7 @@ describe("onReorderShadow source filter", () => { previewIframeRef: { current: null }, timelineElements: [], setSelectedTimelineElementId: vi.fn(), + setTimelineSelectionSet: vi.fn(), setRightCollapsed: vi.fn(), setRightPanelTab: vi.fn(), showToast: vi.fn(), @@ -328,6 +329,7 @@ describe("bulk segment ease commits", () => { previewIframeRef: { current: null }, timelineElements: [], setSelectedTimelineElementId: vi.fn(), + setTimelineSelectionSet: vi.fn(), setRightCollapsed: vi.fn(), setRightPanelTab: vi.fn(), showToast: vi.fn(), diff --git a/packages/studio/src/hooks/useDomEditSession.ts b/packages/studio/src/hooks/useDomEditSession.ts index efec011f95..5b11e467a7 100644 --- a/packages/studio/src/hooks/useDomEditSession.ts +++ b/packages/studio/src/hooks/useDomEditSession.ts @@ -38,6 +38,7 @@ export interface UseDomEditSessionParams { previewIframeRef: React.MutableRefObject; timelineElements: TimelineElement[]; setSelectedTimelineElementId: (id: string | null, options?: SelectElementOptions) => void; + setTimelineSelectionSet: (ids: Set) => void; setRightCollapsed: (collapsed: boolean) => void; setRightPanelTab: (tab: RightPanelTab) => void; showToast: (message: string, tone?: "error" | "info") => void; @@ -80,6 +81,7 @@ export function useDomEditSession({ previewIframeRef, timelineElements, setSelectedTimelineElementId, + setTimelineSelectionSet, setRightCollapsed, setRightPanelTab, showToast, @@ -127,6 +129,7 @@ export function useDomEditSession({ buildDomSelectionForTimelineElement, handleTimelineElementSelect, refreshDomEditSelectionFromPreview, + refreshDomEditGroupSelectionsFromPreview, applyMarqueeSelection, } = useDomSelection({ projectId, @@ -137,6 +140,7 @@ export function useDomEditSession({ previewIframeRef, timelineElements, setSelectedTimelineElementId, + setTimelineSelectionSet, setRightCollapsed, setRightPanelTab, previewIframe, @@ -225,6 +229,7 @@ export function useDomEditSession({ handleDomHtmlAttributeCommit, handleDomAttributesCommit, handleDomTextCommit, + handleDomRichTextCommit, handleDomTextFieldStyleCommit, handleDomAddTextField, handleDomRemoveTextField, @@ -382,6 +387,8 @@ export function useDomEditSession({ activeCompPath, domEditSelection, domEditSelectionRef, + domEditGroupSelectionsRef, + refreshDomEditGroupSelectionsFromPreview, previewIframeRef, previewIframe, captionEditMode, @@ -493,6 +500,7 @@ export function useDomEditSession({ handleDomRotationCommit: handleGsapAwareRotationCommit, handleDomManualEditsReset, handleDomTextCommit, + handleDomRichTextCommit, handleDomTextFieldStyleCommit, handleDomAddTextField, handleDomRemoveTextField, diff --git a/packages/studio/src/hooks/useDomEditTextCommits.ts b/packages/studio/src/hooks/useDomEditTextCommits.ts index 7111356279..dda80f816f 100644 --- a/packages/studio/src/hooks/useDomEditTextCommits.ts +++ b/packages/studio/src/hooks/useDomEditTextCommits.ts @@ -11,6 +11,7 @@ import { ensureImportedFontFace, } from "../utils/studioFontHelpers"; import { + buildDomEditRichTextPatchOperation, buildDomEditStylePatchOperation, buildDomEditTextPatchOperation, findElementForSelection, @@ -23,11 +24,9 @@ import { } from "../components/editor/domEditing"; import type { ImportedFontAsset } from "../components/editor/fontAssets"; import type { PersistDomEditOperations } from "./domEditCommitTypes"; +import { canEditElementTextInline } from "../components/editor/domEditInlineText"; import { buildTextFieldChildOperations } from "./domEditTextFieldCommitOps"; -import { - DomEditPersistUnsupportedTextStructureError, - reportDomEditPersistFailure, -} from "./domEditPersistFailure"; +import { reportDomEditPersistFailure } from "./domEditPersistFailure"; import { bumpDomEditCommitMapVersion, bumpDomEditCommitVersion, @@ -102,9 +101,20 @@ function planDomTextCommit( const childOperations = usesSerializedTextFields ? buildTextFieldChildOperations(originalTextFields, nextTextFields) : null; + // Per-child operations when the layers still line up one-for-one, and the + // element's whole markup when they do not. + // + // `buildTextFieldChildOperations` can only address children that already + // exist, so it returns null for any change in how many there are β€” which is + // every delete and every add. That used to end here with "Couldn't save this + // text structure change": the panel offered a remove button and an Add text + // field row, and neither could ever save. A structure change has one honest + // operation, which is to write the structure. const operations = childOperations ?? - (usesSerializedTextFields ? [] : [buildDomEditTextPatchOperation(nextContent)]); + (usesSerializedTextFields + ? [buildDomEditRichTextPatchOperation(nextContent)] + : [buildDomEditTextPatchOperation(nextContent)]); return { usesSerializedTextFields, @@ -269,9 +279,6 @@ export function useDomEditTextCommits({ } }, persist: async () => { - if (textCommit.usesSerializedTextFields && textCommit.childOperations === null) { - throw new DomEditPersistUnsupportedTextStructureError(); - } await persistDomEditOperations(domEditSelection, textCommit.operations, { label: "Edit text", skipRefresh: true, @@ -307,6 +314,85 @@ export function useDomEditTextCommits({ ], ); + /** + * Persist an element's own markup, for a text edit that styled part of it. + * + * Its own commit rather than a mode of the one above: that one plans a change + * to the text-field model, which escapes markup on the way out and refuses a + * change in child structure, and both of those are correct for the design + * panel. Styling a run of characters is neither of those things. The element + * already holds what the user typed, so there is nothing to apply, only + * something to save and something to put back if saving fails. + */ + const handleDomRichTextCommit = useCallback( + async (html: string) => { + if (!domEditSelection) return; + // The same gate that let the edit open, not the design panel's. + // + // The panel's rule is about its text fields, and it has none for an + // element whose text contains a line break: a `` holding `
`s + // is not a leaf, so nothing inside is a field and the element reports no + // editable text at all. Editing in place does not use fields β€” it + // rewrites the element's own markup β€” so refusing on that rule refused + // elements the caret had just been opened in, and every colour the user + // chose was dropped on the way out with nothing said about it. + if (!canEditElementTextInline(domEditSelection.element)) return; + const isLatestTextCommit = bumpDomEditCommitVersion(domTextCommitVersionRef); + const operations: PatchOperation[] = [{ type: "rich-text", property: "", value: html }]; + const iframe = previewIframeRef.current; + const doc = iframe?.contentDocument; + let editedElement: HTMLElement | null = null; + let previousInnerHtml: string | null = null; + + await runDomEditCommit({ + capture: () => { + if (!doc) return; + const el = findElementForSelection(doc, domEditSelection, activeCompPath); + if (!el) return; + editedElement = el; + previousInnerHtml = el.innerHTML; + }, + apply: () => { + // Idempotent: the caret put this there. Assigned anyway so a commit + // raised from anywhere but the element itself still lands. + if (editedElement) editedElement.innerHTML = html; + }, + persist: async () => { + await persistDomEditOperations(domEditSelection, operations, { + label: "Edit text", + skipRefresh: true, + shouldSave: isLatestTextCommit, + }); + }, + shouldRevert: () => isLatestTextCommit(), + revert: () => { + if (!editedElement || previousInnerHtml === null) return; + editedElement.innerHTML = previousInnerHtml; + }, + onError: (error) => + reportDomEditPersistFailure(domEditSelection, operations, error, showToast), + shouldResync: isLatestTextCommit, + resync: () => + resyncDomTextSelectionFromPreview( + doc, + domEditSelection, + activeCompPath, + buildDomSelectionFromTarget, + applyDomSelection, + ), + }); + }, + [ + activeCompPath, + applyDomSelection, + buildDomSelectionFromTarget, + domEditSelection, + persistDomEditOperations, + previewIframeRef, + showToast, + ], + ); + const commitDomTextFields = useCallback( async ( selection: DomEditSelection, @@ -342,9 +428,6 @@ export function useDomEditTextCommits({ } }, persist: async () => { - if (textCommit.usesSerializedTextFields && textCommit.childOperations === null) { - throw new DomEditPersistUnsupportedTextStructureError(); - } await persistDomEditOperations(selection, textCommit.operations, { label: "Edit text", skipRefresh: true, @@ -477,6 +560,7 @@ export function useDomEditTextCommits({ handleDomHtmlAttributeCommit, handleDomAttributesCommit, handleDomTextCommit, + handleDomRichTextCommit, commitDomTextFields, handleDomTextFieldStyleCommit, handleDomAddTextField, diff --git a/packages/studio/src/hooks/useDomEditWiring.ts b/packages/studio/src/hooks/useDomEditWiring.ts index fbd049482e..9b49f16fa7 100644 --- a/packages/studio/src/hooks/useDomEditWiring.ts +++ b/packages/studio/src/hooks/useDomEditWiring.ts @@ -23,6 +23,8 @@ export interface UseDomEditWiringParams { activeCompPath: string | null; domEditSelection: DomEditSelection | null; domEditSelectionRef: React.MutableRefObject; + domEditGroupSelectionsRef: React.MutableRefObject; + refreshDomEditGroupSelectionsFromPreview: (selections: DomEditSelection[]) => Promise; previewIframeRef: React.RefObject; previewIframe: HTMLIFrameElement | null; captionEditMode: boolean; @@ -115,6 +117,8 @@ export function useDomEditWiring({ activeCompPath, domEditSelection, domEditSelectionRef, + domEditGroupSelectionsRef, + refreshDomEditGroupSelectionsFromPreview, previewIframeRef, previewIframe, captionEditMode, @@ -254,8 +258,10 @@ export function useDomEditWiring({ activeCompPath, captionEditMode, domEditSelectionRef, + domEditGroupSelectionsRef, domEditSelection, applyDomSelection, + refreshDomEditGroupSelectionsFromPreview, buildDomSelectionFromTarget, refreshPreviewDocumentVersion, syncPreviewHistoryHotkey, diff --git a/packages/studio/src/hooks/useDomSelection.test.ts b/packages/studio/src/hooks/useDomSelection.test.ts index 53f4524644..02297938ec 100644 --- a/packages/studio/src/hooks/useDomSelection.test.ts +++ b/packages/studio/src/hooks/useDomSelection.test.ts @@ -5,6 +5,7 @@ import { createRoot } from "react-dom/client"; import { describe, expect, it, vi } from "vitest"; import { installReactActEnvironment, makeSelection } from "./domSelectionTestHarness"; import { useDomSelection } from "./useDomSelection"; +import type { TimelineElement } from "../player"; installReactActEnvironment(); @@ -14,11 +15,24 @@ interface HarnessProps { refreshKey: number; } -function renderHarness(initialProps: HarnessProps): { +interface TimelineSpies { + setSelectedTimelineElementId: ReturnType; + setTimelineSelectionSet: ReturnType; +} + +function renderHarness( + initialProps: HarnessProps, + options: { timelineElements?: TimelineElement[] } = {}, +): { current: () => ReturnType; rerender: (props: HarnessProps) => void; cleanup: () => void; + timeline: TimelineSpies; } { + const timeline: TimelineSpies = { + setSelectedTimelineElementId: vi.fn(), + setTimelineSelectionSet: vi.fn(), + }; const host = document.createElement("div"); document.body.append(host); const root = createRoot(host); @@ -32,8 +46,9 @@ function renderHarness(initialProps: HarnessProps): { compIdToSrc: new Map(), captionEditMode: false, previewIframeRef: { current: null }, - timelineElements: [], - setSelectedTimelineElementId: vi.fn(), + timelineElements: options.timelineElements ?? [], + setSelectedTimelineElementId: timeline.setSelectedTimelineElementId, + setTimelineSelectionSet: timeline.setTimelineSelectionSet, setRightCollapsed: vi.fn(), setRightPanelTab: vi.fn(), previewIframe: null, @@ -61,6 +76,7 @@ function renderHarness(initialProps: HarnessProps): { act(() => root.unmount()); host.remove(); }, + timeline, }; } @@ -77,6 +93,92 @@ function setupSelectedHarness() { return { selection, harness }; } +function timelineElement(domId: string): TimelineElement { + return { + id: domId, + key: domId, + domId, + tag: "div", + start: 0, + duration: 1, + track: 0, + sourceFile: "index.html", + } as TimelineElement; +} + +/** + * A marquee builds the group correctly and then used to lose it: it announced only + * the primary to the timeline, the timeline is the source of truth for what is + * selected, and the sync back to the canvas replaced the group with that one + * element a moment after the drop. The whole set has to be announced, with the + * primary as its anchor rather than as a new single selection. + */ +describe("useDomSelection marquee", () => { + it("announces every marquee'd element to the timeline, anchored on the primary", () => { + const first = document.createElement("div"); + first.id = "card"; + const second = document.createElement("div"); + second.id = "chip"; + document.body.append(first, second); + const harness = renderHarness( + { activeCompPath: "index.html", projectId: "project-1", refreshKey: 0 }, + { timelineElements: [timelineElement("card"), timelineElement("chip")] }, + ); + + act(() => + harness + .current() + .applyMarqueeSelection( + [makeSelection("Card", first), makeSelection("Chip", second)], + false, + ), + ); + + expect(harness.current().domEditGroupSelections).toHaveLength(2); + expect(harness.timeline.setTimelineSelectionSet).toHaveBeenCalledWith( + new Set(["card", "chip"]), + ); + expect(harness.timeline.setSelectedTimelineElementId).toHaveBeenCalledWith("card", { + preserveSet: true, + }); + harness.cleanup(); + }); +}); + +/** + * Adding a second element announced only that element, with preserveSet β€” and + * preserving a set that does not contain the id empties it. An empty timeline + * selection syncs back as "nothing is selected", so growing a group could wipe + * it instead, and so could re-resolving one after a move. + */ +describe("useDomSelection additive", () => { + it("announces both members when a second element joins the selection", () => { + const first = document.createElement("div"); + first.id = "card"; + const second = document.createElement("div"); + second.id = "chip"; + document.body.append(first, second); + const harness = renderHarness( + { activeCompPath: "index.html", projectId: "project-1", refreshKey: 0 }, + { timelineElements: [timelineElement("card"), timelineElement("chip")] }, + ); + + act(() => harness.current().applyDomSelection(makeSelection("Card", first))); + act(() => + harness.current().applyDomSelection(makeSelection("Chip", second), { additive: true }), + ); + + expect(harness.current().domEditGroupSelections).toHaveLength(2); + expect(harness.timeline.setTimelineSelectionSet).toHaveBeenLastCalledWith( + new Set(["card", "chip"]), + ); + expect(harness.timeline.setSelectedTimelineElementId).toHaveBeenLastCalledWith("chip", { + preserveSet: true, + }); + harness.cleanup(); + }); +}); + describe("useDomSelection", () => { it("clears a committed selection when the active composition path changes", () => { const { selection, harness } = setupSelectedHarness(); diff --git a/packages/studio/src/hooks/useDomSelection.ts b/packages/studio/src/hooks/useDomSelection.ts index 32682b0810..e24c34c637 100644 --- a/packages/studio/src/hooks/useDomSelection.ts +++ b/packages/studio/src/hooks/useDomSelection.ts @@ -4,11 +4,7 @@ import { getAllPreviewTargetsFromPointer, getPreviewTargetFromPointer, } from "../utils/studioPreviewHelpers"; -import { - findMatchingTimelineElementId, - findTimelineIdByAncestor, - type RightPanelTab, -} from "../utils/studioHelpers"; +import { type RightPanelTab } from "../utils/studioHelpers"; import { domEditSelectionsTargetSame, domEditSelectionInGroup, @@ -24,6 +20,8 @@ import { } from "../components/editor/domEditing"; import { reapplyPositionEditsAfterSeek } from "../components/editor/manualEdits"; import { useStudioTestHooks } from "./useStudioTestHooks"; +import { logSelect } from "../utils/selectDebug"; +import { announceTimelineSelection as announceSelectionToTimeline } from "./domSelectionTimelineMirror"; // ── Types ── @@ -48,6 +46,8 @@ export interface UseDomSelectionParams { previewIframeRef: React.MutableRefObject; timelineElements: TimelineElement[]; setSelectedTimelineElementId: (id: string | null, options?: SelectElementOptions) => void; + /** Publishes a whole multi-selection to the timeline; the anchor is set separately. */ + setTimelineSelectionSet: (ids: Set) => void; setRightCollapsed: (collapsed: boolean) => void; setRightPanelTab: (tab: RightPanelTab) => void; previewIframe: HTMLIFrameElement | null; @@ -110,6 +110,7 @@ export function useDomSelection({ previewIframeRef, timelineElements, setSelectedTimelineElementId, + setTimelineSelectionSet, setRightCollapsed, setRightPanelTab, previewIframe, @@ -145,6 +146,16 @@ export function useDomSelection({ // ── Callbacks ── + const announceTimelineSelection = useCallback( + (group: DomEditSelection[], primary: DomEditSelection | null) => + announceSelectionToTimeline( + { timelineElements, setSelectedTimelineElementId, setTimelineSelectionSet }, + group, + primary, + ), + [setSelectedTimelineElementId, setTimelineSelectionSet, timelineElements], + ); + const applyDomSelection = useCallback( // fallow-ignore-next-line complexity ( @@ -156,11 +167,12 @@ export function useDomSelection({ }, ) => { if (!selection) { + logSelect("clear", { hadGroup: domEditGroupSelectionsRef.current.length }); domEditSelectionRef.current = null; domEditGroupSelectionsRef.current = []; setDomEditSelection(null); setDomEditGroupSelections([]); - setSelectedTimelineElementId(null); + announceTimelineSelection([], null); return; } @@ -186,6 +198,13 @@ export function useDomSelection({ : (nextGroup[0] ?? null) : selection; + logSelect("apply", { + additive: isAdditiveSelection, + target: selection.selector ?? selection.id ?? null, + wasInGroup, + prevGroup: previousGroup.length, + nextGroup: nextGroup.length, + }); domEditSelectionRef.current = nextSelection; domEditGroupSelectionsRef.current = nextGroup; setDomEditSelection(nextSelection); @@ -208,21 +227,13 @@ export function useDomSelection({ setRightPanelTab("design"); } } - const nextSelectedTimelineId = - findMatchingTimelineElementId(nextSelection, timelineElements) ?? - findTimelineIdByAncestor( - nextSelection.element, - timelineElements, - nextSelection.sourceFile || "index.html", - ); - // Late marquee notify: a primary already in the live set must not collapse it. - setSelectedTimelineElementId(nextSelectedTimelineId, { preserveSet: true }); + announceTimelineSelection(nextGroup, nextSelection); return; } - setSelectedTimelineElementId(null); + announceTimelineSelection([], null); }, - [setSelectedTimelineElementId, timelineElements, setRightCollapsed, setRightPanelTab], + [announceTimelineSelection, setRightCollapsed, setRightPanelTab], ); const clearDomSelection = useCallback(() => { @@ -375,6 +386,13 @@ export function useDomSelection({ [applyDomSelection, buildDomSelectionForTimelineElement], ); + // Forward handle to the group refresher defined below: the single-selection + // refresher falls back to it when the primary is gone, and a ref keeps that from + // forcing either callback to be declared in the other's dependency list. + const refreshDomEditGroupSelectionsFromPreviewRef = useRef< + (selections: DomEditSelection[]) => Promise + >(async () => {}); + const refreshDomEditSelectionFromPreview = useCallback( // fallow-ignore-next-line complexity async (selection: DomEditSelection) => { @@ -389,6 +407,17 @@ export function useDomSelection({ const element = findElementForSelection(doc, selection, activeCompPath); if (!element) { + // Losing the primary is not losing the selection. When a group is live, + // re-resolve it and keep whoever still exists rather than wiping the lot. + const group = domEditGroupSelectionsRef.current; + logSelect("refresh-lost", { + target: selection.selector ?? selection.id ?? null, + group: group.length, + }); + if (group.length > 1) { + await refreshDomEditGroupSelectionsFromPreviewRef.current(group); + return; + } applyDomSelection(null, { revealPanel: false }); return; } @@ -436,23 +465,13 @@ export function useDomSelection({ setDomEditSelection(nextSelection); setDomEditGroupSelections(nextGroup); - if (nextSelection) { - setSelectedTimelineElementId( - findMatchingTimelineElementId(nextSelection, timelineElements), - ); - } else { - setSelectedTimelineElementId(null); - } + announceTimelineSelection(nextGroup, nextSelection); }, - [ - activeCompPath, - buildDomSelectionFromTarget, - setSelectedTimelineElementId, - timelineElements, - previewIframeRef, - ], + [activeCompPath, announceTimelineSelection, buildDomSelectionFromTarget, previewIframeRef], ); + refreshDomEditGroupSelectionsFromPreviewRef.current = refreshDomEditGroupSelectionsFromPreview; + // ── Effects ── // Clear hover unconditionally on composition/project/preview change @@ -503,6 +522,7 @@ export function useDomSelection({ const applyMarqueeSelection = useCallback( // fallow-ignore-next-line complexity (selections: DomEditSelection[], additive: boolean) => { + logSelect("marquee", { hits: selections.length, additive }); if (selections.length === 0) { if (!additive) applyDomSelection(null, { revealPanel: false }); return; @@ -527,16 +547,9 @@ export function useDomSelection({ domEditGroupSelectionsRef.current = nextGroup; setDomEditSelection(nextSelection); setDomEditGroupSelections(nextGroup); - const nextTimelineId = - findMatchingTimelineElementId(nextSelection, timelineElements) ?? - findTimelineIdByAncestor( - nextSelection.element, - timelineElements, - nextSelection.sourceFile || "index.html", - ); - setSelectedTimelineElementId(nextTimelineId); + announceTimelineSelection(nextGroup, nextSelection); }, - [applyDomSelection, timelineElements, setSelectedTimelineElementId], + [applyDomSelection, announceTimelineSelection], ); return { diff --git a/packages/studio/src/hooks/useDomSelectionSelectionGuards.test.ts b/packages/studio/src/hooks/useDomSelectionSelectionGuards.test.ts index dce0655946..dfed3d834c 100644 --- a/packages/studio/src/hooks/useDomSelectionSelectionGuards.test.ts +++ b/packages/studio/src/hooks/useDomSelectionSelectionGuards.test.ts @@ -49,6 +49,7 @@ interface HarnessProps { iframe: HTMLIFrameElement | null; timelineElements: TimelineElement[]; setSelectedTimelineElementId?: (id: string | null, options?: SelectElementOptions) => void; + setTimelineSelectionSet?: (ids: Set) => void; } function renderHarness(props: HarnessProps) { @@ -67,6 +68,8 @@ function renderHarness(props: HarnessProps) { previewIframeRef: { current: props.iframe }, timelineElements: props.timelineElements, setSelectedTimelineElementId: props.setSelectedTimelineElementId ?? vi.fn(), + setTimelineSelectionSet: + props.setTimelineSelectionSet ?? usePlayerStore.getState().setSelectedElementIds, setRightCollapsed: vi.fn(), setRightPanelTab: props.setRightPanelTab, previewIframe: props.iframe, diff --git a/packages/studio/src/hooks/useElementLifecycleOps.ts b/packages/studio/src/hooks/useElementLifecycleOps.ts index 4e17c5af36..3df6bf402e 100644 --- a/packages/studio/src/hooks/useElementLifecycleOps.ts +++ b/packages/studio/src/hooks/useElementLifecycleOps.ts @@ -21,6 +21,7 @@ import { } from "../components/editor/useLayerRevealOverride"; import type { CommitDomEditPatchBatches, DomEditPatchBatch } from "./domEditCommitTypes"; import { cutoverCommittedOrThrow, type CutoverResult } from "../utils/sdkCutover"; +import { studioWriteHeaders } from "../utils/studioFileVersion"; interface UseElementLifecycleOpsParams extends DomEditCommitBaseParams { /** Route delete through SDK when session resolves the hf-id. */ @@ -115,7 +116,7 @@ export function useElementLifecycleOps({ `/api/projects/${pid}/file-mutations/remove-element/${encodeURIComponent(targetPath)}`, { method: "POST", - headers: { "Content-Type": "application/json" }, + headers: { "Content-Type": "application/json", ...studioWriteHeaders() }, body: JSON.stringify({ target: patchTarget }), }, ); diff --git a/packages/studio/src/hooks/useExternalFileChangeCoordinator.ts b/packages/studio/src/hooks/useExternalFileChangeCoordinator.ts index 4d16842859..b1de697b09 100644 --- a/packages/studio/src/hooks/useExternalFileChangeCoordinator.ts +++ b/packages/studio/src/hooks/useExternalFileChangeCoordinator.ts @@ -4,6 +4,7 @@ import { StudioFileConflictError } from "../utils/studioSaveDiagnostics"; import type { ExternalConflictSnapshot } from "../utils/externalConflictStorage"; import { isSelfWriteEcho } from "./sdkSelfWriteRegistry"; import { consumeStudioWriteToken } from "../utils/studioFileVersion"; +import { logReload } from "../utils/reloadDebug"; type ExternalChangeDrainResult = | { status: "clean" } @@ -194,6 +195,7 @@ export function useExternalFileChangeCoordinator({ const reloadAcceptedGeneration = useCallback( (path: string) => { + logReload("reload", { path, by: "external-change coordinator" }); reloadPreview(); reloadSdkSession(path); }, @@ -221,11 +223,22 @@ export function useExternalFileChangeCoordinator({ pendingTimelinePaths.delete(path); const content = readFileChangeContent(payload); - if (consumeStudioWriteToken(readFileChangeWriteToken(payload))) return; - if (content != null && isSelfWriteEcho(path, content)) return; + const token = readFileChangeWriteToken(payload); + logReload("file-change", { path, token: token ?? null, hasContent: content != null }); + if (consumeStudioWriteToken(token)) { + logReload("suppressed", { path, why: "own write token" }); + return; + } + if (content != null && isSelfWriteEcho(path, content)) { + logReload("suppressed", { path, why: "own content echo" }); + return; + } const identity = eventIdentity(path, payload); - if (!allowDuplicate && identity != null && identity === lastEventIdentityRef.current) return; + if (!allowDuplicate && identity != null && identity === lastEventIdentityRef.current) { + logReload("suppressed", { path, why: "duplicate event" }); + return; + } lastEventIdentityRef.current = identity; const generation = ++generationRef.current; const result = await drainPendingChanges(); diff --git a/packages/studio/src/hooks/useGroupCommits.ts b/packages/studio/src/hooks/useGroupCommits.ts index 6e838f1dbe..1ac24bd14e 100644 --- a/packages/studio/src/hooks/useGroupCommits.ts +++ b/packages/studio/src/hooks/useGroupCommits.ts @@ -5,6 +5,7 @@ import { type DomEditCommitBaseParams, } from "../utils/studioFileHistory"; import { buildDomEditPatchTarget, type DomEditSelection } from "../components/editor/domEditing"; +import { studioWriteHeaders } from "../utils/studioFileVersion"; interface UseGroupCommitsParams extends DomEditCommitBaseParams { /** Resync the SDK session after a server-side write (the wrapper/unwrap changes @@ -75,7 +76,7 @@ async function commitStructuralMutation( `/api/projects/${pid}/file-mutations/${route}/${encodeURIComponent(targetPath)}`, { method: "POST", - headers: { "Content-Type": "application/json" }, + headers: { "Content-Type": "application/json", ...studioWriteHeaders() }, body: JSON.stringify(body), }, ); diff --git a/packages/studio/src/hooks/useGsapAwareEditing.ts b/packages/studio/src/hooks/useGsapAwareEditing.ts index 0e84c15826..d23ae3b020 100644 --- a/packages/studio/src/hooks/useGsapAwareEditing.ts +++ b/packages/studio/src/hooks/useGsapAwareEditing.ts @@ -24,7 +24,11 @@ import { useGsapSaveFailureTelemetry, useSafeGsapCommitMutation, } from "./useSafeGsapCommitMutation"; -import type { CommitMutation } from "./gsapScriptCommitTypes"; +import type { + CommitMutation, + CommitMutationCall, + CommitMutationOptions, +} from "./gsapScriptCommitTypes"; import { setElementGsapPosition } from "../utils/elementGsap"; import { logResize, logResizeSettle } from "../utils/resizeDebug"; import type { DomEditGroupPathOffsetCommit } from "../components/editor/DomEditOverlay"; @@ -155,37 +159,73 @@ export function useGsapAwareEditing({ // it survives the N sequential server round-trips) onto each commit β€” // otherwise each member records its own entry and it takes N presses to undo. const coalesceKey = `group-drag:${++groupDragCommitCounter}`; - const coalescedCommit: typeof gsapCommitMutation = (selection, mutation, options) => - gsapCommitMutation(selection, mutation, { - ...options, - coalesceKey, - coalesceMs: Number.POSITIVE_INFINITY, + // Members are written one at a time, and a write that re-renders the preview + // re-runs the whole script β€” which still holds the OLD position of every + // member not yet written. Those members snap back to where they started and + // stay there until their own write lands, which is the single element seen + // jumping mid-commit while the rest of the group sat still. The drafted + // positions are already on screen, so holding the render until the last + // member has been written costs nothing and never shows a half-moved group. + let renderOnCommit = false; + const withGroupOptions = (options: CommitMutationOptions): CommitMutationOptions => ({ + ...options, + coalesceKey, + coalesceMs: Number.POSITIVE_INFINITY, + deferPreviewSync: !renderOnCommit, + }); + // Every member writes the same file. Queue their mutations and send them as + // ONE request instead of one round trip per member: the server reads, parses + // and writes the composition once, and the preview patches once. + const queued: CommitMutationCall[] = []; + const flushQueued = async () => { + if (queued.length === 0) return; + const calls = queued.splice(0, queued.length); + if (!gsapCommitMutation.batch) { + for (const call of calls) { + await gsapCommitMutation(call.selection, call.mutation, call.options); + } + return; + } + await gsapCommitMutation.batch(calls, { + ...(calls.at(-1)?.options ?? { label: "Move animated layer (group)" }), + label: "Move animated layer (group)", }); + }; + const coalescedCommit: typeof gsapCommitMutation = (selection, mutation, options) => { + queued.push({ selection, mutation, options: withGroupOptions(options) }); + return Promise.resolve(); + }; const preflightAnimations = new Map(); // Editability is user-atomic: prove every member can be written before // the first source mutation. Network failures after this point retain the // existing multi-request semantics, but a blocked member can never leave // earlier siblings partially moved. - for (const { selection } of updates) { - try { - const animations = await makeFetchFallback(selection, { failOnFetchError: true })(); - preflightAnimations.set(selection, animations); - const outcome = await tryGsapDragIntercept( - selection, - { x: 0, y: 0 }, - animations, - previewIframeRef.current, - coalescedCommit, - undefined, - { preflightOnly: true }, - ); - assertGsapEditPersisted(outcome); - } catch (error) { - trackGsapInteractionFailure(error, selection, "drag", "Move animated layer (group)"); - throw error; - } - } - for (const { selection, next } of updates) { + // Every member reads the same file, and a preflight writes nothing β€” so run + // them together. The parse layer shares one in-flight request per file, which + // turns N sequential round trips into one. + await Promise.all( + updates.map(async ({ selection }) => { + try { + const animations = await makeFetchFallback(selection, { failOnFetchError: true })(); + preflightAnimations.set(selection, animations); + const outcome = await tryGsapDragIntercept( + selection, + { x: 0, y: 0 }, + animations, + previewIframeRef.current, + coalescedCommit, + undefined, + { preflightOnly: true }, + ); + assertGsapEditPersisted(outcome); + } catch (error) { + trackGsapInteractionFailure(error, selection, "drag", "Move animated layer (group)"); + throw error; + } + }), + ); + for (const [index, { selection, next }] of updates.entries()) { + renderOnCommit = index === updates.length - 1; try { const outcome = await tryGsapDragIntercept( selection, @@ -193,7 +233,13 @@ export function useGsapAwareEditing({ preflightAnimations.get(selection) ?? [], previewIframeRef.current, coalescedCommit, - makeFetchFallback(selection), + // The intercept re-reads the file to resolve a stale or shared tween. + // Anything already queued has to be on disk before that read, or it + // resolves against a file missing writes it is about to build on. + async () => { + await flushQueued(); + return makeFetchFallback(selection)(); + }, { preflightPassed: true }, ); assertGsapEditPersisted(outcome); @@ -202,6 +248,15 @@ export function useGsapAwareEditing({ throw error; } } + try { + await flushQueued(); + } catch (error) { + const selection = updates.at(-1)?.selection; + if (selection) { + trackGsapInteractionFailure(error, selection, "drag", "Move animated layer (group)"); + } + throw error; + } }, [gsapCommitMutation, previewIframeRef, makeFetchFallback, trackGsapInteractionFailure], ); diff --git a/packages/studio/src/hooks/useGsapScriptCommits.test.tsx b/packages/studio/src/hooks/useGsapScriptCommits.test.tsx index 6236d032ab..76ca2129da 100644 --- a/packages/studio/src/hooks/useGsapScriptCommits.test.tsx +++ b/packages/studio/src/hooks/useGsapScriptCommits.test.tsx @@ -73,14 +73,77 @@ describe("applyPreviewSync", () => { syncDragPreview(result(), reloadPreview); - expect(patchRuntimeTweenInPlace).toHaveBeenCalledWith(FAKE_IFRAME, "#a", { - kind: "set", - props: { x: 10 }, - }); + expect(patchRuntimeTweenInPlace).toHaveBeenCalledWith( + FAKE_IFRAME, + "#a", + { + kind: "set", + props: { x: 10 }, + }, + undefined, + false, + ); + expect(applySoftReload).not.toHaveBeenCalled(); + expect(reloadPreview).not.toHaveBeenCalled(); + }); + + it("instantPatches: patches every element the batch wrote, rendering once at the end", () => { + patchRuntimeTweenInPlace.mockReturnValue(true); + const reloadPreview = vi.fn(); + + applyPreviewSync( + FAKE_IFRAME, + result(), + { + label: "Move animated layer (group)", + softReload: true, + instantPatches: [ + { selector: "#a", change: { kind: "set" as const, props: { x: 1 } } }, + { selector: "#b", change: { kind: "set" as const, props: { x: 2 } } }, + { selector: "#c", change: { kind: "set" as const, props: { x: 3 } } }, + ], + }, + reloadPreview, + ); + + // Only the last patch re-renders β€” the earlier two defer their seek, so the + // group repaints once instead of once per member. + expect(patchRuntimeTweenInPlace.mock.calls.map((call) => [call[1], call[4]])).toEqual([ + ["#a", true], + ["#b", true], + ["#c", false], + ]); expect(applySoftReload).not.toHaveBeenCalled(); expect(reloadPreview).not.toHaveBeenCalled(); }); + it("instantPatches: one patch that misses falls the whole batch back to the reload", () => { + patchRuntimeTweenInPlace.mockImplementation((_iframe, selector) => selector !== "#b"); + applySoftReload.mockReturnValue("applied"); + const reloadPreview = vi.fn(); + + applyPreviewSync( + FAKE_IFRAME, + result({ scriptText: "SCRIPT" }), + { + label: "Move animated layer (group)", + softReload: true, + instantPatches: [ + { selector: "#a", change: { kind: "set" as const, props: { x: 1 } } }, + { selector: "#b", change: { kind: "set" as const, props: { x: 2 } } }, + ], + }, + reloadPreview, + ); + + // A half-patched preview is worse than a reloaded one: "#a" landed, "#b" did + // not, so the reload repaints both from the written source. + expect(applySoftReload).toHaveBeenCalled(); + expect(trackStudioEvent).toHaveBeenCalledWith("gsap_instant_patch_fallback", { + selector: "#b", + }); + }); + it("instantPatch + patch fails: falls back to the soft reload, passing onAsyncFailure", () => { patchRuntimeTweenInPlace.mockReturnValue(false); applySoftReload.mockReturnValue("applied"); @@ -338,10 +401,16 @@ describe("runCommit β€” instantPatch wiring", () => { // The file already matched (changed:false) but the runtime patch deferred // from the paired first commit must still land. - expect(patchRuntimeTweenInPlace).toHaveBeenCalledWith(FAKE_IFRAME, "#a", { - kind: "set", - props: { x: 485, y: 311 }, - }); + expect(patchRuntimeTweenInPlace).toHaveBeenCalledWith( + FAKE_IFRAME, + "#a", + { + kind: "set", + props: { x: 485, y: 311 }, + }, + undefined, + false, + ); expect(deps.reloadPreview).not.toHaveBeenCalled(); }); diff --git a/packages/studio/src/hooks/useGsapScriptCommits.ts b/packages/studio/src/hooks/useGsapScriptCommits.ts index b4b7d6f636..fb042945d6 100644 --- a/packages/studio/src/hooks/useGsapScriptCommits.ts +++ b/packages/studio/src/hooks/useGsapScriptCommits.ts @@ -36,6 +36,7 @@ import { useGsapSaveFailureTelemetry, useSafeGsapCommitMutation, } from "./useSafeGsapCommitMutation"; +import { studioWriteHeaders } from "../utils/studioFileVersion"; async function mutateGsapScript( projectId: string, @@ -46,7 +47,7 @@ async function mutateGsapScript( `/api/projects/${encodeURIComponent(projectId)}/gsap-mutations/${encodeURIComponent(sourceFile)}`, { method: "POST", - headers: { "Content-Type": "application/json" }, + headers: { "Content-Type": "application/json", ...studioWriteHeaders() }, body: JSON.stringify(mutation), }, ); @@ -65,7 +66,7 @@ async function mutateGsapScriptBatch( `/api/projects/${encodeURIComponent(projectId)}/gsap-mutations-batch/${encodeURIComponent(sourceFile)}`, { method: "POST", - headers: { "Content-Type": "application/json" }, + headers: { "Content-Type": "application/json", ...studioWriteHeaders() }, body: JSON.stringify({ mutations }), }, ); @@ -248,19 +249,28 @@ export function applyPreviewSync( options: CommitMutationOptions, reloadPreview: () => void, ): void { - if (options.instantPatch) { - const patched = patchRuntimeTweenInPlace( - iframe, - options.instantPatch.selector, - options.instantPatch.change, + const patches = options.instantPatches ?? (options.instantPatch ? [options.instantPatch] : []); + if (patches.length > 0) { + const deferSeek = options.deferPreviewSync === true; + const missed = patches.find( + (patch, index) => + !patchRuntimeTweenInPlace( + iframe, + patch.selector, + patch.change, + undefined, + deferSeek || index < patches.length - 1, + ), ); - // Patched in place β€” element is already correct on screen; no reload needed. - if (patched) return; + // Patched in place β€” elements are already correct on screen; no reload needed. + if (!missed) return; // The instant path couldn't patch in place β€” record the fallback so we can // track how often the fast path misses before the soft/full reload below. - trackStudioEvent("gsap_instant_patch_fallback", { selector: options.instantPatch.selector }); + trackStudioEvent("gsap_instant_patch_fallback", { selector: missed.selector }); // Fall through to the soft/full reload path below. } + // Written, but the caller has more writes to make and will render after the last. + if (options.deferPreviewSync) return; if (options.softReload && result.scriptText) { // A soft-reloadable edit escalates to a full iframe remount ONLY on the // PERMANENT "cannot-soft-reload" result (the preview is genuinely stale/ @@ -355,7 +365,12 @@ export function useGsapScriptCommits({ projectIdRef, activeCompPath, previewIfra ); if (!result) return; options.onResult?.(result); - await finalizeSuccessfulMutation(pid, compositionPath, last.selection, last.mutation, targetPath, result, options); + // Each call brings its own fast-path patch; the batch wrote them all, so the + // preview sync applies them all rather than just the last call's. + const instantPatches = calls + .map(({ options: callOptions }) => callOptions.instantPatch) + .filter((patch) => patch !== undefined); + await finalizeSuccessfulMutation(pid, compositionPath, last.selection, last.mutation, targetPath, result, instantPatches.length > 0 ? { ...options, instantPatches } : options); }, [showToast, finalizeSuccessfulMutation]); // Every GSAP-script commit is a read-modify-write of one file. Overlapping diff --git a/packages/studio/src/hooks/useInlineTextEdit.test.tsx b/packages/studio/src/hooks/useInlineTextEdit.test.tsx new file mode 100644 index 0000000000..125980fd99 --- /dev/null +++ b/packages/studio/src/hooks/useInlineTextEdit.test.tsx @@ -0,0 +1,432 @@ +// @vitest-environment happy-dom + +import React, { act } from "react"; +import { createRoot } from "react-dom/client"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import { useInlineTextEdit, type InlineTextEditControls } from "./useInlineTextEdit"; + +(globalThis as unknown as { IS_REACT_ACT_ENVIRONMENT: boolean }).IS_REACT_ACT_ENVIRONMENT = true; + +afterEach(() => { + document.body.innerHTML = ""; +}); + +/** A heading in the document, standing in for one in the preview. */ +function heading(text = "Motion Playground"): HTMLElement { + const element = document.createElement("h1"); + element.textContent = text; + document.body.append(element); + return element; +} + +function mount(onCommit = vi.fn(), onPause = vi.fn()) { + const controls: { current: InlineTextEditControls | null } = { current: null }; + function Probe() { + controls.current = useInlineTextEdit({ onCommit, onPause }); + return null; + } + const host = document.createElement("div"); + document.body.append(host); + const root = createRoot(host); + act(() => root.render()); + return { controls: () => controls.current!, root, onCommit, onPause }; +} + +describe("useInlineTextEdit", () => { + // Selecting the whole text would mean the next keystroke destroys it, which + // is a bad thing to do to someone who double-clicked to fix a typo. + it("leaves the caret after the last character, with nothing selected", async () => { + const element = heading("Motion Playground"); + const { controls, root } = mount(); + + act(() => { + controls().start(element); + }); + await act(async () => { + await new Promise((resolve) => requestAnimationFrame(() => resolve(null))); + }); + + const selection = document.getSelection()!; + expect(selection.toString()).toBe(""); + expect(selection.isCollapsed).toBe(true); + expect(selection.anchorOffset).toBe(element.textContent!.length); + act(() => root.unmount()); + }); + + it("makes the element editable, and focuses it once the press has finished", async () => { + const element = heading(); + const { controls, root, onPause } = mount(); + + act(() => { + controls().start(element); + }); + + // Not `plaintext-only`: that would make it impossible to give three + // characters a colour, which is the point of editing in the composition. + expect(element.getAttribute("contenteditable")).toBe("true"); + expect(controls().session?.element).toBe(element); + // The frame being edited is the one the user chose to edit on. + expect(onPause).toHaveBeenCalledTimes(1); + + // Focus lands on the next frame, after the press that opened this and the + // click that follows it have both been and gone. + await act(async () => { + await new Promise((resolve) => requestAnimationFrame(() => resolve(null))); + }); + expect(document.activeElement).toBe(element); + act(() => root.unmount()); + }); + + it("hands the current text over exactly once when it commits", () => { + const element = heading(); + const { controls, root, onCommit } = mount(); + + act(() => { + controls().start(element); + }); + element.textContent = "Motion Playground Live"; + act(() => controls().commit()); + + expect(onCommit.mock.calls).toEqual([["Motion Playground Live"]]); + act(() => root.unmount()); + }); + + // Cancelling must leave the preview exactly as it found it: the element was + // being mutated live, and nothing was persisted. + it("puts the original text back on cancel, and commits nothing", () => { + const element = heading("Motion Playground"); + const { controls, root, onCommit } = mount(); + + act(() => { + controls().start(element); + }); + element.textContent = "half-typed"; + act(() => controls().cancel()); + + expect(element.textContent).toBe("Motion Playground"); + expect(onCommit).not.toHaveBeenCalled(); + act(() => root.unmount()); + }); + + it.each([ + ["commit", (c: InlineTextEditControls) => c.commit()], + ["cancel", (c: InlineTextEditControls) => c.cancel()], + ])("stops being editable after %s", (_name, close) => { + const element = heading(); + const { controls, root } = mount(); + + act(() => { + controls().start(element); + }); + act(() => close(controls())); + + expect(element.hasAttribute("contenteditable")).toBe(false); + expect(controls().session).toBeNull(); + act(() => root.unmount()); + }); + + // A session that failed to close would leave the canvas unable to select. + it("tears down once when it is closed twice", () => { + const element = heading(); + const { controls, root, onCommit } = mount(); + + act(() => { + controls().start(element); + }); + act(() => controls().commit()); + act(() => controls().commit()); + + expect(onCommit).toHaveBeenCalledTimes(1); + act(() => root.unmount()); + }); + + it("refuses to open a second session over an open one", () => { + const first = heading("first"); + const second = heading("second"); + const { controls, root } = mount(); + + let opened = false; + act(() => { + controls().start(first); + opened = controls().start(second); + }); + + expect(opened).toBe(false); + expect(controls().session?.element).toBe(first); + expect(second.hasAttribute("contenteditable")).toBe(false); + act(() => root.unmount()); + }); + + // The composition reloads while an edit is open often enough to matter. + it("closes without throwing when the element has left the document", () => { + const element = heading(); + const { controls, root } = mount(); + + act(() => { + controls().start(element); + }); + element.remove(); + + expect(() => act(() => controls().cancel())).not.toThrow(); + expect(controls().session).toBeNull(); + act(() => root.unmount()); + }); + + describe("the keys that end it", () => { + function press(element: HTMLElement, key: string, shiftKey = false) { + act(() => { + element.dispatchEvent(new KeyboardEvent("keydown", { key, shiftKey, bubbles: true })); + }); + } + + it("commits on Enter", () => { + const element = heading(); + const { controls, root, onCommit } = mount(); + act(() => { + controls().start(element); + }); + + element.textContent = "Renamed"; + press(element, "Enter"); + + expect(onCommit.mock.calls).toEqual([["Renamed"]]); + expect(controls().session).toBeNull(); + act(() => root.unmount()); + }); + + // A multi-line element still needs a way to get a line break. + it("leaves Shift+Enter alone", () => { + const element = heading(); + const { controls, root, onCommit } = mount(); + act(() => { + controls().start(element); + }); + + press(element, "Enter", true); + + expect(onCommit).not.toHaveBeenCalled(); + expect(controls().session).not.toBeNull(); + act(() => root.unmount()); + }); + + it("cancels on Escape", () => { + const element = heading("Original"); + const { controls, root, onCommit } = mount(); + act(() => { + controls().start(element); + }); + + element.textContent = "half-typed"; + press(element, "Escape"); + + expect(element.textContent).toBe("Original"); + expect(onCommit).not.toHaveBeenCalled(); + act(() => root.unmount()); + }); + + it("commits when the element loses focus", () => { + const element = heading(); + const { controls, root, onCommit } = mount(); + act(() => { + controls().start(element); + }); + + element.textContent = "Clicked away"; + act(() => element.dispatchEvent(new FocusEvent("blur"))); + + expect(onCommit.mock.calls).toEqual([["Clicked away"]]); + act(() => root.unmount()); + }); + + it("stops listening once the session is over", () => { + const element = heading(); + const { controls, root, onCommit } = mount(); + act(() => { + controls().start(element); + }); + act(() => controls().commit()); + + press(element, "Enter"); + + expect(onCommit).toHaveBeenCalledTimes(1); + act(() => root.unmount()); + }); + }); + + // The mark that says the caret is in the text, not that the element is + // selected. It has to live in the same document as the caret to read so. + it("outlines the element while it is being edited, and puts it back after", () => { + const element = heading(); + element.style.outline = "1px dotted red"; + const { controls, root } = mount(); + + act(() => { + controls().start(element); + }); + // Serialisation order is the browser's; what matters is that it is the + // accent, solid, and thicker than whatever it replaced. + expect(element.style.outline).toContain("#3CE6AC"); + expect(element.style.outline).toContain("2px"); + + act(() => controls().cancel()); + expect(element.style.outline).toContain("dotted"); + expect(element.style.outline).toContain("red"); + expect(element.style.getPropertyValue("outline-offset")).toBe(""); + act(() => root.unmount()); + }); + + // Opening on a point is what makes this feel like text rather than a dialog. + it("opens the caret where the press landed when it can resolve one", async () => { + const element = heading("Motion Playground"); + const range = document.createRange(); + range.setStart(element.firstChild!, 6); + range.collapse(true); + const doc = document as Document & { caretRangeFromPoint?: unknown }; + const original = doc.caretRangeFromPoint; + doc.caretRangeFromPoint = () => range; + + const { controls, root } = mount(); + act(() => { + controls().start(element, { x: 120, y: 40 }); + }); + await act(async () => { + await new Promise((resolve) => requestAnimationFrame(() => resolve(null))); + }); + + expect(document.getSelection()?.anchorOffset).toBe(6); + doc.caretRangeFromPoint = original; + act(() => root.unmount()); + }); + + // A point that resolves outside the element (a rotated glyph, a gap) must + // not put the caret in someone else's text. + it("falls back to the end when the point lands outside the element", async () => { + const element = heading("Motion Playground"); + const stranger = heading("Somewhere else"); + const strayRange = document.createRange(); + strayRange.setStart(stranger.firstChild!, 3); + strayRange.collapse(true); + const doc = document as Document & { caretRangeFromPoint?: unknown }; + const original = doc.caretRangeFromPoint; + doc.caretRangeFromPoint = () => strayRange; + + const { controls, root } = mount(); + act(() => { + controls().start(element, { x: 9999, y: 9999 }); + }); + await act(async () => { + await new Promise((resolve) => requestAnimationFrame(() => resolve(null))); + }); + + const selection = document.getSelection()!; + expect(selection.anchorNode?.parentElement).toBe(element); + expect(selection.anchorOffset).toBe(element.textContent!.length); + doc.caretRangeFromPoint = original; + act(() => root.unmount()); + }); + + // Double click takes the word and triple click takes the lot, in this element + // exactly as in any other text field, because nothing here interferes with + // either. Claiming the double click for select-all cost the word selection. + it("leaves double and triple click to the browser", async () => { + const element = heading("Motion Playground"); + const { controls, root } = mount(); + act(() => { + controls().start(element); + }); + await act(async () => { + await new Promise((resolve) => requestAnimationFrame(() => resolve(null))); + }); + const before = document.getSelection()?.toString(); + + act(() => element.dispatchEvent(new MouseEvent("dblclick", { bubbles: true }))); + + // No handler ran, so the selection is untouched: the real browser would + // have set it to the word under the pointer before this ever fired. + expect(document.getSelection()?.toString()).toBe(before); + act(() => root.unmount()); + }); +}); + +// Styling a run of characters is what the element is edited in place for, and +// it only counts once the markup survives the trip out of the element. +describe("useInlineTextEdit with styled runs", () => { + it("hands over the markup, not just the words", () => { + const element = heading("hello"); + const { controls, onCommit, root } = mount(); + + act(() => { + controls().start(element); + }); + element.innerHTML = 'hello'; + act(() => controls().commit()); + + expect(onCommit).toHaveBeenCalledWith('hello'); + act(() => root.unmount()); + }); + + it("cleans what it hands over, so the preview shows what will be saved", () => { + const element = heading("hello"); + const { controls, onCommit, root } = mount(); + + act(() => { + controls().start(element); + }); + element.innerHTML = 'hi'; + act(() => controls().commit()); + + expect(onCommit).toHaveBeenCalledWith("hi"); + // Cleaned in the element too, not only on the way out. + expect(element.innerHTML).toBe("hi"); + act(() => root.unmount()); + }); + + it("puts the styling back on cancel, not just the letters", () => { + const element = heading(); + element.innerHTML = 'before'; + const { controls, onCommit, root } = mount(); + + act(() => { + controls().start(element); + }); + element.innerHTML = "after"; + act(() => controls().cancel()); + + expect(element.innerHTML).toBe('before'); + expect(onCommit).not.toHaveBeenCalled(); + act(() => root.unmount()); + }); + + it("keeps the element markup-free when nothing was styled", () => { + const element = heading("plain words"); + const { controls, onCommit, root } = mount(); + + act(() => { + controls().start(element); + }); + act(() => controls().commit()); + + expect(onCommit).toHaveBeenCalledWith("plain words"); + act(() => root.unmount()); + }); + + it("pastes the words, not the page they came from", () => { + const element = heading("hi"); + const { controls, root } = mount(); + act(() => { + controls().start(element); + }); + + const insertText = vi.fn(); + (document as Document & { execCommand: unknown }).execCommand = insertText; + const paste = new Event("paste", { bubbles: true, cancelable: true }) as ClipboardEvent; + Object.defineProperty(paste, "clipboardData", { + value: { getData: (type: string) => (type === "text/plain" ? "pasted" : "pasted") }, + }); + act(() => void element.dispatchEvent(paste)); + + expect(paste.defaultPrevented).toBe(true); + expect(insertText).toHaveBeenCalledWith("insertText", false, "pasted"); + act(() => root.unmount()); + }); +}); diff --git a/packages/studio/src/hooks/useInlineTextEdit.ts b/packages/studio/src/hooks/useInlineTextEdit.ts new file mode 100644 index 0000000000..3282bbbc94 --- /dev/null +++ b/packages/studio/src/hooks/useInlineTextEdit.ts @@ -0,0 +1,265 @@ +import { useCallback, useEffect, useRef, useState } from "react"; +import { sanitizeRichTextChildren } from "@hyperframes/core/rich-text-sanitize"; + +/** + * Editing an element's text where it sits, in the composition itself. + * + * The alternative is an input positioned over the element, which has to + * reproduce its font, size, weight, spacing, colour, alignment and wrapping to + * look right, and is subtly wrong the moment any of those is missed. The + * preview is a same-origin document holding the real element, and the commit + * path already mutates that exact node, so the element is both the most + * accurate surface to type into and the one the rest of the code understands. + * + * The session owns the element's editable state for its whole life, and tears + * down the same way whichever way it ends. A session that failed to close + * would leave the canvas unable to select anything. + */ + +/** + * `true`, not `plaintext-only`. + * + * `plaintext-only` was what kept a text edit from becoming a structural one, + * and it also made it impossible to give three characters a colour, which is + * the point of editing in the composition rather than in a field. The guard it + * was providing is rebuilt as two narrower ones that do not cost the feature: + * paste arrives as plain text, and what leaves the element goes through the + * sanitiser before anyone writes it to a file. + */ +const EDITABLE = "true"; +/** Studio's accent, so the mark belongs to Studio rather than to the design. */ +const EDITING_OUTLINE = "2px solid #3CE6AC"; + +export interface InlineTextEditSession { + element: HTMLElement; + /** + * The element's markup when editing started, for putting back on cancel. + * Markup rather than text: cancelling an edit that recoloured a word has to + * restore the colours it replaced, not just the letters. + */ + original: string; + /** The element's own outline, to put back when the session ends. */ + outline: string; +} + +export interface InlineTextEditControls { + session: InlineTextEditSession | null; + /** + * Begin editing this element. `caretAt` is a point in the element's own + * document, so the caret can open where the user pointed rather than at a + * fixed end. Returns false when a session is already open. + */ + start: (element: HTMLElement, caretAt?: { x: number; y: number }) => boolean; + /** Hand the current text to the commit function and close. */ + commit: () => void; + /** Put the original text back and close, persisting nothing. */ + cancel: () => void; +} + +export function useInlineTextEdit({ + onCommit, + onPause, +}: { + /** Where the edited text goes. The caller owns persistence. */ + onCommit: (text: string) => void; + /** Stop playback, so the element is not animating under the caret. */ + onPause?: () => void; +}): InlineTextEditControls { + const [session, setSession] = useState(null); + // The teardown reads this rather than the state, so an exit path that runs + // before React re-renders still sees the element it has to clean up. + const openRef = useRef(null); + /** The pending caret placement, so a session that closes first can drop it. */ + const framesRef = useRef(null); + + const teardown = useCallback((): InlineTextEditSession | null => { + const open = openRef.current; + if (!open) return null; + if (framesRef.current !== null) { + open.element.ownerDocument.defaultView?.cancelAnimationFrame(framesRef.current); + framesRef.current = null; + } + openRef.current = null; + setSession(null); + // An element removed from the document mid-session is not an error, it is + // just nothing left to clean up. + if (open.element.isConnected) { + open.element.removeAttribute("contenteditable"); + // Restored rather than cleared: the composition may have authored one. + open.element.style.outline = open.outline; + open.element.style.removeProperty("outline-offset"); + open.element.blur(); + } + return open; + }, []); + + const start = useCallback( + (element: HTMLElement, caretAt?: { x: number; y: number }): boolean => { + if (openRef.current) return false; + + const open = { + element, + original: element.innerHTML, + outline: element.style.outline, + }; + // Drawn on the element itself, not in Studio's overlay above it. This is + // the only mark that says the caret is in the TEXT rather than the + // element being selected, and it has to sit in the same document as the + // caret to read that way. + element.style.outline = EDITING_OUTLINE; + element.style.outlineOffset = "2px"; + openRef.current = open; + setSession(open); + onPause?.(); + + element.setAttribute("contenteditable", EDITABLE); + // Focused and selected on the next frame, not now. The press that opened + // this is still in flight: the canvas overlay takes focus on its own + // pointer-down, and the click that follows puts a caret in the element + // and collapses any selection. Doing it after all of that is what lands. + const view = element.ownerDocument.defaultView; + const raf = view?.requestAnimationFrame(() => { + element.focus({ preventScroll: true }); + placeCaret(element, caretAt); + }); + framesRef.current = raf ?? null; + return true; + }, + [onPause], + ); + + const commit = useCallback(() => { + const open = openRef.current; + if (!open) return; + // Sanitised here, in the element, so the preview shows exactly what will be + // saved rather than something the server will quietly cut down. + sanitizeRichTextChildren(open.element); + const html = open.element.innerHTML; + teardown(); + // After teardown, so the commit path's own resync does not fight an + // element that is still editable. + onCommit(html); + }, [onCommit, teardown]); + + const cancel = useCallback(() => { + const open = openRef.current; + if (!open) return; + if (open.element.isConnected) open.element.innerHTML = open.original; + teardown(); + }, [teardown]); + + // The keys belong to the element, not to the document: the element lives in + // the preview's own document, so a listener on Studio's would never see them. + useEffect(() => { + const element = session?.element; + if (!element) return; + + const onKeyDown = (event: KeyboardEvent) => { + // Shift+Enter is a line break in a multi-line element, and is left alone. + if (event.key === "Enter" && !event.shiftKey) { + event.preventDefault(); + commit(); + return; + } + if (event.key === "Escape") { + event.preventDefault(); + cancel(); + } + }; + // Clicking away keeps the work, which is what every other field in Studio + // does and what a user who has just typed something expects. + const onBlur = () => commit(); + // Nothing here for double or triple click: the browser already takes the + // word on two and the whole text on three, which is what a text field does + // everywhere else. Overriding the double click to take everything cost the + // word selection and gained nothing the triple click did not already do. + // Dropping `plaintext-only` means the browser would otherwise paste a whole + // web page's markup straight in. What arrives is the words. + const onPaste = (event: ClipboardEvent) => { + event.preventDefault(); + const text = event.clipboardData?.getData("text/plain") ?? ""; + if (text) element.ownerDocument.execCommand("insertText", false, text); + }; + + element.addEventListener("keydown", onKeyDown); + element.addEventListener("blur", onBlur); + element.addEventListener("paste", onPaste); + return () => { + element.removeEventListener("keydown", onKeyDown); + element.removeEventListener("blur", onBlur); + element.removeEventListener("paste", onPaste); + }; + }, [session, commit, cancel]); + + return { session, start, commit, cancel }; +} + +/** + * Put the caret where the user pointed, or after the last character. + * + * Opening on a point is what makes this feel like text rather than a dialog: + * the caret lands between the two letters that were clicked, exactly as it + * would in any other editor. + */ +function placeCaret(element: HTMLElement, at?: { x: number; y: number }): void { + const doc = element.ownerDocument; + const selection = doc.defaultView?.getSelection(); + if (!selection) return; + + const range = at ? caretRangeAt(doc, at) : null; + if (range && element.contains(range.startContainer)) { + selection.removeAllRanges(); + selection.addRange(range); + return; + } + placeCaretAtEnd(element); +} + +/** The caret position under a point, across the two APIs browsers expose. */ +function caretRangeAt(doc: Document, at: { x: number; y: number }): Range | null { + const legacy = doc as Document & { + caretRangeFromPoint?: (x: number, y: number) => Range | null; + }; + if (typeof legacy.caretRangeFromPoint === "function") { + return legacy.caretRangeFromPoint(at.x, at.y); + } + const standard = doc as Document & { + caretPositionFromPoint?: (x: number, y: number) => { offsetNode: Node; offset: number } | null; + }; + const position = standard.caretPositionFromPoint?.(at.x, at.y); + if (!position) return null; + const range = doc.createRange(); + range.setStart(position.offsetNode, position.offset); + range.collapse(true); + return range; +} + +/** + * Put the caret after the last character, with nothing selected. + * + * Selecting the whole text would mean the next keystroke silently destroys it, + * which is a bad thing to do to someone who double-clicked to fix a typo. A + * caret at the end is where a person who wants to keep typing expects to be, + * and everything else stays available: click anywhere to move it, drag to + * select, Cmd+A to take the lot. + */ +function placeCaretAtEnd(element: HTMLElement): void { + const doc = element.ownerDocument; + const selection = doc.defaultView?.getSelection(); + if (!selection) return; + const range = doc.createRange(); + // Into the text node, not just past the last child: collapsing the element's + // contents leaves the caret at a node boundary, which types in the right + // place but reports itself as "after child 0" and is a different position + // from the one the user sees at the end of the word. + const last = element.lastChild; + if (last && last.nodeType === 3) { + range.setStart(last, last.textContent?.length ?? 0); + range.collapse(true); + } else { + range.selectNodeContents(element); + range.collapse(false); + } + selection.removeAllRanges(); + selection.addRange(range); +} diff --git a/packages/studio/src/hooks/useStudioUrlState.ts b/packages/studio/src/hooks/useStudioUrlState.ts index e67e6d346a..263182676c 100644 --- a/packages/studio/src/hooks/useStudioUrlState.ts +++ b/packages/studio/src/hooks/useStudioUrlState.ts @@ -22,6 +22,8 @@ interface UseStudioUrlStateParams { rightCollapsed: boolean; activeCompPathHydrated: boolean; domEditSelection: DomEditSelection | null; + domEditGroupSelections: DomEditSelection[]; + applyMarqueeSelection: (selections: DomEditSelection[], additive: boolean) => void; buildDomSelectionFromTarget: ( target: HTMLElement, options?: { preferClipAncestor?: boolean }, @@ -38,14 +40,25 @@ interface UseStudioUrlStateParams { initialState: StudioUrlState; } -function toPersistedSelection(selection: DomEditSelection | null): StudioUrlSelectionState | null { +function toPersistedSelection( + selection: DomEditSelection | null, + // Optional: a caller that only ever has one selection has nothing to add, and + // the URL must still carry that one rather than throwing on the way out. + group: DomEditSelection[] = [], +): StudioUrlSelectionState | null { if (!selection) return null; if (!selection.id && !selection.selector) return null; + // The primary is already carried by selId; the rest ride along so the link + // reopens the same multi-selection instead of a single element. + const groupIds = group + .filter((member) => member.id && member.id !== selection.id) + .map((member) => member.id as string); return { sourceFile: selection.sourceFile || undefined, id: selection.id || undefined, selector: selection.selector || undefined, selectorIndex: selection.selectorIndex ?? undefined, + groupIds: groupIds.length > 0 ? groupIds : undefined, }; } @@ -67,6 +80,8 @@ export function useStudioUrlState({ rightCollapsed, activeCompPathHydrated, domEditSelection, + domEditGroupSelections, + applyMarqueeSelection, buildDomSelectionFromTarget, applyDomSelection, setRightPanelTab, @@ -91,10 +106,10 @@ export function useStudioUrlState({ rightCollapsed, timelineVisible: null, selection: hydratedSelectionRef.current - ? toPersistedSelection(domEditSelection) + ? toPersistedSelection(domEditSelection, domEditGroupSelections) : pendingSelectionRef.current, }), - [activeCompPath, domEditSelection, rightCollapsed, rightPanelTab], + [activeCompPath, domEditGroupSelections, domEditSelection, rightCollapsed, rightPanelTab], ); // Resolve a URL selection to a live element and apply it. Shared by the initial @@ -128,12 +143,32 @@ export function useStudioUrlState({ applyDomSelection(null, { revealPanel: false }); return true; } - void buildDomSelectionFromTarget(element, { preferClipAncestor: false }).then((resolved) => { - applyDomSelection(resolved, { revealPanel: false }); - }); + const groupIds = selection.groupIds ?? []; + void (async () => { + const primary = await buildDomSelectionFromTarget(element, { preferClipAncestor: false }); + if (!primary) return applyDomSelection(null, { revealPanel: false }); + if (groupIds.length === 0) return applyDomSelection(primary, { revealPanel: false }); + // Restore the whole multi-selection, primary first so it stays the anchor. + // Members whose element is gone are dropped rather than failing the rest. + const members = [primary]; + for (const memberId of groupIds) { + const memberEl = doc.getElementById(memberId); + const resolved = memberEl + ? await buildDomSelectionFromTarget(memberEl, { preferClipAncestor: false }) + : null; + if (resolved) members.push(resolved); + } + applyMarqueeSelection(members, false); + })(); return true; }, - [activeCompPath, applyDomSelection, buildDomSelectionFromTarget, previewIframeRef], + [ + activeCompPath, + applyDomSelection, + applyMarqueeSelection, + buildDomSelectionFromTarget, + previewIframeRef, + ], ); useEffect(() => { diff --git a/packages/studio/src/hooks/useTimelineEditing.ts b/packages/studio/src/hooks/useTimelineEditing.ts index bf846f06fe..10f0da7dee 100644 --- a/packages/studio/src/hooks/useTimelineEditing.ts +++ b/packages/studio/src/hooks/useTimelineEditing.ts @@ -36,6 +36,7 @@ import { serializeZLaneGesture } from "../components/nle/zLaneGesture"; import { cutoverCommittedOrThrow, sdkTimingPersist } from "../utils/sdkCutover"; import type { UseTimelineEditingOptions } from "./useTimelineEditingTypes"; import { getStudioSaveErrorMessage } from "../utils/studioSaveDiagnostics"; +import { studioWriteHeaders } from "../utils/studioFileVersion"; type TimelineMoveUpdates = Pick & { stackingReorder?: TimelineStackingReorderIntent | null; @@ -412,7 +413,7 @@ export function useTimelineEditing({ `/api/projects/${pid}/file-mutations/remove-element/${encodeURIComponent(targetPath)}`, { method: "POST", - headers: { "Content-Type": "application/json" }, + headers: { "Content-Type": "application/json", ...studioWriteHeaders() }, body: JSON.stringify({ target: patchTarget }), }, ); diff --git a/packages/studio/src/hooks/useTimelineSelectionPreviewSync.ts b/packages/studio/src/hooks/useTimelineSelectionPreviewSync.ts index 656568cb93..f57ebcd2e6 100644 --- a/packages/studio/src/hooks/useTimelineSelectionPreviewSync.ts +++ b/packages/studio/src/hooks/useTimelineSelectionPreviewSync.ts @@ -2,6 +2,7 @@ import { useEffect, useMemo, useRef } from "react"; import type { TimelineElement } from "../player"; import type { DomEditSelection } from "../components/editor/domEditing"; import { resolveTimelineIdForSelection } from "../utils/studioHelpers"; +import { logSelect } from "../utils/selectDebug"; interface UseTimelineSelectionPreviewSyncParams { selectedElementId: string | null; @@ -93,6 +94,13 @@ export function useTimelineSelectionPreviewSync({ if (selectedIds.length === 0) { missingSelectionKeyRef.current = ""; + // The timeline holds nothing, so the canvas is about to hold nothing either. + // This is the path that silently drops a selection the user can still see. + logSelect("timeline-empty", { + had: currentIds.length, + previousKey: previousSelectedKey.length > 0, + clearing: previousSelectedKey.length > 0 && currentIds.length > 0, + }); if (previousSelectedKey.length > 0 && currentIds.length > 0) { applyDomSelection(null, { revealPanel: false }); } @@ -127,6 +135,11 @@ export function useTimelineSelectionPreviewSync({ return; } missingSelectionKeyRef.current = ""; + logSelect("timeline-sync", { + wanted: selectedIds.length, + had: currentIds.length, + resolved: selections.length, + }); if (selections.length === 0) { applyDomSelection(null, { revealPanel: false }); } else if (selections.length === 1) { diff --git a/packages/studio/src/player/hooks/useTimelinePlayer.ts b/packages/studio/src/player/hooks/useTimelinePlayer.ts index 4ac8e6be0e..11fc23824f 100644 --- a/packages/studio/src/player/hooks/useTimelinePlayer.ts +++ b/packages/studio/src/player/hooks/useTimelinePlayer.ts @@ -4,6 +4,7 @@ import { useMountEffect } from "../../hooks/useMountEffect"; import { usePlaybackKeyboard } from "./usePlaybackKeyboard"; import { useTimelineSyncCallbacks } from "./useTimelineSyncCallbacks"; import { useTimelinePlayerLoop } from "./useTimelinePlayerLoop"; +import { logReload } from "../../utils/reloadDebug"; export type { ClipManifestClip } from "../lib/playbackTypes"; export { createStaticSeekPlaybackAdapter } from "../lib/playbackAdapter"; @@ -445,6 +446,7 @@ export function useTimelinePlayer() { const refreshPlayer = useCallback(() => { const iframe = iframeRef.current; if (!iframe) return; + logReload("refreshPlayer", { stack: new Error("refreshPlayer").stack }); saveSeekPosition(); // Hide the iframe across the full reload so the user never sees the reloading // document's RAW DOM (every clip stacked and visible) in the window between the diff --git a/packages/studio/src/player/lib/playbackShortcuts.ts b/packages/studio/src/player/lib/playbackShortcuts.ts index e30e7192be..da312899d6 100644 --- a/packages/studio/src/player/lib/playbackShortcuts.ts +++ b/packages/studio/src/player/lib/playbackShortcuts.ts @@ -6,15 +6,13 @@ * is active and the user is navigating caption segments). */ +import { isTypingTarget } from "../../utils/typingTarget"; + const PLAYBACK_FRAME_STEP_CODES = new Set(["ArrowLeft", "ArrowRight"]); const PLAYBACK_SHORTCUT_IGNORED_SELECTOR = [ - "input", - "textarea", - "select", "button", "a[href]", - "[contenteditable='true']", "[role='button']", "[role='checkbox']", "[role='combobox']", @@ -27,6 +25,9 @@ const PLAYBACK_SHORTCUT_IGNORED_SELECTOR = [ ].join(","); export function shouldIgnorePlaybackShortcutTarget(target: EventTarget | null): boolean { + // Anything the user is typing into owns its keys outright, editable elements + // included: a letter claimed here never reaches the text. + if (isTypingTarget(target)) return true; if (!target || typeof target !== "object") return false; const candidate = target as { closest?: unknown }; if (typeof candidate.closest !== "function") return false; diff --git a/packages/studio/src/utils/dragDebug.ts b/packages/studio/src/utils/dragDebug.ts new file mode 100644 index 0000000000..72bb69b8fc --- /dev/null +++ b/packages/studio/src/utils/dragDebug.ts @@ -0,0 +1,83 @@ +// Canvas drag diagnostics β€” grep [hf-drag]. Off by default; opt in per session +// with `localStorage.setItem("hf-drag-debug", "1")` (then reload). +// +// A drag that "jumps" is a position that changed without the pointer asking. The +// pointer delta, what snapping did to it, what each member was told to move, and +// where each member actually ended up are logged at every stage, so the frame the +// position diverges from the pointer is visible rather than inferred. +import { makeStudioDebugLogger } from "./studioDebug"; + +export const logDrag = makeStudioDebugLogger("drag"); + +let moveN = 0; + +/** Per-pointermove logging, throttled: the first move then every 8th. */ +export function logDragMove(data: Record): void { + moveN += 1; + if (moveN % 8 === 1) logDrag("move", { n: moveN, ...data }); +} + +export function resetDragMoveLog(): void { + moveN = 0; +} + +/** Where these elements are rendered right now, in preview-document pixels. */ +export function readDragPositions( + elements: Array<{ key: string; element: HTMLElement }>, +): Record { + const positions: Record = {}; + for (const { key, element } of elements) { + const rect = element.getBoundingClientRect(); + positions[key] = `${Math.round(rect.left)},${Math.round(rect.top)}`; + } + return positions; +} + +/** + * Members whose screen movement disagrees with the rest of the group this frame. + * + * A group moves as one object, so every member travels the same distance; one + * that does not is the whole bug, and averaged-looking samples hide it. Compares + * each member's movement against the group's median and names the outliers, so a + * single element drifting shows up as itself rather than as "the group jumped". + */ +export function findNonRigidMembers( + before: Record, + after: Record, +): string[] { + const moves = new Map(); + for (const key of Object.keys(after)) { + const from = before[key]?.split(",").map(Number); + const to = after[key]?.split(",").map(Number); + if (!from || !to || from.length !== 2 || to.length !== 2) continue; + moves.set(key, `${Math.round(to[0]! - from[0]!)},${Math.round(to[1]! - from[1]!)}`); + } + const counts = new Map(); + for (const move of moves.values()) counts.set(move, (counts.get(move) ?? 0) + 1); + let common = ""; + let best = 0; + for (const [move, count] of counts) { + if (count > best) [common, best] = [move, count]; + } + return [...moves] + .filter(([, move]) => move !== common) + .map(([key, move]) => `${key.split("|")[2] ?? key} moved ${move}, group moved ${common}`); +} + +/** + * Sample the group now and again after the commit has had time to land. The drop + * is the one moment a jump can hide: the source write, the preview reload and the + * timeline resume all happen within a few frames of each other, and any of them + * can put the elements back where they started before the new position arrives. + */ +export function logDragSettle( + stage: string, + elements: Array<{ key: string; element: HTMLElement }>, +): void { + logDrag(stage, { at: readDragPositions(elements) }); + const win = elements[0]?.element.ownerDocument.defaultView; + if (!win) return; + win.setTimeout(() => logDrag(`${stage}+120ms`, { at: readDragPositions(elements) }), 120); + win.setTimeout(() => logDrag(`${stage}+400ms`, { at: readDragPositions(elements) }), 400); + win.setTimeout(() => logDrag(`${stage}+900ms`, { at: readDragPositions(elements) }), 900); +} diff --git a/packages/studio/src/utils/reloadDebug.ts b/packages/studio/src/utils/reloadDebug.ts new file mode 100644 index 0000000000..c9f4594dd5 --- /dev/null +++ b/packages/studio/src/utils/reloadDebug.ts @@ -0,0 +1,10 @@ +// Preview full-reload diagnostics β€” grep [hf-reload]. Off by default; opt in per +// session with `localStorage.setItem("hf-reload-debug", "1")` (then reload). +// +// A full reload blanks the stage for ~100-300ms, so any reload the user did not +// ask for reads as a flash. These lines answer the only question that matters +// when one appears: who asked for it, and why the write that triggered it was +// not recognised as Studio's own. +import { makeStudioDebugLogger } from "./studioDebug"; + +export const logReload = makeStudioDebugLogger("reload"); diff --git a/packages/studio/src/utils/selectDebug.ts b/packages/studio/src/utils/selectDebug.ts new file mode 100644 index 0000000000..364f16e85e --- /dev/null +++ b/packages/studio/src/utils/selectDebug.ts @@ -0,0 +1,9 @@ +// Canvas selection diagnostics β€” grep [hf-select]. Off by default; opt in with +// `localStorage.setItem("hf-select-debug", "1")` (then reload). +// +// Selection failures are silent by nature: a handler returns early and nothing +// happens, which looks identical to a click that never landed. These lines say +// which branch ran and what it decided. +import { makeStudioDebugLogger } from "./studioDebug"; + +export const logSelect = makeStudioDebugLogger("select"); diff --git a/packages/studio/src/utils/sourcePatcher.ts b/packages/studio/src/utils/sourcePatcher.ts index c020ff7ab2..b2b9c97fdf 100644 --- a/packages/studio/src/utils/sourcePatcher.ts +++ b/packages/studio/src/utils/sourcePatcher.ts @@ -87,7 +87,11 @@ function splitInlineStyleDeclarations(style: string): string[] { } export interface PatchOperation { - type: "inline-style" | "attribute" | "text-content" | "html-attribute"; + // `rich-text` is the only member that carries markup. It is deliberately + // separate from `text-content`, whose contract is "this value is text": the + // design panel and every other caller rely on that, and widening it would + // have turned all of them into markup sinks at once. + type: "inline-style" | "attribute" | "text-content" | "html-attribute" | "rich-text"; property: string; value: string | null; childSelector?: string; diff --git a/packages/studio/src/utils/studioDebug.ts b/packages/studio/src/utils/studioDebug.ts new file mode 100644 index 0000000000..ee54b04777 --- /dev/null +++ b/packages/studio/src/utils/studioDebug.ts @@ -0,0 +1,26 @@ +// Opt-in diagnostic channels β€” one per question worth tracing, all off by +// default. Turn one on for the session with `localStorage.setItem("hf--debug", +// "1")` and reload, then grep the console for `[hf-]`. +// +// These exist because the interesting failures here are decisions, not crashes: +// a preview that reloads when it should not, a shift-click that selects nothing. +// Nothing is thrown and nothing is logged by default, so without a trace of the +// decision the only way to find the cause is to guess. +type DebugLogger = (stage: string, data?: Record) => void; + +export function makeStudioDebugLogger(name: string): DebugLogger { + let enabled: boolean | null = null; + return (stage, data = {}) => { + if (enabled === null) { + try { + enabled = localStorage.getItem(`hf-${name}-debug`) === "1"; + } catch { + enabled = false; + } + } + if (!enabled) return; + console.log( + `[hf-${name}] ${JSON.stringify({ stage, t: Math.round(performance.now()), ...data })}`, + ); + }; +} diff --git a/packages/studio/src/utils/studioFileVersion.ts b/packages/studio/src/utils/studioFileVersion.ts index 2737c069b0..db89a42166 100644 --- a/packages/studio/src/utils/studioFileVersion.ts +++ b/packages/studio/src/utils/studioFileVersion.ts @@ -51,3 +51,18 @@ export async function studioExpectedFileVersion( export function createStudioWriteToken(): string { return globalThis.crypto.randomUUID(); } + +/** + * Headers that claim the write a mutation request is about to make as our own. + * + * The token is marked BEFORE the request goes out on purpose: the server writes + * the file and the watcher broadcasts it while the request is still in flight, so + * a token marked from the response can arrive after the echo it was meant to + * match. An unmatched echo reads as an external change and costs a full preview + * reload, which the user sees as a flash right after their own edit. + */ +export function studioWriteHeaders(): Record { + const token = createStudioWriteToken(); + markStudioWriteToken(token); + return { "X-Hyperframes-Write-Token": token }; +} diff --git a/packages/studio/src/utils/studioHelpers.ts b/packages/studio/src/utils/studioHelpers.ts index 5cef1add34..489bf8ea0b 100644 --- a/packages/studio/src/utils/studioHelpers.ts +++ b/packages/studio/src/utils/studioHelpers.ts @@ -1,3 +1,4 @@ +import { isTypingTarget } from "./typingTarget"; import type { TimelineElement } from "../player/store/playerStore"; import type { DomEditSelection } from "../components/editor/domEditing"; import type { TimelineAssetKind } from "./timelineAssetDrop"; @@ -114,11 +115,7 @@ export function getEventTargetElement(target: EventTarget | null): HTMLElement | } export function shouldIgnoreHistoryShortcut(target: EventTarget | null): boolean { - const el = getEventTargetElement(target); - if (!el) return false; - return Boolean( - el.closest("input, textarea, select, [contenteditable='true'], [role='textbox'], .cm-editor"), - ); + return isTypingTarget(target); } export function getHistoryShortcutLabel(action: "undo" | "redo"): string { diff --git a/packages/studio/src/utils/studioUrlState.test.ts b/packages/studio/src/utils/studioUrlState.test.ts index 4d2c6eb801..22246970a9 100644 --- a/packages/studio/src/utils/studioUrlState.test.ts +++ b/packages/studio/src/utils/studioUrlState.test.ts @@ -77,6 +77,8 @@ function renderStudioUrlStateHarness( rightCollapsed: true, activeCompPathHydrated: true, domEditSelection: null, + domEditGroupSelections: [], + applyMarqueeSelection: () => {}, buildDomSelectionFromTarget: () => Promise.resolve(null), applyDomSelection: () => {}, initialState: { @@ -132,9 +134,38 @@ describe("studio url state", () => { id: "hero", selector: undefined, selectorIndex: undefined, + groupIds: undefined, }); }); + /** + * A link to a bug hit while several elements were selected has to carry the + * whole selection. Without the group the URL reopens one element, the report + * cannot be reproduced from it, and it reads as "works for me". + */ + it("round-trips a multi-selection through the hash", () => { + const hash = buildStudioHash("demo", { + activeCompPath: null, + currentTime: null, + rightPanelTab: null, + rightCollapsed: null, + timelineVisible: null, + selection: { + sourceFile: "index.html", + id: "chip", + groupIds: ["card", "dot-b"], + }, + }); + + expect(hash).toContain("selGroup=card%2Cdot-b"); + expect(parseStudioUrlStateFromHash(hash).selection?.groupIds).toEqual(["card", "dot-b"]); + }); + + it("reads a single selection as having no group", () => { + const hash = parseStudioUrlStateFromHash("#project/demo?v=1&selFile=index.html&selId=hero"); + expect(hash.selection?.groupIds).toBeUndefined(); + }); + it("builds a project hash with persisted studio state", () => { expect( buildStudioHash("demo", { diff --git a/packages/studio/src/utils/studioUrlState.ts b/packages/studio/src/utils/studioUrlState.ts index e295ca578b..89624ec386 100644 --- a/packages/studio/src/utils/studioUrlState.ts +++ b/packages/studio/src/utils/studioUrlState.ts @@ -7,6 +7,13 @@ export interface StudioUrlSelectionState { id?: string; selector?: string; selectorIndex?: number; + /** + * The other members of a multi-selection, by element id, primary excluded. + * A link to a bug in a group edit is only reproducible if it carries the group; + * without this, opening the URL lands on one element and the report reads as + * "works for me". + */ + groupIds?: string[]; } export interface StudioUrlState { @@ -63,19 +70,28 @@ function parseTab(value: string | null): RightPanelTab | null { return VALID_TABS.includes(value as RightPanelTab) ? (value as RightPanelTab) : null; } +/** The other members of a multi-selection, dropping blanks a hand-edited URL leaves. */ +function parseGroupIds(value: string | null): string[] | undefined { + const ids = (value ?? "") + .split(",") + .map((id) => id.trim()) + .filter(Boolean); + return ids.length > 0 ? ids : undefined; +} + function normalizeSelection(params: URLSearchParams): StudioUrlSelectionState | null { const sourceFile = params.get("selFile") || undefined; const id = params.get("selId") || undefined; const selector = params.get("selSelector") || undefined; - const selectorIndex = parseNumber(params.get("selIndex")); - if (!sourceFile && !id && !selector) return null; + const selectorIndex = parseNumber(params.get("selIndex")); return { sourceFile, id, selector, selectorIndex: selectorIndex != null ? Math.max(0, Math.floor(selectorIndex)) : undefined, + groupIds: parseGroupIds(params.get("selGroup")), }; } @@ -130,6 +146,9 @@ export function buildStudioHash(projectId: string, state: StudioUrlState): strin if (typeof state.selection.selectorIndex === "number") { params.set("selIndex", String(Math.max(0, Math.floor(state.selection.selectorIndex)))); } + if (state.selection.groupIds?.length) { + params.set("selGroup", state.selection.groupIds.join(",")); + } } return buildProjectHash(projectId, params); diff --git a/packages/studio/src/utils/timelineCompositionInsert.ts b/packages/studio/src/utils/timelineCompositionInsert.ts index 144b6c4efb..275905a6d9 100644 --- a/packages/studio/src/utils/timelineCompositionInsert.ts +++ b/packages/studio/src/utils/timelineCompositionInsert.ts @@ -2,6 +2,7 @@ import { createStudioSaveHttpError } from "./studioSaveDiagnostics"; import { serializeStudioFileMutation } from "./studioFileMutationCoordinator"; import type { RecordEditInput } from "./studioFileHistory"; import { buildProjectApiPath } from "./projectRouting"; +import { studioWriteHeaders } from "./studioFileVersion"; interface TimelineCompositionInsertionResult { path: string; @@ -34,7 +35,7 @@ async function insertTimelineComposition(input: { ), { method: "POST", - headers: { "Content-Type": "application/json" }, + headers: { "Content-Type": "application/json", ...studioWriteHeaders() }, body: JSON.stringify({ sourcePath: input.sourcePath, start: input.start, diff --git a/packages/studio/src/utils/typingTarget.test.ts b/packages/studio/src/utils/typingTarget.test.ts new file mode 100644 index 0000000000..5f5e1211f8 --- /dev/null +++ b/packages/studio/src/utils/typingTarget.test.ts @@ -0,0 +1,47 @@ +// @vitest-environment happy-dom + +import { afterEach, describe, expect, it } from "vitest"; +import { isTypingTarget } from "./typingTarget"; + +afterEach(() => { + document.body.innerHTML = ""; +}); + +function mount(html: string): HTMLElement { + document.body.innerHTML = html; + return document.body.firstElementChild as HTMLElement; +} + +describe("isTypingTarget", () => { + // The bug this exists for: inline text editing uses plaintext-only, an + // attribute selector for 'true' missed it, and the playback shortcuts ate + // every letter typed into the composition. + it("recognises a plaintext-only editable, not just contenteditable=true", () => { + expect(isTypingTarget(mount('

Hi

'))).toBe(true); + expect(isTypingTarget(mount('

Hi

'))).toBe(true); + expect(isTypingTarget(mount("

Hi

"))).toBe(true); + }); + + it("recognises a child of an editable, which a selector on the target alone misses", () => { + const host = mount('
inner
'); + expect(isTypingTarget(host.querySelector("span"))).toBe(true); + }); + + it("recognises the ordinary fields too", () => { + expect(isTypingTarget(mount(""))).toBe(true); + expect(isTypingTarget(mount(""))).toBe(true); + expect(isTypingTarget(mount(""))).toBe(true); + expect(isTypingTarget(mount('
'))).toBe(true); + }); + + it("leaves the keys alone for anything that is not being typed into", () => { + expect(isTypingTarget(mount("
plain
"))).toBe(false); + expect(isTypingTarget(mount(""))).toBe(false); + expect(isTypingTarget(mount('

Hi

'))).toBe(false); + }); + + it("says no to nothing at all", () => { + expect(isTypingTarget(null)).toBe(false); + expect(isTypingTarget({} as EventTarget)).toBe(false); + }); +}); diff --git a/packages/studio/src/utils/typingTarget.ts b/packages/studio/src/utils/typingTarget.ts new file mode 100644 index 0000000000..eb91626c62 --- /dev/null +++ b/packages/studio/src/utils/typingTarget.ts @@ -0,0 +1,39 @@ +/** + * Whether a keystroke is going somewhere the user is typing. + * + * Every keyboard shortcut in Studio has to ask this before claiming a key, and + * they were each asking it slightly differently. The version that matched + * `[contenteditable='true']` missed `contenteditable="plaintext-only"`, which + * is what inline text editing uses, so the playback shortcuts kept claiming + * letters out of it: `a` seeked to the in-point and `e` to the out-point, + * `preventDefault` and all, and the character never reached the text. + * + * `isContentEditable` is the property to ask, not the attribute to match: it is + * true for every editable value and for an element made editable by an + * ancestor, which an attribute selector on the target alone cannot see. + */ +export function isTypingTarget(target: EventTarget | null): boolean { + const element = asElement(target); + if (!element) return false; + if (element.isContentEditable) return true; + return element.closest(TYPING_SELECTOR) !== null; +} + +/** + * Things a keystroke belongs to rather than to a shortcut. `contenteditable` is + * matched by value as well, for a host whose own property is not yet true. + */ +const TYPING_SELECTOR = [ + "input", + "textarea", + "select", + "[contenteditable]:not([contenteditable='false'])", + "[role='textbox']", + ".cm-editor", +].join(","); + +function asElement(target: EventTarget | null): HTMLElement | null { + if (!target || typeof target !== "object") return null; + const candidate = target as { closest?: unknown; isContentEditable?: unknown }; + return typeof candidate.closest === "function" ? (target as HTMLElement) : null; +}