fix(studio): keyframe bug bash — 7 fixes rebased onto refactor

miguel-heygen · miguel-heygen · commit 4140770a023c · 2026-06-09T20:50:11.000-04:00
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
diff --git a/docs/bugs/2026-06-09-keyframes-bug-bash.md b/docs/bugs/2026-06-09-keyframes-bug-bash.md
@@ -0,0 +1,55 @@
+# Keyframes Bug Bash — 2026-06-09
+
+Attendees: Executive Suite (SF Office), James Russo, Miguel Angel Simon Sierra, Terence Cho, Ular Kimsanov, Vance Ingalls, Xiaye Wang
+
+## Bugs
+
+### Keyframe editing
+
+- [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`._
+
+- [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._
+
+- [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._
+
+- [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._
+
+- [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`._
+
+- [ ] **Scale doesn't respect the keyframe property at the moment it was created** — When modifying scale at a keyframe position, the value doesn't stick correctly; it snaps back or applies globally instead of at that keyframe.
+
+- [ ] **Baked properties conflict with GSAP tween positions** — Manual dragging properties (path offsets) interfere with the animated position of GSAP elements. After editing, the animation doesn't play back correctly until the page is refreshed.
+
+### Gesture recording
+
+- [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._
+
+- [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`._
+
+- [ ] **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.
+
+### Keyframe conversion / creation
+
+- [ ] **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.
+
+- [ ] **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.
+
+### Timeline UX
+
+- [ ] **Timeline jumps around when clicking small keyframe diamonds** — When zoomed in and clicking a keyframe diamond that's too small, the timeline view scrolls/jumps erratically.
+
+- [ ] **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.
+
+- [ ] **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.
+
+### Animation section confusion
+
+- [ ] **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. Editing in the animation section modifies the tween directly (one line of code), while editing in the layout section modifies the keyframe. This distinction is unclear to users.
+
+- [ ] **Scale value resets to 1 after editing** — Setting scale to 4 in the animation section caused it to snap back to 1 (the CSS maximum). The value didn't persist correctly.
+
+## Summary
+
+- **Total bugs found**: 16
+- **Fixed in PR #1312**: 7
+- **Remaining**: 9
diff --git a/packages/studio/src/App.tsx b/packages/studio/src/App.tsx
@@ -35,6 +35,7 @@ import { AskAgentModal } from "./components/AskAgentModal";
 import { StudioGlobalDragOverlay } from "./components/StudioGlobalDragOverlay";
 import { StudioHeader } from "./components/StudioHeader";
 import { useGestureCommit } from "./hooks/useGestureCommit";
+import { STUDIO_KEYFRAMES_ENABLED } from "./components/editor/manualEditingAvailability";
 
 import { GestureTrailOverlay } from "./components/editor/GestureTrailOverlay";
 import { StudioLeftSidebar } from "./components/StudioLeftSidebar";
@@ -251,7 +252,9 @@ export function StudioApp() {
     onResetKeyframes: () => resetKeyframesRef.current(),
     onDeleteSelectedKeyframes: () => deleteSelectedKeyframesRef.current(),
     onAfterUndoRedo: () => invalidateGsapCacheRef.current(),
-    onToggleRecording: () => handleToggleRecordingRef.current(),
+    onToggleRecording: STUDIO_KEYFRAMES_ENABLED
+      ? () => handleToggleRecordingRef.current()
+      : undefined,
   });
   const selectSidebarTabStable = useCallback(
     (tab: SidebarTab) => leftSidebarRef.current?.selectTab(tab),
@@ -529,7 +532,7 @@ export function StudioApp() {
                     }}
                     recordingState={gestureState}
                     recordingDuration={gestureRecording.recordingDuration}
-                    onToggleRecording={handleToggleRecording}
+                    onToggleRecording={STUDIO_KEYFRAMES_ENABLED ? handleToggleRecording : undefined}
                   />
                 )}
               </div>
diff --git a/packages/studio/src/components/editor/PropertyPanel.tsx b/packages/studio/src/components/editor/PropertyPanel.tsx
@@ -1,4 +1,4 @@
-import { memo, useRef, useState } from "react";
+import { memo, useEffect, useRef, useState } from "react";
 import { Eye, Layers, Move, X } from "../../icons/SystemIcons";
 import { useStudioContext } from "../../contexts/StudioContext";
 import { readStudioBoxSize, readStudioPathOffset, readStudioRotation } from "./manualEdits";
@@ -17,7 +17,7 @@ import { GsapAnimationSection } from "./GsapAnimationSection";
 import { PropertyPanel3dTransform } from "./propertyPanel3dTransform";
 import { KeyframeNavigation } from "./KeyframeNavigation";
 import { STUDIO_GSAP_PANEL_ENABLED, STUDIO_KEYFRAMES_ENABLED } from "./manualEditingAvailability";
-import { usePlayerStore } from "../../player";
+import { usePlayerStore, liveTime } from "../../player";
 import { TimingSection } from "./propertyPanelTimingSection";
 import { type PropertyPanelProps } from "./propertyPanelHelpers";
 
@@ -88,7 +88,26 @@ export const PropertyPanel = memo(function PropertyPanel({
   const { showToast } = useStudioContext();
   const [clipboardCopied, setClipboardCopied] = useState(false);
   const clipboardTimerRef = useRef<ReturnType<typeof setTimeout>>(undefined);
-  const currentTime = usePlayerStore((s) => s.currentTime);
+  const storeTime = usePlayerStore((s) => s.currentTime);
+  const [liveRenderTime, setLiveRenderTime] = useState(storeTime);
+  useEffect(() => {
+    setLiveRenderTime(storeTime);
+  }, [storeTime]);
+  useEffect(() => {
+    let rafId = 0;
+    const unsub = liveTime.subscribe((t) => {
+      if (!rafId)
+        rafId = requestAnimationFrame(() => {
+          rafId = 0;
+          setLiveRenderTime(t);
+        });
+    });
+    return () => {
+      unsub();
+      cancelAnimationFrame(rafId);
+    };
+  }, []);
+  const currentTime = liveRenderTime;
 
   if (!element) {
     return (
diff --git a/packages/studio/src/components/editor/manualEditsDom.ts b/packages/studio/src/components/editor/manualEditsDom.ts
@@ -527,10 +527,14 @@ function reapplyPathOffsets(doc: Document): void {
     const x = el.style.getPropertyValue(STUDIO_OFFSET_X_PROP);
     const y = el.style.getPropertyValue(STUDIO_OFFSET_Y_PROP);
     if (x || y) {
-      applyStudioPathOffset(el, {
-        x: Number.parseFloat(x) || 0,
-        y: Number.parseFloat(y) || 0,
-      });
+      applyStudioPathOffset(
+        el,
+        {
+          x: Number.parseFloat(x) || 0,
+          y: Number.parseFloat(y) || 0,
+        },
+        { updateBase: false },
+      );
     }
   }
 }
diff --git a/packages/studio/src/components/renders/RenderQueueItem.tsx b/packages/studio/src/components/renders/RenderQueueItem.tsx
@@ -142,53 +142,54 @@ export const RenderQueueItem = memo(function RenderQueueItem({
           )}
         </div>
 
-        {/* Actions */}
-        {hovered && (
-          <div className="flex items-center gap-1 flex-shrink-0">
-            {isComplete && (
-              <button
-                onClick={handleDownload}
-                className="p-1 rounded text-panel-text-4 hover:text-panel-accent transition-colors"
-                title="Download"
-              >
-                <svg
-                  width="12"
-                  height="12"
-                  viewBox="0 0 24 24"
-                  fill="none"
-                  stroke="currentColor"
-                  strokeWidth="2"
-                  strokeLinecap="round"
-                  strokeLinejoin="round"
-                >
-                  <path d="M21 15v4a2 2 0 01-2 2H5a2 2 0 01-2-2v-4" />
-                  <polyline points="7 10 12 15 17 10" />
-                  <line x1="12" y1="15" x2="12" y2="3" />
-                </svg>
-              </button>
-            )}
-            <button
-              onClick={(e) => {
-                e.stopPropagation();
-                onDelete();
-              }}
-              className="p-1 rounded text-panel-text-4 hover:text-red-400 transition-colors"
-              title="Remove"
+        {/* Actions — always visible to prevent layout shifts */}
+        <div className="flex items-center gap-1 flex-shrink-0">
+          <button
+            onClick={isComplete ? handleDownload : undefined}
+            className={`p-1 rounded transition-colors ${
+              isComplete
+                ? "text-panel-text-5 hover:text-panel-accent"
+                : "text-panel-text-5/30 pointer-events-none"
+            }`}
+            title={isComplete ? "Download" : "Rendering..."}
+            disabled={!isComplete}
+          >
+            <svg
+              width="12"
+              height="12"
+              viewBox="0 0 24 24"
+              fill="none"
+              stroke="currentColor"
+              strokeWidth="2"
+              strokeLinecap="round"
+              strokeLinejoin="round"
             >
-              <svg
-                width="12"
-                height="12"
-                viewBox="0 0 24 24"
-                fill="none"
-                stroke="currentColor"
-                strokeWidth="2"
-                strokeLinecap="round"
-              >
-                <path d="M18 6L6 18M6 6l12 12" />
-              </svg>
-            </button>
-          </div>
-        )}
+              <path d="M21 15v4a2 2 0 01-2 2H5a2 2 0 01-2-2v-4" />
+              <polyline points="7 10 12 15 17 10" />
+              <line x1="12" y1="15" x2="12" y2="3" />
+            </svg>
+          </button>
+          <button
+            onClick={(e) => {
+              e.stopPropagation();
+              onDelete();
+            }}
+            className="p-1 rounded text-panel-text-5 hover:text-red-400 transition-colors"
+            title="Remove"
+          >
+            <svg
+              width="12"
+              height="12"
+              viewBox="0 0 24 24"
+              fill="none"
+              stroke="currentColor"
+              strokeWidth="2"
+              strokeLinecap="round"
+            >
+              <path d="M18 6L6 18M6 6l12 12" />
+            </svg>
+          </button>
+        </div>
       </div>
     </div>
   );
diff --git a/packages/studio/src/hooks/gsapRuntimeBridge.ts b/packages/studio/src/hooks/gsapRuntimeBridge.ts
@@ -11,7 +11,6 @@
 import type { GsapAnimation } from "@hyperframes/core/gsap-parser";
 import type { DomEditSelection } from "../components/editor/domEditingTypes";
 
-import { resolveTweenStart, resolveTweenDuration } from "../utils/globalTimeCompiler";
 import { readAllAnimatedProperties, readGsapProperty } from "./gsapRuntimeReaders";
 import {
   commitGsapPositionFromDrag,
@@ -125,6 +124,10 @@ export async function tryGsapDragIntercept(
   const selector = selectorForSelection(selection);
   if (!selector) return false;
 
+  // Keyframe writes at 0%/100% when outside the tween range. Acceptable
+  // trade-off — CSS path must NEVER touch GSAP-targeted elements because
+  // changing the CSS offset corrupts all existing keyframes (baked mismatch).
+
   const gsapPos = readGsapPositionFromIframe(iframe, selector);
   if (!gsapPos) return false;
 
diff --git a/packages/studio/src/hooks/useAnimatedPropertyCommit.ts b/packages/studio/src/hooks/useAnimatedPropertyCommit.ts
@@ -45,6 +45,29 @@ function computePercentage(selection: DomEditSelection): number {
     : 0;
 }
 
+function pickBestAnimation(
+  animations: GsapAnimation[],
+  selector: string | null,
+): GsapAnimation | undefined {
+  if (animations.length <= 1) return animations[0];
+  const currentTime = usePlayerStore.getState().currentTime;
+
+  const scored = animations.map((a) => {
+    let score = 0;
+    if (a.keyframes) score += 10;
+    // Prefer single-element selectors over comma-separated groups
+    if (selector && a.targetSelector === selector) score += 5;
+    else if (a.targetSelector.includes(",")) score -= 3;
+    // Prefer tweens active at the current time
+    const pos = typeof a.position === "number" ? a.position : 0;
+    const dur = a.duration ?? 0;
+    if (currentTime >= pos && currentTime <= pos + dur) score += 8;
+    return { anim: a, score };
+  });
+  scored.sort((a, b) => b.score - a.score);
+  return scored[0]?.anim;
+}
+
 function selectorFor(selection: DomEditSelection): string | null {
   if (selection.id) return `#${selection.id}`;
   if (selection.selector) return selection.selector;
@@ -72,8 +95,7 @@ export function useAnimatedPropertyCommit(deps: CommitAnimatedPropertyDeps) {
       const selector = selectorFor(selection);
       const pct = computePercentage(selection);
 
-      let anim: GsapAnimation | undefined =
-        selectedGsapAnimations.find((a) => a.keyframes) ?? selectedGsapAnimations[0];
+      let anim: GsapAnimation | undefined = pickBestAnimation(selectedGsapAnimations, selector);
 
       // Case 3: No animation — create one first
       if (!anim) {
@@ -112,15 +134,26 @@ export function useAnimatedPropertyCommit(deps: CommitAnimatedPropertyDeps) {
       }
       backfillDefaults[property] = typeof value === "number" ? value : value;
 
+      const existingKf = anim.keyframes?.keyframes.some(
+        (kf) => Math.abs(kf.percentage - pct) < 0.05,
+      );
+
       await gsapCommitMutation(
         selection,
-        {
-          type: "add-keyframe",
-          animationId: anim.id,
-          percentage: pct,
-          properties,
-          backfillDefaults,
-        },
+        existingKf
+          ? {
+              type: "update-keyframe",
+              animationId: anim.id,
+              percentage: pct,
+              properties,
+            }
+          : {
+              type: "add-keyframe",
+              animationId: anim.id,
+              percentage: pct,
+              properties,
+              backfillDefaults,
+            },
         { label: `Edit ${property} (keyframe ${pct}%)`, softReload: true },
       );
     },
diff --git a/packages/studio/src/hooks/useGestureRecording.ts b/packages/studio/src/hooks/useGestureRecording.ts

Original file line number	Diff line number	Diff line change
`@@ -527,10 +527,14 @@ function reapplyPathOffsets(doc: Document): void {`
`527`	`527`	`const x = el.style.getPropertyValue(STUDIO_OFFSET_X_PROP);`
`528`	`528`	`const y = el.style.getPropertyValue(STUDIO_OFFSET_Y_PROP);`
`529`	`529`	`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`	`+ );`
`534`	`538`	`}`
`535`	`539`	`}`
`536`	`540`	`}`