From 8b38a8562e64bb4367878dddd07e842da19fdcf4 Mon Sep 17 00:00:00 2001 From: Miguel Angel Simon Sierra Date: Mon, 27 Jul 2026 15:34:14 +0200 Subject: [PATCH] fix(studio): close the timeline regressions the QA triage attributed here MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- .../nle/useTimelineEditCallbacks.test.tsx | 40 +++-- .../nle/useTimelineEditCallbacks.ts | 36 ++--- .../hooks/gsapKeyframeCacheHelpers.test.ts | 43 ++++++ .../src/hooks/gsapKeyframeCacheHelpers.ts | 28 ++++ .../studio/src/hooks/useGsapTweenCache.ts | 4 + .../player/components/LayerDisclosureRow.tsx | 4 + .../components/TimelineClipDiamonds.test.tsx | 7 +- .../components/TimelineClipDiamonds.tsx | 143 ++++++----------- .../components/TimelineDiamondConnectors.tsx | 145 ++++++++++++++++++ .../src/player/components/TimelineLanes.tsx | 5 - .../components/TimelineTrackHeader.test.tsx | 13 +- .../player/components/TimelineTrackHeader.tsx | 54 ++----- 12 files changed, 335 insertions(+), 187 deletions(-) create mode 100644 packages/studio/src/player/components/TimelineDiamondConnectors.tsx diff --git a/packages/studio/src/components/nle/useTimelineEditCallbacks.test.tsx b/packages/studio/src/components/nle/useTimelineEditCallbacks.test.tsx index 78aeb1227..1589f7061 100644 --- a/packages/studio/src/components/nle/useTimelineEditCallbacks.test.tsx +++ b/packages/studio/src/components/nle/useTimelineEditCallbacks.test.tsx @@ -205,7 +205,7 @@ describe("useTimelineEditCallbacks — flat tween keyframe lanes", () => { view.unmount(); }); - it("deletes a non-selected element flat boundary through the clicked element's selection", async () => { + it("refuses a non-selected element flat boundary instead of deleting the tween", async () => { const circle: TimelineElement = { ...element, id: "circle", @@ -229,12 +229,15 @@ describe("useTimelineEditCallbacks — flat tween keyframe lanes", () => { await Promise.resolve(); }); - // Persisted through the CLICKED element's own selection, not the current one. - expect(mocks.actions.handleGsapDeleteAnimation).toHaveBeenCalledWith( + // Persisted through the CLICKED element's own selection, not the current one, + // and as a remove-keyframe the writer can refuse — never a whole-tween delete. + expect(mocks.actions.handleGsapRemoveKeyframe).toHaveBeenCalledWith( otherFlatAnimation.id, + 0, + undefined, mocks.selection, ); - expect(mocks.actions.handleGsapRemoveKeyframe).not.toHaveBeenCalled(); + expect(mocks.actions.handleGsapDeleteAnimation).not.toHaveBeenCalled(); view.unmount(); }); @@ -313,7 +316,7 @@ describe("useTimelineEditCallbacks — flat tween keyframe lanes", () => { view.unmount(); }); - it("keeps selected-element flat boundary deletion on the animation delete path", () => { + it("keeps a selected-element flat boundary on the remove-keyframe path", () => { const view = renderCallbacks(); act(() => { @@ -325,15 +328,17 @@ describe("useTimelineEditCallbacks — flat tween keyframe lanes", () => { }); }); - expect(mocks.actions.handleGsapDeleteAnimation).toHaveBeenCalledWith( + expect(mocks.actions.handleGsapRemoveKeyframe).toHaveBeenCalledWith( flatAnimation.id, + 0, + undefined, undefined, ); - expect(mocks.actions.handleGsapRemoveKeyframe).not.toHaveBeenCalled(); + expect(mocks.actions.handleGsapDeleteAnimation).not.toHaveBeenCalled(); view.unmount(); }); - it("routes the flat lane-header remove toggle through the guarded delete path", async () => { + it("routes the flat lane-header remove toggle through the refusable remove path", async () => { const view = renderCallbacks(); await act(async () => { @@ -346,19 +351,20 @@ describe("useTimelineEditCallbacks — flat tween keyframe lanes", () => { }); }); - expect(mocks.actions.handleGsapDeleteAnimation).toHaveBeenCalledWith( + expect(mocks.actions.handleGsapRemoveKeyframe).toHaveBeenCalledWith( flatAnimation.id, + 100, + undefined, mocks.selection, ); - expect(mocks.actions.handleGsapRemoveKeyframe).not.toHaveBeenCalled(); + expect(mocks.actions.handleGsapDeleteAnimation).not.toHaveBeenCalled(); view.unmount(); }); // The lane-header toggle fires on whichever element owns the lane, which need - // not be the selected one. Looking the flat tween up in the selected element's - // animations misses, and the miss silently takes the remove-one-keyframe - // branch, which strands the flat tween instead of deleting it. - it("removes a non-selected element's flat tween through that element's own animations", async () => { + // not be the selected one. It must still commit through that element's own + // selection, and it must never escalate a flat tween to a whole-tween delete. + it("removes a non-selected element's flat tween through that element's own selection", async () => { const circle: TimelineElement = { ...element, id: "circle", @@ -381,11 +387,13 @@ describe("useTimelineEditCallbacks — flat tween keyframe lanes", () => { }); }); - expect(mocks.actions.handleGsapDeleteAnimation).toHaveBeenCalledWith( + expect(mocks.actions.handleGsapRemoveKeyframe).toHaveBeenCalledWith( otherFlatAnimation.id, + 0, + undefined, mocks.selection, ); - expect(mocks.actions.handleGsapRemoveKeyframe).not.toHaveBeenCalled(); + expect(mocks.actions.handleGsapDeleteAnimation).not.toHaveBeenCalled(); view.unmount(); }); diff --git a/packages/studio/src/components/nle/useTimelineEditCallbacks.ts b/packages/studio/src/components/nle/useTimelineEditCallbacks.ts index 3173efcfc..050b37339 100644 --- a/packages/studio/src/components/nle/useTimelineEditCallbacks.ts +++ b/packages/studio/src/components/nle/useTimelineEditCallbacks.ts @@ -116,7 +116,6 @@ export function useTimelineEditCallbacks({ handleGsapAddKeyframeBatch, handleGsapConvertToKeyframes, handleGsapRemoveAllKeyframes, - handleGsapDeleteAnimation, buildDomSelectionForTimelineElement, } = useDomEditActionsContext(); @@ -167,20 +166,16 @@ export function useTimelineEditCallbacks({ ); const removeKeyframeTarget = useCallback( - ( - animationId: string, - percentage: number, - animations: GsapAnimation[], - selectionOverride?: DomEditSelection | null, - ) => { - const animation = animations.find((candidate) => candidate.id === animationId); - if (animation && !animation.keyframes) { - handleGsapDeleteAnimation(animationId, selectionOverride); - return; - } + (animationId: string, percentage: number, selectionOverride?: DomEditSelection | null) => { + // A flat tween's two diamonds are SYNTHESIZED endpoints, not authored + // keyframes, so "remove keyframe" has nothing to remove. Escalating to a + // whole-animation delete here destroyed the authored tween and its source + // comment on a single click, with no undo beyond the editor's own stack. + // Always post remove-keyframe: the writer refuses it for a flat tween + // (`changed:false`, file untouched), which is the correct no-op. handleGsapRemoveKeyframe(animationId, percentage, undefined, selectionOverride); }, - [handleGsapDeleteAnimation, handleGsapRemoveKeyframe], + [handleGsapRemoveKeyframe], ); return useMemo( @@ -211,15 +206,14 @@ export function useTimelineEditCallbacks({ if (!target) return; const element = usePlayerStore.getState().elements.find((el) => (el.key ?? el.id) === elId); if (!element) { - removeKeyframeTarget(target.animId, target.tweenPct, animations); + removeKeyframeTarget(target.animId, target.tweenPct); return; } // Persist through the CLICKED element's own selection so a deletion on a // non-selected element (especially one in a different source file) commits // against the right element instead of the current domEditSelection. void buildDomSelectionForTimelineElement(element).then((selection) => { - if (selection) - removeKeyframeTarget(target.animId, target.tweenPct, animations, selection); + if (selection) removeKeyframeTarget(target.animId, target.tweenPct, selection); }); }, // Retime the keyframe to the playhead, preserving its value + ease. The @@ -341,15 +335,7 @@ export function useTimelineEditCallbacks({ const selection = await buildDomSelectionForTimelineElement(element); if (!selection) return; if (target.remove) { - // The clicked element's animations, not the selected element's: this - // lookup decides delete-the-flat-tween vs remove-one-keyframe, and a - // miss silently takes the keyframe branch, stranding the flat tween. - removeKeyframeTarget( - target.animationId, - target.tweenPercentage, - resolveElementAnimations(element.key ?? element.id), - selection, - ); + removeKeyframeTarget(target.animationId, target.tweenPercentage, selection); return; } await handleGsapAddKeyframeBatch( diff --git a/packages/studio/src/hooks/gsapKeyframeCacheHelpers.test.ts b/packages/studio/src/hooks/gsapKeyframeCacheHelpers.test.ts index 9bc45eec2..05e5c93da 100644 --- a/packages/studio/src/hooks/gsapKeyframeCacheHelpers.test.ts +++ b/packages/studio/src/hooks/gsapKeyframeCacheHelpers.test.ts @@ -4,6 +4,7 @@ import { usePlayerStore, type KeyframeCacheEntry } from "../player/store/playerS import { clearKeyframeCacheForElement, clearKeyframeCacheForFile, + pruneKeyframeCacheToFiles, updateKeyframeCacheFromParsed, } from "./gsapKeyframeCacheHelpers"; @@ -111,6 +112,48 @@ describe("clearKeyframeCacheForFile", () => { }); }); +describe("pruneKeyframeCacheToFiles", () => { + // Switching composition leaves the previous comp's elements cached with no + // owner left to clear them: each file only ever clears its own entries. + it("drops every element of a file the next scan no longer covers", () => { + seed("index.html#stress-1"); + seed("stress-1"); + seed("index.html#stress-2"); + seed("stress-2"); + seed("kf200.html#kf200"); + seed("index.html#kf200"); + seed("kf200"); + + pruneKeyframeCacheToFiles(["kf200.html"]); + + for (const key of ["index.html#stress-1", "stress-1", "index.html#stress-2", "stress-2"]) { + expect(cache().has(key)).toBe(false); + } + expect(cache().has("kf200.html#kf200")).toBe(true); + }); + + it("prunes gsapAnimations alongside keyframeCache", () => { + usePlayerStore.getState().setGsapAnimations("index.html#stress-1", [animWithKeyframes("t")]); + usePlayerStore.getState().setGsapAnimations("kf200.html#kf200", [animWithKeyframes("u")]); + + pruneKeyframeCacheToFiles(["kf200.html"]); + + const animations = usePlayerStore.getState().gsapAnimations; + expect(animations.has("index.html#stress-1")).toBe(false); + expect(animations.has("kf200.html#kf200")).toBe(true); + }); + + it("keeps everything when every cached file is still covered", () => { + seed("index.html#hero"); + seed("comp.html#a"); + + pruneKeyframeCacheToFiles(["index.html", "comp.html"]); + + expect(cache().has("index.html#hero")).toBe(true); + expect(cache().has("comp.html#a")).toBe(true); + }); +}); + describe("updateKeyframeCacheFromParsed", () => { it("serializes a multi-keyframe tween with a stable shape and animation identity", () => { const animation: GsapAnimation = { diff --git a/packages/studio/src/hooks/gsapKeyframeCacheHelpers.ts b/packages/studio/src/hooks/gsapKeyframeCacheHelpers.ts index efe0941df..929960a16 100644 --- a/packages/studio/src/hooks/gsapKeyframeCacheHelpers.ts +++ b/packages/studio/src/hooks/gsapKeyframeCacheHelpers.ts @@ -113,6 +113,34 @@ export function clearKeyframeCacheForFile(sourceFile: string): void { } } +/** + * Drop every cached element owned by a file that is no longer on screen. Each + * file only ever clears its OWN entries (see clearKeyframeCacheForFile), so + * switching composition left the previous composition's elements cached forever + * — 240 entries per switch on a 120-clip comp, in both keyframeCache and + * gsapAnimations, with nothing to evict them. Called once before a re-scan, with + * the full set of files that scan covers. + */ +export function pruneKeyframeCacheToFiles(files: readonly string[]): void { + const keep = new Set(files); + const { keyframeCache, gsapAnimations } = usePlayerStore.getState(); + const stale = new Map>(); + for (const key of [...keyframeCache.keys(), ...gsapAnimations.keys()]) { + const hash = key.indexOf("#"); + // Bare-id aliases carry no owner; clearKeyframeCacheForElement takes them + // with their prefixed key, so skipping them here loses nothing. + if (hash < 0) continue; + const sourceFile = key.slice(0, hash); + if (keep.has(sourceFile)) continue; + const ids = stale.get(sourceFile) ?? new Set(); + ids.add(key.slice(hash + 1)); + stale.set(sourceFile, ids); + } + for (const [sourceFile, ids] of stale) { + for (const id of ids) clearKeyframeCacheForElement(sourceFile, id); + } +} + /** Every cache key a write for this element sets, in read-preference order. */ export function elementCacheKeys(sourceFile: string, elementId: string): string[] { return sourceFile === "index.html" diff --git a/packages/studio/src/hooks/useGsapTweenCache.ts b/packages/studio/src/hooks/useGsapTweenCache.ts index daa22260b..0f89be655 100644 --- a/packages/studio/src/hooks/useGsapTweenCache.ts +++ b/packages/studio/src/hooks/useGsapTweenCache.ts @@ -4,6 +4,7 @@ import { usePlayerStore } from "../player/store/playerStore"; import { readRuntimeKeyframes, scanAllRuntimeKeyframes } from "./gsapRuntimeBridge"; import { clearKeyframeCacheForElement, + pruneKeyframeCacheToFiles, writeGsapAnimationsForElement, } from "./gsapKeyframeCacheHelpers"; import { toAbsoluteTime, toClipPercentage } from "./gsapShared"; @@ -390,6 +391,9 @@ export function usePopulateKeyframeCacheForFile( new Set([sourceFile, ...(compositionSrcKey ? compositionSrcKey.split("|") : [])]), ); const doc = iframeRef?.current?.contentDocument; + // Everything the previous scan cached for a file this one no longer covers + // (the composition just switched away from) has no owner left to clear it. + pruneKeyframeCacheToFiles(files); Promise.all(files.map((sf) => populateKeyframeCacheFromAst(projectId, sf, doc))).then(() => { astFetchDoneRef.current = fetchKey; }); diff --git a/packages/studio/src/player/components/LayerDisclosureRow.tsx b/packages/studio/src/player/components/LayerDisclosureRow.tsx index e9087e11a..596abd197 100644 --- a/packages/studio/src/player/components/LayerDisclosureRow.tsx +++ b/packages/studio/src/player/components/LayerDisclosureRow.tsx @@ -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} + {children} ); } diff --git a/packages/studio/src/player/components/TimelineClipDiamonds.test.tsx b/packages/studio/src/player/components/TimelineClipDiamonds.test.tsx index 85804deb1..11c83585b 100644 --- a/packages/studio/src/player/components/TimelineClipDiamonds.test.tsx +++ b/packages/studio/src/player/components/TimelineClipDiamonds.test.tsx @@ -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("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()); diff --git a/packages/studio/src/player/components/TimelineClipDiamonds.tsx b/packages/studio/src/player/components/TimelineClipDiamonds.tsx index b3007f979..8509db281 100644 --- a/packages/studio/src/player/components/TimelineClipDiamonds.tsx +++ b/packages/studio/src/player/components/TimelineClipDiamonds.tsx @@ -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 ( - -
- {onSelectSegment && !kf.easeAmbiguous && kf.animationId !== undefined && ( -
- -
- )} - - ); - })} + {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)}%`} > 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(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 ( + +
+ {onSelectSegment && !kf.easeAmbiguous && kf.animationId !== undefined && ( +
+ +
+ )} + + ); + })} + + ); +} diff --git a/packages/studio/src/player/components/TimelineLanes.tsx b/packages/studio/src/player/components/TimelineLanes.tsx index 3d9769afd..d8b622f2c 100644 --- a/packages/studio/src/player/components/TimelineLanes.tsx +++ b/packages/studio/src/player/components/TimelineLanes.tsx @@ -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) { diff --git a/packages/studio/src/player/components/TimelineTrackHeader.test.tsx b/packages/studio/src/player/components/TimelineTrackHeader.test.tsx index 56a077cd0..46d4372b5 100644 --- a/packages/studio/src/player/components/TimelineTrackHeader.test.tsx +++ b/packages/studio/src/player/components/TimelineTrackHeader.test.tsx @@ -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 }); diff --git a/packages/studio/src/player/components/TimelineTrackHeader.tsx b/packages/studio/src/player/components/TimelineTrackHeader.tsx index b808d8d21..205203901 100644 --- a/packages/studio/src/player/components/TimelineTrackHeader.tsx +++ b/packages/studio/src/player/components/TimelineTrackHeader.tsx @@ -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 (
setHoveredGroup(lane.group)} - onPointerLeave={() => setHoveredGroup(null)} > {/* Tree connector: vertical spine (top-half on the last lane) + branch tick. */} -
); } @@ -293,15 +265,12 @@ export function TimelineTrackHeader({ currentTime, isTrackHidden, isAudioTrack, - isActive, - isHovered, theme, onToggleClipExpanded, onToggleTrackHidden, onTogglePropertyGroupKeyframe, onSeek, }: TimelineTrackHeaderProps) { - const [hoveredGroup, setHoveredGroup] = useState(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. */} +