fix(studio): reconcile keyframe-seek timing basis between flat Layout and Motion groups

Whole-plan coherence review (Plan 3a Layout + Plan 3b Motion) found that
Layout's keyframe gutter and Motion's Timing row independently derived an
element's start/duration and disagreed whenever an element had animations
but no explicit data-duration: Motion correctly inferred the range from the
element's GSAP tweens, while Layout's keyframe gutter fell back to a naive
`duration ?? 1`, so clicking a keyframe percentage in Layout could seek to a
different absolute time than what Motion's Timing row displayed.

Extract deriveElementTiming (propertyPanelFlatTimingDerivation.ts) as the
single shared basis both paths now consume: FlatTimingRow (Motion) and
PropertyPanelFlat's own elStart/elDuration (Layout's keyframe gutter and 3D
Transform block). PropertyPanelFlat now recomputes this basis itself from
its own element/gsapAnimations props instead of trusting the parent's naive
value, so PropertyPanel.tsx (and its legacy non-flat panel) is untouched.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
Vance Ingalls
2026-07-09 12:58:00 -07:00
co-authored by Claude Sonnet 5
parent 710a2488f1
commit 684ec4e875
5 changed files with 241 additions and 26 deletions
@@ -143,6 +143,40 @@ function animatedElement() {
};
}
// Inferred-timing fixture (whole-plan coherence fix): NO explicit data-start
// or data-duration — sections.timing must turn on via animationCount (fed
// from gsapAnimations.length), not an authored attribute, so both the Motion
// Timing row and the Layout keyframe gutter are forced to infer the range
// from the element's own GSAP tween instead of reading it off an attribute.
function inferredMotionElement() {
return {
...baseElement(),
id: "inferred-anim",
selector: "#inferred-anim",
label: "Inferred Anim",
};
}
// A single "to" tween running from t=2 to t=5 (position 2, duration 3), with
// keyframes on "x" at 0/50/100% — enough to drive both FlatTimingRow's
// inference and the Layout "x" row's keyframe-seek gutter.
const INFERRED_TIMING_ANIMATION = {
id: "a1",
targetSelector: "#inferred-anim",
method: "to",
position: 2,
duration: 3,
properties: { x: 100 },
keyframes: {
format: "percentage",
keyframes: [
{ percentage: 0, properties: { x: 0 } },
{ percentage: 50, properties: { x: 50 } },
{ percentage: 100, properties: { x: 100 } },
],
},
} as never;
async function renderPanel(
flatEnabled: boolean,
elementOverride: ReturnType<typeof baseElement> = baseElement(),
@@ -418,3 +452,65 @@ describe("PropertyPanel — Motion group (Plan 3b)", () => {
RENDER_TIMEOUT_MS,
);
});
// Whole-plan coherence fix: Layout's keyframe-seek basis and Motion's Timing
// row basis must agree on the same start/duration for an element that has
// animations but no explicit data-duration — before the fix, Layout fell back
// to a naive `duration ?? 1` while Motion correctly inferred the range from
// the tween (position 2, duration 3 -> start 2 / duration 3 / end 5).
describe("PropertyPanel — flat Layout/Motion timing agreement (whole-plan coherence fix)", () => {
it(
"Motion's Timing row shows the inferred start/end/duration for an element with animations but no explicit duration",
async () => {
const { host, root } = await renderPanel(true, inferredMotionElement(), {
gsapAnimations: [INFERRED_TIMING_ANIMATION],
});
openFlatGroup(host, "Motion");
const motionGroup = host.querySelector('[data-flat-group-open="true"]');
if (!motionGroup) throw new Error("expected the Motion group to be open");
expect(motionGroup.textContent).toContain("Inferred");
const inputs = motionGroup.querySelectorAll<HTMLInputElement>("input");
// FlatTimingRow renders Start, End, Duration in that order.
expect(inputs[0]?.value).toBe("2.00s");
expect(inputs[1]?.value).toBe("5.00s");
expect(inputs[2]?.value).toBe("3.00s");
act(() => root.unmount());
},
RENDER_TIMEOUT_MS,
);
it(
"Layout's X-row keyframe gutter seeks to the SAME absolute time Motion's Timing row shows as the midpoint (50% of an inferred 2s-5s range = 3.5s)",
async () => {
const onSeekToTime = vi.fn();
const { host, root } = await renderPanel(true, inferredMotionElement(), {
gsapAnimations: [INFERRED_TIMING_ANIMATION],
onSeekToTime,
});
openFlatGroup(host, "Layout");
const layoutGroup = host.querySelector('[data-flat-group-open="true"]');
if (!layoutGroup) throw new Error("expected the Layout group to be open");
const xRow = Array.from(layoutGroup.querySelectorAll<HTMLElement>(".group")).find(
(el) => el.querySelector("span")?.textContent === "X",
);
if (!xRow) throw new Error("expected an X row");
const gutter = xRow.querySelector('[data-flat-kf-gutter="true"]');
if (!gutter) throw new Error("expected a keyframe gutter on the X row");
// The diamond button always carries a `title`; the two plain arrow
// buttons don't. At currentPct=0 with keyframes at 0/50/100%, the prev
// arrow is disabled (no earlier keyframe) and the next arrow seeks to
// the 50% keyframe — exactly the case the coherence bug affected.
const nextArrow = Array.from(gutter.querySelectorAll<HTMLButtonElement>("button")).find(
(b) => !b.title && !b.disabled,
);
if (!nextArrow) throw new Error("expected an enabled next-keyframe arrow button");
act(() => nextArrow.dispatchEvent(new MouseEvent("click", { bubbles: true })));
// Same basis as the Timing row: start 2 + 50% * duration 3 = 3.5.
expect(onSeekToTime).toHaveBeenCalledWith(3.5);
act(() => root.unmount());
},
RENDER_TIMEOUT_MS,
);
});
@@ -11,6 +11,7 @@ import { FlatTextSection } from "./propertyPanelFlatTextSection";
import { FlatStyleSection } from "./propertyPanelFlatStyleSections";
import { FlatLayoutSection } from "./propertyPanelFlatLayoutSection";
import { FlatMotionSection } from "./propertyPanelFlatMotionSection";
import { deriveElementTiming } from "./propertyPanelFlatTimingDerivation";
import { createGsapLivePreview } from "./gsapLivePreview";
import { formatTextFieldPreview, StyleSections } from "./propertyPanelSections";
import { STUDIO_GSAP_PANEL_ENABLED } from "./manualEditingAvailability";
@@ -96,8 +97,12 @@ export function PropertyPanelFlat({
currentPct,
animIdForProp,
gsapRuntimeValues,
elStart,
elDuration,
// Renamed: PropertyPanel.tsx still computes/passes these for its own legacy
// (non-flat) panel, but the flat path recomputes its own basis below via
// deriveElementTiming so it agrees with Motion's Timing row — ignore the
// parent's naive `elDuration ?? 1` fallback.
elStart: _elStart,
elDuration: _elDuration,
onCommitAnimatedProperty,
onCommitAnimatedProperties,
onSeekToTime,
@@ -222,6 +227,12 @@ export function PropertyPanelFlat({
setPinnedGroupIds((current) =>
current.includes(groupId) ? current.filter((id) => id !== groupId) : [...current, groupId],
);
// Basis for the Layout keyframe gutter (X/Y/W/H/Angle + 3D Transform) —
// must agree with Motion's Timing row (FlatTimingRow), which infers the
// range from animations when there's no explicit data-duration. Computed
// here (not threaded from PropertyPanel) both to keep that file under its
// 600-LOC gate and because element/gsapAnimations are already in scope.
const { start: elStart, duration: elDuration } = deriveElementTiming(element, gsapAnimations);
// Trivial percentage→time seek, derived here rather than threaded from
// PropertyPanel (keeps that file under its 600-LOC gate).
const seekFromKfPct = (pct: number) => onSeekToTime?.(elStart + (pct / 100) * elDuration);
@@ -7,21 +7,7 @@ import { CommitField } from "./propertyPanelPrimitives";
import { AnimationCard } from "./AnimationCard";
import { ADD_METHODS, ADD_METHOD_LABELS, METHOD_TOOLTIPS } from "./gsapAnimationConstants";
import type { GsapAnimationEditCallbacks } from "./gsapAnimationCallbacks";
function deriveTimingFromAnimations(
animations: GsapAnimation[],
): { start: number; duration: number } | null {
let lo = Infinity;
let hi = -Infinity;
for (const a of animations) {
const s = a.resolvedStart ?? (typeof a.position === "number" ? a.position : 0);
const d = a.duration ?? 0;
lo = Math.min(lo, s);
hi = Math.max(hi, s + d);
}
if (!Number.isFinite(lo) || !Number.isFinite(hi) || hi <= lo) return null;
return { start: lo, duration: hi - lo };
}
import { deriveElementTiming } from "./propertyPanelFlatTimingDerivation";
export function FlatTimingRow({
element,
@@ -32,15 +18,7 @@ export function FlatTimingRow({
animations?: GsapAnimation[];
onSetAttribute: (attr: string, value: string) => void | Promise<void>;
}) {
const explicitStart = Number.parseFloat(element.dataAttributes.start ?? "0") || 0;
const explicitDuration =
Number.parseFloat(
element.dataAttributes.duration ?? element.dataAttributes["hf-authored-duration"] ?? "0",
) || 0;
const derived = explicitDuration > 0 ? null : deriveTimingFromAnimations(animations);
const start = derived ? derived.start : explicitStart;
const duration = derived ? derived.duration : explicitDuration;
const { start, duration, inferred: derived } = deriveElementTiming(element, animations);
const end = start + duration;
const commitStart = (nextValue: string) => {
@@ -0,0 +1,71 @@
import { describe, expect, it } from "vitest";
import { deriveElementTiming } from "./propertyPanelFlatTimingDerivation";
import type { DomEditSelection } from "./domEditingTypes";
import type { GsapAnimation } from "@hyperframes/core/gsap-parser";
function withDataAttributes(
dataAttributes: Record<string, string>,
): Pick<DomEditSelection, "dataAttributes"> {
return { dataAttributes };
}
describe("deriveElementTiming", () => {
it("uses the explicit data-start/data-duration attributes when duration is authored", () => {
const result = deriveElementTiming(withDataAttributes({ start: "8", duration: "4" }));
expect(result).toEqual({ start: 8, duration: 4, inferred: false });
});
it("infers start/duration from animations when there is no explicit data-duration", () => {
const animations = [{ position: 2, duration: 3 } as unknown as GsapAnimation];
const result = deriveElementTiming(
withDataAttributes({ start: "0", duration: "0" }),
animations,
);
expect(result).toEqual({ start: 2, duration: 3, inferred: true });
});
it("spans the earliest tween start to the latest tween end across multiple animations", () => {
const animations = [
{ position: 1, duration: 2 } as unknown as GsapAnimation, // 1 -> 3
{ position: 2, duration: 4 } as unknown as GsapAnimation, // 2 -> 6
];
const result = deriveElementTiming(withDataAttributes({}), animations);
expect(result).toEqual({ start: 1, duration: 5, inferred: true });
});
it("prefers an explicit data-duration over inference even when animations exist", () => {
const animations = [{ position: 2, duration: 3 } as unknown as GsapAnimation];
const result = deriveElementTiming(
withDataAttributes({ start: "0", duration: "10" }),
animations,
);
expect(result).toEqual({ start: 0, duration: 10, inferred: false });
});
it("falls back to hf-authored-duration when data-duration is absent", () => {
const result = deriveElementTiming(
withDataAttributes({ start: "1", "hf-authored-duration": "6" }),
);
expect(result).toEqual({ start: 1, duration: 6, inferred: false });
});
it("returns a zero-duration, non-inferred result with no attributes and no animations", () => {
const result = deriveElementTiming(withDataAttributes({}));
expect(result).toEqual({ start: 0, duration: 0, inferred: false });
});
// This is the exact bug from the whole-plan coherence review: Layout's
// keyframe-seek basis must land on the same absolute time that Motion's
// Timing row displays as the element's midpoint.
it("agrees with a keyframe-percentage seek: 50% lands on the same midpoint the Timing row would show", () => {
const animations = [{ position: 2, duration: 3 } as unknown as GsapAnimation];
const timing = deriveElementTiming(
withDataAttributes({ start: "0", duration: "0" }),
animations,
);
const seekTimeAt50Pct = timing.start + (50 / 100) * timing.duration;
const timingRowMidpoint = timing.start + timing.duration / 2;
expect(seekTimeAt50Pct).toBe(timingRowMidpoint);
expect(seekTimeAt50Pct).toBe(3.5);
});
});
@@ -0,0 +1,59 @@
import type { GsapAnimation } from "@hyperframes/core/gsap-parser";
import type { DomEditSelection } from "./domEditingTypes";
/**
* The single source of truth for an element's clip start/duration in the flat
* inspector. Both the Motion group's Timing row (`FlatTimingRow`) and the
* Layout group's keyframe gutter (fed via `elStart`/`elDuration` from
* `PropertyPanel.tsx` through `PropertyPanelFlat.tsx`) must derive this the
* same way — otherwise a keyframe-percentage seek in Layout lands on a
* different absolute time than the range Motion displays for the same
* element (found by the Plan 3a+3b whole-plan coherence review).
*
* Precedence: an explicit `data-duration` (or `data-hf-authored-duration`)
* wins outright. Only when neither is present do we infer the range from the
* element's own GSAP tweens (earliest tween start → latest tween end).
*
* Scoped to the FLAT inspector only — the legacy (non-flat) panel keeps its
* own, unrelated `elStart`/`elDuration ?? 1` computation in `PropertyPanel.tsx`
* untouched.
*/
export interface ElementTiming {
start: number;
duration: number;
/** True when duration/start came from `deriveTimingFromAnimations`, not an authored attribute. */
inferred: boolean;
}
function deriveTimingFromAnimations(
animations: GsapAnimation[],
): { start: number; duration: number } | null {
let lo = Infinity;
let hi = -Infinity;
for (const a of animations) {
const s = a.resolvedStart ?? (typeof a.position === "number" ? a.position : 0);
const d = a.duration ?? 0;
lo = Math.min(lo, s);
hi = Math.max(hi, s + d);
}
if (!Number.isFinite(lo) || !Number.isFinite(hi) || hi <= lo) return null;
return { start: lo, duration: hi - lo };
}
export function deriveElementTiming(
element: Pick<DomEditSelection, "dataAttributes">,
animations: GsapAnimation[] = [],
): ElementTiming {
const explicitStart = Number.parseFloat(element.dataAttributes.start ?? "0") || 0;
const explicitDuration =
Number.parseFloat(
element.dataAttributes.duration ?? element.dataAttributes["hf-authored-duration"] ?? "0",
) || 0;
const derived = explicitDuration > 0 ? null : deriveTimingFromAnimations(animations);
return {
start: derived ? derived.start : explicitStart,
duration: derived ? derived.duration : explicitDuration,
inferred: derived !== null,
};
}