mirror of
https://github.com/heygen-com/hyperframes.git
synced 2026-09-03 23:03:09 +00:00
fix(studio): stop injecting inline z-index on all clips during timeline edits (#959)
## Summary - Remove the z-index injection loops from timeline move, delete, and asset-drop commit paths — only timing/track attributes are now patched on the affected clip, leaving all other clips untouched - Fix `patchInlineStyleInTag` to handle self-closing void elements (`<img />`, `<audio />`) — the old code produced malformed `<img ... / style="z-index: 7">` output - Delete the now-unused `buildTrackZIndexMap` helper and its tests ## Root cause Three timeline operations (`handleTimelineElementMove`, `handleTimelineElementDelete`, `handleTimelineAssetDrop`) looped over every clip in the file on each commit and injected `style="z-index: N"` derived from an inverted `data-track-index` mapping via `buildTrackZIndexMap`. This 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. ## Test plan - [x] Reproduction test covering old bug behavior (inline z-index injection on all clips, inverted layering) - [x] Verification tests confirming move/delete only patches the affected clip's timing attributes - [x] Void element tests confirming `<img ... style="..." />` output (not `<img ... / style="...">`) - [x] End-to-end browser test: opened Studio, dragged badge clip in timeline via CDP, verified only `data-start` changed on the dragged clip with zero inline z-index injections - [x] Full test suite: 585 tests pass (54 files) - [x] Build, lint, format, typecheck all green Closes #958
This commit is contained in:
Miguel Ángel
@@ -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