ext/hyperframes
2
0
Fork 0
mirror of https://github.com/heygen-com/hyperframes.git synced 2026-09-11 14:50:02 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(studio): close the timeline regressions the QA triage attributed here

Browse Source 
The QA fleet's 295 findings were replayed against the merge-base. Most were
pre-existing, but these were caused by this stack:

Deleting one keyframe destroyed the whole tween. The lane-header remove toggle
escalated a flat tween to a whole-animation delete, which took the authored
`tl.to(...)` and its source comment with it on a single click. The base build
posts remove-keyframe and lets the writer refuse it; restore that.

A keyframed layer could not be hidden at all. The visibility eye had moved off
the always-mounted layer row onto a hover-gated property-group row, so it only
existed while the lanes were expanded AND the pointer was over that lane. A
keyboard-only user could reach no eye at all, and its label named a track its
row did not act on. It goes back on the layer row.

A drag from the centre of a clip bar did nothing, because the 16px inline ease
button sits exactly there and swallowed the press. It now lets the press through
to the clip and keeps only the click, dropped if the pointer travelled.

Dragging a diamond onto a neighbour silently discarded the retime. The clamp
bounded the dragged keyframe by the whole merged row, so two animations
colliding at one percentage pinned each other in place and the drag resolved
back to a click. Clamp against the dragged keyframe's own tween instead.

Also: floor the diamond hit box at 12px (the gap-derived size fell to ~7px at
the zoom floor), round the diamond tooltip percentage, and prune the keyframe
caches when a composition switch drops a file from the scan set — each file only
ever cleared its own entries, so the previous composition leaked every element
into both keyframeCache and gsapAnimations, with nothing to evict it.
This commit is contained in:
Miguel Angel Simon Sierra
2026-07-28 00:40:45 +02:00
parent 538264463c
commit 8b38a8562e
12 changed files with 335 additions and 187 deletions
Download Patch File Download Diff File Expand all files Collapse all files
packages/studio/src/player/components/LayerDisclosureRow.tsx
+4
View File
@@ -11,12 +11,15 @@ export function LayerDisclosureRow({
isExpanded,
gutterBackground,
onToggleClipExpanded,
children,
}: {
keyframeClip: TimelineElement;
clipCount: number;
isExpanded: boolean;
gutterBackground: string;
onToggleClipExpanded: () => void;
/** Trailing controls that act on the LAYER (the visibility eye), not on a lane. */
children?: React.ReactNode;
}) {
const name = keyframeClip.label ?? keyframeClip.domId ?? keyframeClip.id;
return (
@@ -57,6 +60,7 @@ export function LayerDisclosureRow({
{name}
</span>
<TrackClipCount clipCount={clipCount} />
{children}
</div>
);
}
packages/studio/src/player/components/TimelineClipDiamonds.test.tsx
+5 -2
View File
@@ -46,7 +46,10 @@ function renderDiamonds(onClickKeyframe = vi.fn()) {
}
describe("TimelineClipDiamonds", () => {
it("keeps dense keyframe hit regions and visuals from overlapping", () => {
// Dense rows narrow the DIAMOND so neighbours stay individually readable, but
// the hit box floors at KF_MIN_HIT_W — a gap-sized target gets unusable
// (~7px) at the zoom floor.
it("narrows dense keyframe visuals while flooring their hit regions", () => {
const host = document.createElement("div");
document.body.append(host);
const root = createRoot(host);
@@ -76,7 +79,7 @@ describe("TimelineClipDiamonds", () => {
const diamonds = Array.from(host.querySelectorAll<HTMLButtonElement>("button[title]"));
expect(diamonds).toHaveLength(3);
for (const diamond of diamonds) {
expect(Number.parseFloat(diamond.style.width)).toBeCloseTo(10.8);
expect(Number.parseFloat(diamond.style.width)).toBeCloseTo(12);
expect(Number(diamond.querySelector("svg")?.getAttribute("width"))).toBeCloseTo(8.8);
}
act(() => root.unmount());
packages/studio/src/player/components/TimelineClipDiamonds.tsx
+46 -97
View File
@@ -1,11 +1,11 @@
import { Fragment, memo, useEffect, useRef, useState } from "react";
import { memo, useEffect, useRef, useState } from "react";
import { BEAT_BAND_H } from "./BeatStrip";
import {
KEYFRAME_DRAG_THRESHOLD_PX,
previewClipPct,
resolveKeyframeDrag,
} from "../../components/editor/keyframeDrag";
import { MiniCurveSvg } from "../../components/editor/EaseCurveSection";
import { TimelineDiamondConnectors } from "./TimelineDiamondConnectors";
import { clipToTweenPercentage } from "../../components/editor/KeyframeNavigation";
import { LANE_H } from "./timelineLayout";
import { timelineKeyframeSelectionKey } from "./timelineKeyframeIdentity";
@@ -16,11 +16,24 @@ import {
keyframeTarget,
type DragState,
type TimelineClipDiamondsProps,
type TimelineDiamondKeyframe,
type TimelineDiamondLaneProps,
} from "./timelineDiamondTypes";
export type { TimelineDiamondKeyframe } from "./timelineDiamondTypes";
// Floor for a diamond's clickable width. The visual size still narrows to the
// neighbour gap so packed diamonds stay individually readable, but the hit box
// stops there: at the zoom floor the gap alone left a ~7px target, which is
// neither hittable nor selectable with any accuracy. Boxes may overlap slightly
// below this width; each diamond still owns the half-gap around its own centre.
const KF_MIN_HIT_W = 12;
/** A clip-% is a float division, so a raw tooltip reads `25.032499999999995%`. */
function roundPct(percentage: number): number {
return Math.round(percentage * 1000) / 1000;
}
export const TimelineDiamondLane = memo(function TimelineDiamondLane({
keyframesData,
clipWidthPx,
@@ -115,9 +128,15 @@ export const TimelineDiamondLane = memo(function TimelineDiamondLane({
const sorted = keyframesData.keyframes
.filter((kf) => kf.percentage >= KF_MIN_PCT && kf.percentage <= KF_MAX_PCT)
.sort((a, b) => a.percentage - b.percentage);
// Clip-%s of the sorted keyframes — the neighbour clamp (preview + drop) needs
// the whole row to bound the dragged diamond between its immediate siblings.
const sortedClipPcts = sorted.map((k) => k.percentage);
// The neighbour clamp bounds a dragged diamond between its immediate siblings
// so a retime can't reorder the tween. Siblings means "keyframes of the SAME
// tween": a merged row interleaves several animations, and two of them
// colliding at one percentage would otherwise pin each other's diamonds in
// place — the drag clamped back onto its own position and resolved to a click.
const siblingRowOf = (keyframe: TimelineDiamondKeyframe) =>
keyframe.animationId === undefined
? sorted
: sorted.filter((k) => k.animationId === keyframe.animationId);
const centerXOf = (percentage: number) =>
Math.max(0, Math.min(clipWidthPx, (percentage / 100) * clipWidthPx));
// One record per diamond, carrying its own geometry, so the connector and
@@ -129,12 +148,12 @@ export const TimelineDiamondLane = memo(function TimelineDiamondLane({
const previousGap = previous ? centerX - centerXOf(previous.percentage) : Infinity;
const nextGap = next ? centerXOf(next.percentage) - centerX : Infinity;
const nearestGap = Math.max(1, Math.min(previousGap, nextGap));
const hitWidth = Math.min(diamondSize, nearestGap);
const gapWidth = Math.min(diamondSize, nearestGap);
return {
keyframe,
centerX,
hitWidth,
visualSize: hitWidth === diamondSize ? diamondSize : Math.max(2, hitWidth - 2),
hitWidth: Math.max(KF_MIN_HIT_W, gapWidth),
visualSize: gapWidth === diamondSize ? diamondSize : Math.max(2, gapWidth - 2),
};
});
const baseColor = isSelected ? accentColor : "#a3a3a3";
@@ -155,95 +174,25 @@ export const TimelineDiamondLane = memo(function TimelineDiamondLane({
pointerEvents: "none",
}}
>
{markers.map((marker, i) => {
const previous = markers[i - 1];
if (!previous) return null;
const kf = marker.keyframe;
const x1 = previous.centerX;
const x2 = marker.centerX;
if (x2 - x1 < 1) return null;
const connectorLeft = x1 + previous.visualSize / 2;
const connectorWidth = x2 - x1 - previous.visualSize / 2 - marker.visualSize / 2;
// The ease button targets one segment, so it needs the keyframe's own
// animationId/tweenPercentage. On a merged inline row the button is
// hidden where the segment is ambiguous (two source animations collide
// at this % with different eases; see easeAmbiguous) or the keyframe has
// no source animation id (runtime-scanned) so there is no tween to target.
const target = keyframeTarget(kf);
const ease = kf.ease ?? globalEase;
return (
<Fragment key={`line-${i}-${previous.keyframe.percentage}-${kf.percentage}`}>
<div
className="absolute"
data-keyframe-connector={groupAware ? "" : undefined}
style={{
left: connectorLeft,
top: centerY,
width: Math.max(0, connectorWidth),
height: 2,
transform: "translateY(-1px)",
background: baseColor,
opacity: baseOpacity,
borderRadius: 1,
}}
/>
{onSelectSegment && !kf.easeAmbiguous && kf.animationId !== undefined && (
<div
className="group absolute"
data-keyframe-ease-segment=""
style={{
left: x1,
top: centerY,
width: x2 - x1,
height: 18,
transform: "translateY(-50%)",
// Own a stacking context above the diamond buttons. At fit
// zoom the 16px ease control can overlap its neighbouring
// diamond; without a z-index here the later diamond wins the
// hit test even though the child button has z-index 3.
zIndex: 3,
// Only the centered control is interactive. The transparent
// segment wrapper must not swallow connector/clip gestures.
pointerEvents: "none",
}}
>
<button
type="button"
data-keyframe-ease-button=""
aria-label={`Edit ${ease} easing`}
title={`Edit ${ease} easing`}
className="absolute flex items-center justify-center rounded opacity-0 transition-opacity group-hover:opacity-100 focus-visible:opacity-100"
style={{
left: "50%",
top: "50%",
width: 16,
height: 16,
transform: "translate(-50%, -50%)",
zIndex: 3,
pointerEvents: "auto",
padding: 0,
border: "1px solid rgba(255, 255, 255, 0.14)",
background: "#171717",
cursor: "pointer",
}}
onPointerDown={(e) => e.stopPropagation()}
onClick={(e) => {
e.stopPropagation();
onSelectSegment(target);
}}
>
<MiniCurveSvg ease={ease} active size={12} />
</button>
</div>
)}
</Fragment>
);
})}
<TimelineDiamondConnectors
markers={markers}
centerY={centerY}
baseColor={baseColor}
baseOpacity={baseOpacity}
groupAware={groupAware}
globalEase={globalEase}
keyframeTarget={keyframeTarget}
onSelectSegment={onSelectSegment}
/>
{markers.map((marker, i) => {
const kf = marker.keyframe;
const target = keyframeTarget(kf);
const kfKey = timelineKeyframeSelectionKey(elementId, target);
// Clamp against this keyframe's own tween, not the whole merged row.
const siblingRow = siblingRowOf(kf);
const siblingClipPcts = siblingRow.map((k) => k.percentage);
const siblingIndex = siblingRow.indexOf(kf);
// While dragging this diamond, render it at the live preview clip-%.
const renderPct = preview?.kfKey === kfKey ? preview.clipPct : kf.percentage;
// Center the marker's non-overlapping hit region ON its keyframe %, so
@@ -266,7 +215,7 @@ export const TimelineDiamondLane = memo(function TimelineDiamondLane({
kfKey,
startX: e.clientX,
lastX: e.clientX,
index: i,
index: siblingIndex,
fromClipPct: pendingRetimeRef.current.get(kfKey)?.clipPct ?? kf.percentage,
moved: false,
};
@@ -292,7 +241,7 @@ export const TimelineDiamondLane = memo(function TimelineDiamondLane({
clipWidthPx,
draggedClipPct: live.fromClipPct,
draggedIndex: live.index,
sortedClipPcts,
sortedClipPcts: siblingClipPcts,
}),
});
});
@@ -329,8 +278,8 @@ export const TimelineDiamondLane = memo(function TimelineDiamondLane({
pointerUpX: e.clientX,
clipWidthPx,
draggedClipPct: d.fromClipPct,
draggedIndex: i,
sortedClipPcts,
draggedIndex: siblingIndex,
sortedClipPcts: siblingClipPcts,
});
if (res.kind === "click" || res.kind === "noop") {
// "noop" is a press with enough pointer jitter to arm a drag (canDrag
@@ -445,7 +394,7 @@ export const TimelineDiamondLane = memo(function TimelineDiamondLane({
e.stopPropagation();
onContextMenuKeyframe?.(e, target);
}}
title={`${kf.percentage}%`}
title={`${roundPct(kf.percentage)}%`}
>
<svg
width={marker.visualSize}
packages/studio/src/player/components/TimelineDiamondConnectors.tsx
+145
View File
@@ -0,0 +1,145 @@
import { Fragment, useRef } from "react";
import { KEYFRAME_DRAG_THRESHOLD_PX } from "../../components/editor/keyframeDrag";
import { MiniCurveSvg } from "../../components/editor/EaseCurveSection";
import type { TimelineKeyframeTarget } from "./timelineKeyframeIdentity";
import type { TimelineDiamondKeyframe } from "./TimelineClipDiamonds";
/** One diamond's geometry within its row, as computed by the lane. */
export interface TimelineDiamondMarker {
keyframe: TimelineDiamondKeyframe;
centerX: number;
hitWidth: number;
visualSize: number;
}
/**
* The line between each pair of adjacent diamonds, plus the ease control that
* sits at the segment's midpoint. Split out of TimelineClipDiamonds only to keep
* that file inside the repo's file-size cap; it has no state of its own beyond
* the press-position guard below.
*/
export function TimelineDiamondConnectors({
markers,
centerY,
baseColor,
baseOpacity,
groupAware,
globalEase,
keyframeTarget,
onSelectSegment,
}: {
markers: readonly TimelineDiamondMarker[];
centerY: number;
baseColor: string;
baseOpacity: number;
groupAware: boolean;
globalEase: string;
keyframeTarget: (keyframe: TimelineDiamondKeyframe) => TimelineKeyframeTarget;
onSelectSegment?: (target: TimelineKeyframeTarget) => void;
}) {
// The ease button sits dead centre of its segment, which on a two-keyframe clip
// is the centre of the clip bar — the natural place to grab a clip and drag it.
// Swallowing pointerdown there made that grab a no-op. Instead the press falls
// through to the clip (so the drag starts normally) and the button keeps only
// the click, which we drop if the pointer actually travelled.
const pressXRef = useRef<number | null>(null);
return (
<>
{markers.map((marker, i) => {
const previous = markers[i - 1];
if (!previous) return null;
const kf = marker.keyframe;
const x1 = previous.centerX;
const x2 = marker.centerX;
if (x2 - x1 < 1) return null;
const connectorLeft = x1 + previous.visualSize / 2;
const connectorWidth = x2 - x1 - previous.visualSize / 2 - marker.visualSize / 2;
// The ease button targets one segment, so it needs the keyframe's own
// animationId/tweenPercentage. On a merged inline row the button is
// hidden where the segment is ambiguous (two source animations collide
// at this % with different eases; see easeAmbiguous) or the keyframe has
// no source animation id (runtime-scanned) so there is no tween to target.
const target = keyframeTarget(kf);
const ease = kf.ease ?? globalEase;
return (
<Fragment key={`line-${i}-${previous.keyframe.percentage}-${kf.percentage}`}>
<div
className="absolute"
data-keyframe-connector={groupAware ? "" : undefined}
style={{
left: connectorLeft,
top: centerY,
width: Math.max(0, connectorWidth),
height: 2,
transform: "translateY(-1px)",
background: baseColor,
opacity: baseOpacity,
borderRadius: 1,
}}
/>
{onSelectSegment && !kf.easeAmbiguous && kf.animationId !== undefined && (
<div
className="group absolute"
data-keyframe-ease-segment=""
style={{
left: x1,
top: centerY,
width: x2 - x1,
height: 18,
transform: "translateY(-50%)",
// Own a stacking context above the diamond buttons. At fit
// zoom the 16px ease control can overlap its neighbouring
// diamond; without a z-index here the later diamond wins the
// hit test even though the child button has z-index 3.
zIndex: 3,
// Only the centered control is interactive. The transparent
// segment wrapper must not swallow connector/clip gestures.
pointerEvents: "none",
}}
>
<button
type="button"
data-keyframe-ease-button=""
aria-label={`Edit ${ease} easing`}
title={`Edit ${ease} easing`}
className="absolute flex items-center justify-center rounded opacity-0 transition-opacity group-hover:opacity-100 focus-visible:opacity-100"
style={{
left: "50%",
top: "50%",
width: 16,
height: 16,
transform: "translate(-50%, -50%)",
zIndex: 3,
pointerEvents: "auto",
padding: 0,
border: "1px solid rgba(255, 255, 255, 0.14)",
background: "#171717",
cursor: "pointer",
}}
onPointerDown={(e) => {
pressXRef.current = e.clientX;
}}
onClick={(e) => {
e.stopPropagation();
const pressX = pressXRef.current;
pressXRef.current = null;
if (
pressX !== null &&
Math.abs(e.clientX - pressX) >= KEYFRAME_DRAG_THRESHOLD_PX
) {
return;
}
onSelectSegment(target);
}}
>
<MiniCurveSvg ease={ease} active size={12} />
</button>
</div>
)}
</Fragment>
);
})}
</>
);
}
packages/studio/src/player/components/TimelineLanes.tsx
-5
View File
@@ -238,11 +238,6 @@ export function TimelineLanes({
currentTime={currentTime}
isTrackHidden={isTrackHidden}
isAudioTrack={isAudioTrack}
isActive={
keyframeClipKey != null &&
(selectedElementId === keyframeClipKey || selectedElementIds.has(keyframeClipKey))
}
isHovered={keyframeClipKey != null && hoveredClip === keyframeClipKey}
theme={theme}
onToggleClipExpanded={() => {
if (keyframeClipKey) {
packages/studio/src/player/components/TimelineTrackHeader.test.tsx
+11 -2
View File
@@ -90,8 +90,6 @@ function renderHeader(options: RenderHeaderOptions = {}): {
currentTime={next.currentTime ?? 0}
isTrackHidden={false}
isAudioTrack={false}
isActive
isHovered={false}
theme={defaultTimelineTheme}
onToggleClipExpanded={vi.fn()}
onToggleTrackHidden={vi.fn()}
@@ -123,6 +121,17 @@ describe("TimelineTrackHeader", () => {
act(() => view.root.unmount());
});
// The eye acts on the layer, so it has to be reachable without a pointer and
// in every disclosure state — a hover-gated eye is unusable by keyboard.
it("keeps the visibility eye mounted whether the layer is expanded or collapsed", () => {
const view = renderHeader({ expanded: true });
expect(view.host.querySelector('button[aria-label="Hide track 0"]')).not.toBeNull();
view.rerender({ expanded: false });
expect(view.host.querySelector('button[aria-label="Hide track 0"]')).not.toBeNull();
act(() => view.root.unmount());
});
it("adds and removes a keyframe on the explicitly targeted property-group tween", () => {
const onTogglePropertyGroupKeyframe = vi.fn();
const view = renderHeader({ currentTime: 0.5, onTogglePropertyGroupKeyframe });
packages/studio/src/player/components/TimelineTrackHeader.tsx
+14 -40
View File
@@ -1,6 +1,5 @@
import { useState } from "react";
import { Eye, EyeSlash } from "@phosphor-icons/react";
import type { GsapAnimation, PropertyGroupName } from "@hyperframes/core/gsap-parser";
import type { GsapAnimation } from "@hyperframes/core/gsap-parser";
import { Music } from "../../icons/SystemIcons";
import type { TimelineElement } from "../store/playerStore";
import type { TimelineEditCallbacks } from "./timelineCallbacks";
@@ -30,8 +29,6 @@ interface TimelineTrackHeaderProps {
currentTime: number;
isTrackHidden: boolean;
isAudioTrack: boolean;
isActive: boolean;
isHovered: boolean;
theme: TimelineTheme;
onToggleClipExpanded: () => void;
onToggleTrackHidden: TimelineEditCallbacks["onToggleTrackHidden"];
@@ -180,14 +177,7 @@ function PropertyGroupHeaderRow({
expandedElement,
currentTime,
clipPercentage,
hoveredGroup,
setHoveredGroup,
isActive,
isHovered,
isTrackHidden,
trackNumber,
gutterBackground,
onToggleTrackHidden,
onTogglePropertyGroupKeyframe,
onSeek,
}: {
@@ -197,14 +187,7 @@ function PropertyGroupHeaderRow({
expandedElement: TimelineElement;
currentTime: number;
clipPercentage: number;
hoveredGroup: PropertyGroupName | null;
setHoveredGroup: (group: PropertyGroupName | null) => void;
isActive: boolean;
isHovered: boolean;
isTrackHidden: boolean;
trackNumber: number;
gutterBackground: string;
onToggleTrackHidden: TimelineEditCallbacks["onToggleTrackHidden"];
onTogglePropertyGroupKeyframe?: TimelineEditCallbacks["onTogglePropertyGroupKeyframe"];
onSeek?: (time: number) => void;
}) {
@@ -213,9 +196,6 @@ function PropertyGroupHeaderRow({
currentTime,
clipPercentage,
);
const showEye =
hoveredGroup === lane.group ||
(hoveredGroup === null && laneIndex === 0 && (isActive || isHovered));
return (
<div
@@ -228,8 +208,6 @@ function PropertyGroupHeaderRow({
height: LANE_H,
background: gutterBackground,
}}
onPointerEnter={() => setHoveredGroup(lane.group)}
onPointerLeave={() => setHoveredGroup(null)}
>
{/* Tree connector: vertical spine (top-half on the last lane) + branch tick. */}
<span className="relative h-full w-3 shrink-0" aria-hidden="true">
@@ -272,12 +250,6 @@ function PropertyGroupHeaderRow({
>
{valueReadout(lane.group, values)}
</span>
<VisibilityButton
hidden={isTrackHidden}
trackNumber={trackNumber}
visible={showEye}
onToggle={onToggleTrackHidden}
/>
</div>
);
}
@@ -293,15 +265,12 @@ export function TimelineTrackHeader({
currentTime,
isTrackHidden,
isAudioTrack,
isActive,
isHovered,
theme,
onToggleClipExpanded,
onToggleTrackHidden,
onTogglePropertyGroupKeyframe,
onSeek,
}: TimelineTrackHeaderProps) {
const [hoveredGroup, setHoveredGroup] = useState<PropertyGroupName | null>(null);
const clipPercentage = keyframeClip
? ((currentTime - keyframeClip.start) / keyframeClip.duration) * 100
: 0;
@@ -346,7 +315,19 @@ export function TimelineTrackHeader({
isExpanded={isExpanded}
gutterBackground={theme.gutterBackground}
onToggleClipExpanded={onToggleClipExpanded}
/>
>
{/* The eye belongs to the LAYER, so it lives on the always-mounted
layer row exactly like a plain track's. Hanging it off a lane row
(hover-gated, and only while expanded) left a keyframed track with
no way to be hidden at all by keyboard, and put the control on a
row it does not act on. */}
<VisibilityButton
hidden={isTrackHidden}
trackNumber={trackNumber}
visible
onToggle={onToggleTrackHidden}
/>
</LayerDisclosureRow>
{isExpanded &&
lanes.map((lane, laneIndex) => (
<PropertyGroupHeaderRow
@@ -357,14 +338,7 @@ export function TimelineTrackHeader({
expandedElement={keyframeClip}
currentTime={currentTime}
clipPercentage={clipPercentage}
hoveredGroup={hoveredGroup}
setHoveredGroup={setHoveredGroup}
isActive={isActive}
isHovered={isHovered}
isTrackHidden={isTrackHidden}
trackNumber={trackNumber}
gutterBackground={theme.gutterBackground}
onToggleTrackHidden={onToggleTrackHidden}
onTogglePropertyGroupKeyframe={onTogglePropertyGroupKeyframe}
onSeek={onSeek}
/>