mirror of
https://github.com/heygen-com/hyperframes.git
synced 2026-09-03 12:54:29 +00:00
refactor(studio): single-source the timeline stacking key + guard audio reorder
The element stacking key (element.key ?? id) was recomputed in four places (reorder-intent generation, row ordering, the commit-time sibling lookup via a threaded keyOf param, and resolveTimelineMove). Any drift would silently break the sibling lookup and no-op the reorder. Route all of them through the existing getTimelineElementIdentity owner, share one toStackingOrderItem mapper between row ordering and reorder intent, and drop the keyOf parameter. Also enforce the audio side-effect invariant in the single mutation owner (applyTimelineStackingReorder): dragging an audio clip has no visual layer to restack, so it never writes z-index. Covered by a new hook test.
This commit is contained in:
@@ -6,6 +6,7 @@ import {
|
||||
type TimelineStackingReorderIntent,
|
||||
} from "../player/components/timelineEditing";
|
||||
import { computeReorderZValues, getElementZIndex } from "../player/lib/layerOrdering";
|
||||
import { getTimelineElementIdentity } from "../player/lib/timelineElementHelpers";
|
||||
import { saveProjectFilesWithHistory } from "../utils/studioFileHistory";
|
||||
import { selectedKeyframePercentagesForElement } from "../utils/keyframeSelection";
|
||||
import type { EditHistoryKind } from "../utils/editHistory";
|
||||
@@ -19,9 +20,10 @@ function isHTMLElement(element: Element | null): element is HTMLElement {
|
||||
* Resolve a timeline vertical move to a z-index stacking reorder and commit it
|
||||
* through the shared layers-panel reorder path. Reads live sibling z-index from
|
||||
* the preview DOM, remaps with the dup-preserving reorder math, and writes only
|
||||
* z-index (never data-track-index). No-op when the move isn't a reorder or the
|
||||
* live siblings can't be resolved. Extracted from StudioApp's timeline hook to
|
||||
* keep it under the studio 600-LOC cap.
|
||||
* z-index (never data-track-index). No-op when the move isn't a reorder, the
|
||||
* dragged clip is audio (no visual layer to restack), or the live siblings can't
|
||||
* be resolved. Extracted from StudioApp's timeline hook to keep it under the
|
||||
* studio 600-LOC cap.
|
||||
*/
|
||||
export function applyTimelineStackingReorder(input: {
|
||||
element: TimelineElement;
|
||||
@@ -31,8 +33,10 @@ export function applyTimelineStackingReorder(input: {
|
||||
iframe: HTMLIFrameElement | null;
|
||||
activeCompPath: string | null;
|
||||
commit: TimelineZIndexReorderCommit | null | undefined;
|
||||
keyOf: (element: TimelineElement) => string;
|
||||
}): void {
|
||||
// Audio has no visual stacking; a vertical drag on it must never write z-index.
|
||||
if (input.element.tag === "audio") return;
|
||||
|
||||
const intent =
|
||||
input.stackingReorder ??
|
||||
(input.targetTrack !== input.element.track
|
||||
@@ -44,7 +48,9 @@ export function applyTimelineStackingReorder(input: {
|
||||
: null);
|
||||
if (intent == null || intent.fromIndex === intent.toIndex) return;
|
||||
|
||||
const siblingByKey = new Map(input.timelineElements.map((el) => [input.keyOf(el), el]));
|
||||
const siblingByKey = new Map(
|
||||
input.timelineElements.map((el) => [getTimelineElementIdentity(el), el]),
|
||||
);
|
||||
const orderedSiblings = intent.siblingKeys
|
||||
.map((key) => siblingByKey.get(key) ?? null)
|
||||
.filter((sibling): sibling is TimelineElement => sibling != null);
|
||||
|
||||
@@ -49,12 +49,17 @@ function createPreviewIframe(
|
||||
return iframe;
|
||||
}
|
||||
|
||||
function timelineElement(input: { id: string; track: number; zIndex: number }): TimelineElement {
|
||||
function timelineElement(input: {
|
||||
id: string;
|
||||
track: number;
|
||||
zIndex: number;
|
||||
tag?: string;
|
||||
}): TimelineElement {
|
||||
return {
|
||||
id: input.id,
|
||||
domId: input.id,
|
||||
hfId: `hf-${input.id}`,
|
||||
tag: "div",
|
||||
tag: input.tag ?? "div",
|
||||
start: 0,
|
||||
duration: 2,
|
||||
track: input.track,
|
||||
@@ -235,6 +240,29 @@ describe("useTimelineEditing timeline z-index reorder", () => {
|
||||
unmount();
|
||||
});
|
||||
|
||||
it("never writes z-index when the dragged clip is audio (no visual layer)", async () => {
|
||||
const iframe = createPreviewIframe([
|
||||
{ id: "front", track: 0 },
|
||||
{ id: "music", track: 1 },
|
||||
]);
|
||||
const front = timelineElement({ id: "front", track: 0, zIndex: 0 });
|
||||
const music = timelineElement({ id: "music", track: 1, zIndex: 0, tag: "audio" });
|
||||
const commit = vi.fn<(entries: ZIndexEntry[]) => void>();
|
||||
const { move, unmount } = renderTimelineEditingHook({
|
||||
timelineElements: [front, music],
|
||||
iframe,
|
||||
onZIndexCommit: commit,
|
||||
});
|
||||
|
||||
await act(async () => {
|
||||
await move(music, { start: music.start, track: front.track });
|
||||
});
|
||||
|
||||
expect(commit).not.toHaveBeenCalled();
|
||||
|
||||
unmount();
|
||||
});
|
||||
|
||||
it("remaps distinct z-index values onto the reordered sibling group", async () => {
|
||||
const iframe = createPreviewIframe([
|
||||
{ id: "front", track: 0, style: "position: relative; z-index: 10" },
|
||||
|
||||
@@ -140,7 +140,6 @@ export function useTimelineEditing({
|
||||
iframe: previewIframeRef.current,
|
||||
activeCompPath,
|
||||
commit: handleDomZIndexReorderCommitRef?.current,
|
||||
keyOf: (el) => el.key ?? el.id,
|
||||
});
|
||||
|
||||
if (!startChanged) return;
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
import { formatTime } from "../lib/time";
|
||||
import { roundToCenti } from "../../utils/rounding";
|
||||
import { resolveContextOrder, resolveStackingContextKey } from "../lib/layerOrdering";
|
||||
import { getTimelineElementIdentity } from "../lib/timelineElementHelpers";
|
||||
|
||||
const roundToCentiseconds = roundToCenti;
|
||||
|
||||
@@ -32,7 +33,7 @@ export interface TimelineStackingReorderIntent {
|
||||
siblingKeys: string[];
|
||||
}
|
||||
|
||||
interface TimelineStackingOrderItem {
|
||||
export interface TimelineStackingOrderItem {
|
||||
key: string;
|
||||
track: number;
|
||||
zIndex: number;
|
||||
@@ -41,9 +42,9 @@ interface TimelineStackingOrderItem {
|
||||
compositionAncestors: readonly string[];
|
||||
}
|
||||
|
||||
function toStackingOrderItem(element: TimelineStackingElement): TimelineStackingOrderItem {
|
||||
export function toStackingOrderItem(element: TimelineStackingElement): TimelineStackingOrderItem {
|
||||
return {
|
||||
key: element.key ?? element.id,
|
||||
key: getTimelineElementIdentity(element),
|
||||
track: element.track,
|
||||
zIndex: element.zIndex ?? 0,
|
||||
stackingContextId: element.stackingContextId ?? null,
|
||||
@@ -77,8 +78,9 @@ export function resolveTimelineStackingReorderByTargetTrack(args: {
|
||||
}): TimelineStackingReorderIntent | null {
|
||||
const orderedSiblings = resolveContextSiblings(args.element, args.elements);
|
||||
if (orderedSiblings.length <= 1) return null;
|
||||
const draggedKey = args.element.key ?? args.element.id;
|
||||
const fromIndex = orderedSiblings.findIndex((sibling) => sibling.key === draggedKey);
|
||||
const fromIndex = orderedSiblings.findIndex(
|
||||
(sibling) => sibling.key === getTimelineElementIdentity(args.element),
|
||||
);
|
||||
if (fromIndex < 0) return null;
|
||||
const toIndex = orderedSiblings.findIndex((sibling) => sibling.track === args.targetTrack);
|
||||
if (toIndex < 0) return null;
|
||||
@@ -178,7 +180,7 @@ export function resolveTimelineMove(
|
||||
// stacking context (top = front), rather than changing the raw track number.
|
||||
if (input.stackingElement && input.stackingElements) {
|
||||
const orderedSiblings = resolveContextSiblings(input.stackingElement, input.stackingElements);
|
||||
const draggedKey = input.stackingElement.key ?? input.stackingElement.id;
|
||||
const draggedKey = getTimelineElementIdentity(input.stackingElement);
|
||||
const fromIndex = orderedSiblings.findIndex((sibling) => sibling.key === draggedKey);
|
||||
if (fromIndex >= 0 && orderedSiblings.length > 1) {
|
||||
const toIndex = clamp(fromIndex + deltaTrack, 0, orderedSiblings.length - 1);
|
||||
|
||||
@@ -1,39 +1,23 @@
|
||||
import { type TimelineElement } from "../store/playerStore";
|
||||
import {
|
||||
resolveContextOrder,
|
||||
resolveStackingContextKey,
|
||||
type ContextOrderItem,
|
||||
} from "../lib/layerOrdering";
|
||||
import { resolveContextOrder, resolveStackingContextKey } from "../lib/layerOrdering";
|
||||
import { getTimelineElementIdentity } from "../lib/timelineElementHelpers";
|
||||
import { toStackingOrderItem, type TimelineStackingOrderItem } from "./timelineEditing";
|
||||
|
||||
/**
|
||||
* Pure timeline track-ordering logic. Timeline rows are ordered by scoped
|
||||
* stacking (z-index per stacking context, top = front), with data-track-index
|
||||
* used only to split time-overlapping clips of equal rank onto separate rows.
|
||||
* Extracted from Timeline.tsx to keep the component under the studio 600-LOC cap.
|
||||
*
|
||||
* Key derivation and stacking-descriptor mapping are owned by timelineEditing so
|
||||
* the row order here and the reorder intent there interpret every element the
|
||||
* same way.
|
||||
*/
|
||||
|
||||
interface TimelineTrackOrderItem extends ContextOrderItem {
|
||||
key: string;
|
||||
track: number;
|
||||
start: number;
|
||||
duration: number;
|
||||
}
|
||||
|
||||
function getTimelineElementKey(element: TimelineElement): string {
|
||||
return element.key ?? element.id;
|
||||
}
|
||||
type TimelineTrackOrderItem = TimelineStackingOrderItem & { start: number; duration: number };
|
||||
|
||||
function toTimelineTrackOrderItem(element: TimelineElement): TimelineTrackOrderItem {
|
||||
return {
|
||||
key: getTimelineElementKey(element),
|
||||
track: element.track,
|
||||
start: element.start,
|
||||
duration: element.duration,
|
||||
zIndex: element.zIndex ?? 0,
|
||||
stackingContextId: element.stackingContextId ?? null,
|
||||
parentCompositionId: element.parentCompositionId ?? null,
|
||||
compositionAncestors: element.compositionAncestors ?? [],
|
||||
};
|
||||
return { ...toStackingOrderItem(element), start: element.start, duration: element.duration };
|
||||
}
|
||||
|
||||
function timelineElementsOverlap(
|
||||
@@ -51,7 +35,7 @@ function trackFrontOrderIndex(
|
||||
for (const element of elements) {
|
||||
orderIndex = Math.min(
|
||||
orderIndex,
|
||||
orderIndexByKey.get(getTimelineElementKey(element)) ?? Number.POSITIVE_INFINITY,
|
||||
orderIndexByKey.get(getTimelineElementIdentity(element)) ?? Number.POSITIVE_INFINITY,
|
||||
);
|
||||
}
|
||||
return orderIndex;
|
||||
|
||||
@@ -245,7 +245,7 @@ export function buildTimelineElementIdentity(params: {
|
||||
return { id, key };
|
||||
}
|
||||
|
||||
export function getTimelineElementIdentity(element: TimelineElement): string {
|
||||
export function getTimelineElementIdentity(element: { key?: string | null; id: string }): string {
|
||||
return element.key ?? element.id;
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user