fix(studio): propagate z-index reorder save failures and drop dead targetTrack

handleDomZIndexReorderCommit no longer swallows per-entry save failures: it settles every
patch, and on any rejection rolls back the eager DOM z-index/position and the optimistic
store zIndex before rejecting, so a failed save cannot leave the UI showing a stacking order
that never persisted or let an ordered-after timing write proceed.

Also removes the dead targetTrack parameter threaded through the timeline edit helpers;
vertical placement is owned by the z-index intent.
This commit is contained in:
Miguel Angel Simon Sierra
2026-07-09 16:53:26 -04:00
parent 51dd8a0753
commit 6080f5ad3e
6 changed files with 168 additions and 12 deletions
@@ -39,7 +39,6 @@ describe("applyTimelineStackingReorder", () => {
applyTimelineStackingReorder({
element: el({ id: "chip", tag: "div" }),
targetTrack: 0,
stackingReorder: {
contextKey: "scene",
placement: { type: "above", layerId: "layer:scene:x" },
@@ -76,7 +75,6 @@ describe("applyTimelineStackingReorder", () => {
applyTimelineStackingReorder({
element: el({ id: "track", tag: "audio" }),
targetTrack: 0,
stackingReorder: {
contextKey: "main",
placement: { type: "above", layerId: "layer:main:x" },
@@ -33,7 +33,6 @@ function isHTMLElement(element: Element | null): element is HTMLElement {
// fallow-ignore-next-line complexity
export function applyTimelineStackingReorder(input: {
element: TimelineElement;
targetTrack: number;
stackingReorder: TimelineStackingReorderIntent | null | undefined;
timelineElements: readonly TimelineElement[];
iframe: HTMLIFrameElement | null;
@@ -89,12 +88,7 @@ export function applyTimelineStackingReorder(input: {
}
if (commitEntries.length === 0) return Promise.resolve();
const persisted = input.commit?.(commitEntries) ?? Promise.resolve();
const store = usePlayerStore.getState();
for (const entry of commitEntries) {
store.updateElement(entry.key, { zIndex: entry.zIndex, hasExplicitZIndex: true });
}
return persisted;
return input.commit?.(commitEntries) ?? Promise.resolve();
}
/**
@@ -160,6 +160,7 @@ export function useElementLifecycleOps({
// persistDomEditOperations → onTrySdkPersist, so it is already SDK-cut-over as setStyle.
// No SDK reorder/reparent op exists; DOM sibling order stays server-authoritative if ever needed.
const handleDomZIndexReorderCommit = useCallback(
// fallow-ignore-next-line complexity
(
entries: Array<{
element: HTMLElement;
@@ -168,6 +169,7 @@ export function useElementLifecycleOps({
selector?: string;
selectorIndex?: number;
sourceFile: string;
key?: string;
}>,
) => {
if (entries.length === 0) return Promise.resolve();
@@ -178,8 +180,15 @@ export function useElementLifecycleOps({
);
const coalesceKey = `z-reorder:${entries.map((e) => e.id ?? e.selector ?? e.element.getAttribute("data-hf-id") ?? "el").join(":")}`;
const saves: Array<Promise<void>> = [];
const rollbacks: Array<() => void> = [];
for (let i = 0; i < entries.length; i++) {
const entry = entries[i];
const priorZIndex = entry.element.style.zIndex;
const priorPosition = entry.element.style.position;
const priorStoreEntry = entry.key
? usePlayerStore.getState().elements.find((el) => (el.key ?? el.id) === entry.key)
: undefined;
let positionChanged = false;
entry.element.style.zIndex = String(entry.zIndex);
const patches: Array<{ type: "inline-style"; property: string; value: string }> = [
{ type: "inline-style", property: "z-index", value: String(entry.zIndex) },
@@ -188,11 +197,27 @@ export function useElementLifecycleOps({
const win = entry.element.ownerDocument?.defaultView;
if (win && win.getComputedStyle(entry.element).position === "static") {
entry.element.style.position = "relative";
positionChanged = true;
patches.push({ type: "inline-style", property: "position", value: "relative" });
}
} catch {
/* cross-origin or detached — skip */
}
if (entry.key) {
usePlayerStore
.getState()
.updateElement(entry.key, { zIndex: entry.zIndex, hasExplicitZIndex: true });
}
rollbacks.push(() => {
entry.element.style.zIndex = priorZIndex;
if (positionChanged) entry.element.style.position = priorPosition;
if (entry.key && priorStoreEntry) {
usePlayerStore.getState().updateElement(entry.key, {
zIndex: priorStoreEntry.zIndex,
hasExplicitZIndex: priorStoreEntry.hasExplicitZIndex,
});
}
});
saves.push(
commitPositionPatchToHtml(
{
@@ -209,12 +234,21 @@ export function useElementLifecycleOps({
coalesceKey,
skipRefresh: i < entries.length - 1,
},
).catch(() => undefined),
),
);
}
// Resolves once every z-index patch is persisted so a same-file timing write
// can be ordered after it (see applyTimelineStackingReorder callers).
return Promise.all(saves).then(() => undefined);
return Promise.allSettled(saves).then((settled) => {
const rejected = settled.find(
(result): result is PromiseRejectedResult => result.status === "rejected",
);
if (rejected) {
for (const rollback of rollbacks) rollback();
return Promise.reject(rejected.reason);
}
return undefined;
});
},
[commitPositionPatchToHtml, onReorderShadow],
);
@@ -493,6 +493,136 @@ describe("useTimelineEditing timeline z-index reorder", () => {
unmount();
});
it("rejects and rolls back DOM and store z-index changes when a reorder save fails", async () => {
const iframe = createPreviewIframe([
{ id: "front", track: 0, style: "position: relative; z-index: 7" },
{ id: "back", track: 1, style: "position: static" },
]);
const front = timelineElement({ id: "front", track: 0, zIndex: 7 });
const back = timelineElement({ id: "back", track: 1, zIndex: 0 });
usePlayerStore.getState().setElements([
{ ...front, hasExplicitZIndex: true },
{ ...back, hasExplicitZIndex: false },
]);
const saveError = new Error("save failed");
const commitPositionPatchToHtml = vi
.fn<(...args: unknown[]) => Promise<void>>()
.mockResolvedValueOnce(undefined)
.mockRejectedValueOnce(saveError);
const { move, unmount } = renderTimelineEditingHookWithLifecycle({
timelineElements: [front, back],
iframe,
commitPositionPatchToHtml,
});
const doc = iframe.contentDocument;
if (!doc) throw new Error("Expected iframe document");
const frontElement = doc.getElementById("front") as HTMLElement | null;
const backElement = doc.getElementById("back") as HTMLElement | null;
if (!frontElement || !backElement) throw new Error("Expected reordered elements");
let rejection: unknown;
await act(async () => {
try {
await move(back, {
start: back.start,
track: back.track,
stackingReorder: {
contextKey: "root",
placement: { type: "above", layerId: "front" },
zIndexChanges: [
{ key: "front", zIndex: 2 },
{ key: "back", zIndex: 5 },
],
},
});
} catch (error) {
rejection = error;
}
await flushAsyncWork();
});
expect(rejection).toBe(saveError);
expect(frontElement.style.zIndex).toBe("7");
expect(frontElement.style.position).toBe("relative");
expect(backElement.style.zIndex).toBe("");
expect(backElement.style.position).toBe("static");
const storeEntries = usePlayerStore.getState().elements;
expect(storeEntries.find((entry) => entry.id === "front")).toMatchObject({
zIndex: 7,
hasExplicitZIndex: true,
});
expect(storeEntries.find((entry) => entry.id === "back")).toMatchObject({
zIndex: 0,
hasExplicitZIndex: false,
});
unmount();
});
it("waits for every lifecycle z-index save before resolving a reorder", async () => {
const iframe = createPreviewIframe([
{ id: "front", track: 0, style: "position: relative; z-index: 1" },
{ id: "back", track: 1, style: "position: relative; z-index: 0" },
]);
const front = timelineElement({ id: "front", track: 0, zIndex: 1 });
const back = timelineElement({ id: "back", track: 1, zIndex: 0 });
let releaseFirst!: () => void;
let releaseSecond!: () => void;
const firstSave = new Promise<void>((resolve) => {
releaseFirst = resolve;
});
const secondSave = new Promise<void>((resolve) => {
releaseSecond = resolve;
});
const commitPositionPatchToHtml = vi
.fn<(...args: unknown[]) => Promise<void>>()
.mockReturnValueOnce(firstSave)
.mockReturnValueOnce(secondSave);
const { move, unmount } = renderTimelineEditingHookWithLifecycle({
timelineElements: [front, back],
iframe,
commitPositionPatchToHtml,
});
let settled = false;
let movePromise!: Promise<void>;
await act(async () => {
movePromise = move(back, {
start: back.start,
track: back.track,
stackingReorder: {
contextKey: "root",
placement: { type: "above", layerId: "front" },
zIndexChanges: [
{ key: "front", zIndex: 2 },
{ key: "back", zIndex: 3 },
],
},
}).then(() => {
settled = true;
});
await flushAsyncWork();
});
expect(commitPositionPatchToHtml).toHaveBeenCalledTimes(2);
expect(settled).toBe(false);
await act(async () => {
releaseFirst();
await flushAsyncWork();
});
expect(settled).toBe(false);
await act(async () => {
releaseSecond();
await movePromise;
await flushAsyncWork();
});
expect(settled).toBe(true);
unmount();
});
it("keeps horizontal-only drag on the timing and GSAP shift path without z-index writes", async () => {
const iframe = createPreviewIframe([{ id: "clip", track: 0 }]);
const clip = timelineElement({ id: "clip", track: 0, zIndex: 0 });
@@ -136,7 +136,6 @@ export function useTimelineEditing({
const reorderDone = applyTimelineStackingReorder({
element,
targetTrack: updates.track,
stackingReorder: updates.stackingReorder,
timelineElements,
iframe: previewIframeRef.current,
@@ -20,6 +20,7 @@ export type TimelineZIndexReorderCommit = (
selector?: string;
selectorIndex?: number;
sourceFile: string;
key?: string;
}>,
) => Promise<void>;