diff --git a/packages/studio/src/player/components/timelineAuthoredMoveTarget.ts b/packages/studio/src/player/components/timelineAuthoredMoveTarget.ts index 6dc93c7968..c0adffcdfb 100644 --- a/packages/studio/src/player/components/timelineAuthoredMoveTarget.ts +++ b/packages/studio/src/player/components/timelineAuthoredMoveTarget.ts @@ -11,6 +11,7 @@ export function canMoveTimelineElement(element: TimelineElement): boolean { duration: element.duration, domId: element.domId, selector: element.selector, + hfId: element.hfId, compositionSrc: element.compositionSrc, playbackStart: element.playbackStart, playbackStartAttr: element.playbackStartAttr, diff --git a/packages/studio/src/player/components/timelineEditCapabilities.ts b/packages/studio/src/player/components/timelineEditCapabilities.ts index 9d211f4a2f..061faf58ae 100644 --- a/packages/studio/src/player/components/timelineEditCapabilities.ts +++ b/packages/studio/src/player/components/timelineEditCapabilities.ts @@ -23,8 +23,17 @@ function isDeterministicTimelineWindow(input: { return ["video", "audio", "img"].includes(input.tag.toLowerCase()); } -export function hasPatchableTimelineTarget(input: { domId?: string; selector?: string }): boolean { - return Boolean(input.domId || input.selector); +export function hasPatchableTimelineTarget(input: { + domId?: string; + selector?: string; + hfId?: string; +}): boolean { + // hfId counts as a stable target: Studio stamps data-hf-id into the source and + // findElementForSelection resolves it before id/selector, so a clip carrying only + // an hfId is just as patchable as one with an author-written id. Omitting it here + // made those clips report canMove:false and surface "This clip can't be moved or + // resized from the timeline yet", even though the write path fully supported them. + return Boolean(input.domId || input.selector || input.hfId); } export function getTimelineEditCapabilities(input: { @@ -33,6 +42,7 @@ export function getTimelineEditCapabilities(input: { duration: number; domId?: string; selector?: string; + hfId?: string; compositionSrc?: string; playbackStart?: number; playbackStartAttr?: "media-start" | "playback-start"; diff --git a/packages/studio/src/player/components/timelineEditing.test.ts b/packages/studio/src/player/components/timelineEditing.test.ts index 50b749c153..d3d2943db5 100644 --- a/packages/studio/src/player/components/timelineEditing.test.ts +++ b/packages/studio/src/player/components/timelineEditing.test.ts @@ -377,12 +377,58 @@ describe("hasPatchableTimelineTarget", () => { expect(hasPatchableTimelineTarget({ selector: ".hero-card" })).toBe(true); }); + it("returns true when the clip only has an hfId", () => { + // Studio stamps data-hf-id into the source and resolves it ahead of id/selector, + // so an hfId-only clip is patchable. Regression: it used to report false, which + // blocked dragging any authored clip the user had not given an id. + expect(hasPatchableTimelineTarget({ hfId: "hf-6wbt" })).toBe(true); + }); + it("returns false when the clip has no stable patch target", () => { expect(hasPatchableTimelineTarget({})).toBe(false); }); }); describe("getTimelineEditCapabilities", () => { + it("lets an hfId-only authored clip move and trim", () => { + // A plain
with no author id: the + // exact shape that produced "This clip can't be moved or resized from the + // timeline yet" while the DOM-edit path considered the same element editable. + expect( + getTimelineEditCapabilities({ + tag: "div", + kind: "element", + duration: 2.6, + hfId: "hf-opwy", + timingSource: "authored", + }), + ).toEqual({ canMove: true, canTrimStart: true, canTrimEnd: true }); + }); + + it("still refuses an hfId-only clip whose timing is implicit", () => { + expect( + getTimelineEditCapabilities({ + tag: "div", + kind: "element", + duration: 2.6, + hfId: "hf-opwy", + timingSource: "implicit", + }).canMove, + ).toBe(false); + }); + + it("still refuses an hfId-only clip on a locked row", () => { + expect( + getTimelineEditCapabilities({ + tag: "div", + kind: "element", + duration: 2.6, + hfId: "hf-opwy", + timelineLocked: true, + }).canMove, + ).toBe(false); + }); + it("does not disable editable audio just because it spans multiple scenes", () => { expect( getTimelineEditCapabilities({ diff --git a/packages/studio/src/player/components/timelineGroupEditing.ts b/packages/studio/src/player/components/timelineGroupEditing.ts index 567a98cd3e..76e4dac886 100644 --- a/packages/studio/src/player/components/timelineGroupEditing.ts +++ b/packages/studio/src/player/components/timelineGroupEditing.ts @@ -193,6 +193,7 @@ function canTrimEdge(element: TimelineElement, edge: TimelineGroupResizeEdge): b duration: element.duration, domId: element.domId, selector: element.selector, + hfId: element.hfId, compositionSrc: element.compositionSrc, playbackStart: element.playbackStart, playbackStartAttr: element.playbackStartAttr, diff --git a/packages/studio/src/player/lib/timelineDOM.test.ts b/packages/studio/src/player/lib/timelineDOM.test.ts index 78a0bc7ec6..82275f5735 100644 --- a/packages/studio/src/player/lib/timelineDOM.test.ts +++ b/packages/studio/src/player/lib/timelineDOM.test.ts @@ -6,6 +6,8 @@ import { createImplicitTimelineLayersFromDOM, mergeTimelineElementsPreservingDowngrades, } from "./timelineDOM"; +import { findTimelineDomNodeForClip } from "./timelineElementHelpers"; +import type { ClipManifestClip } from "./playbackTypes"; import type { TimelineElement } from "../store/playerStore"; function el(id: string, extra: Partial = {}): TimelineElement { @@ -329,3 +331,69 @@ describe("audio FX attributes on parsed elements", () => { expect(bgm?.automation).toBeUndefined(); }); }); + +describe("findTimelineDomNodeForClip — positional fallback never crosses tags", () => { + // Regression: a clip whose own element carries no [data-start] is not a + // candidate here. Before the tag guard it took candidates[fallbackIndex] — an + // unrelated scene
— which corrupted two clips at once: the img clip got a + // host that was not its element, and the div clip that owned that node was + // starved to null, losing hfId/domId/selector and therefore canMove. That is + // what surfaced as "This clip can't be moved or resized from the timeline yet". + const html = ` +
+
+
+
`; + + const imgClip: ClipManifestClip = { + id: null, + label: "Site Hero", + start: 0, + duration: 14, + track: 2, + kind: "image", + tagName: "img", + compositionId: null, + parentCompositionId: null, + compositionSrc: null, + assetUrl: null, + }; + + it("returns null instead of stealing a div for an img clip", () => { + const doc = makeDoc(html); + expect(findTimelineDomNodeForClip(doc, imgClip, 0)).toBeNull(); + }); + + it("leaves the div clips resolvable after an unmatched img clip", () => { + const doc = makeDoc(html); + const used = new Set(); + const stolen = findTimelineDomNodeForClip(doc, imgClip, 0, used); + if (stolen) used.add(stolen); + + const divClip: ClipManifestClip = { + ...imgClip, + label: "S", + start: 6, + duration: 2.6, + track: 1, + kind: "element", + tagName: "div", + }; + const host = findTimelineDomNodeForClip(doc, divClip, 1, used); + expect(host?.getAttribute("data-hf-id")).toBe("hf-b"); + }); + + it("still allows a same-tag positional fallback when attributes drift", () => { + const doc = makeDoc(html); + const drifted: ClipManifestClip = { + ...imgClip, + label: "S", + start: 99, + duration: 99, + track: 9, + kind: "element", + tagName: "div", + }; + expect(findTimelineDomNodeForClip(doc, drifted, 0)?.getAttribute("data-hf-id")).toBe("hf-a"); + }); +}); diff --git a/packages/studio/src/player/lib/timelineElementHelpers.ts b/packages/studio/src/player/lib/timelineElementHelpers.ts index fc403f9b2f..7a2dcc2c33 100644 --- a/packages/studio/src/player/lib/timelineElementHelpers.ts +++ b/packages/studio/src/player/lib/timelineElementHelpers.ts @@ -419,7 +419,18 @@ export function findTimelineDomNodeForClip( const exact = candidates.find((node) => nodeMatchesManifestClip(node, clip)); if (exact) return exact; - return candidates[fallbackIndex] ?? null; + // Positional fallback, but never across tags. A clip whose element carries no + // [data-start] of its own (an expanded child row, say) is not a candidate here, + // so without this guard it takes candidates[fallbackIndex] — an unrelated node — + // and corrupts TWO clips at once: the thief gets a host that is not its element, + // and the rightful owner is starved to null, losing hfId/domId/selector and with + // them canMove. That is what produced "This clip can't be moved or resized from + // the timeline yet" on clips that were perfectly well-formed. + const positional = candidates[fallbackIndex]; + if (!positional) return null; + const clipTag = clip.tagName?.toLowerCase(); + if (clipTag && positional.tagName.toLowerCase() !== clipTag) return null; + return positional; } // ---------------------------------------------------------------------------