fix(studio): recompute keyframe currentPct from the corrected timing basis in the flat Layout group

Follow-up to 684ec4e87: that fix corrected the seek target for Layout's
keyframe gutter via deriveElementTiming, but currentPct — which drives
KeyframeNavigation's diamond active/inactive state and prev/next arrow
targeting — still used PropertyPanel's naive elStart=0/elDuration=1
basis. For an element with animations but no explicit data-duration,
seeking to a keyframe's real absolute time no longer lit that
keyframe's diamond as active, and the prev/next arrows targeted the
wrong keyframes.

Thread currentTime into PropertyPanelFlat (swapping the now-redundant
currentPct prop 1-for-1, so PropertyPanel.tsx's line count is
unchanged) and recompute currentPct there from the same
deriveElementTiming basis already used for the seek fix.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
Vance Ingalls
2026-07-14 15:50:43 -07:00
co-authored by Claude Sonnet 5
parent b155ed46b6
commit 4b592832ab
3 changed files with 117 additions and 8 deletions
@@ -181,6 +181,7 @@ async function renderPanel(
flatEnabled: boolean,
elementOverride: ReturnType<typeof baseElement> = baseElement(),
propsOverride: Partial<PropertyPanelProps> = {},
currentTime?: number,
) {
vi.resetModules();
vi.doMock("./manualEditingAvailability", async () => {
@@ -189,6 +190,13 @@ async function renderPanel(
);
return { ...actual, STUDIO_FLAT_INSPECTOR_ENABLED: flatEnabled };
});
// Seed the playhead on the SAME store instance PropertyPanel.tsx will read via
// usePlayerStore (module-fresh since the resetModules() above) — must happen
// before PropertyPanel is imported/rendered so its initial render sees it.
if (currentTime !== undefined) {
const { usePlayerStore } = await import("../../player/store/playerStore");
usePlayerStore.getState().setCurrentTime(currentTime);
}
const { PropertyPanel } = await import("./PropertyPanel");
const host = document.createElement("div");
document.body.append(host);
@@ -483,10 +491,20 @@ describe("PropertyPanel — flat Layout/Motion timing agreement (whole-plan cohe
"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,
});
// Seed the playhead at the clip's real start (t=2, the 0% keyframe's
// absolute time) — now that the follow-up fix also recomputes
// `currentPct` from the corrected elStart/elDuration basis, "current
// position is at the 0% keyframe" must be expressed as an actual t=2
// seek rather than relying on the store's untouched t=0 default (which,
// post-fix, resolves to a currentPct of -66.7% — well outside the 0%
// keyframe's tolerance window, and no longer "the case the coherence
// bug affected" that this test documents).
const { host, root } = await renderPanel(
true,
inferredMotionElement(),
{ gsapAnimations: [INFERRED_TIMING_ANIMATION], onSeekToTime },
2,
);
openFlatGroup(host, "Layout");
const layoutGroup = host.querySelector('[data-flat-group-open="true"]');
if (!layoutGroup) throw new Error("expected the Layout group to be open");
@@ -498,7 +516,7 @@ describe("PropertyPanel — flat Layout/Motion timing agreement (whole-plan cohe
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
// buttons don't. At currentPct=0 (playhead on the 0% keyframe), 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(
@@ -514,3 +532,86 @@ describe("PropertyPanel — flat Layout/Motion timing agreement (whole-plan cohe
RENDER_TIMEOUT_MS,
);
});
// Follow-up fix (review of 684ec4e87): the seek-basis fix above corrected
// WHERE a keyframe click seeks to, but `currentPct` — the value that drives
// KeyframeNavigation's diamond active/inactive state and prev/next arrow
// targeting — still used the OLD naive basis. For an inferred-duration
// element, seeking to a keyframe's actual absolute time no longer lit that
// keyframe's diamond as active. Prove the round-trip here: seek to the exact
// absolute time of the 50% keyframe (2 + 0.5*3 = 3.5) and confirm its diamond
// renders "active" (title="Remove x keyframe"), not "inactive"/"ghost".
describe("PropertyPanel — flat Layout currentPct basis (currentPct follow-up fix)", () => {
it(
"lights the X-row keyframe diamond as active when the playhead is seeked to that keyframe's real absolute time (inferred 2s-5s range, 50% keyframe = 3.5s)",
async () => {
const { host, root } = await renderPanel(
true,
inferredMotionElement(),
{ gsapAnimations: [INFERRED_TIMING_ANIMATION] },
3.5,
);
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");
const diamond = gutter.querySelector<HTMLButtonElement>("button[title]");
if (!diamond) throw new Error("expected a keyframe diamond button");
// KeyframeDiamond's title mapping: active -> "Remove ... keyframe",
// inactive -> "Add ... keyframe", ghost -> "Convert ... to keyframes".
// Before this fix, currentPct was computed against the naive
// elStart=0/elDuration=1 basis, so t=3.5 produced currentPct=350% —
// nowhere near the 50% keyframe within KeyframeNavigation's tolerance —
// and the diamond stayed "inactive" even though the playhead was
// exactly on that keyframe's real time.
expect(diamond.title).toBe("Remove x keyframe");
act(() => root.unmount());
},
RENDER_TIMEOUT_MS,
);
it(
"prev/next arrows re-center on the current keyframe once currentPct agrees with the corrected seek basis",
async () => {
const onSeekToTime = vi.fn();
const { host, root } = await renderPanel(
true,
inferredMotionElement(),
{ gsapAnimations: [INFERRED_TIMING_ANIMATION], onSeekToTime },
3.5,
);
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");
const buttons = Array.from(gutter.querySelectorAll<HTMLButtonElement>("button"));
const [prevArrow, , nextArrow] = buttons;
if (!prevArrow || !nextArrow) throw new Error("expected prev/next arrow buttons");
// At the 50% keyframe (t=3.5), prev should target the 0% keyframe
// (absolute t=2) and next should target the 100% keyframe (absolute
// t=5) — both only resolvable once currentPct agrees with elStart=2/
// elDuration=3, the same basis the seek fix already uses.
expect(prevArrow.disabled).toBe(false);
act(() => prevArrow.dispatchEvent(new MouseEvent("click", { bubbles: true })));
expect(onSeekToTime).toHaveBeenLastCalledWith(2);
expect(nextArrow.disabled).toBe(false);
act(() => nextArrow.dispatchEvent(new MouseEvent("click", { bubbles: true })));
expect(onSeekToTime).toHaveBeenLastCalledWith(5);
act(() => root.unmount());
},
RENDER_TIMEOUT_MS,
);
});
@@ -293,7 +293,7 @@ export const PropertyPanel = memo(function PropertyPanel(props: PropertyPanelPro
commitManualRotation={commitManualRotation}
gsapAnimId={gsapAnimId}
navKeyframes={navKeyframes}
currentPct={currentPct}
currentTime={currentTime}
animIdForProp={animIdForProp}
gsapRuntimeValues={gsap3dValues}
elStart={elStart}
@@ -94,7 +94,7 @@ export function PropertyPanelFlat({
commitManualRotation,
gsapAnimId,
navKeyframes,
currentPct,
currentTime,
animIdForProp,
gsapRuntimeValues,
// Renamed: PropertyPanel.tsx still computes/passes these for its own legacy
@@ -186,7 +186,6 @@ export function PropertyPanelFlat({
| "commitManualRotation"
| "gsapAnimId"
| "navKeyframes"
| "currentPct"
| "animIdForProp"
| "gsapRuntimeValues"
| "elStart"
@@ -207,6 +206,7 @@ export function PropertyPanelFlat({
selectedElementId: string | null;
clipboardCopied: boolean;
onCopyElementInfo: () => void;
currentTime: number;
}) {
// Lazy initializer: pick whichever group actually renders for this element
// (Text if text-editable, else Style if style-editable, else none open) so a
@@ -236,6 +236,14 @@ export function PropertyPanelFlat({
// 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);
// Playhead position within the SAME corrected elStart/elDuration basis as
// seekFromKfPct above — recomputed here (not threaded as `currentPct` from
// PropertyPanel, which still derives it against its own naive basis for the
// legacy panel) so KeyframeNavigation's diamond active-state and prev/next
// arrow targeting agree with where a keyframe click actually seeks to
// (follow-up fix to 684ec4e87, which corrected the seek basis but left this
// one still naive).
const currentPct = elDuration > 0 ? ((currentTime - elStart) / elDuration) * 100 : 0;
// Motion group double-gate — reproduces the legacy PropertyPanel gate exactly:
// • Timing (sections.timing) shows via resolveEditingSections, same as today.