Skip to content

Commit e4f8126

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 e4f8126

30 files changed

Lines changed: 873 additions & 378 deletions
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/lint/rules/gsap.ts

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -867,4 +867,33 @@ export const gsapRules: LintRule<LintContext>[] = [
867867
}
868868
return findings;
869869
},
870+
871+
// gsap_group_selector_keyframes
872+
({ scripts }) => {
873+
const findings: HyperframeLintFinding[] = [];
874+
for (const script of scripts) {
875+
const content = stripJsComments(script.content);
876+
const pattern = /\.(?:to|from|fromTo)\(\s*["']([^"']+,\s*[^"']+)["']\s*,\s*\{[^}]*keyframes/g;
877+
let match: RegExpExecArray | null;
878+
while ((match = pattern.exec(content)) !== null) {
879+
const selector = match[1]!;
880+
const count = selector.split(",").length;
881+
const contextStart = Math.max(0, match.index - 20);
882+
const contextEnd = Math.min(content.length, match.index + match[0].length + 40);
883+
findings.push({
884+
code: "gsap_group_selector_keyframes",
885+
severity: "warning",
886+
message:
887+
`GSAP tween targets ${count} elements with shared keyframes ("${truncateSnippet(selector, 60)}"). ` +
888+
`Editing one element's keyframes in Studio will affect all ${count} elements. ` +
889+
`Split into individual tweens for per-element keyframe control.`,
890+
fixHint:
891+
`Replace the group selector with individual tl.to() calls per element, ` +
892+
`each with their own keyframes object.`,
893+
snippet: truncateSnippet(content.slice(contextStart, contextEnd)),
894+
});
895+
}
896+
}
897+
return findings;
898+
},
870899
];

packages/core/src/parsers/gsapParser.ts

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1623,18 +1623,18 @@ export function removeAllKeyframesFromScript(script: string, animationId: string
16231623
const kfNode = findKeyframesObjectNode(loc.target.call.varsArg);
16241624
if (!kfNode) return script;
16251625

1626-
// Collect all percentage keyframe entries, sorted
16271626
const kfEntries = filterPercentageProps(kfNode)
16281627
.map((p: any) => ({ pct: percentageFromKey(propKeyName(p)!), prop: p }))
16291628
.filter((e) => !Number.isNaN(e.pct))
16301629
.sort((a, b) => a.pct - b.pct);
16311630
if (kfEntries.length === 0) return script;
16321631

1633-
const lastRecord = objectExpressionToRecord(
1634-
kfEntries[kfEntries.length - 1]!.prop.value,
1635-
loc.parsed.scope,
1636-
);
1637-
collapseKeyframesToFlat(loc.target.call.varsArg, lastRecord);
1632+
// For to()/set(): collapse to last keyframe (the destination = visible state).
1633+
// For from(): collapse to first keyframe (the starting state).
1634+
const method = loc.target.call.method;
1635+
const collapseEntry = method === "from" ? kfEntries[0]! : kfEntries[kfEntries.length - 1]!;
1636+
const record = objectExpressionToRecord(collapseEntry.prop.value, loc.parsed.scope);
1637+
collapseKeyframesToFlat(loc.target.call.varsArg, record);
16381638

16391639
return recast.print(loc.parsed.ast).code;
16401640
}

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: 12 additions & 3 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),
@@ -330,7 +333,11 @@ export function StudioApp() {
330333
});
331334

332335
const compositionDimensions = useCompositionDimensions();
333-
const { lintModal, linting, handleLint, closeLintModal } = useLintModal(projectId);
336+
const { lintModal, linting, handleLint, closeLintModal, findingsByElement, findingsByFile } =
337+
useLintModal(projectId, refreshKey);
338+
useEffect(() => {
339+
usePlayerStore.getState().setLintFindingsByElement(findingsByElement);
340+
}, [findingsByElement]);
334341
const frameCapture = useFrameCapture({
335342
projectId,
336343
activeCompPath,
@@ -482,6 +489,8 @@ export function StudioApp() {
482489
onPreviewBlock={setBlockPreview}
483490
onLint={handleLint}
484491
linting={linting}
492+
lintFindingCount={lintModal?.length ?? findingsByFile.size}
493+
lintFindingsByFile={findingsByFile}
485494
/>
486495
<StudioPreviewArea
487496
timelineToolbar={timelineToolbar}
@@ -529,7 +538,7 @@ export function StudioApp() {
529538
}}
530539
recordingState={gestureState}
531540
recordingDuration={gestureRecording.recordingDuration}
532-
onToggleRecording={handleToggleRecording}
541+
onToggleRecording={STUDIO_KEYFRAMES_ENABLED ? handleToggleRecording : undefined}
533542
/>
534543
)}
535544
</div>

packages/studio/src/components/StudioLeftSidebar.tsx

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@ export interface StudioLeftSidebarProps {
1616
onPreviewBlock?: (preview: BlockPreviewInfo | null) => void;
1717
onLint: () => void;
1818
linting: boolean;
19+
lintFindingCount?: number;
20+
lintFindingsByFile?: Map<string, { count: number; messages: string[] }>;
1921
}
2022

2123
// fallow-ignore-next-line complexity
@@ -26,6 +28,8 @@ export function StudioLeftSidebar({
2628
onPreviewBlock,
2729
onLint,
2830
linting,
31+
lintFindingCount,
32+
lintFindingsByFile,
2933
}: StudioLeftSidebarProps) {
3034
const {
3135
leftCollapsed,
@@ -129,6 +133,8 @@ export function StudioLeftSidebar({
129133
isRendering={renderQueue.isRendering}
130134
onLint={onLint}
131135
linting={linting}
136+
lintFindingCount={lintFindingCount}
137+
lintFindingsByFile={lintFindingsByFile}
132138
onToggleCollapse={toggleLeftSidebar}
133139
onAddBlock={onAddBlock}
134140
onPreviewBlock={onPreviewBlock}

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: 54 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,26 @@ 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 label={enabled ? "Auto-keyframe ON" : "Auto-keyframe OFF"}>
19+
<button
20+
type="button"
21+
onClick={() => usePlayerStore.getState().setAutoKeyframeEnabled(!enabled)}
22+
className={`flex h-7 w-7 items-center justify-center rounded transition-colors ${
23+
enabled ? "text-red-400" : "text-neutral-600 hover:text-neutral-400"
24+
}`}
25+
>
26+
<svg width="14" height="14" viewBox="0 0 14 14" fill="none">
27+
<circle cx="7" cy="7" r="5" stroke="currentColor" strokeWidth="1.5" />
28+
{enabled && <circle cx="7" cy="7" r="3" fill="currentColor" />}
29+
</svg>
30+
</button>
31+
</Tooltip>
32+
);
33+
}
34+
1535
interface DomEditSessionSlice extends EnableKeyframesSession {
1636
domEditSelection: DomEditSelection | null;
1737
selectedGsapAnimations: GsapAnimation[];
@@ -74,40 +94,43 @@ export function TimelineToolbar({
7494
Timeline
7595
</div>
7696
{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 ${
97+
<>
98+
<Tooltip
99+
label={
90100
keyframeState === "active"
91-
? "text-studio-accent"
101+
? "Remove keyframe at playhead"
92102
: keyframeState === "inactive"
93-
? "text-neutral-400 hover:text-studio-accent"
94-
: "text-neutral-600 hover:text-neutral-400"
95-
}`}
103+
? "Add keyframe at playhead"
104+
: "Enable keyframes"
105+
}
96106
>
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>
107+
<button
108+
type="button"
109+
onClick={onToggleKeyframe}
110+
className={`flex h-7 w-7 items-center justify-center rounded transition-colors ${
111+
keyframeState === "active"
112+
? "text-studio-accent"
113+
: keyframeState === "inactive"
114+
? "text-neutral-400 hover:text-studio-accent"
115+
: "text-neutral-600 hover:text-neutral-400"
116+
}`}
117+
>
118+
<svg width="18" height="18" viewBox="0 0 10 10" fill="currentColor">
119+
{keyframeState === "active" ? (
120+
<path d="M5 0.5L9.5 5L5 9.5L0.5 5Z" />
121+
) : (
122+
<path
123+
d="M5 1.2L8.8 5L5 8.8L1.2 5Z"
124+
fill="none"
125+
stroke="currentColor"
126+
strokeWidth="1.2"
127+
/>
128+
)}
129+
</svg>
130+
</button>
131+
</Tooltip>
132+
<AutoKeyframeToggle />
133+
</>
111134
)}
112135
{onSplitElement &&
113136
(() => {

0 commit comments

Comments
 (0)