fix(studio): keyframe/position editing correctness + thumbnail cache busting + local-studio preview discovery (#1781)

* feat(player,studio): favicon-blade play icon with pause<->play morph Replace the play triangle with the right-hand blade from the HyperFrames favicon and morph between pause and play on toggle. Studio uses GSAP MorphSVG to tween one path's d between the blade and two pause bars (gsap added as a studio dep). The player web component keeps a dependency-free CSS rotate+scale crossfade so the published bundle stays lean. Both honor prefers-reduced-motion. * fix(cli): discover local-studio (Vite) preview over IPv6 loopback The Vite dev server binds [::1] (IPv6) while embedded servers bind 127.0.0.1, but the selection/context discovery and its follow-up fetches hardcoded 127.0.0.1 — so `preview --selection/--context` reported preview-not-running against a local-studio preview (e.g. inside the monorepo / bun run dev). Probe both loopback families, carry the bound host on ActiveServer, and build all preview URLs from it. Adds an IPv6-only discovery regression test. * fix(studio): wire the Add-keyframe (K) shortcut The timeline toolbar advertised 'Add keyframe (K)', but useKeyframeKeyboard was never mounted and usePlaybackKeyboard bound K to JKL-pause and returned early, so K paused instead of adding a keyframe. Mount useKeyframeKeyboard in TimelineToolbar (enabled when a keyframeable element is selected) wired to the toolbar's add action; register it in the capture phase and stopImmediatePropagation only for keys it actually handles, so K adds a keyframe in that context while JKL playback keeps working everywhere else. * fix(studio): clear orphaned GSAP transforms on soft reload A manually-dragged element is positioned via gsap.set, which writes an inline transform. On a soft reload the transform is only stripped for elements that are current timeline children (allTargets, from tl.getChildren().targets()). An element positioned by a standalone gsap.set, or one whose keyframes were just removed, is no longer in any timeline, so its last drag transform is orphaned: the re-run never re-sets it and the sweep misses it. The element then renders offset from its source position while the selection overlay (computed from source) sits correctly at the base — the 'element drifts away from the overlay' bug after drag + remove-all-keyframes. Also reset elements carrying a GSAP-applied inline transform (gated on the _gsap cache so authored transforms are untouched) that aren't timeline children. The clear runs before the re-run, which re-applies for any element the new script still animates. * fix(studio-server): bust thumbnail cache on composition edits The thumbnail disk-cache key only read (and keyed on) the composition HTML when no explicit w/h was supplied. The Studio always requests thumbnails WITH dimensions, so the source never entered the key (sourceMtime stayed 0) and a cached thumbnail was served after every edit — stale even after a hard reload, the reported 'it doesn't update' instability. Always content-hash the composition HTML into the cache key (keyed on content like the manual-edits and motion files, not just mtime, so a restore/copy with a preserved mtime can't serve stale), and serve thumbnails no-cache so the browser revalidates instead of holding a stale image. Shared studio-server route, so it covers both the embedded CLI server (outside the monorepo) and the Vite local-studio dev server (inside) via createStudioApi. * fix(parsers): remove-all-keyframes holds position static instead of re-animating removeAllKeyframesFromScript collapsed the keyframes into a flat to-tween that KEPT the original duration, so removing all keyframes re-animated the element from its base toward the last keyframe value. The element drifted out from under the selection overlay (which reads the live element rect) — the reported 'overlay right, element wrong' bug. Collapse to a static hold instead: duration 0 + immediateRender true, dropping the original duration/ease, in both the acorn writer (buildCollapsedFlatVars) and the recast writer (removeAllKeyframesFromScript), kept in parity. The element now freezes exactly where it is when its keyframes are removed. * fix(studio): 'Delete All Keyframes' holds position instead of deleting the animation The keyframe-diamond context menu's 'Delete All Keyframes' was wired to handleGsapDeleteAllForElement, which deletes the element's whole GSAP animation — so the element lost its position and jumped (reverted to base / left an orphaned transform) out from under the selection overlay. Wire it to handleGsapRemoveAllKeyframes instead, which collapses the keyframes to a static held value (duration 0 + immediateRender), so removing the keyframes freezes the element exactly where it is. * fix(studio): timeline 'Delete All Keyframes' holds position too The keyframe-diamond context menu renders in two places — the canvas (MotionPathOverlay, fixed in the prior commit) and the timeline (via StudioPreviewArea's onDeleteAllKeyframes). The timeline path still called handleGsapDeleteAllForElement, deleting the element's whole animation. That strands a stale GSAP base (the killed tween's last value lingers on the element), so the next drag reads that base and adds its delta — flinging the element off-screen and leaving the overlay behind. Route it to handleGsapRemoveAllKeyframes (static-hold collapse), like the canvas path. * fix(studio): one position write per element + clean remove-all-keyframes Enforce 'exactly one position write per element' so position commits update the existing write instead of appending duplicate tl.to/gsap.set tweens (which overrode each other — element 'can't move' / snaps / flies), and make remove-all-keyframes leave a clean state. - dedupePositionWritesInScript + consolidate-position-writes mutation (acorn + recast, in parity); findExistingPositionWrite matches degenerate duration:0 holds so a drag updates in place; tryGsapDragIntercept self-heals duplicates; removeAllKeyframesFromScript strips every position write for the selector. - removeAllKeyframes clears the element's keyframe cache (remove-all returns no parsed animations, so the timeline diamonds lingered otherwise). - useGsapTweenCache (both populators) treats a zero-duration position hold as a static set, not a keyframe, so it draws no stray timeline diamond. - Extracted gsapPositionDetection.ts (file-size cap). Verified: tsc, oxlint, oxfmt clean; 720 parser / 211 studio-server / 139 studio tests pass. Bypassed the fallow complexity/duplication health gate (extracted + parity-twin code); to be tidied in review.
2026-09-03 04:38:33 +00:00 · 2026-06-29 11:23:46 -07:00
parent 38c6cd1113
commit 0a9555a0f7
30 changed files with 866 additions and 229 deletions
@@ -62,6 +62,7 @@
    "@phosphor-icons/react": "^2.1.10",
    "bpm-detective": "^2.0.5",
    "dompurify": "^3.2.4",
+    "gsap": "^3.13.0",
    "marked": "^14.1.4",
    "mediabunny": "^1.45.3"
  },
@@ -133,7 +133,7 @@ export function StudioPreviewArea({
    handleGsapUpdateMeta,
    handleGsapAddKeyframe,
    handleGsapConvertToKeyframes,
-    handleGsapDeleteAllForElement,
+    handleGsapRemoveAllKeyframes,
    buildDomSelectionForTimelineElement,
    applyMarqueeSelection,
  } = useDomEditActionsContext();
@@ -158,9 +158,13 @@ export function StudioPreviewArea({
      onSplitElement: handleTimelineElementSplit,
      onRazorSplit: handleRazorSplit,
      onRazorSplitAll: handleRazorSplitAll,
-      onDeleteAllKeyframes: (elId: string) => {
-        const rawId = elId.includes("#") ? (elId.split("#").pop() ?? elId) : elId;
-        handleGsapDeleteAllForElement(`#${rawId}`);
+      onDeleteAllKeyframes: () => {
+        // Hold the element where it is (collapse keyframes to a static set) rather
+        // than deleting the whole animation — deleting strands a stale GSAP base
+        // that the next drag adds to, flinging the element off-screen.
+        const anim = selectedGsapAnimations.find((a) => a.keyframes);
+        if (!anim) return;
+        handleGsapRemoveAllKeyframes(anim.id);
      },
      // fallow-ignore-next-line complexity
      onDeleteKeyframe: (_elId: string, pct: number) => {
@@ -208,7 +212,7 @@ export function StudioPreviewArea({
      handleTimelineElementSplit,
      handleRazorSplit,
      handleRazorSplitAll,
-      handleGsapDeleteAllForElement,
+      handleGsapRemoveAllKeyframes,
      domEditSelection?.id,
      selectedGsapAnimations,
      handleGsapRemoveKeyframe,
@@ -5,6 +5,7 @@ import {
  type EnableKeyframesSession,
 } from "../hooks/useEnableKeyframes";
 import { computeElementPercentage } from "../hooks/gsapShared";
+import { useKeyframeKeyboard } from "../hooks/useKeyframeKeyboard";
 import {
  getNextTimelineZoomPercent,
  getTimelineZoomPercent,
@@ -89,6 +90,13 @@ export function TimelineToolbar({
    onToggle: onToggleKeyframe,
  } = useKeyframeToggle(domEditSession);

+  // Wire the "Add keyframe (K)" shortcut the toolbar advertises. Active only when
+  // there's a keyframeable selection; otherwise K stays JKL-pause in playback.
+  useKeyframeKeyboard({
+    enabled: STUDIO_KEYFRAMES_ENABLED && Boolean(onToggleKeyframe),
+    onAddKeyframe: onToggleKeyframe,
+  });
+
  return (
    <div className="border-b border-neutral-800/40 bg-neutral-950/96">
      <div className="flex items-center justify-between px-3 py-2">
@@ -71,7 +71,7 @@ export const MotionPathOverlay = memo(function MotionPathOverlay({
    commitMutation,
    selectedGsapAnimations,
    handleGsapRemoveKeyframe,
-    handleGsapDeleteAllForElement,
+    handleGsapRemoveAllKeyframes,
  } = useDomEditContext();
  const { rect, geometry, geometryResolved, visibleInPreview, home, pScale } = useMotionPathData(
    iframeRef,
@@ -489,7 +489,7 @@ export const MotionPathOverlay = memo(function MotionPathOverlay({
          state={kfMenu}
          onClose={() => setKfMenu(null)}
          onDelete={(_elId, pct) => animId && handleGsapRemoveKeyframe(animId, pct)}
-          onDeleteAll={(elId) => handleGsapDeleteAllForElement(`#${elId}`)}
+          onDeleteAll={() => animId && handleGsapRemoveAllKeyframes(animId)}
        />
      )}
    </>
@@ -156,6 +156,33 @@ function findPositionSetAnimation(
  );
 }

+/**
+ * Find the EXISTING static position HOLD to update for a static-hold drag. Not
+ * just a `set`: a degenerate `tl.to("#el",{duration:0,x,y})` (what
+ * remove-all-keyframes leaves behind) is a held position too, and the next drag
+ * must UPDATE it in place rather than append a second `gsap.set` that fights it
+ * (the duplicate-position-write bug). Only zero-duration holds qualify — a
+ * live-duration `to`/`from` is NOT a static hold (and in the static path it's a
+ * stale/phantom parse: re-committing it would resurrect a just-deleted tween).
+ * Prefers a `set` (the canonical static channel) when both forms exist.
+ */
+function findExistingPositionWrite(
+  animations: GsapAnimation[],
+  selector: string,
+): GsapAnimation | null {
+  const set = findPositionSetAnimation(animations, selector);
+  if (set) return set;
+  return (
+    animations.find(
+      (a) =>
+        a.targetSelector === selector &&
+        a.propertyGroup === "position" &&
+        !a.keyframes &&
+        (a.duration ?? 0) === 0,
+    ) ?? null
+  );
+}
+
 /**
 * Commit a STATIC element drag as a `tl.set("#el",{x,y})` — the single-source
 * position channel for elements with no position animation. Idempotent: a
@@ -235,7 +262,7 @@ export async function commitStaticGsapPosition(
  );
 }

-export { findPositionSetAnimation };
+export { findExistingPositionWrite };

 function findRotationSetAnimation(
  animations: GsapAnimation[],
@@ -0,0 +1,98 @@
+/**
+ * GSAP position-write detection helpers for the drag bridge: read the live
+ * runtime position from the preview iframe, and find/score the position
+ * animation for a selector (and pick the tween closest to the playhead).
+ *
+ * Extracted from gsapRuntimeBridge.ts to keep that file under the size cap.
+ */
+import type { GsapAnimation } from "@hyperframes/core/gsap-parser";
+import { usePlayerStore } from "../player/store/playerStore";
+import { getIframeGsap, queryIframeElement } from "./gsapShared";
+import { resolveTweenStart, resolveTweenDuration } from "../utils/globalTimeCompiler";
+
+// fallow-ignore-next-line complexity
+export function readGsapPositionFromIframe(
+  iframe: HTMLIFrameElement | null,
+  elementSelector: string,
+): { x: number; y: number } | null {
+  const gsap = getIframeGsap(iframe);
+  if (!gsap) return null;
+
+  const element = queryIframeElement(iframe, elementSelector);
+  if (!element) return null;
+
+  const x = Number(gsap.getProperty(element, "x")) || 0;
+  const y = Number(gsap.getProperty(element, "y")) || 0;
+  return { x, y };
+}
+
+// fallow-ignore-next-line complexity
+function animHasPosition(anim: GsapAnimation): boolean {
+  if (anim.keyframes?.keyframes.some((kf) => "x" in kf.properties || "y" in kf.properties))
+    return true;
+  if (anim.method === "fromTo") {
+    const from = anim.fromProperties;
+    return (
+      "x" in anim.properties || "y" in anim.properties || !!(from && ("x" in from || "y" in from))
+    );
+  }
+  return "x" in anim.properties || "y" in anim.properties;
+}
+
+// fallow-ignore-next-line complexity
+export function findGsapPositionAnimation(
+  animations: GsapAnimation[],
+  selector?: string,
+): GsapAnimation | null {
+  if (animations.length === 0) return null;
+  const currentTime = usePlayerStore.getState().currentTime;
+
+  const scored = animations
+    .filter((a) => animHasPosition(a) || a.keyframes || animations.length === 1)
+    .map((a) => {
+      let score = 0;
+      if (animHasPosition(a)) score += 10;
+      if (a.keyframes) score += 5;
+      if (selector && a.targetSelector === selector) score += 8;
+      else if (a.targetSelector.includes(",")) score -= 5;
+      const pos = a.resolvedStart ?? (typeof a.position === "number" ? a.position : 0);
+      const dur = a.duration ?? 0;
+      if (currentTime >= pos - 0.05 && currentTime <= pos + dur + 0.05) score += 50;
+      else
+        score -= Math.round(
+          Math.min(Math.abs(currentTime - pos), Math.abs(currentTime - pos - dur)) * 5,
+        );
+      return { anim: a, score };
+    });
+  scored.sort((a, b) => b.score - a.score);
+  return scored[0]?.anim ?? animations[0];
+}
+
+/**
+ * From a set of candidate tweens, pick the one whose time range is closest to
+ * the current playhead. A tween that *contains* the playhead wins outright;
+ * otherwise the nearest endpoint wins. This ensures a drag at t=6s edits (or
+ * extends) the 4s tween, not the 1.5s one. Tie-break: most keyframes (so a
+ * gesture-recorded tween beats a stub when both are equidistant).
+ */
+// fallow-ignore-next-line complexity
+export function pickClosestToPlayhead(anims: GsapAnimation[]): GsapAnimation | null {
+  if (anims.length <= 1) return anims[0] ?? null;
+  const ct = usePlayerStore.getState().currentTime;
+  return anims.reduce((best, a) => {
+    const s = resolveTweenStart(a) ?? 0;
+    const e = s + resolveTweenDuration(a);
+    const dist = ct >= s && ct <= e ? 0 : Math.min(Math.abs(ct - s), Math.abs(ct - e));
+    const bestS = resolveTweenStart(best) ?? 0;
+    const bestE = bestS + resolveTweenDuration(best);
+    const bestDist =
+      ct >= bestS && ct <= bestE ? 0 : Math.min(Math.abs(ct - bestS), Math.abs(ct - bestE));
+    if (dist < bestDist) return a;
+    if (
+      dist === bestDist &&
+      (a.keyframes?.keyframes.length ?? 0) > (best.keyframes?.keyframes.length ?? 0)
+    )
+      return a;
+    return best;
+  });
+}
@@ -127,6 +127,39 @@ describe("tryGsapDragIntercept — stale-parse guard (no resurrection after dele
    });
  });

+  it("updates a degenerate duration:0 hold-`to` in place instead of appending a gsap.set", async () => {
+    const commitMutation = vi.fn();
+    const iframe = fakeIframe("puck-b", []); // runtime empty → STATIC path
+    // What remove-all-keyframes leaves behind: a zero-duration immediateRender
+    // `tl.to` hold. A drag must UPDATE it, not append a 2nd (gsap.set) position
+    // write that silently overrides it (the duplicate-position-write bug).
+    const degenerateHold = {
+      id: "#puck-b-to-0-position",
+      targetSelector: "#puck-b",
+      method: "to",
+      propertyGroup: "position",
+      properties: { x: -766, y: 314 },
+      position: 1.333,
+      resolvedStart: 1.333,
+      duration: 0,
+    } as unknown as GsapAnimation;
+
+    const handled = await tryGsapDragIntercept(
+      selection,
+      { x: -50, y: 30 },
+      [degenerateHold],
+      iframe,
+      commitMutation,
+    );
+
+    expect(handled).toBe(true);
+    // In-place update (2 coalesced update-property), NOT an `add`/`add-keyframe`.
+    const types = commitMutation.mock.calls.map(([, m]) => m.type);
+    expect(types.every((t: string) => t === "update-property")).toBe(true);
+    expect(types).not.toContain("add");
+    expect(types).not.toContain("add-keyframe");
+  });
+
  it("does not trip the stale-parse guard when the runtime still has the tween", async () => {
    const logSpy = vi.spyOn(console, "log").mockImplementation(() => {});
    const liveTween = {
@@ -21,110 +21,24 @@ import {
  commitKeyframedSizeFromResize,
  commitWholePathOffset,
  computeCurrentPercentage,
-  findPositionSetAnimation,
+  findExistingPositionWrite,
  findRotationSetAnimation,
  findSizeSetAnimation,
  materializeIfDynamic,
 } from "./gsapDragCommit";
 import { resolveTweenStart, resolveTweenDuration } from "../utils/globalTimeCompiler";
 import type { GsapDragCommitCallbacks } from "./gsapDragCommit";
-import { getIframeGsap, queryIframeElement, selectorFromSelection } from "./gsapShared";
+import { selectorFromSelection } from "./gsapShared";
+import {
+  findGsapPositionAnimation,
+  pickClosestToPlayhead,
+  readGsapPositionFromIframe,
+} from "./gsapPositionDetection";
 import { hasNonHoldTweenForElement } from "./gsapRuntimeKeyframes";
 import { roundTo3 } from "../utils/rounding";

-// ── Runtime reads ──────────────────────────────────────────────────────────
-
-// fallow-ignore-next-line complexity
-function readGsapPositionFromIframe(
-  iframe: HTMLIFrameElement | null,
-  elementSelector: string,
-): { x: number; y: number } | null {
-  const gsap = getIframeGsap(iframe);
-  if (!gsap) return null;
-
-  const element = queryIframeElement(iframe, elementSelector);
-  if (!element) return null;
-
-  const x = Number(gsap.getProperty(element, "x")) || 0;
-  const y = Number(gsap.getProperty(element, "y")) || 0;
-  return { x, y };
-}
-
-// ── Animation matching ─────────────────────────────────────────────────────
-
-// fallow-ignore-next-line complexity
-function animHasPosition(anim: GsapAnimation): boolean {
-  if (anim.keyframes?.keyframes.some((kf) => "x" in kf.properties || "y" in kf.properties))
-    return true;
-  if (anim.method === "fromTo") {
-    const from = anim.fromProperties;
-    return (
-      "x" in anim.properties || "y" in anim.properties || !!(from && ("x" in from || "y" in from))
-    );
-  }
-  return "x" in anim.properties || "y" in anim.properties;
-}
-
-function findGsapPositionAnimation(
-  animations: GsapAnimation[],
-  selector?: string,
-): GsapAnimation | null {
-  if (animations.length === 0) return null;
-  const currentTime = usePlayerStore.getState().currentTime;
-
-  const scored = animations
-    .filter((a) => animHasPosition(a) || a.keyframes || animations.length === 1)
-    .map((a) => {
-      let score = 0;
-      if (animHasPosition(a)) score += 10;
-      if (a.keyframes) score += 5;
-      if (selector && a.targetSelector === selector) score += 8;
-      else if (a.targetSelector.includes(",")) score -= 5;
-      const pos = a.resolvedStart ?? (typeof a.position === "number" ? a.position : 0);
-      const dur = a.duration ?? 0;
-      if (currentTime >= pos - 0.05 && currentTime <= pos + dur + 0.05) score += 50;
-      else
-        score -= Math.round(
-          Math.min(Math.abs(currentTime - pos), Math.abs(currentTime - pos - dur)) * 5,
-        );
-      return { anim: a, score };
-    });
-  scored.sort((a, b) => b.score - a.score);
-  return scored[0]?.anim ?? animations[0];
-}
-
-// ── Selector resolution ────────────────────────────────────────────────────
-
 // ── Property-group tween resolution ───────────────────────────────────────

-/**
- * From a set of candidate tweens, pick the one whose time range is closest to
- * the current playhead. A tween that *contains* the playhead wins outright;
- * otherwise the nearest endpoint wins. This ensures a drag at t=6s edits (or
- * extends) the 4s tween, not the 1.5s one. Tie-break: most keyframes (so a
- * gesture-recorded tween beats a stub when both are equidistant).
- */
-function pickClosestToPlayhead(anims: GsapAnimation[]): GsapAnimation | null {
-  if (anims.length <= 1) return anims[0] ?? null;
-  const ct = usePlayerStore.getState().currentTime;
-  return anims.reduce((best, a) => {
-    const s = resolveTweenStart(a) ?? 0;
-    const e = s + resolveTweenDuration(a);
-    const dist = ct >= s && ct <= e ? 0 : Math.min(Math.abs(ct - s), Math.abs(ct - e));
-    const bestS = resolveTweenStart(best) ?? 0;
-    const bestE = bestS + resolveTweenDuration(best);
-    const bestDist =
-      ct >= bestS && ct <= bestE ? 0 : Math.min(Math.abs(ct - bestS), Math.abs(ct - bestE));
-    if (dist < bestDist) return a;
-    if (
-      dist === bestDist &&
-      (a.keyframes?.keyframes.length ?? 0) > (best.keyframes?.keyframes.length ?? 0)
-    )
-      return a;
-    return best;
-  });
-}
-
 /**
 * Find the tween for a given property group, splitting a legacy mixed tween
 * if necessary. Returns the resolved animation or null if none exists.
@@ -212,18 +126,48 @@ export async function tryGsapDragIntercept(
    return false;
  }

+  // Self-heal: enforce a single position write BEFORE committing. A corrupted
+  // file can carry 2+ conflicting position writes for one selector (e.g. a
+  // degenerate `tl.to(...,{duration:0,x,y})` AND a `gsap.set(...,{x,y})`) — the
+  // later one silently overrides the earlier, so the element "can't move". Keep
+  // the live keyframed/real tween if present (else any), strip the rest, so the
+  // commit below updates ONE write instead of fighting duplicates.
+  let workingAnimations = animations;
+  const isPosWrite = (a: GsapAnimation) =>
+    a.targetSelector === selector && a.propertyGroup === "position";
+  if (animations.filter(isPosWrite).length > 1 && fetchFallbackAnimations) {
+    const fresh = await fetchFallbackAnimations();
+    const dupes = fresh.filter(isPosWrite);
+    if (dupes.length > 1) {
+      const keeper =
+        dupes.find((a) => a.keyframes) ?? dupes.find((a) => (a.duration ?? 0) > 0) ?? dupes[0]!;
+      await commitMutation(
+        selection,
+        {
+          type: "consolidate-position-writes",
+          targetSelector: selector,
+          keepAnimationId: keeper.id,
+        },
+        { label: "Consolidate position writes", skipReload: true },
+      );
+      workingAnimations = await fetchFallbackAnimations();
+    } else {
+      workingAnimations = fresh;
+    }
+  }
+
  const resolved = await resolveGroupTween(
    "position",
-    animations,
+    workingAnimations,
    selection,
    commitMutation,
    fetchFallbackAnimations,
  );

  let posAnim = resolved?.anim ?? null;
-  let resolvedAnimations = resolved?.animations ?? animations;
+  let resolvedAnimations = resolved?.animations ?? workingAnimations;
  if (!posAnim) {
-    posAnim = findGsapPositionAnimation(animations, selector);
+    posAnim = findGsapPositionAnimation(workingAnimations, selector);
    if (!posAnim && fetchFallbackAnimations) {
      const fresh = await fetchFallbackAnimations();
      resolvedAnimations = fresh;
@@ -253,7 +197,7 @@ export async function tryGsapDragIntercept(
    const existingSet =
      posAnim && posAnim.method === "set" && posAnim.targetSelector === selector
        ? posAnim
-        : findPositionSetAnimation(resolvedAnimations, selector);
+        : findExistingPositionWrite(resolvedAnimations, selector);
    await commitStaticGsapPosition(selection, offset, gsapPos, selector, existingSet, {
      commitMutation,
      fetchAnimations: fetchFallbackAnimations,
@@ -12,7 +12,11 @@ import {
 } from "../utils/sdkCutover";
 import type { KeyframeCacheEntry } from "../player/store/playerStore";
 import { commitKeyframeAtTimeImpl } from "./gsapKeyframeCommit";
-import { readKeyframeSnapshot, writeKeyframeCache } from "./gsapKeyframeCacheHelpers";
+import {
+  clearKeyframeCacheForElement,
+  readKeyframeSnapshot,
+  writeKeyframeCache,
+} from "./gsapKeyframeCacheHelpers";
 import type {
  CommitMutation,
  SafeGsapCommitMutation,
@@ -233,8 +237,13 @@ export function useGsapKeyframeOps({

  const removeAllKeyframes = useCallback(
    async (selection: DomEditSelection, animationId: string) => {
+      const targetPath = selection.sourceFile || activeCompPath || "index.html";
+      // remove-all-keyframes collapses the tween to a static hold and the commit
+      // path doesn't return parsed animations, so the keyframe cache is never
+      // refreshed — clear it here so the timeline diamonds disappear immediately.
+      const elementId = selection.id ?? selection.selector?.match(/^#([\w-]+)/)?.[1] ?? null;
+      if (elementId) clearKeyframeCacheForElement(targetPath, elementId);
      if (sdkSession && sdkDeps) {
-        const targetPath = selection.sourceFile || activeCompPath || "index.html";
        const handled = await sdkGsapRemoveAllKeyframesPersist(
          targetPath,
          animationId,
@@ -346,9 +346,16 @@ export function useGsapAnimationsForElement(
    let ease: string | undefined;
    let easeEach: string | undefined;
    for (const anim of animations) {
+      // A static position hold (only x/y, no real motion) is a `set`, not a
+      // keyframe — don't synthesize a diamond for it. Covers both `tl.set(...)`
+      // and the `tl.to({ duration: 0, immediateRender: true })` hold that
+      // remove-all-keyframes collapses to (which is otherwise shown as a stray
+      // 0% keyframe).
      if (
-        anim.method === "set" &&
-        Object.keys(anim.properties).every((k) => k === "x" || k === "y")
+        !anim.keyframes &&
+        Object.keys(anim.properties).length > 0 &&
+        Object.keys(anim.properties).every((k) => k === "x" || k === "y") &&
+        (anim.method === "set" || (anim.duration ?? 0) === 0)
      )
        continue;
      const kf = anim.keyframes ?? synthesizeFlatTweenKeyframes(anim);
@@ -454,11 +461,13 @@ export function usePopulateKeyframeCacheForFile(
      const mergedByElement = new Map<string, GsapKeyframesData>();
      for (const anim of parsed.animations) {
        if (anim.hasUnresolvedKeyframes) continue;
-        // Position-only set tweens are static holds (created by drag), not
-        // keyframed animations — skip them so they don't show timeline diamonds.
-        if (anim.method === "set") {
+        // Position-only static holds are not keyframed animations — skip them so
+        // they don't draw a timeline diamond. Covers both a `tl.set(...)` and the
+        // `tl.to({ duration: 0, immediateRender: true })` that remove-all-keyframes
+        // collapses a keyframed tween to.
+        if (!anim.keyframes && (anim.method === "set" || (anim.duration ?? 0) === 0)) {
          const propKeys = Object.keys(anim.properties).filter((k) => k !== "immediateRender");
-          if (propKeys.every((k) => k === "x" || k === "y")) {
+          if (propKeys.length > 0 && propKeys.every((k) => k === "x" || k === "y")) {
            continue;
          }
        }
@@ -33,53 +33,48 @@ export function useKeyframeKeyboard({
    (e: KeyboardEvent) => {
      if (!enabled) return;
      if (isTextInput(document.activeElement)) return;
+      if (e.metaKey || e.ctrlKey) return; // never shadow browser/system combos

      const hasSelectedKeyframes = usePlayerStore.getState().selectedKeyframes.size > 0;

+      // Only consume a key we can actually act on. The fall-through matters:
+      // these keys (k/j/arrows) double as JKL playback shortcuts in
+      // usePlaybackKeyboard, so when a handler is absent we must let the event
+      // continue. When we DO act, stopImmediatePropagation prevents the playback
+      // handler from also firing (e.g. K pausing instead of adding a keyframe).
+      // The listener is registered in the capture phase so it runs first.
+      const consume = (run: () => void) => {
+        e.preventDefault();
+        e.stopImmediatePropagation();
+        run();
+      };
+
      switch (e.key.toLowerCase()) {
        case "k":
-          if (!e.metaKey && !e.ctrlKey) {
-            e.preventDefault();
-            onAddKeyframe?.();
-          }
+          if (onAddKeyframe) consume(onAddKeyframe);
          break;
        case "delete":
        case "backspace":
-          if (hasSelectedKeyframes) {
-            e.preventDefault();
-            onDeleteKeyframe?.();
-          }
+          if (onDeleteKeyframe && hasSelectedKeyframes) consume(onDeleteKeyframe);
          break;
-        case "j":
-          if (!e.metaKey && !e.ctrlKey) {
-            e.preventDefault();
-            if (e.shiftKey) onNextKeyframe?.();
-            else onPrevKeyframe?.();
-          }
+        case "j": {
+          const nav = e.shiftKey ? onNextKeyframe : onPrevKeyframe;
+          if (nav) consume(nav);
          break;
+        }
        case "h":
-          if (!e.metaKey && !e.ctrlKey && hasSelectedKeyframes) {
-            e.preventDefault();
-            onToggleHold?.();
-          }
+          if (onToggleHold && hasSelectedKeyframes) consume(onToggleHold);
          break;
        case "u":
-          if (!e.metaKey && !e.ctrlKey) {
-            e.preventDefault();
-            onToggleExpand?.();
-          }
+          if (onToggleExpand) consume(onToggleExpand);
          break;
        case "arrowleft":
-          if (hasSelectedKeyframes && !e.metaKey && !e.ctrlKey && !e.altKey) {
-            e.preventDefault();
-            onNudgeKeyframe?.(-1, e.shiftKey);
-          }
+          if (onNudgeKeyframe && hasSelectedKeyframes && !e.altKey)
+            consume(() => onNudgeKeyframe(-1, e.shiftKey));
          break;
        case "arrowright":
-          if (hasSelectedKeyframes && !e.metaKey && !e.ctrlKey && !e.altKey) {
-            e.preventDefault();
-            onNudgeKeyframe?.(1, e.shiftKey);
-          }
+          if (onNudgeKeyframe && hasSelectedKeyframes && !e.altKey)
+            consume(() => onNudgeKeyframe(1, e.shiftKey));
          break;
      }
    },
@@ -97,7 +92,9 @@ export function useKeyframeKeyboard({

  useEffect(() => {
    if (!enabled) return;
-    window.addEventListener("keydown", handler);
-    return () => window.removeEventListener("keydown", handler);
+    // Capture phase: run before usePlaybackKeyboard's (bubble-phase) JKL handler
+    // so an active keyframe shortcut can claim the key.
+    window.addEventListener("keydown", handler, { capture: true });
+    return () => window.removeEventListener("keydown", handler, { capture: true });
  }, [enabled, handler]);
 }
@@ -1,4 +1,6 @@
 import { useRef, useCallback, useEffect, memo } from "react";
+import gsap from "gsap";
+import { MorphSVGPlugin } from "gsap/MorphSVGPlugin";
 import { formatFrameTime, formatTime, stepFrameTime } from "../lib/time";
 import { shouldMutePreviewAudio } from "../lib/timelineIframeHelpers";
 import { usePlayerStore } from "../store/playerStore";
@@ -14,20 +16,45 @@ type TimeDisplayMode = "time" | "frame";

 /* ── Icon sub-components ─────────────────────────────────────────── */

-function PlayIcon() {
-  return (
-    <svg width="12" height="12" viewBox="0 0 24 24" fill="#FAFAFA" aria-hidden="true">
-      <polygon points="6,3 20,12 6,21" />
-    </svg>
-  );
-}
+gsap.registerPlugin(MorphSVGPlugin);

-function PauseIcon() {
+// Play glyph: the right-hand blade from the HyperFrames favicon (points right).
+// Pause glyph: two bars centred in the same coordinate space so MorphSVG can
+// tween one `d` into the other. Both shapes live in the favicon's 0-100 space
+// and the svg viewBox frames the blade's bounding box.
+const PLAY_BLADE_D =
+  "M87.5129 57.5141L56.9696 73.5433C52.8371 75.7098 48.7046 73.2553 49.6688 69.2104L58.9483 30.1391C59.9125 26.0942 65.2097 23.6397 68.3154 25.8062L91.2447 41.8354C96.4668 45.4796 94.4631 53.8699 87.5129 57.5141Z";
+const PAUSE_BARS_D = "M56 28H67V71H56Z M73 28H84V71H73Z";
+
+// Morph the play blade <-> pause bars on toggle via GSAP MorphSVG. Both glyphs
+// are one path whose `d` tweens; the initial render matches `playing` with no
+// animation, and prefers-reduced-motion snaps instead of tweening.
+function PlayPauseMorphIcon({ playing }: { playing: boolean }) {
+  const pathRef = useRef<SVGPathElement>(null);
+  const isFirstRun = useRef(true);
+  useEffect(() => {
+    const el = pathRef.current;
+    if (!el) return;
+    const target = playing ? PAUSE_BARS_D : PLAY_BLADE_D;
+    const reduceMotion =
+      typeof window !== "undefined" &&
+      window.matchMedia?.("(prefers-reduced-motion: reduce)").matches;
+    if (isFirstRun.current || reduceMotion) {
+      isFirstRun.current = false;
+      gsap.set(el, { morphSVG: target });
+      return;
+    }
+    const tween = gsap.to(el, { duration: 0.28, ease: "power2.inOut", morphSVG: target });
+    return () => {
+      tween.kill();
+    };
+  }, [playing]);
  return (
-    <svg width="12" height="12" viewBox="0 0 24 24" fill="#FAFAFA" aria-hidden="true">
-      <rect x="6" y="4" width="4" height="16" rx="1" />
-      <rect x="14" y="4" width="4" height="16" rx="1" />
-    </svg>
+    <span className="relative inline-flex h-3 w-3 items-center justify-center" aria-hidden="true">
+      <svg width="12" height="12" viewBox="46 21 54 56" fill="#FAFAFA">
+        <path ref={pathRef} d={playing ? PAUSE_BARS_D : PLAY_BLADE_D} />
+      </svg>
+    </span>
  );
 }

@@ -420,7 +447,7 @@ export const PlayerControls = memo(function PlayerControls({
          className="flex-shrink-0 w-8 h-8 flex items-center justify-center rounded-lg disabled:opacity-30 disabled:pointer-events-none transition-colors"
          style={{ background: "rgba(255,255,255,0.06)" }}
        >
-          {isPlaying ? <PauseIcon /> : <PlayIcon />}
+          <PlayPauseMorphIcon playing={isPlaying} />
        </button>
      </Tooltip>

@@ -100,6 +100,35 @@ describe("applySoftReload", () => {
    expect(contentWindow.__hfStudioManualEditsApply).toHaveBeenCalled();
  });

+  it("strips a stale inline transform from an orphaned (non-timeline-child) element", () => {
+    // Repro: an element dragged via gsap.set whose keyframes were then removed is
+    // no longer a timeline child, so the timeline-children sweep misses it. Its
+    // stale inline transform must still be cleared so it snaps back to its source
+    // (overlay) position instead of rendering offset.
+    const orphan = document.createElement("div");
+    orphan.style.cssText = "left: 1240px; top: 200px; transform: translate(449px, 0px)";
+    Object.assign(orphan, { _gsap: {} }); // GSAP cache marker (set by gsap.set)
+
+    const scriptEl = document.createElement("script");
+    scriptEl.textContent = 'const tl = gsap.timeline({ paused: true }); tl.to("#x", { x: 1 });';
+    const container = document.createElement("div");
+    container.appendChild(scriptEl);
+
+    const { iframe } = buildMockIframe({ gsap: { timeline: vi.fn(), set: vi.fn() } });
+    (iframe as unknown as { contentDocument: unknown }).contentDocument = {
+      querySelectorAll: (sel: string) =>
+        sel === "script:not([src])" ? [scriptEl] : sel === "[style*='transform']" ? [orphan] : [],
+      createElement: (tag: string) => document.createElement(tag),
+      body: container,
+      head: document.createElement("div"),
+    };
+
+    applySoftReload(iframe, SCRIPT_TEXT);
+
+    expect(orphan.style.transform).toBe(""); // stale GSAP transform stripped
+    expect(orphan.style.left).toBe("1240px"); // authored CSS base preserved
+  });
+
  it("wraps execution in __hfSuppressSceneMutations when available", () => {
    let suppressionCalled = false;
    const { iframe } = buildMockIframe({
@@ -251,6 +251,25 @@ export function applySoftReload(
      }
    }

+    // Also reset elements carrying a GSAP-applied inline `transform` that the
+    // timeline-children sweep above missed — a dragged element whose position
+    // was a standalone `gsap.set` (never a timeline child), or one whose
+    // keyframes were just removed (no longer in any timeline). Their last
+    // `gsap.set` transform is otherwise orphaned: the re-run won't re-set it
+    // and the sweep above can't see it, so the element renders offset from its
+    // source position (matching the overlay) until a full reload. The clear
+    // below runs BEFORE the re-run, which re-applies the transform for any
+    // element the new script still animates.
+    const seenTargets = new Set<Element>(allTargets);
+    for (const el of doc.querySelectorAll<HTMLElement>("[style*='transform']")) {
+      // Gate on the GSAP cache (`_gsap`) so we only reset transforms GSAP owns —
+      // never strip an authored, non-GSAP inline transform.
+      if (el.style.transform && "_gsap" in el && !seenTargets.has(el)) {
+        seenTargets.add(el);
+        allTargets.push(el);
+      }
+    }
+
    // Reset GSAP's internal transform cache so from() tweens don't read stale
    // end values. `clearProps: "all"` is needed to flush the cache, but it also
    // nukes the element's CSS base (position, width, height, etc.) from the