fix(studio): resolve keyframe fallbacks against the clicked element

Three review follow-ups on the editor-callback consolidation.

The keyframe-target resolve now takes the clicked element's key and reads
that element's keyframe cache. The diamond context menu and move-to-playhead
pass no explicit target, so they fell through to the cache of whatever
element happened to be selected: opening the menu on a non-selected
element's diamond resolved against the wrong keyframes.

PropertyPanelFlat opens the Motion group by adjusting state during render
instead of in an effect, so the AnimationCard mounts on the same commit the
focus request arrives on rather than a frame later.

Both animation sections pass a module-level focus consumer instead of a
fresh inline arrow, so AnimationCard's focus effect stops re-running on
every parent render.
This commit is contained in:
Miguel Angel Simon Sierra
2026-07-27 19:52:07 +02:00
parent 2d1b905a56
commit ba8df2661c
6 changed files with 99 additions and 19 deletions
@@ -6,6 +6,7 @@ import { AnimationCard } from "./AnimationCard";
import {
type GsapAnimationEditCallbacks,
withTrackedGsapAnimationCallbacks,
clearFocusedEaseSegment,
} from "./gsapAnimationCallbacks";
import { useTrackDesignInput } from "../../contexts/DesignPanelInputContext";
import { usePlayerStore } from "../../player";
@@ -29,7 +30,6 @@ export const GsapAnimationSection = memo(function GsapAnimationSection({
const [addMenuOpen, setAddMenuOpen] = useState(false);
const trackedCallbacks = withTrackedGsapAnimationCallbacks(callbacks, track);
const focusedEaseSegment = usePlayerStore((s) => s.focusedEaseSegment);
const setFocusedEaseSegment = usePlayerStore((s) => s.setFocusedEaseSegment);
return (
<Section title="Animation" icon={<Film size={15} />}>
@@ -57,7 +57,7 @@ export const GsapAnimationSection = memo(function GsapAnimationSection({
focusedSegment={
focusedEaseSegment?.animationId === anim.id ? focusedEaseSegment : null
}
onFocusSegmentConsumed={() => setFocusedEaseSegment(null)}
onFocusSegmentConsumed={clearFocusedEaseSegment}
/>
))}
@@ -249,18 +249,22 @@ export function PropertyPanelFlat({
// force the Motion group open so its AnimationCard (which only mounts while
// the group is expanded) can consume the focus and reveal the ease editor.
const focusedEaseSegment = usePlayerStore((s) => s.focusedEaseSegment);
// Identity of the element THIS panel actually renders (not the store's
// selectedElementId, which flips synchronously on selection while the panel
// still renders the previous element during async DOM-selection resolution):
// a stale panel would otherwise consume a focus request meant for its
// successor when both share a class-selector animation id.
// The element THIS panel renders, not the store's selectedElementId: that
// flips synchronously while the panel still renders its predecessor, so a
// stale panel would consume a request meant for its successor whenever the
// two share a class-selector animation id.
const renderedElementId = `${element.sourceFile}#${element.id}`;
useEffect(() => {
if (!focusedEaseSegment || focusedEaseSegment.elementId !== renderedElementId) return;
if (gsapAnimations.some((a) => a.id === focusedEaseSegment.animationId)) {
setOpenGroupId("motion");
}
}, [focusedEaseSegment, gsapAnimations, renderedElementId]);
// Adjusted during render (not an effect) so the card mounts on the same
// commit the request lands on. Keyed on request identity: a group the user
// closes afterwards stays closed.
const [consumedFocus, setConsumedFocus] = useState(focusedEaseSegment);
if (focusedEaseSegment !== consumedFocus) {
setConsumedFocus(focusedEaseSegment);
const focusesThisPanel =
focusedEaseSegment?.elementId === renderedElementId &&
gsapAnimations.some((a) => a.id === focusedEaseSegment.animationId);
if (focusesThisPanel) setOpenGroupId("motion");
}
const [justToggledIds, setJustToggledIds] = useState<string[]>([]);
const justToggledTimeoutRef = useRef<ReturnType<typeof setTimeout> | null>(null);
@@ -1,4 +1,5 @@
import type { ArcPathSegment } from "@hyperframes/parsers/gsap-parser";
import { usePlayerStore } from "../../player";
/**
* Edit callbacks shared by GsapAnimationSection and each AnimationCard it
@@ -155,3 +156,12 @@ export function withTrackedGsapAnimationCallbacks(
: undefined,
};
}
/**
* Stable consumer for the store's one-shot ease-focus request. Module-level on
* purpose: an inline arrow in the section components is a dep of AnimationCard's
* focus effect, so a fresh identity each render re-runs that effect every render.
*/
export function clearFocusedEaseSegment(): void {
usePlayerStore.getState().setFocusedEaseSegment(null);
}
@@ -9,6 +9,7 @@ import { AnimationCard } from "./AnimationCard";
import {
type GsapAnimationEditCallbacks,
withTrackedGsapAnimationCallbacks,
clearFocusedEaseSegment,
} from "./gsapAnimationCallbacks";
import { deriveElementTiming } from "./propertyPanelFlatTimingDerivation";
import { usePlayerStore } from "../../player";
@@ -138,7 +139,6 @@ export function FlatMotionSection({
const [addMenuOpen, setAddMenuOpen] = useState(false);
const trackedCallbacks = withTrackedGsapAnimationCallbacks(callbacks, track);
const focusedEaseSegment = usePlayerStore((s) => s.focusedEaseSegment);
const setFocusedEaseSegment = usePlayerStore((s) => s.setFocusedEaseSegment);
// Only consume a focus request aimed at the element THIS panel renders (not
// the store's selectedElementId, which flips synchronously during async
// selection resolution), so a shared class-selector animation id can't open
@@ -182,7 +182,7 @@ export function FlatMotionSection({
defaultExpanded={index === 0}
flat
focusedSegment={focusedHere?.animationId === anim.id ? focusedHere : null}
onFocusSegmentConsumed={() => setFocusedEaseSegment(null)}
onFocusSegmentConsumed={clearFocusedEaseSegment}
/>
))}
<GsapAddAnimationControl
@@ -244,6 +244,59 @@ describe("useTimelineEditCallbacks — flat tween keyframe lanes", () => {
view.unmount();
});
// The diamond context menu opens on whatever diamond was clicked, which need
// not belong to the selected element, and it passes no explicit target — so
// the resolve falls back to the cache. Reading the SELECTED element's cache
// there resolves against the wrong element's keyframes.
it("resolves a cache fallback against the clicked element, not the selected one", () => {
const circle: TimelineElement = {
...element,
id: "circle",
key: "scenes/main.html#circle",
domId: "circle",
};
usePlayerStore.setState({
elements: [element, circle],
gsapAnimations: new Map([["scenes/main.html#circle", [otherKeyframedAnimation]]]),
keyframeCache: new Map([
// Decoy at the same clip-% under the selected element's key.
[
"box",
{
format: "percentage",
keyframes: [{ percentage: 100, tweenPercentage: 100, properties: { x: 420 } }],
},
],
[
"scenes/main.html#circle",
{
format: "percentage",
keyframes: [
{
percentage: 100,
tweenPercentage: 100,
propertyGroup: "position",
animationId: otherKeyframedAnimation.id,
properties: { x: 420 },
},
],
},
],
]),
});
const view = renderCallbacks();
act(() => {
view.callbacks.onMoveKeyframeToPlayhead?.("scenes/main.html#circle", 100);
});
expect(mocks.actions.handleGsapMoveKeyframeToPlayhead).toHaveBeenCalledWith(
otherKeyframedAnimation.id,
100,
);
view.unmount();
});
it("keeps selected-element flat boundary deletion on the animation delete path", () => {
const view = renderCallbacks();
@@ -143,12 +143,18 @@ export function useTimelineEditCallbacks({
tweenPercentage?: number,
animationId?: string,
animations: GsapAnimation[] = selectedGsapAnimations,
elementKey?: string,
): { animId: string; tweenPct: number } | null => {
const explicitTarget =
propertyGroup !== undefined || tweenPercentage !== undefined || animationId !== undefined
? [{ percentage: pct, propertyGroup, tweenPercentage, animationId }]
: undefined;
const cached = usePlayerStore.getState().keyframeCache.get(domEditSelection?.id ?? "");
// The clicked element's own cache when the caller knows it: the diamond
// context menu can open on an element that is not the selected one, and
// reading the selection's cache there resolves against the wrong element.
const cached = usePlayerStore
.getState()
.keyframeCache.get(elementKey ?? domEditSelection?.id ?? "");
return resolveTimelineKeyframeTarget(
pct,
explicitTarget ?? cached?.keyframes ?? [],
@@ -198,7 +204,7 @@ export function useTimelineEditCallbacks({
},
onDeleteKeyframe: (elId, pct, group, tweenPct, animationId) => {
const animations = resolveElementAnimations(elId);
const target = resolveKeyframeTarget(pct, group, tweenPct, animationId, animations);
const target = resolveKeyframeTarget(pct, group, tweenPct, animationId, animations, elId);
if (!target) return;
const element = usePlayerStore.getState().elements.find((el) => (el.key ?? el.id) === elId);
if (!element) {
@@ -213,8 +219,15 @@ export function useTimelineEditCallbacks({
});
},
// Retime the keyframe to the playhead, preserving its value + ease.
onMoveKeyframeToPlayhead: (_elId, pct, group, tweenPct, animationId) => {
const target = resolveKeyframeTarget(pct, group, tweenPct, animationId);
onMoveKeyframeToPlayhead: (elId, pct, group, tweenPct, animationId) => {
const target = resolveKeyframeTarget(
pct,
group,
tweenPct,
animationId,
resolveElementAnimations(elId),
elId,
);
if (target) handleGsapMoveKeyframeToPlayhead(target.animId, target.tweenPct);
},
// Drag-to-retime. The diamond reports clip-%s; resolveKeyframeTarget gives