diff --git a/packages/studio/src/components/editor/KeyframeEaseList.tsx b/packages/studio/src/components/editor/KeyframeEaseList.tsx index 4574433d7..9e53dd611 100644 --- a/packages/studio/src/components/editor/KeyframeEaseList.tsx +++ b/packages/studio/src/components/editor/KeyframeEaseList.tsx @@ -93,7 +93,11 @@ export function KeyframeEaseList({ ? "Custom" : (EASE_LABELS[segEase] ?? segEase); return ( -
+
- + {/* Decorative: the disclosure button above already names the row's keyframe + state, and aria-label on a plain span is not exposed reliably anyway. */} + diff --git a/packages/studio/src/player/components/Timeline.test.ts b/packages/studio/src/player/components/Timeline.test.ts index ed0310814..b1978443d 100644 --- a/packages/studio/src/player/components/Timeline.test.ts +++ b/packages/studio/src/player/components/Timeline.test.ts @@ -108,7 +108,7 @@ function renderBasicTimeline() { } describe("Timeline provider boundary", () => { - it("keeps all-collapsed horizontal positions at the 32px gutter", () => { + it("keeps all-collapsed horizontal positions at the gutter plus the pre-t=0 pad", () => { usePlayerStore.setState({ duration: 11, timelineReady: true, @@ -121,13 +121,13 @@ describe("Timeline provider boundary", () => { const { root, clip, trackHeader, rulerTick, rulerOrigin, playhead } = renderTimelineGeometry("clip-1"); - expect(trackHeader.style.width).toBe("32px"); + expect(trackHeader.style.width).toBe(`${GUTTER + TRACKS_LEFT_PAD}px`); expect(clip.style.left).toBe("1000px"); expect(clip.style.height).toBe(""); expect(clip.style.bottom).toBe(`${CLIP_Y}px`); - expect(rulerOrigin.style.width).toBe("32px"); + expect(rulerOrigin.style.width).toBe(`${GUTTER + TRACKS_LEFT_PAD}px`); expect(rulerTick.style.left).toBe("999.5px"); - expect(playhead.style.left).toBe(`${1032 - PLAYHEAD_HEAD_W / 2}px`); + expect(playhead.style.left).toBe(`${GUTTER + TRACKS_LEFT_PAD + 1000 - PLAYHEAD_HEAD_W / 2}px`); expect(playhead.style.width).toBe(`${PLAYHEAD_HEAD_W}px`); expect( resolveTimelineAssetDrop( diff --git a/packages/studio/src/player/components/Timeline.tsx b/packages/studio/src/player/components/Timeline.tsx index fb111dfc2..5279eeb31 100644 --- a/packages/studio/src/player/components/Timeline.tsx +++ b/packages/studio/src/player/components/Timeline.tsx @@ -21,7 +21,7 @@ import { useTimelineEditPinning } from "./useTimelineEditPinning"; import { useTimelineStackingSync } from "./useTimelineStackingSync"; import { useTimelineGeometry } from "./useTimelineGeometry"; import { useAutoExpandKeyframedClips } from "./useAutoExpandKeyframedClips"; -import { GUTTER, LABEL_COL_W, generateTicks } from "./timelineLayout"; +import { GUTTER, LABEL_COL_W, TRACKS_LEFT_PAD, generateTicks } from "./timelineLayout"; import { useTimelineScrollViewport } from "./useTimelineScrollViewport"; import { STUDIO_PREVIEW_FPS } from "../lib/time"; import { useResolvedTimelineEditCallbacks } from "./useResolvedTimelineEditCallbacks"; @@ -127,7 +127,10 @@ export const Timeline = memo(function Timeline({ [gsapAnimations], ); const labelMode = STUDIO_KEYFRAMES_ENABLED && hasKeyframedClips; - const contentOrigin = labelMode ? LABEL_COL_W + GUTTER : GUTTER; + // Without the label column the pre-t=0 breathing room is still TRACKS_LEFT_PAD + // (dropping it would jam clip 0 against the gutter on every non-keyframed + // composition); in label mode the 232px label column already provides it. + const contentOrigin = labelMode ? LABEL_COL_W + GUTTER : GUTTER + TRACKS_LEFT_PAD; const contentGutter = labelMode ? GUTTER : 0; const setSelectedElementId = usePlayerStore((s) => s.setSelectedElementId); const currentTime = usePlayerStore((s) => s.currentTime); diff --git a/packages/studio/src/player/components/TimelineCanvas.tsx b/packages/studio/src/player/components/TimelineCanvas.tsx index 181ca290b..bdf63bbc5 100644 --- a/packages/studio/src/player/components/TimelineCanvas.tsx +++ b/packages/studio/src/player/components/TimelineCanvas.tsx @@ -47,6 +47,11 @@ export const TimelineCanvas = memo(function TimelineCanvas(props: TimelineCanvas const draggedRowIndex = draggedClip?.started === true ? displayTrackOrder.indexOf(draggedClip.previewTrack) : -1; const draggedRowHeight = getTimelineRowHeight(draggedRowIndex, props.rowHeights); + // A clip bar in an EXPANDED row still renders at TRACK_H (the property lanes + // occupy the rest of the row — see TimelineLanes' clipHeight), so the drag + // ghost and drop placeholder must clamp to it or they stretch to the full + // expanded row height and stop matching the clip being dragged. + const draggedClipHeight = Math.min(draggedRowHeight, TRACK_H) - CLIP_Y * 2; const { onResizeElement, onMoveElement, @@ -177,7 +182,7 @@ export const TimelineCanvas = memo(function TimelineCanvas(props: TimelineCanvas top: getTimelineRowTop(draggedRowIndex, props.rowHeights) + CLIP_Y, left: props.contentOrigin + draggedClip.previewStart * props.pps, width: Math.max(draggedClip.element.duration * props.pps, 4), - height: draggedRowHeight - CLIP_Y * 2, + height: draggedClipHeight, border: "1px solid rgba(60,230,172,0.55)", background: "rgba(60,230,172,0.12)", borderRadius: 4, @@ -230,7 +235,7 @@ export const TimelineCanvas = memo(function TimelineCanvas(props: TimelineCanvas top: activeDraggedPosition.top, left: activeDraggedPosition.left, width: Math.max(activeDraggedElement.duration * props.pps, 4), - height: draggedRowHeight - CLIP_Y * 2, + height: draggedClipHeight, zIndex: 40, }} > diff --git a/packages/studio/src/player/components/TimelineClipDiamonds.tsx b/packages/studio/src/player/components/TimelineClipDiamonds.tsx index 146aeb1fb..b3007f979 100644 --- a/packages/studio/src/player/components/TimelineClipDiamonds.tsx +++ b/packages/studio/src/player/components/TimelineClipDiamonds.tsx @@ -375,12 +375,23 @@ export const TimelineDiamondLane = memo(function TimelineDiamondLane({ pendingRetimeRef.current.delete(kfKey); } }; + // A rejected drop (the destination time is already occupied) snaps + // the diamond back to its source position, so the pending entry AND + // the selection have to revert with it — parking on the ghost drop + // position strands the playhead + selection on a keyframe that does + // not exist there. + const revertRetime = () => { + clearPending(); + onClickKeyframe?.(fromTarget); + }; void onMoveKeyframe?.(fromTarget, res.toClipPct).then((committed) => { - if (!committed) clearPending(); - }, clearPending); + if (!committed) revertRetime(); + }, revertRetime); // A retime still targeted this exact diamond — park/select it at its // new position, same as a plain click, or a drag that actually moved - // something looks identical to one that silently did nothing. + // something looks identical to one that silently did nothing. Done + // optimistically so the gesture stays responsive; revertRetime puts + // it back if the move is rejected. onClickKeyframe?.({ ...target, percentage: res.toClipPct, diff --git a/packages/studio/src/player/components/TimelinePropertyLanes.tsx b/packages/studio/src/player/components/TimelinePropertyLanes.tsx index bd518e772..6d9f02815 100644 --- a/packages/studio/src/player/components/TimelinePropertyLanes.tsx +++ b/packages/studio/src/player/components/TimelinePropertyLanes.tsx @@ -1,4 +1,4 @@ -import type { MouseEvent as ReactMouseEvent, RefObject } from "react"; +import { useMemo, type MouseEvent as ReactMouseEvent, type RefObject } from "react"; import { classifyPropertyGroup, type GsapAnimation, @@ -59,6 +59,14 @@ function sourceGroups(animations: readonly GsapAnimation[]) { return groups; } +/** Resolve the ease from THIS keyframe's own source tween. A lane can merge + * several tweens, so a shared lane-level fallback would label a segment with a + * different animation's ease than the one the ease editor targets (it routes + * by animationId). */ +function keyframeEase(keyframe: { ease?: string }, animation: GsapAnimation): string | undefined { + return keyframe.ease ?? animation.keyframes?.easeEach ?? animation.ease; +} + function groupKeyframes( animations: readonly GsapAnimation[], group: PropertyGroupName, @@ -79,6 +87,7 @@ function groupKeyframes( tweenPercentage: keyframe.percentage, propertyGroup: group, animationId: animation.id, + ease: keyframeEase(keyframe, animation), }); } } @@ -116,15 +125,33 @@ export function TimelinePropertyLanes({ onMoveKeyframe, suppressClickRef, }: TimelinePropertyLanesProps) { - if (clipWidthPx < 20 || clipDuration <= 0) return null; - const lanes = getTimelinePropertyLanes(animations, clipStart, clipDuration); + // Memoized: TimelineDiamondLane is React.memo'd, and rebuilding the lanes (and + // a fresh keyframesData literal per lane) on every render would re-render every + // diamond in every expanded clip on each playhead tick. + const lanes = useMemo( + () => + clipWidthPx < 20 || clipDuration <= 0 + ? [] + : getTimelinePropertyLanes(animations, clipStart, clipDuration), + [animations, clipStart, clipDuration, clipWidthPx], + ); + const laneData = useMemo( + () => + lanes.map((lane) => ({ + ...lane, + keyframesData: { format: "percentage" as const, keyframes: lane.keyframes }, + })), + [lanes], + ); - if (lanes.length === 0) return null; + if (laneData.length === 0) return null; return ( <> - {lanes.map(({ group, animations: groupAnimations, keyframes }, laneIndex) => ( + {laneData.map(({ group, keyframesData }, laneIndex) => (
; + ease?: string; }>, ): GsapAnimation { return { @@ -126,7 +127,7 @@ describe("TimelineTrackHeader", () => { const onTogglePropertyGroupKeyframe = vi.fn(); const view = renderHeader({ currentTime: 0.5, onTogglePropertyGroupKeyframe }); - click(view.host, "Toggle Opacity keyframe"); + click(view.host, "Add Opacity keyframe"); expect(onTogglePropertyGroupKeyframe).toHaveBeenLastCalledWith( ELEMENT, expect.objectContaining({ @@ -139,7 +140,7 @@ describe("TimelineTrackHeader", () => { ); view.rerender({ currentTime: 1, onTogglePropertyGroupKeyframe }); - click(view.host, "Toggle Opacity keyframe"); + click(view.host, "Remove Opacity keyframe"); expect(onTogglePropertyGroupKeyframe).toHaveBeenLastCalledWith( ELEMENT, expect.objectContaining({ @@ -214,13 +215,13 @@ describe("TimelineTrackHeader", () => { it("fills the toggle diamond exactly at that group's keyframe", () => { const view = renderHeader({ currentTime: 0.5 }); const positionToggle = view.host.querySelector( - 'button[aria-label="Toggle Position keyframe"]', + 'button[aria-label="Add Position keyframe"]', ); expect(positionToggle?.textContent).toBe("◇"); view.rerender({ currentTime: 1 }); expect( - view.host.querySelector('button[aria-label="Toggle Position keyframe"]') + view.host.querySelector('button[aria-label="Remove Position keyframe"]') ?.textContent, ).toBe("◆"); act(() => view.root.unmount()); @@ -241,6 +242,36 @@ describe("TimelineTrackHeader", () => { act(() => view.root.unmount()); }); + it("samples mid-segment values along the segment's ease, not linearly", () => { + // GSAP hangs a segment's ease on the keyframe it arrives at, so 0% -> 50% + // runs power2.in. Half way through that segment power2.in(0.5) = 0.125, so + // the readout is 12.5/6.25 and NOT the linear 50/25. + const eased = animation("eased-position", "position", [ + { percentage: 0, properties: { x: 0, y: 0 } }, + { percentage: 50, properties: { x: 100, y: 50 }, ease: "power2.in" }, + { percentage: 100, properties: { x: 200, y: 100 }, ease: "power2.in" }, + ]); + const onTogglePropertyGroupKeyframe = vi.fn(); + const view = renderHeader({ + animations: [eased], + currentTime: 0.5, + onTogglePropertyGroupKeyframe, + }); + + expect(view.host.querySelector('[data-property-group="position"]')?.textContent).toContain( + "12.5, 6.25", + ); + + // The same sampled value is what an added keyframe gets stamped with, so a + // header insert lands on the existing curve instead of deforming it. + click(view.host, "Add Position keyframe"); + expect(onTogglePropertyGroupKeyframe).toHaveBeenCalledOnce(); + expect(onTogglePropertyGroupKeyframe.mock.calls[0][1]).toMatchObject({ + properties: { x: 12.5, y: 6.25 }, + }); + act(() => view.root.unmount()); + }); + it("disables the previous chevron at or before the group's first keyframe", () => { const view = renderHeader({ currentTime: 0 }); const prevAt0 = view.host.querySelector( diff --git a/packages/studio/src/player/components/TimelineTrackHeader.tsx b/packages/studio/src/player/components/TimelineTrackHeader.tsx index 7364fb7c6..b808d8d21 100644 --- a/packages/studio/src/player/components/TimelineTrackHeader.tsx +++ b/packages/studio/src/player/components/TimelineTrackHeader.tsx @@ -129,6 +129,14 @@ function PropertyGroupNavigation({ onSeek?: (time: number) => void; children: React.ReactNode; }) { + // The 12x20px glyph is all the lane row has room for, so the WCAG 24x24 + // target is met with a centered transparent ::before overlay instead of a + // bigger box; focus-visible matches every other control in this header. + const CHEVRON_BUTTON_CLASS = + "relative h-5 w-3 border-0 bg-transparent p-0 text-white/55 hover:text-white disabled:text-white/15 " + + "focus-visible:outline focus-visible:outline-1 focus-visible:outline-[#3CE6AC] " + + "before:absolute before:left-1/2 before:top-1/2 before:h-6 before:w-6 " + + "before:-translate-x-1/2 before:-translate-y-1/2 before:content-['']"; const seekTo = (keyframe: { percentage: number } | null) => { if (keyframe) { onSeek?.(expandedElement.start + (keyframe.percentage / 100) * expandedElement.duration); @@ -140,7 +148,7 @@ function PropertyGroupNavigation({ type="button" aria-label={`Previous ${label} keyframe`} disabled={!navigation.prevKeyframe} - className="h-5 w-3 border-0 bg-transparent p-0 text-white/55 hover:text-white disabled:text-white/15" + className={CHEVRON_BUTTON_CLASS} onClick={(event) => { event.stopPropagation(); seekTo(navigation.prevKeyframe); @@ -153,7 +161,7 @@ function PropertyGroupNavigation({ type="button" aria-label={`Next ${label} keyframe`} disabled={!navigation.nextKeyframe} - className="h-5 w-3 border-0 bg-transparent p-0 text-white/55 hover:text-white disabled:text-white/15" + className={CHEVRON_BUTTON_CLASS} onClick={(event) => { event.stopPropagation(); seekTo(navigation.nextKeyframe); @@ -242,7 +250,8 @@ function PropertyGroupHeaderRow({ >