Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
14 changes: 12 additions & 2 deletions packages/studio/src/player/components/timelineEditCapabilities.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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: {
Expand All @@ -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";
Expand Down
46 changes: 46 additions & 0 deletions packages/studio/src/player/components/timelineEditing.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 <div class="clip" data-start data-duration> 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({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
68 changes: 68 additions & 0 deletions packages/studio/src/player/lib/timelineDOM.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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> = {}): TimelineElement {
Expand Down Expand Up @@ -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 <div> — 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 = `
<div data-composition-id="root" data-start="0" data-duration="14">
<div data-hf-id="hf-a" class="s clip" data-start="0" data-duration="3" data-track-index="1"></div>
<div data-hf-id="hf-b" class="s clip" data-start="6" data-duration="2.6" data-track-index="1"></div>
</div>`;

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<Element>();
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");
});
});
13 changes: 12 additions & 1 deletion packages/studio/src/player/lib/timelineElementHelpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

// ---------------------------------------------------------------------------
Expand Down
Loading