diff --git a/packages/studio/src/hooks/timelineEditingHelpers.test.ts b/packages/studio/src/hooks/timelineEditingHelpers.test.ts new file mode 100644 index 000000000..e120fe2d0 --- /dev/null +++ b/packages/studio/src/hooks/timelineEditingHelpers.test.ts @@ -0,0 +1,88 @@ +// @vitest-environment jsdom +import { describe, expect, it, vi } from "vitest"; +import { applyTimelineStackingReorder } from "./timelineEditingHelpers"; +import type { TimelineElement } from "../player/store/playerStore"; + +function makeIframeWith(html: string): HTMLIFrameElement { + const iframe = document.createElement("iframe"); + document.body.append(iframe); + const doc = iframe.contentDocument; + if (!doc) throw new Error("expected iframe document"); + doc.body.innerHTML = html; + return iframe; +} + +function el(input: Partial & { id: string; tag: string }): TimelineElement { + return { + label: input.id, + start: 0, + duration: 5, + track: 0, + zIndex: 0, + hasExplicitZIndex: false, + stackingContextId: null, + ...input, + }; +} + +describe("applyTimelineStackingReorder", () => { + it("commits via the change's own locator even when the element is not in timelineElements", () => { + // Sub-comp children live in the preview iframe but NOT in the top-level + // timelineElements list — the intent must be self-contained. + const iframe = makeIframeWith(`
`); + const commit = vi.fn<(entries: unknown[]) => void>(); + + applyTimelineStackingReorder({ + element: el({ id: "chip", tag: "div" }), + targetTrack: 0, + stackingReorder: { + contextKey: "scene", + placement: { type: "above", layerId: "layer:scene:x" }, + zIndexChanges: [ + { + key: "scenes/scene.html#chip", + zIndex: 5, + domId: "chip", + sourceFile: "scenes/scene.html", + }, + ], + }, + timelineElements: [], // element intentionally absent from the top-level list + iframe, + activeCompPath: "index.html", + commit, + }); + + expect(commit).toHaveBeenCalledTimes(1); + const entries = commit.mock.calls[0]![0] as Array<{ + zIndex: number; + id?: string; + sourceFile: string; + }>; + expect(entries).toHaveLength(1); + expect(entries[0]!.zIndex).toBe(5); + expect(entries[0]!.id).toBe("chip"); + expect(entries[0]!.sourceFile).toBe("scenes/scene.html"); + }); + + it("never commits when the dragged clip is audio", () => { + const iframe = makeIframeWith(``); + const commit = vi.fn<(entries: unknown[]) => void>(); + + applyTimelineStackingReorder({ + element: el({ id: "track", tag: "audio" }), + targetTrack: 0, + stackingReorder: { + contextKey: "main", + placement: { type: "above", layerId: "layer:main:x" }, + zIndexChanges: [{ key: "track", zIndex: 5, domId: "track" }], + }, + timelineElements: [], + iframe, + activeCompPath: "index.html", + commit, + }); + + expect(commit).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/studio/src/hooks/timelineEditingHelpers.ts b/packages/studio/src/hooks/timelineEditingHelpers.ts index 6a403a6b9..2dfe41b81 100644 --- a/packages/studio/src/hooks/timelineEditingHelpers.ts +++ b/packages/studio/src/hooks/timelineEditingHelpers.ts @@ -29,6 +29,7 @@ function isHTMLElement(element: Element | null): element is HTMLElement { * be resolved. Extracted from StudioApp's timeline hook to keep it under the * studio 600-LOC cap. */ +// fallow-ignore-next-line complexity export function applyTimelineStackingReorder(input: { element: TimelineElement; targetTrack: number; @@ -44,9 +45,19 @@ export function applyTimelineStackingReorder(input: { const intent = input.stackingReorder ?? null; if (intent == null || intent.zIndexChanges.length === 0) return; + // Resolve each change's live element from the change's OWN locator (the intent + // is self-contained), falling back to the top-level element list. Sub-comp + // children aren't in `timelineElements`, so a list-only lookup would miss them. const siblingByKey = new Map( input.timelineElements.map((el) => [getTimelineElementIdentity(el), el]), ); + const doc = input.iframe?.contentDocument ?? null; + const findLive = (domId?: string, selector?: string, selectorIndex?: number): Element | null => { + if (!doc) return null; + if (domId) return doc.getElementById(domId); + if (selector) return doc.querySelectorAll(selector)[selectorIndex ?? 0] ?? null; + return null; + }; const commitEntries: Array<{ element: HTMLElement; zIndex: number; @@ -59,18 +70,20 @@ export function applyTimelineStackingReorder(input: { for (const change of intent.zIndexChanges) { const sibling = siblingByKey.get(change.key); - if (!sibling) return; - const element = findTimelineElementInIframe(input.iframe, sibling); + const domId = change.domId ?? sibling?.domId; + const selector = change.selector ?? sibling?.selector; + const selectorIndex = change.selectorIndex ?? sibling?.selectorIndex; + const element = findLive(domId, selector, selectorIndex); if (!isHTMLElement(element)) return; if (getElementZIndex(element) === change.zIndex) continue; commitEntries.push({ element, zIndex: change.zIndex, - id: sibling.domId ?? sibling.id, - selector: sibling.selector, - selectorIndex: sibling.selectorIndex, - sourceFile: sibling.sourceFile || input.activeCompPath || "index.html", - key: getTimelineElementIdentity(sibling), + id: domId ?? sibling?.id ?? change.key, + selector, + selectorIndex, + sourceFile: change.sourceFile ?? sibling?.sourceFile ?? input.activeCompPath ?? "index.html", + key: change.key, }); } diff --git a/packages/studio/src/player/components/timelineLayerDrag.ts b/packages/studio/src/player/components/timelineLayerDrag.ts index 6cb542596..d87bf7ad2 100644 --- a/packages/studio/src/player/components/timelineLayerDrag.ts +++ b/packages/studio/src/player/components/timelineLayerDrag.ts @@ -27,11 +27,18 @@ function layerContainsElement(layer: StackingTimelineLayer, key: string): boolea function addElementChange( changes: TimelineStackingZIndexChange[], - key: string, - currentZIndex: number, + element: TimelineStackingElement, zIndex: number, ): void { - if (currentZIndex !== zIndex) changes.push({ key, zIndex }); + if ((element.zIndex ?? 0) === zIndex) return; + changes.push({ + key: getTimelineElementIdentity(element), + zIndex, + domId: element.domId, + selector: element.selector, + selectorIndex: element.selectorIndex, + sourceFile: element.sourceFile, + }); } function addLayerChanges( @@ -45,7 +52,7 @@ function addLayerChanges( const key = getTimelineElementIdentity(element); if (key === excludedKey) continue; const before = changes.length; - addElementChange(changes, key, element.zIndex ?? 0, zIndex); + addElementChange(changes, element, zIndex); if (changes.length > before) count += 1; } return count; @@ -70,12 +77,7 @@ function resolvePlacementZIndexChanges(input: { targetZIndex: number; }): TimelineStackingZIndexChange[] { const changes: TimelineStackingZIndexChange[] = []; - addElementChange( - changes, - getTimelineElementIdentity(input.element), - input.element.zIndex ?? 0, - input.targetZIndex, - ); + addElementChange(changes, input.element, input.targetZIndex); return changes; } diff --git a/packages/studio/src/player/components/timelineStacking.ts b/packages/studio/src/player/components/timelineStacking.ts index 11c82156c..5235cca19 100644 --- a/packages/studio/src/player/components/timelineStacking.ts +++ b/packages/studio/src/player/components/timelineStacking.ts @@ -9,6 +9,13 @@ export interface TimelineStackingElement { stackingContextId?: string | null; parentCompositionId?: string | null; compositionAncestors?: string[]; + // Locator for resolving the live element at commit time (sub-comp children + // aren't in the top-level element list, so the reorder intent must be + // self-contained rather than re-looked-up by identity). + domId?: string; + selector?: string; + selectorIndex?: number; + sourceFile?: string; } export interface TimelineStackingOrderItem { @@ -29,6 +36,10 @@ export type TimelineLayerDropPlacement = export interface TimelineStackingZIndexChange { key: string; zIndex: number; + domId?: string; + selector?: string; + selectorIndex?: number; + sourceFile?: string; } export interface TimelineStackingReorderIntent {