mirror of
https://github.com/heygen-com/hyperframes.git
synced 2026-09-01 19:42:03 +00:00
fix(studio): stop injecting inline z-index on all clips during timeline edits
Timeline move, delete, and asset-drop operations were looping over every clip in the file and writing style="z-index: N" derived from an inverted data-track-index mapping. This silently overrode the author's CSS z-index — contradicting the documented contract that data-track-index does not affect visual layering — and persisted the corruption in the source HTML. Remove the z-index injection loops from all three timeline commit paths. Move and delete now only patch timing/track attributes on the affected clip. Asset drop still sets z-index on the newly created element via the generated HTML, without touching existing clips. Delete the now-unused buildTrackZIndexMap helper. Also fix patchInlineStyleInTag to handle self-closing void elements: the old code produced malformed `<img ... / style="z-index: 7">` because it didn't account for the trailing `/` before appending the style attribute. Closes #958
This commit is contained in:
@@ -5,5 +5,7 @@ packages/studio/src/components/editor/manualEdits.test.ts
|
||||
packages/studio/src/player/hooks/useTimelinePlayer.test.ts
|
||||
packages/studio/src/components/editor/manualEditsDom.ts
|
||||
packages/studio/src/utils/sourcePatcher.ts
|
||||
packages/studio/src/utils/sourcePatcher.test.ts
|
||||
packages/studio/src/App.tsx
|
||||
packages/studio/src/player/components/Timeline.tsx
|
||||
packages/studio/src/player/components/timelineEditing.test.ts
|
||||
|
||||
@@ -2,10 +2,7 @@ import { useCallback, useRef } from "react";
|
||||
import type { TimelineElement } from "../player";
|
||||
import { usePlayerStore } from "../player";
|
||||
import { applyPatchByTarget, readAttributeByTarget } from "../utils/sourcePatcher";
|
||||
import {
|
||||
buildTrackZIndexMap,
|
||||
formatTimelineAttributeNumber,
|
||||
} from "../player/components/timelineEditing";
|
||||
import { formatTimelineAttributeNumber } from "../player/components/timelineEditing";
|
||||
import {
|
||||
buildTimelineAssetId,
|
||||
buildTimelineAssetInsertHtml,
|
||||
@@ -101,16 +98,6 @@ export function useTimelineEditing({
|
||||
throw new Error(`Timeline element ${element.id} is missing a patchable target`);
|
||||
}
|
||||
|
||||
const resolvedTargetPath = targetPath || "index.html";
|
||||
const relevantElements = timelineElements
|
||||
.map((te) =>
|
||||
(te.key ?? te.id) === (element.key ?? element.id)
|
||||
? { ...te, start: updates.start, track: updates.track }
|
||||
: te,
|
||||
)
|
||||
.filter((te) => (te.sourceFile || activeCompPath || "index.html") === resolvedTargetPath);
|
||||
const trackZIndices = buildTrackZIndexMap(relevantElements.map((te) => te.track));
|
||||
|
||||
let patchedContent = applyPatchByTarget(originalContent, patchTarget, {
|
||||
type: "attribute",
|
||||
property: "start",
|
||||
@@ -121,17 +108,6 @@ export function useTimelineEditing({
|
||||
property: "track-index",
|
||||
value: String(updates.track),
|
||||
});
|
||||
for (const te of relevantElements) {
|
||||
const elementTarget = buildPatchTarget(te);
|
||||
if (!elementTarget) continue;
|
||||
const nextZIndex = trackZIndices.get(te.track);
|
||||
if (nextZIndex == null) continue;
|
||||
patchedContent = applyPatchByTarget(patchedContent, elementTarget, {
|
||||
type: "inline-style",
|
||||
property: "z-index",
|
||||
value: String(nextZIndex),
|
||||
});
|
||||
}
|
||||
|
||||
if (patchedContent === originalContent) {
|
||||
throw new Error(`Unable to patch timeline element ${element.id} in ${targetPath}`);
|
||||
@@ -150,14 +126,7 @@ export function useTimelineEditing({
|
||||
|
||||
reloadPreview();
|
||||
},
|
||||
[
|
||||
activeCompPath,
|
||||
recordEdit,
|
||||
timelineElements,
|
||||
writeProjectFile,
|
||||
domEditSaveTimestampRef,
|
||||
reloadPreview,
|
||||
],
|
||||
[activeCompPath, recordEdit, writeProjectFile, domEditSaveTimestampRef, reloadPreview],
|
||||
);
|
||||
|
||||
const handleTimelineElementResize = useCallback(
|
||||
@@ -247,14 +216,6 @@ export function useTimelineEditing({
|
||||
throw new Error(`Timeline element ${element.id} is missing a patchable target`);
|
||||
}
|
||||
|
||||
const resolvedTargetPath = targetPath || "index.html";
|
||||
const remainingElements = timelineElements.filter(
|
||||
(te) =>
|
||||
(te.key ?? te.id) !== (element.key ?? element.id) &&
|
||||
(te.sourceFile || activeCompPath || "index.html") === resolvedTargetPath,
|
||||
);
|
||||
const trackZIndices = buildTrackZIndexMap(remainingElements.map((te) => te.track));
|
||||
|
||||
const removeResponse = await fetch(
|
||||
`/api/projects/${pid}/file-mutations/remove-element/${encodeURIComponent(targetPath)}`,
|
||||
{
|
||||
@@ -271,19 +232,8 @@ export function useTimelineEditing({
|
||||
changed?: boolean;
|
||||
content?: string;
|
||||
};
|
||||
let patchedContent =
|
||||
const patchedContent =
|
||||
typeof removeData.content === "string" ? removeData.content : originalContent;
|
||||
for (const te of remainingElements) {
|
||||
const elementTarget = buildPatchTarget(te);
|
||||
if (!elementTarget) continue;
|
||||
const nextZIndex = trackZIndices.get(te.track);
|
||||
if (nextZIndex == null) continue;
|
||||
patchedContent = applyPatchByTarget(patchedContent, elementTarget, {
|
||||
type: "inline-style",
|
||||
property: "z-index",
|
||||
value: String(nextZIndex),
|
||||
});
|
||||
}
|
||||
|
||||
domEditSaveTimestampRef.current = Date.now();
|
||||
await saveProjectFilesWithHistory({
|
||||
@@ -352,26 +302,10 @@ export function useTimelineEditing({
|
||||
const relevantElements = timelineElements.filter(
|
||||
(te) => (te.sourceFile || activeCompPath || "index.html") === resolvedTargetPath,
|
||||
);
|
||||
const trackZIndices = buildTrackZIndexMap([
|
||||
...relevantElements.map((te) => te.track),
|
||||
placement.track,
|
||||
]);
|
||||
const newElementZIndex = Math.max(1, relevantElements.length + 1);
|
||||
|
||||
let patchedContent = originalContent;
|
||||
for (const te of relevantElements) {
|
||||
const elementTarget = buildPatchTarget(te);
|
||||
if (!elementTarget) continue;
|
||||
const nextZIndex = trackZIndices.get(te.track);
|
||||
if (nextZIndex == null) continue;
|
||||
patchedContent = applyPatchByTarget(patchedContent, elementTarget, {
|
||||
type: "inline-style",
|
||||
property: "z-index",
|
||||
value: String(nextZIndex),
|
||||
});
|
||||
}
|
||||
|
||||
patchedContent = insertTimelineAssetIntoSource(
|
||||
patchedContent,
|
||||
const patchedContent = insertTimelineAssetIntoSource(
|
||||
originalContent,
|
||||
buildTimelineAssetInsertHtml({
|
||||
id: newId,
|
||||
assetPath: resolvedAssetSrc,
|
||||
@@ -379,7 +313,7 @@ export function useTimelineEditing({
|
||||
start: normalizedStart,
|
||||
duration: normalizedDuration,
|
||||
track: placement.track,
|
||||
zIndex: trackZIndices.get(placement.track) ?? 1,
|
||||
zIndex: newElementZIndex,
|
||||
geometry: resolveTimelineAssetInitialGeometry(originalContent),
|
||||
}),
|
||||
);
|
||||
|
||||
@@ -4,7 +4,6 @@ import {
|
||||
buildPromptCopyText,
|
||||
buildTimelineElementAgentPrompt,
|
||||
buildTimelineAgentPrompt,
|
||||
buildTrackZIndexMap,
|
||||
canOffsetTrimClipStart,
|
||||
getTimelineEditCapabilities,
|
||||
hasPatchableTimelineTarget,
|
||||
@@ -159,29 +158,6 @@ describe("resolveTimelineMove", () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe("buildTrackZIndexMap", () => {
|
||||
it("maps visually higher tracks onto higher z-index values", () => {
|
||||
expect(buildTrackZIndexMap([-2, -1, 0, 3])).toEqual(
|
||||
new Map([
|
||||
[-2, 4],
|
||||
[-1, 3],
|
||||
[0, 2],
|
||||
[3, 1],
|
||||
]),
|
||||
);
|
||||
});
|
||||
|
||||
it("deduplicates tracks before assigning z-index values", () => {
|
||||
expect(buildTrackZIndexMap([-1, 0, -1, 3, 3])).toEqual(
|
||||
new Map([
|
||||
[-1, 3],
|
||||
[0, 2],
|
||||
[3, 1],
|
||||
]),
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe("canOffsetTrimClipStart", () => {
|
||||
it("allows front trim for clips that carry playback offset metadata", () => {
|
||||
expect(
|
||||
|
||||
@@ -114,12 +114,6 @@ export function resolveTimelineMove(
|
||||
};
|
||||
}
|
||||
|
||||
export function buildTrackZIndexMap(tracks: number[]): Map<number, number> {
|
||||
const uniqueTracks = Array.from(new Set(tracks)).sort((a, b) => a - b);
|
||||
const maxZIndex = uniqueTracks.length;
|
||||
return new Map(uniqueTracks.map((track, index) => [track, maxZIndex - index]));
|
||||
}
|
||||
|
||||
export function resolveTimelineResize(
|
||||
input: TimelineResizeInput,
|
||||
edge: "start" | "end",
|
||||
|
||||
@@ -5,10 +5,7 @@ import {
|
||||
resolveTimelineAssetInitialGeometry,
|
||||
} from "./timelineAssetDrop";
|
||||
import { collectHtmlIds } from "./studioHelpers";
|
||||
import {
|
||||
buildTrackZIndexMap,
|
||||
formatTimelineAttributeNumber,
|
||||
} from "../player/components/timelineEditing";
|
||||
import { formatTimelineAttributeNumber } from "../player/components/timelineEditing";
|
||||
import { saveProjectFilesWithHistory } from "./studioFileHistory";
|
||||
import type { EditHistoryKind } from "./editHistory";
|
||||
|
||||
@@ -127,8 +124,7 @@ export async function addBlockToProject(
|
||||
? Math.max(...relevantElements.map((te) => te.track)) + 1
|
||||
: 1);
|
||||
|
||||
const trackZIndices = buildTrackZIndexMap([...relevantElements.map((te) => te.track), track]);
|
||||
const zIndex = trackZIndices.get(track) ?? 1;
|
||||
const zIndex = Math.max(1, relevantElements.length + 1);
|
||||
|
||||
const width = isBlock
|
||||
? (block as { dimensions: { width: number } }).dimensions.width
|
||||
|
||||
@@ -35,6 +35,28 @@ describe("applyPatchByTarget", () => {
|
||||
);
|
||||
});
|
||||
|
||||
it("adds inline style to a self-closing void element without malforming it", () => {
|
||||
const html = `<img id="gif-img" class="clip" data-start="1" src="earth.gif" alt="earth" />`;
|
||||
const op: PatchOperation = { type: "inline-style", property: "z-index", value: "3" };
|
||||
|
||||
const result = applyPatch(html, "gif-img", op);
|
||||
expect(result).toBe(
|
||||
`<img id="gif-img" class="clip" data-start="1" src="earth.gif" alt="earth" style="z-index: 3" />`,
|
||||
);
|
||||
expect(result).not.toContain("/ style");
|
||||
});
|
||||
|
||||
it("adds inline style to a self-closing void element matched by selector", () => {
|
||||
const html = `<img class="clip hero" data-start="0" src="bg.png" alt="" />`;
|
||||
const op: PatchOperation = { type: "inline-style", property: "opacity", value: "0.5" };
|
||||
|
||||
const result = applyPatchByTarget(html, { selector: ".hero" }, op);
|
||||
expect(result).toBe(
|
||||
`<img class="clip hero" data-start="0" src="bg.png" alt="" style="opacity: 0.5" />`,
|
||||
);
|
||||
expect(result).not.toContain("/ style");
|
||||
});
|
||||
|
||||
it("patches inline move styles by target", () => {
|
||||
const html = `<div id="card" style="position: absolute; left: 108px; top: 112px"></div>`;
|
||||
|
||||
|
||||
@@ -203,9 +203,9 @@ function patchInlineStyleInTag(
|
||||
} else {
|
||||
// No existing style attribute
|
||||
if (value === null) return html; // nothing to remove
|
||||
// Add one
|
||||
const newTag =
|
||||
tag.replace(/>$/, "") + ` style="${prop}: ${escapeStyleAttributeValue(value, '"')}"`;
|
||||
const selfClosing = /\s*\/$/.test(tag);
|
||||
const base = selfClosing ? tag.replace(/\s*\/$/, "") : tag;
|
||||
const newTag = `${base} style="${prop}: ${escapeStyleAttributeValue(value, '"')}"${selfClosing ? " /" : ""}`;
|
||||
return html.replace(tag, newTag);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -0,0 +1,155 @@
|
||||
import { describe, expect, it } from "vitest";
|
||||
import { applyPatchByTarget, applyPatch } from "./sourcePatcher";
|
||||
|
||||
/**
|
||||
* Reproduction tests for https://github.com/heygen-com/hyperframes/issues/958
|
||||
*
|
||||
* The bug: dragging a clip in the Studio timeline rewrites index.html with
|
||||
* inline style="z-index: N" on EVERY clip, overriding the author's CSS z-index.
|
||||
* Additionally, void elements (img, audio) get malformed self-closing tags.
|
||||
*/
|
||||
|
||||
const ISSUE_HTML = `<style>
|
||||
* { margin: 0; padding: 0; box-sizing: border-box; }
|
||||
html, body { margin: 0; width: 1920px; height: 1080px; overflow: hidden; background: #000; }
|
||||
#bg-video { position: absolute; inset: 0; width: 100%; height: 100%; object-fit: cover; z-index: 0; }
|
||||
#title { position: absolute; top: 50%; left: 50%; transform: translate(-50%,-50%);
|
||||
font: 700 120px sans-serif; color: #fff; z-index: 20; }
|
||||
</style>
|
||||
|
||||
<div id="root" data-composition-id="main" data-start="0" data-duration="10"
|
||||
data-width="1920" data-height="1080">
|
||||
<video id="bg-video" data-start="0" data-track-index="0" src="some-bg.mp4" muted playsinline></video>
|
||||
<div id="title" class="clip" data-start="0" data-duration="10" data-track-index="1">TITLE</div>
|
||||
</div>`;
|
||||
|
||||
const VOID_ELEMENT_HTML = `<div id="root" data-composition-id="main" data-start="0" data-duration="14"
|
||||
data-width="1920" data-height="1080">
|
||||
<video id="bg-video" data-start="0" data-track-index="0" src="some-bg.mp4" muted playsinline></video>
|
||||
<img id="gif-img" class="clip" data-start="1" data-duration="13" data-track-index="2" src="assets/earth.gif" alt="rotating earth gif" />
|
||||
<div id="title" class="clip" data-start="0" data-duration="10" data-track-index="1">TITLE</div>
|
||||
</div>`;
|
||||
|
||||
describe("issue #958 — timeline drag must not inject inline z-index", () => {
|
||||
it("reproduces the old bug: z-index injection on all clips overrides CSS layering", () => {
|
||||
// Simulate the OLD behavior: buildTrackZIndexMap + loop over all clips
|
||||
function buildTrackZIndexMap(tracks: number[]): Map<number, number> {
|
||||
const uniqueTracks = Array.from(new Set(tracks)).sort((a, b) => a - b);
|
||||
const maxZIndex = uniqueTracks.length;
|
||||
return new Map(uniqueTracks.map((track, index) => [track, maxZIndex - index]));
|
||||
}
|
||||
|
||||
const elements = [
|
||||
{ id: "bg-video", track: 0 },
|
||||
{ id: "title", track: 1 },
|
||||
];
|
||||
const trackZIndices = buildTrackZIndexMap(elements.map((e) => e.track));
|
||||
|
||||
// Apply the old z-index injection loop
|
||||
let broken = ISSUE_HTML;
|
||||
for (const el of elements) {
|
||||
const nextZIndex = trackZIndices.get(el.track);
|
||||
if (nextZIndex == null) continue;
|
||||
broken = applyPatch(broken, el.id, {
|
||||
type: "inline-style",
|
||||
property: "z-index",
|
||||
value: String(nextZIndex),
|
||||
});
|
||||
}
|
||||
|
||||
// Verify the bug: bg-video gets z-index: 2, title gets z-index: 1
|
||||
// This INVERTS the intended layering (CSS had bg-video: 0, title: 20)
|
||||
expect(broken).toContain('id="bg-video"');
|
||||
expect(broken).toContain('style="z-index: 2"');
|
||||
expect(broken).toContain('id="title"');
|
||||
expect(broken).toContain('style="z-index: 1"');
|
||||
|
||||
// The title (z-index: 1) is now BEHIND the video (z-index: 2)
|
||||
// — the opposite of the author's intent (CSS z-index: 20 vs 0)
|
||||
});
|
||||
|
||||
it("verifies the fix: moving a clip only patches data-start and data-track-index", () => {
|
||||
// Simulate the NEW behavior: only patch the moved clip's timing attributes
|
||||
const movedElement = { id: "bg-video" };
|
||||
const updates = { start: "2.5", track: "0" };
|
||||
|
||||
let fixed = applyPatchByTarget(
|
||||
ISSUE_HTML,
|
||||
{ id: movedElement.id },
|
||||
{ type: "attribute", property: "start", value: updates.start },
|
||||
);
|
||||
fixed = applyPatchByTarget(
|
||||
fixed,
|
||||
{ id: movedElement.id },
|
||||
{ type: "attribute", property: "track-index", value: updates.track },
|
||||
);
|
||||
|
||||
// The moved clip's timing changed
|
||||
expect(fixed).toContain('id="bg-video" data-start="2.5" data-track-index="0"');
|
||||
|
||||
// No inline z-index was injected on ANY element
|
||||
expect(fixed).not.toContain('style="z-index');
|
||||
|
||||
// The title clip is completely untouched
|
||||
expect(fixed).toContain(
|
||||
'<div id="title" class="clip" data-start="0" data-duration="10" data-track-index="1">TITLE</div>',
|
||||
);
|
||||
|
||||
// CSS z-index declarations in <style> are preserved
|
||||
expect(fixed).toContain("z-index: 0;");
|
||||
expect(fixed).toContain("z-index: 20;");
|
||||
});
|
||||
|
||||
it("verifies the fix: deleting a clip does not inject z-index on remaining clips", () => {
|
||||
// After the element removal API call returns the content without bg-video,
|
||||
// the old code would loop over remaining clips and inject z-index.
|
||||
// The new code just uses the removal result as-is.
|
||||
const afterRemoval = ISSUE_HTML.replace(/<video id="bg-video"[^>]*><\/video>\n /, "");
|
||||
|
||||
// No z-index injection step — the result is used directly
|
||||
expect(afterRemoval).not.toContain('style="z-index');
|
||||
expect(afterRemoval).toContain(
|
||||
'<div id="title" class="clip" data-start="0" data-duration="10" data-track-index="1">TITLE</div>',
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe("issue #958 — void element inline style injection", () => {
|
||||
it("reproduces the old bug: self-closing tags get malformed when style is injected", () => {
|
||||
// The old code did: tag.replace(/>$/, "") + ` style="z-index: 7"`
|
||||
// But `tag` from the regex capture never includes `>`, and for self-closing
|
||||
// elements it ends with `/`. The replace was a no-op, producing:
|
||||
// <img ... / style="z-index: 7">
|
||||
const img = `<img id="gif-img" class="clip" data-start="1" src="earth.gif" alt="earth" />`;
|
||||
const result = applyPatch(img, "gif-img", {
|
||||
type: "inline-style",
|
||||
property: "z-index",
|
||||
value: "7",
|
||||
});
|
||||
|
||||
// With the fix, the style is inserted before the self-closing slash
|
||||
expect(result).not.toContain("/ style");
|
||||
expect(result).toContain('style="z-index: 7" />');
|
||||
});
|
||||
|
||||
it("verifies inline style on void elements in a full composition", () => {
|
||||
// Even though we no longer inject z-index during move/delete,
|
||||
// the patchInlineStyleInTag fix is still important for other inline style
|
||||
// patches (e.g., position, opacity) that the Studio applies to void elements.
|
||||
const result = applyPatch(VOID_ELEMENT_HTML, "gif-img", {
|
||||
type: "inline-style",
|
||||
property: "opacity",
|
||||
value: "0.8",
|
||||
});
|
||||
|
||||
expect(result).toContain('style="opacity: 0.8" />');
|
||||
expect(result).not.toContain("/ style");
|
||||
// Other elements untouched
|
||||
expect(result).toContain(
|
||||
'<video id="bg-video" data-start="0" data-track-index="0" src="some-bg.mp4" muted playsinline></video>',
|
||||
);
|
||||
expect(result).toContain(
|
||||
'<div id="title" class="clip" data-start="0" data-duration="10" data-track-index="1">TITLE</div>',
|
||||
);
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user