fix(studio): an undo that lands mid-drag keeps the box under the pointer (#4853)

* fix(studio): an undo that lands mid-drag keeps the box under the pointer

* fix(studio): an undo that lands on a pressed layer keeps what its gesture draws

* fix(studio): every gesture mark names what it draws, and a text edit draws nothing

* test(studio): gate the drag, undo, drag cases again now that their race is fixed

* fix(studio): a text edit's gesture mark draws nothing, so an undo under it reverts

* test(studio): keep the one drag, undo, drag case whose press can still jump in quarantine

* test(studio): bank the drag, undo, drag cases that now pass
This commit is contained in:
Miguel Ángel
2026-10-01 14:00:32 -07:00
committed by GitHub
parent d05344b3ee
commit 83035e08b8
10 changed files with 1197 additions and 984 deletions
@@ -219,10 +219,13 @@ export function startGesture(
// Hold a % translate as the same px now, so a growing box can't drag it along mid-frame.
if (result.member.plainTranslate) applyManualOffsetDragDraft(result.member, 0, 0);
} else {
manualEditDragToken = beginStudioManualEditGesture(sel.element);
manualEditDragToken = beginStudioManualEditGesture(sel.element, "resize");
}
} else {
manualEditDragToken = beginStudioManualEditGesture(sel.element);
manualEditDragToken = beginStudioManualEditGesture(
sel.element,
kind === "rotate" ? "rotate" : "resize",
);
}
}
@@ -1,3 +1,4 @@
// fallow-ignore-file code-duplication
import { describe, expect, it, vi } from "vitest";
import { Window } from "happy-dom";
import {
@@ -285,7 +286,7 @@ describe("studio manual edits", () => {
expect(set).not.toHaveBeenCalled();
expect(tickerTick).not.toHaveBeenCalled();
beginStudioManualEditGesture(card);
beginStudioManualEditGesture(card, "move");
applyStudioPathOffsetDraft(card, { x: 35, y: -6 });
expect(readStudioPathOffset(card)).toEqual({ x: 35, y: -6 });
@@ -395,8 +396,8 @@ describe("studio manual edits", () => {
const card = document.getElementById("card") as HTMLElement;
applyStudioPathOffset(card, { x: 40, y: 24 });
const firstToken = beginStudioManualEditGesture(card);
const secondToken = beginStudioManualEditGesture(card);
const firstToken = beginStudioManualEditGesture(card, "move");
const secondToken = beginStudioManualEditGesture(card, "move");
endStudioManualEditGesture(card, firstToken);
// Gesture still active — offset should remain
@@ -35,13 +35,26 @@ import {
import { gsapAnimatesProperty } from "./gsapAnimatesProperty";
import { splitTopLevelWhitespace } from "./manualEditsStyleHelpers";
import { roundTo3, roundToLayoutPx } from "../../utils/rounding";
import { BOX_SIZE_STYLE_PROPS } from "./manualEditsDomPatches";
/* ── Gesture tracking ─────────────────────────────────────────────── */
let studioManualEditGestureId = 0;
export function beginStudioManualEditGesture(element: HTMLElement): string {
export type StudioGestureDraws = "move" | "resize" | "rotate" | "edit";
const MOVE_DRAWS = ["translate", STUDIO_OFFSET_X_PROP, STUDIO_OFFSET_Y_PROP];
const GESTURE_DRAWS: Record<StudioGestureDraws, readonly string[]> = {
move: MOVE_DRAWS,
resize: [...MOVE_DRAWS, STUDIO_WIDTH_PROP, STUDIO_HEIGHT_PROP, ...BOX_SIZE_STYLE_PROPS],
rotate: ["rotate", "transform", "transform-origin", "display", STUDIO_ROTATION_PROP],
edit: [],
};
export function beginStudioManualEditGesture(
element: HTMLElement,
draws: StudioGestureDraws,
): string {
studioManualEditGestureId += 1;
const token = `gesture-${studioManualEditGestureId}`;
const token = `gesture-${studioManualEditGestureId}:${draws}`;
element.setAttribute(STUDIO_MANUAL_EDIT_GESTURE_ATTR, token);
return token;
}
@@ -88,6 +101,12 @@ export function countStudioManualEditSave<R>(element: HTMLElement, save: () => R
return result;
}
export function studioGestureDraws(element: Element): readonly string[] | null {
const token = element.getAttribute(STUDIO_MANUAL_EDIT_GESTURE_ATTR);
if (token === null) return null;
return GESTURE_DRAWS[token.split(":")[1] as StudioGestureDraws] ?? [];
}
function isStudioManualEditGestureActive(element: HTMLElement): boolean {
return element.hasAttribute(STUDIO_MANUAL_EDIT_GESTURE_ATTR);
}
@@ -356,7 +356,10 @@ export function createManualOffsetDragMember(input: {
}
const initialPathOffset = captureStudioPathOffset(input.element);
const gestureToken = beginStudioManualEditGesture(input.element);
const gestureToken = beginStudioManualEditGesture(
input.element,
input.gesture === "resize" ? "resize" : "move",
);
const measured = measureManualOffsetDragScreenToOffsetMatrix(input.element, initialOffset, {
scaleX: input.rect.editScaleX,
scaleY: input.rect.editScaleY,
@@ -5,6 +5,7 @@ import { createRoot } from "react-dom/client";
import { afterEach, describe, expect, it, vi } from "vitest";
import { PreviewReadOnlyProvider } from "../components/editor/previewReadOnlyContext";
import { useInlineTextEdit, type InlineTextEditControls } from "./useInlineTextEdit";
import { applyUndoRestoreToPreview } from "../utils/gsapUndoRestore";
(globalThis as unknown as { IS_REACT_ACT_ENVIRONMENT: boolean }).IS_REACT_ACT_ENVIRONMENT = true;
@@ -643,3 +644,29 @@ describe("useInlineTextEdit with styled runs", () => {
act(() => root.unmount());
});
});
describe("an undo that lands while a text edit is open", () => {
it("marks the edit as drawing nothing, so the undo reverts the move and keeps the edit", () => {
const wrap = (body: string) => `<html><body>${body}</body></html>`;
const moved = `<h1 id="t" style="translate: 90px 60px">Title</h1>`;
document.body.innerHTML = moved;
const element = document.getElementById("t")!;
const { controls, root } = mount();
act(() => {
controls().start(element);
});
const contentWindow = { __player: { seek: vi.fn() }, __hfStudioManualEditsApply: vi.fn() };
const iframe = { contentWindow, contentDocument: document } as unknown as HTMLIFrameElement;
const files = {
"index.html": { previous: wrap(moved), restored: wrap(`<h1 id="t">Title</h1>`) },
};
expect(element.getAttribute("data-hf-studio-manual-edit-gesture")).toMatch(/:edit$/);
expect(applyUndoRestoreToPreview(iframe, "index.html", files, 0, vi.fn())).toBe("soft");
expect(element.style.getPropertyValue("translate")).toBe("");
expect(element.getAttribute("contenteditable")).toBe("true");
expect(element.style.getPropertyValue("outline")).not.toBe("");
act(() => root.unmount());
});
});
@@ -141,7 +141,7 @@ export function useInlineTextEdit({
original: element.innerHTML,
outline: element.style.outline,
outlineOffset: element.style.outlineOffset,
gesture: beginStudioManualEditGesture(element),
gesture: beginStudioManualEditGesture(element, "edit"),
};
// 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
@@ -1,7 +1,14 @@
// @vitest-environment happy-dom
import { describe, it, expect, vi } from "vitest";
import { applyUndoRestoreToPreview, diffSoftReloadableRestore } from "./gsapUndoRestore";
import {
applyUndoRestoreToPreview,
diffSoftReloadableRestore,
showRestoreInPlace,
} from "./gsapUndoRestore";
import { applyPatch } from "./sourcePatcher";
import { beginStudioManualEditGesture } from "../components/editor/manualEdits";
import { writePlainMove, writeTranslatePx } from "../components/editor/plainTranslate";
// ── Bug 2: undo/redo restore soft-apply ──────────────────────────────────────
@@ -420,3 +427,121 @@ describe("applyUndoRestoreToPreview", () => {
expect(reloadPreview).toHaveBeenCalledTimes(1);
});
});
describe("an undo that lands while the layer is being dragged", () => {
const ROOT = "index.html";
const undone = `<div id="a" style="width: 120px; translate: 90px 60px" data-start="2" data-hf-studio-original-inline-translate="">t</div>`;
const restored = wrap(`<div id="a" style="width: 100px" data-start="1">t</div>`);
const files = { [ROOT]: { previous: wrap(undone), restored } };
const saveDrop = (el: HTMLElement, file: string) =>
writePlainMove(el, { x: 20, y: 110 }).reduce((html, op) => applyPatch(html, "a", op), file);
function dragging(drag: boolean) {
const { iframe, doc } = buildLiveIframe(undone);
const el = doc.getElementById("a")!;
if (drag) beginStudioManualEditGesture(el, "move");
if (drag) writeTranslatePx(el, { x: 70, y: 110 });
else el.style.setProperty("opacity", "0.5");
applyUndoRestoreToPreview(iframe, ROOT, files, 3, vi.fn());
return el;
}
it("keeps the translate the drag is drawing and reverts the rest", () => {
const el = dragging(true);
expect(el.style.getPropertyValue("translate")).toBe("70px 110px");
expect(el.style.getPropertyValue("width")).toBe("100px");
expect(el.getAttribute("data-start")).toBe("1");
expect(el.hasAttribute("data-hf-studio-manual-edit-gesture")).toBe(true);
});
it("saves the drop onto the undone file, the same bytes as dragging the undone layer", () => {
const el = dragging(true);
const fresh = new DOMParser().parseFromString(restored, "text/html").getElementById("a")!;
const saved = saveDrop(el, restored);
expect(saved).toBe(saveDrop(fresh, restored));
expect(saved).toContain("width: 100px; translate: 20px 110px");
expect(saved).not.toContain("120px");
});
it("keeps the box where it is when the press has not moved it yet", () => {
const { iframe, doc } = buildLiveIframe(undone);
const el = doc.getElementById("a")!;
beginStudioManualEditGesture(el, "move");
applyUndoRestoreToPreview(iframe, ROOT, files, 3, vi.fn());
expect(el.style.getPropertyValue("translate")).toBe("90px 60px");
expect(el.style.getPropertyValue("width")).toBe("100px");
});
it("reverts the translate under a gesture that draws nothing, such as a text edit", () => {
const { iframe, doc } = buildLiveIframe(undone);
const el = doc.getElementById("a")!;
beginStudioManualEditGesture(el, "edit");
applyUndoRestoreToPreview(iframe, ROOT, files, 3, vi.fn());
expect(el.style.getPropertyValue("translate")).toBe("");
expect(el.hasAttribute("data-hf-studio-manual-edit-gesture")).toBe(true);
});
it("keeps the drag's translate when the undo is shown in place", () => {
const { iframe, doc } = buildLiveIframe(undone);
const el = doc.getElementById("a")!;
beginStudioManualEditGesture(el, "move");
writeTranslatePx(el, { x: 70, y: 110 });
expect(showRestoreInPlace(iframe, ROOT, files, 3)).not.toBeNull();
expect(el.style.getPropertyValue("translate")).toBe("70px 110px");
expect(el.style.getPropertyValue("width")).toBe("100px");
});
it("keeps a drag that started after the undo was shown when the undo is put back", () => {
const { iframe, doc } = buildLiveIframe(undone);
const el = doc.getElementById("a")!;
const putBack = showRestoreInPlace(iframe, ROOT, files, 3)!;
beginStudioManualEditGesture(el, "move");
writeTranslatePx(el, { x: 70, y: 110 });
putBack(3);
expect(el.style.getPropertyValue("translate")).toBe("70px 110px");
expect(el.style.getPropertyValue("width")).toBe("120px");
});
it("keeps the drag's translate when the undo re-runs a GSAP script", () => {
const script = (extra: string) =>
`<script>window.__timelines["root"]=gsap.timeline();${extra}</script>`;
const layer = `<div id="a" data-hf-studio-path-offset="true" style="translate: 40px 30px">t</div>`;
const { iframe, contentWindow, doc } = buildLiveIframe(
`${layer}${script("tl.set('#b',{x:1});")}`,
);
Object.assign(contentWindow.gsap, { set: () => {} });
const el = doc.getElementById("a")!;
beginStudioManualEditGesture(el, "move");
writeTranslatePx(el, { x: 70, y: 110 });
const scripted = {
[ROOT]: {
previous: wrap(`<div id="a">t</div>${script("tl.set('#b',{x:1});")}`),
restored: wrap(`<div id="a">t</div>${script("")}`),
},
};
expect(applyUndoRestoreToPreview(iframe, ROOT, scripted, 3, vi.fn())).toBe("soft");
expect(el.style.getPropertyValue("translate")).toBe("70px 110px");
});
it("restores a layer no gesture is drawing exactly as before", () => {
const el = dragging(false);
const want = new DOMParser().parseFromString(restored, "text/html").getElementById("a")!;
expect(el.getAttributeNames().map((n) => [n, el.getAttribute(n)])).toEqual(
want.getAttributeNames().map((n) => [n, want.getAttribute(n)]),
);
});
});
+57 -17
View File
@@ -12,6 +12,8 @@ import { isCompositionTemplate } from "@hyperframes/parsers/hf-ids";
import { findAuthoredElement, parseSavedSource } from "./authoredSource";
import { STUDIO_EDIT_ATTRS } from "../components/editor/manualEditsSeekReapply";
import { markScenesStale } from "../player/sceneSwap";
import { STUDIO_ORIGINAL_INLINE_TRANSLATE_ATTR } from "../components/editor/manualEditsTypes";
import { studioGestureDraws } from "../components/editor/manualEditsDom";
type PreviewWindow = Window & {
__player?: { seek?: (t: number) => void };
@@ -140,23 +142,55 @@ function diffRestoreDocs(prevDoc: Document, nextDoc: Document): string[] | null
return prevRest === nextRest ? changedElementKeys : null;
}
/** Copy every attribute from `source` onto the live `target`, dropping extras. */
function syncElementAttributes(target: Element, source: Element): void {
/** Copies `source`'s attributes onto `target`; under a gesture mark it merges them (keepDrawn). */
function syncElementAttributes(target: Element, source: Element, base?: Element): void {
const draws = base && studioGestureDraws(target);
const merged = draws ? keepDrawn(target, source, base, draws) : source;
for (const name of [...target.getAttributeNames()]) {
if (!source.hasAttribute(name)) target.removeAttribute(name);
if (!merged.hasAttribute(name)) target.removeAttribute(name);
}
for (const name of source.getAttributeNames()) {
target.setAttribute(name, source.getAttribute(name) ?? "");
for (const name of merged.getAttributeNames()) {
target.setAttribute(name, merged.getAttribute(name) ?? "");
}
}
/** `source`, plus what a gesture in progress draws on `live` (whatever its value) or changed since `base`. */
function keepDrawn(live: Element, source: Element, base: Element, draws: readonly string[]) {
const merged = source.cloneNode(false) as Element;
const keepsMark = draws.includes("translate");
for (const name of live.getAttributeNames().filter((n) => n !== "style")) {
const value = live.getAttribute(name)!;
const mark = keepsMark && name === STUDIO_ORIGINAL_INLINE_TRANSLATE_ATTR;
if (mark || value !== base.getAttribute(name)) merged.setAttribute(name, value);
}
keepDrawnStyle(live as HTMLElement, base as HTMLElement, merged as HTMLElement, draws);
return merged;
}
function keepDrawnStyle(
live: HTMLElement,
base: HTMLElement,
merged: HTMLElement,
draws: readonly string[],
) {
const value = (prop: string) => live.style.getPropertyValue(prop);
const drawn = (prop: string) =>
draws.includes(prop) || value(prop) !== base.style.getPropertyValue(prop);
const props = new Set([...Array.from(live.style), ...Array.from(base.style), ...draws]);
for (const prop of [...props].filter(drawn)) {
if (value(prop))
merged.style.setProperty(prop, value(prop), live.style.getPropertyPriority(prop));
else merged.style.removeProperty(prop);
}
}
// A gesture folded into the script leaves marks on the live element that every seek would re-impose.
function syncStaleEditMarks(doc: Document, restored: string): void {
const restoredDoc = parseSavedSource(restored);
function syncStaleEditMarks(doc: Document, file: UndoRestoreFile): void {
const [restoredDoc, previousDoc] = [file.restored, file.previous].map(parseSavedSource);
for (const live of doc.querySelectorAll(STUDIO_EDIT_ATTRS.map((attr) => `[${attr}]`).join())) {
const source = findAuthoredElement(restoredDoc, live);
if (source && STUDIO_EDIT_ATTRS.some((a) => live.getAttribute(a) !== source.getAttribute(a))) {
syncElementAttributes(live, source);
syncElementAttributes(live, source, findAuthoredElement(previousDoc, live) ?? undefined);
}
}
}
@@ -172,7 +206,7 @@ function hasAmbiguousGsapScriptChange(
);
}
type RestoreTarget = { live: Element; restored: Element };
type RestoreTarget = { live: Element; restored: Element; previous?: Element };
type RestorePlan = { targets: RestoreTarget[]; scripted: boolean };
/** Whether either side has a GSAP script, or null when its scripts rule out an in-place restore. */
@@ -196,6 +230,7 @@ function scopeTargets(
scope: ParentNode,
keys: string[],
restoredByKey: Map<string, Element>,
previousByKey: Map<string, Element>,
): RestoreTarget[] | null {
const liveByKey = identityElementMap(scope);
if (!liveByKey) return null;
@@ -205,7 +240,7 @@ function scopeTargets(
const restored = restoredByKey.get(key);
// The preview rewrites a sub-composition's own root, so its attributes are not the file's.
if (!live || !restored || live.hasAttribute("data-hf-inner-root")) return null;
targets.push({ live, restored });
targets.push({ live, restored, previous: previousByKey.get(key) });
}
return targets;
}
@@ -222,9 +257,10 @@ function fileTargets(
if (scripted === null) return null;
const keys = diffRestoreDocs(prevDoc, nextDoc);
const restoredByKey = identityElementMap(nextDoc);
if (!keys || !restoredByKey) return null;
const previousByKey = identityElementMap(prevDoc);
if (!keys || !restoredByKey || !previousByKey) return null;
const found = liveScopes(doc, path, isActive).map((scope) =>
scopeTargets(scope, keys, restoredByKey),
scopeTargets(scope, keys, restoredByKey, previousByKey),
);
if (!found.length || found.includes(null)) return null;
return { targets: (found as RestoreTarget[][]).flat(), scripted };
@@ -273,11 +309,14 @@ export function showRestoreInPlace(
const win = iframe?.contentWindow as PreviewWindow | null;
const plan = doc && win ? planRestoreTargets(doc, activeCompPath ?? "index.html", files) : null;
if (!iframe || !win || !plan || plan.scripted) return null;
const before = plan.targets.map(({ live }) => [live, live.cloneNode(false) as Element] as const);
for (const { live, restored } of plan.targets) syncElementAttributes(live, restored);
const before = plan.targets.map(
({ live, restored }) => [live, live.cloneNode(false) as Element, restored] as const,
);
for (const { live, restored, previous } of plan.targets)
syncElementAttributes(live, restored, previous);
const putBack = (time: number) => {
if (iframe.contentDocument !== doc) return false;
for (const [live, attributes] of before) syncElementAttributes(live, attributes);
for (const [live, attributes, shown] of before) syncElementAttributes(live, attributes, shown);
return finalizeInPlace(iframe, win, time);
};
if (finalizeInPlace(iframe, win, currentTime)) return putBack;
@@ -338,13 +377,14 @@ export function applyUndoRestoreToPreview(
}
// Sync each changed element's attributes onto the live DOM from the restored
// markup, so the runtime's seek-reapply reads the reverted values.
for (const target of plan.targets) syncElementAttributes(target.live, target.restored);
for (const target of plan.targets)
syncElementAttributes(target.live, target.restored, target.previous);
const active = files[activeDocPath];
const restoredScript = active ? extractGsapScriptText(active.restored) : null;
const previousScript = active ? extractGsapScriptText(active.previous) : null;
if (restoredScript && restoredScript !== previousScript) {
syncStaleEditMarks(doc, active.restored);
syncStaleEditMarks(doc, active);
const result = applySoftReload(iframe, restoredScript, {
onAsyncFailure: reloadPreview,
currentTimeOverride: currentTime,
File diff suppressed because it is too large Load Diff
@@ -50,12 +50,7 @@ export function bankable(runs) {
* The fixing PR deletes its own ids here and re-banks them in the same PR.
*/
export const QUARANTINED = {
"sequndo-none-center-r0-nested-z100": "#4853",
"sequndo-none-center-r0-root-z100": "#4853",
"sequndo-none-pct-r0-nested-z100": "#4853",
"sequndo-none-pct-r0-root-z100": "#4853",
"sequndo-none-px-r0-nested-z100": "#4853",
"sequndo-none-px-r0-root-z100": "#4853",
"sequndo-none-px-r0-root-z100": "part C (#4807 stack)",
"seqrepeat-none-px-r0-nested-z100": "part C (#4807 stack)",
};