feat(studio): keyframe hooks wiring — session, cache, toolbar [5/6]#1171
feat(studio): keyframe hooks wiring — session, cache, toolbar [5/6]#1171miguel-heygen wants to merge 1 commit into
Conversation
|
Warning This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
This stack of pull requests is managed by Graphite. Learn more about stacking. |
56a0a44 to
40826ce
Compare
4bddbdd to
242d54b
Compare
40826ce to
aee3a23
Compare
044c22d to
a4b899e
Compare
554f0ac to
7e14d2b
Compare
a4b899e to
23563ad
Compare
7e14d2b to
dfa9793
Compare
23563ad to
9fb06d5
Compare
dfa9793 to
eee8f91
Compare
766ebbc to
0858462
Compare
eee8f91 to
197a649
Compare
0858462 to
c5b813b
Compare
02cf6cf to
079fcc5
Compare
c5b813b to
0474d74
Compare
079fcc5 to
fa450e6
Compare
0474d74 to
dcf1075
Compare
jrusso1020
left a comment
There was a problem hiding this comment.
Hooks integration layer. +607/-19 across 7 files — wires the keyframe system through the existing Studio session/cache/toolbar machinery. The deletions (-19) are smaller than I'd expect for a "wire through" PR — confirm none of those 19 lines remove a load-bearing escape hatch (e.g., the prior "skip GSAP-animated elements for CSS offset" guard).
Three angles worth checking explicitly:
1. Hook dependency arrays
useDomEditSession, useGsapScriptCommits, useGsapTweenCache, useDomEditCommits, useAppHotkeys all get new wiring. Per useEffect / useCallback dependency rules:
- If any new value is read inside an effect but not in its dependency array → stale closure → silent state drift.
- If any value is added to deps but is referentially unstable (
{...}literal, new array each render) → effect fires every render → cache invalidation thrash.
The hooks chain is non-trivial; a single bad dep array can cause "works on first run, breaks on second" symptoms. Worth eslint-plugin-react-hooks rule check (exhaustive-deps) at minimum — if the project doesn't already enforce, this PR is a good time to flag it.
2. Cache invalidation on undo/redo
PR body says "Undo/redo syncs keyframe cache." Verify the sync goes both ways:
- Undo: keyframe cache reverts to pre-mutation snapshot.
- Redo: keyframe cache re-applies the mutation's result.
- Mutation-after-undo: history is correctly truncated; redo no longer applies.
The interleaving with optimistic updates (from PR #1169) is the risky part. Optimistic state shouldn't be undone separately from server state.
3. "CSS offset never persisted for GSAP elements"
This guard is critical for the stale-offset cleanup from #1168 to remain meaningful. If the test plan checkbox stays unchecked at merge, the cleanup is gating on a behavior that isn't enforced at write time — i.e., the cleanup runs on reload but the offset gets re-written immediately by the next drag interaction. Verify the guard fires before the persistence call, not just at runtime cleanup.
Non-blocking
useAppHotkeys likely now has a new keyframe-toggle binding. Verify it doesn't conflict with any existing Studio hotkey (especially across the new S shortcut for Split in #1189 and any keyframe-add/delete keys this PR adds).
Review by Jerrai (hyperframes specialist)
dcf1075 to
90e4d0b
Compare

Summary
Wire keyframe system through useDomEditSession, useGsapScriptCommits, useGsapTweenCache, useDomEditCommits, useAppHotkeys. Toolbar diamond toggle. Undo/redo cache sync. CSS offset persistence guard.
Part 5 of 6 — hooks integration. Depends on PR 4.
Test plan