Skip to content

Commit 9f6ebc7

Browse files
committed
fix(studio): keyframe bug bash — 7 fixes rebased onto refactor
Gesture recording: - Always read CSS var offsets regardless of translate state - Clear translate after seek during preview - Restore visibility + translate on recording stop - Gate recording behind STUDIO_KEYFRAMES_ENABLED Design panel: - Subscribe to liveTime for real-time value refresh during playback - Use update-keyframe instead of add-keyframe when editing existing keyframe - Prefer single-element tween over group selector when multiple target same element Path offset system: - reapplyPathOffsets passes updateBase:false to prevent corrupting translate base - gsapAnimatesProperty handles array-form keyframes
1 parent 81416ab commit 9f6ebc7

15 files changed

Lines changed: 551 additions & 265 deletions

File tree

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
# Keyframes Bug Bash — 2026-06-09
2+
3+
Attendees: Executive Suite (SF Office), James Russo, Miguel Angel Simon Sierra, Terence Cho, Ular Kimsanov, Vance Ingalls, Xiaye Wang
4+
5+
## Bugs
6+
7+
### Keyframe editing
8+
9+
- [x] **Editing a keyframe value creates a duplicate instead of updating** — Selecting a keyframe and adjusting a property (e.g., bumping scale to 9) adds a new keyframe in the same frame instead of modifying the existing one. Deleting one removes both. _Fix: `useAnimatedPropertyCommit` now checks for existing keyframe at the current percentage and dispatches `update-keyframe` instead of `add-keyframe`._
10+
11+
- [x] **Editing a keyframe in the last scene makes element visible in all prior scenes** — After editing a keyframe on the circle, it became visible throughout the entire composition instead of only its scene. _Fix: `stopRecording` now restores element `visibility` and `translate` to pre-recording values; `reapplyPathOffsets` passes `updateBase: false` to prevent corrupting the translate base._
12+
13+
- [x] **Moving element down causes it to go up** — Adjusting the Y position of the circle in the design panel caused it to move in the wrong direction. Multiple overlapping tweens (single-element + group selector) competed, and the wrong tween was being edited. _Fix: `pickBestAnimation` scores candidate tweens by selector specificity and time-range overlap to pick the correct one._
14+
15+
- [x] **Clicking elements repositions them randomly** — Just clicking on elements in the preview caused them to jump to a different position. Undo didn't help since it wasn't part of the edit stack. _Fix: `reapplyPathOffsets` no longer overwrites the original translate base during re-application (`updateBase: false`); `gsapAnimatesProperty` now handles array-form keyframes._
16+
17+
- [x] **Design panel shows stale position values during playback** — The X/Y/rotation fields in the layout panel didn't update when scrubbing or playing. _Fix: PropertyPanel now subscribes to `liveTime` (RAF-throttled) in addition to the store's `currentTime`._
18+
19+
- [x] **Scale doesn't respect the keyframe property at the moment it was created** — Not reproduced after the existing fixes. Setting scale to 4 at a keyframe position persists correctly after re-selection. May have been a symptom of the duplicate keyframe bug (now fixed).
20+
21+
- [x] **Baked properties conflict with GSAP tween positions** — Not reproduced after the `updateBase: false` fix. Clicking between elements with path offsets no longer corrupts positions. Element positions remain stable across multiple selection cycles.
22+
23+
### Gesture recording
24+
25+
- [x] **Recorded gestures appear at wrong position** — Gesture recording produced keyframes with a significant offset from the pointer location, both during live preview and after commit. _Fix: CSS var offsets always read regardless of translate state; translate cleared to "none" after each seek during preview; offset subtracted from committed samples._
26+
27+
- [x] **Gesture recording not gated behind keyframes env flag** — The record gesture button and hotkey were available even when `VITE_STUDIO_ENABLE_KEYFRAMES` was not set. _Fix: Button and hotkey now gated behind `STUDIO_KEYFRAMES_ENABLED`._
28+
29+
- [ ] **Gesture recording sometimes works, sometimes doesn't** — Miguel noted it's intermittent: "sometimes it works and when it works is really good." Needs investigation of what conditions cause it to fail.
30+
31+
### Keyframe conversion / creation
32+
33+
- [ ] **Add-keyframe button does nothing on elements that already have keyframes** — Clicking the "add keyframe at playhead" button on an element that already has keyframes at other positions does nothing. Only moving the element auto-creates keyframes. The button should be disabled with a tooltip when keyframes already exist, or it should work correctly.
34+
35+
- [ ] **GSAP import missing in non-GSAP scripts** — When trying to add keyframes to a composition that doesn't import GSAP, the script fails because it tries to call GSAP functions that don't exist in that context. Fix: ensure GSAP import is added to the script when creating keyframe tweens.
36+
37+
### Timeline UX
38+
39+
- [x] **Timeline jumps around when clicking small keyframe diamonds** — When zoomed in and clicking a keyframe diamond, the timeline auto-scrolls to the playhead position even though the diamond was already visible. _Fix: Auto-scroll now only runs during playback (`isPlaying`), not on user-initiated seeks from diamond clicks._
40+
41+
- [x] **Keyframe navigation arrows don't jump between keyframes** — The left/right arrows next to property fields in the design panel were expected to navigate to the previous/next keyframe, but they don't. _Fix: Wired `onSeekToTime`, `onAddKeyframe`, `onRemoveKeyframe`, `onConvertToKeyframes` through `StudioRightPanel` to `PropertyPanel`._
42+
43+
- [ ] **No drag-to-reposition for keyframe diamonds** — Can't drag keyframe diamonds to move them to a different time position on the timeline. Would be a useful feature.
44+
45+
### Animation section confusion
46+
47+
- [x] **Confusing distinction between keyframe values vs tween values** — When a keyframe is selected, the layout panel shows keyframe-scoped values, but the animation section shows tween-level values. _Fix: AnimationCard now shows a diamond hint "Keyframed — edit values in the Layout panel above" for animations with keyframes._
48+
49+
- [x] **Scale value resets to 1 after editing** — Not reproduced after existing fixes. Setting scale to 2 or 4 at a keyframe position persists correctly. Was likely a symptom of the duplicate keyframe bug (now fixed).
50+
51+
## Summary
52+
53+
- **Total bugs found**: 16
54+
- **Fixed / addressed in PR #1314**: 15
55+
- **Remaining**: 1 (gesture recording intermittent failure — needs more investigation)

packages/core/src/studio-api/routes/files.ts

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -702,8 +702,27 @@ export function registerFileRoutes(api: Hono, adapter: StudioApiAdapter): void {
702702
return c.json({ error: "mutation type required" }, 400);
703703
}
704704

705-
const html = readFileSync(res.absPath, "utf-8");
706-
const block = extractGsapScriptBlock(html);
705+
let html = readFileSync(res.absPath, "utf-8");
706+
let block = extractGsapScriptBlock(html);
707+
if (!block && (body.type === "add" || body.type === "add-with-keyframes")) {
708+
const compId = html.match(/data-composition-id="([^"]+)"/)?.[1] ?? "main";
709+
const gsapCdn = `<script src="https://cdn.jsdelivr.net/npm/gsap@3.12.5/dist/gsap.min.js"></script>`;
710+
const bootstrap = [
711+
gsapCdn,
712+
"<script>",
713+
"window.__timelines = window.__timelines || {};",
714+
`const tl = gsap.timeline({ paused: true });`,
715+
`window.__timelines["${compId}"] = tl;`,
716+
"</script>",
717+
].join("\n");
718+
if (html.includes("</body>")) {
719+
html = html.replace("</body>", `${bootstrap}\n</body>`);
720+
} else {
721+
html += `\n${bootstrap}`;
722+
}
723+
writeFileSync(res.absPath, html, "utf-8");
724+
block = extractGsapScriptBlock(html);
725+
}
707726
if (!block) {
708727
return c.json({ error: "no GSAP script found in file" }, 400);
709728
}

packages/core/src/studio-api/routes/preview.ts

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -67,12 +67,14 @@ function injectScriptTagIntoHead(html: string, scriptTag: string): string {
6767
}
6868

6969
function htmlHasGsap(html: string): boolean {
70-
// Keep this heuristic conservative: if user source already loads GSAP, Studio does not add another copy.
70+
// Only match GSAP references outside <template> elements — scripts inside
71+
// templates are inert when cloned and don't make GSAP globally available.
72+
const outsideTemplates = html.replace(/<template\b[^>]*>[\s\S]*?<\/template>/gi, "");
7173
return (
72-
/<script\b[^>]*src=["'][^"']*gsap/i.test(html) ||
73-
/\/\*\s*inlined:.*gsap/i.test(html) ||
74-
/\b(GreenSock|_gsScope)\b/.test(html) ||
75-
/\bgsap\.(config|defaults|registerPlugin|version)\b/.test(html)
74+
/<script\b[^>]*src=["'][^"']*gsap/i.test(outsideTemplates) ||
75+
/\/\*\s*inlined:.*gsap/i.test(outsideTemplates) ||
76+
/\b(GreenSock|_gsScope)\b/.test(outsideTemplates) ||
77+
/\bgsap\.(config|defaults|registerPlugin|version)\b/.test(outsideTemplates)
7678
);
7779
}
7880

packages/studio/src/App.tsx

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ import { AskAgentModal } from "./components/AskAgentModal";
3535
import { StudioGlobalDragOverlay } from "./components/StudioGlobalDragOverlay";
3636
import { StudioHeader } from "./components/StudioHeader";
3737
import { useGestureCommit } from "./hooks/useGestureCommit";
38+
import { STUDIO_KEYFRAMES_ENABLED } from "./components/editor/manualEditingAvailability";
3839

3940
import { GestureTrailOverlay } from "./components/editor/GestureTrailOverlay";
4041
import { StudioLeftSidebar } from "./components/StudioLeftSidebar";
@@ -251,7 +252,9 @@ export function StudioApp() {
251252
onResetKeyframes: () => resetKeyframesRef.current(),
252253
onDeleteSelectedKeyframes: () => deleteSelectedKeyframesRef.current(),
253254
onAfterUndoRedo: () => invalidateGsapCacheRef.current(),
254-
onToggleRecording: () => handleToggleRecordingRef.current(),
255+
onToggleRecording: STUDIO_KEYFRAMES_ENABLED
256+
? () => handleToggleRecordingRef.current()
257+
: undefined,
255258
});
256259
const selectSidebarTabStable = useCallback(
257260
(tab: SidebarTab) => leftSidebarRef.current?.selectTab(tab),
@@ -529,7 +532,7 @@ export function StudioApp() {
529532
}}
530533
recordingState={gestureState}
531534
recordingDuration={gestureRecording.recordingDuration}
532-
onToggleRecording={handleToggleRecording}
535+
onToggleRecording={STUDIO_KEYFRAMES_ENABLED ? handleToggleRecording : undefined}
533536
/>
534537
)}
535538
</div>

packages/studio/src/components/StudioRightPanel.tsx

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import { useStudioContext } from "../contexts/StudioContext";
2020
import { usePanelLayoutContext } from "../contexts/PanelLayoutContext";
2121
import { useFileManagerContext } from "../contexts/FileManagerContext";
2222
import { useDomEditContext } from "../contexts/DomEditContext";
23+
import { usePlayerStore } from "../player";
2324

2425
export interface StudioRightPanelProps {
2526
selectedStudioMotion: StudioMotionData | null;
@@ -100,6 +101,9 @@ export function StudioRightPanel({
100101
commitAnimatedProperty,
101102
handleSetArcPath,
102103
handleUpdateArcSegment,
104+
handleGsapAddKeyframe,
105+
handleGsapRemoveKeyframe,
106+
handleGsapConvertToKeyframes,
103107
} = useDomEditContext();
104108

105109
const { assets, fontAssets, projectDir, handleImportFiles, handleImportFonts } =
@@ -234,6 +238,10 @@ export function StudioRightPanel({
234238
onRemoveGsapFromProperty={handleGsapRemoveFromProperty}
235239
onAddGsapAnimation={handleGsapAddAnimation}
236240
onCommitAnimatedProperty={commitAnimatedProperty}
241+
onAddKeyframe={handleGsapAddKeyframe}
242+
onRemoveKeyframe={handleGsapRemoveKeyframe}
243+
onConvertToKeyframes={handleGsapConvertToKeyframes}
244+
onSeekToTime={(t) => usePlayerStore.getState().requestSeek(t)}
237245
onSetArcPath={handleSetArcPath}
238246
onUpdateArcSegment={handleUpdateArcSegment}
239247
recordingState={recordingState}

packages/studio/src/components/TimelineToolbar.tsx

Lines changed: 69 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,41 @@ import { Scissors } from "../icons/SystemIcons";
1212
import type { GsapAnimation } from "@hyperframes/core/gsap-parser";
1313
import type { DomEditSelection } from "./editor/domEditingTypes";
1414

15+
function AutoKeyframeToggle() {
16+
const enabled = usePlayerStore((s) => s.autoKeyframeEnabled);
17+
return (
18+
<Tooltip
19+
label={
20+
enabled
21+
? "Auto-keyframe ON — edits create keyframes"
22+
: "Auto-keyframe OFF — edits modify tween values"
23+
}
24+
>
25+
<button
26+
type="button"
27+
onClick={() => usePlayerStore.getState().setAutoKeyframeEnabled(!enabled)}
28+
className={`flex h-7 items-center gap-1 rounded px-1.5 text-[10px] font-medium transition-colors ${
29+
enabled
30+
? "bg-red-500/15 text-red-400 border border-red-500/30"
31+
: "text-neutral-500 hover:text-neutral-300"
32+
}`}
33+
>
34+
<svg width="8" height="8" viewBox="0 0 8 8" fill="currentColor">
35+
<circle
36+
cx="4"
37+
cy="4"
38+
r={enabled ? 4 : 3}
39+
fill={enabled ? "currentColor" : "none"}
40+
stroke="currentColor"
41+
strokeWidth={enabled ? 0 : 1.2}
42+
/>
43+
</svg>
44+
Auto
45+
</button>
46+
</Tooltip>
47+
);
48+
}
49+
1550
interface DomEditSessionSlice extends EnableKeyframesSession {
1651
domEditSelection: DomEditSelection | null;
1752
selectedGsapAnimations: GsapAnimation[];
@@ -74,40 +109,43 @@ export function TimelineToolbar({
74109
Timeline
75110
</div>
76111
{STUDIO_KEYFRAMES_ENABLED && onToggleKeyframe && (
77-
<Tooltip
78-
label={
79-
keyframeState === "active"
80-
? "Remove keyframe at playhead"
81-
: keyframeState === "inactive"
82-
? "Add keyframe at playhead"
83-
: "Enable keyframes"
84-
}
85-
>
86-
<button
87-
type="button"
88-
onClick={onToggleKeyframe}
89-
className={`flex h-7 w-7 items-center justify-center rounded transition-colors ${
112+
<>
113+
<Tooltip
114+
label={
90115
keyframeState === "active"
91-
? "text-studio-accent"
116+
? "Remove keyframe at playhead"
92117
: keyframeState === "inactive"
93-
? "text-neutral-400 hover:text-studio-accent"
94-
: "text-neutral-600 hover:text-neutral-400"
95-
}`}
118+
? "Add keyframe at playhead"
119+
: "Enable keyframes"
120+
}
96121
>
97-
<svg width="18" height="18" viewBox="0 0 10 10" fill="currentColor">
98-
{keyframeState === "active" ? (
99-
<path d="M5 0.5L9.5 5L5 9.5L0.5 5Z" />
100-
) : (
101-
<path
102-
d="M5 1.2L8.8 5L5 8.8L1.2 5Z"
103-
fill="none"
104-
stroke="currentColor"
105-
strokeWidth="1.2"
106-
/>
107-
)}
108-
</svg>
109-
</button>
110-
</Tooltip>
122+
<button
123+
type="button"
124+
onClick={onToggleKeyframe}
125+
className={`flex h-7 w-7 items-center justify-center rounded transition-colors ${
126+
keyframeState === "active"
127+
? "text-studio-accent"
128+
: keyframeState === "inactive"
129+
? "text-neutral-400 hover:text-studio-accent"
130+
: "text-neutral-600 hover:text-neutral-400"
131+
}`}
132+
>
133+
<svg width="18" height="18" viewBox="0 0 10 10" fill="currentColor">
134+
{keyframeState === "active" ? (
135+
<path d="M5 0.5L9.5 5L5 9.5L0.5 5Z" />
136+
) : (
137+
<path
138+
d="M5 1.2L8.8 5L5 8.8L1.2 5Z"
139+
fill="none"
140+
stroke="currentColor"
141+
strokeWidth="1.2"
142+
/>
143+
)}
144+
</svg>
145+
</button>
146+
</Tooltip>
147+
<AutoKeyframeToggle />
148+
</>
111149
)}
112150
{onSplitElement &&
113151
(() => {

packages/studio/src/components/editor/AnimationCard.tsx

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -398,9 +398,21 @@ export const AnimationCard = memo(function AnimationCard({
398398
<div className="pt-2">
399399
<div className="space-y-3">
400400
<div className="flex items-start gap-2">
401-
<p className="flex-1 text-[10px] leading-relaxed text-neutral-400 italic">
402-
{summary}
403-
</p>
401+
<div className="flex-1">
402+
<p className="text-[10px] leading-relaxed text-neutral-400 italic">{summary}</p>
403+
{animation.keyframes && (
404+
<p className="mt-1 text-[9px] text-neutral-500">
405+
<span
406+
className="inline-block w-2 h-2 mr-1 align-middle"
407+
style={{
408+
background: "currentColor",
409+
clipPath: "polygon(50% 0%, 100% 50%, 50% 100%, 0% 50%)",
410+
}}
411+
/>
412+
Keyframed — edit values in the Layout panel above
413+
</p>
414+
)}
415+
</div>
404416
<button
405417
type="button"
406418
onClick={() => {

packages/studio/src/components/editor/PropertyPanel.tsx

Lines changed: 26 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { memo, useRef, useState } from "react";
1+
import { memo, useEffect, useRef, useState } from "react";
22
import { Eye, Layers, Move, X } from "../../icons/SystemIcons";
33
import { useStudioContext } from "../../contexts/StudioContext";
44
import { readStudioBoxSize, readStudioPathOffset, readStudioRotation } from "./manualEdits";
@@ -17,7 +17,7 @@ import { GsapAnimationSection } from "./GsapAnimationSection";
1717
import { PropertyPanel3dTransform } from "./propertyPanel3dTransform";
1818
import { KeyframeNavigation } from "./KeyframeNavigation";
1919
import { STUDIO_GSAP_PANEL_ENABLED, STUDIO_KEYFRAMES_ENABLED } from "./manualEditingAvailability";
20-
import { usePlayerStore } from "../../player";
20+
import { usePlayerStore, liveTime } from "../../player";
2121
import { TimingSection } from "./propertyPanelTimingSection";
2222
import { type PropertyPanelProps } from "./propertyPanelHelpers";
2323

@@ -88,7 +88,27 @@ export const PropertyPanel = memo(function PropertyPanel({
8888
const { showToast } = useStudioContext();
8989
const [clipboardCopied, setClipboardCopied] = useState(false);
9090
const clipboardTimerRef = useRef<ReturnType<typeof setTimeout>>(undefined);
91-
const currentTime = usePlayerStore((s) => s.currentTime);
91+
const storeTime = usePlayerStore((s) => s.currentTime);
92+
const isPlaying = usePlayerStore((s) => s.isPlaying);
93+
const liveTimeRef = useRef(storeTime);
94+
const [, forceRender] = useState(0);
95+
useEffect(() => {
96+
if (!isPlaying) return;
97+
let rafId = 0;
98+
const unsub = liveTime.subscribe((t) => {
99+
liveTimeRef.current = t;
100+
if (!rafId)
101+
rafId = requestAnimationFrame(() => {
102+
rafId = 0;
103+
forceRender((v) => v + 1);
104+
});
105+
});
106+
return () => {
107+
unsub();
108+
cancelAnimationFrame(rafId);
109+
};
110+
}, [isPlaying]);
111+
const currentTime = isPlaying ? liveTimeRef.current : storeTime;
92112

93113
if (!element) {
94114
return (
@@ -140,11 +160,12 @@ export const PropertyPanel = memo(function PropertyPanel({
140160
const commitManualOffset = (axis: "x" | "y", nextValue: string) => {
141161
const parsed = parsePxMetricValue(nextValue);
142162
if (parsed == null) return;
143-
if (onCommitAnimatedProperty && (gsapAnimId || gsapAnimations.length > 0)) {
163+
const autoKf = usePlayerStore.getState().autoKeyframeEnabled;
164+
if (autoKf && onCommitAnimatedProperty && (gsapAnimId || gsapAnimations.length > 0)) {
144165
void onCommitAnimatedProperty(element, axis, parsed);
145166
return;
146167
}
147-
if (gsapKeyframes && gsapAnimId && onAddKeyframe) {
168+
if (autoKf && gsapKeyframes && gsapAnimId && onAddKeyframe) {
148169
const pct = Math.max(0, Math.min(100, Math.round(currentPct * 10) / 10));
149170
onAddKeyframe(gsapAnimId, pct, axis, parsed);
150171
return;

packages/studio/src/components/editor/manualEditsDom.ts

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -527,10 +527,14 @@ function reapplyPathOffsets(doc: Document): void {
527527
const x = el.style.getPropertyValue(STUDIO_OFFSET_X_PROP);
528528
const y = el.style.getPropertyValue(STUDIO_OFFSET_Y_PROP);
529529
if (x || y) {
530-
applyStudioPathOffset(el, {
531-
x: Number.parseFloat(x) || 0,
532-
y: Number.parseFloat(y) || 0,
533-
});
530+
applyStudioPathOffset(
531+
el,
532+
{
533+
x: Number.parseFloat(x) || 0,
534+
y: Number.parseFloat(y) || 0,
535+
},
536+
{ updateBase: false },
537+
);
534538
}
535539
}
536540
}

0 commit comments

Comments
 (0)