feat(studio): consolidate timeline editor callbacks#2786
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. |
7f86018 to
86e9735
Compare
dd77b83 to
041ead6
Compare
vanceingalls
left a comment
There was a problem hiding this comment.
Adversarial R1 review — head 041ead68
Verdict: COMMENT (2 × P2, 2 × P3). No blockers; deferred-findings block absorbs the perf/nit surface, and the tests around the identity-carrying keyframe resolution are the load-bearing correctness signal. Flagging two golden-rule violations and a fallback-path caveat that the deferred list already names.
Adversarial lenses
- Lens A — signature stability. PASSED. The consolidated callbacks (
onDeleteKeyframe,onMoveKeyframeToPlayhead,onMoveKeyframe) acceptpropertyGroup,tweenPercentage,animationIdas optional params inpackages/studio/src/player/components/timelineCallbacks.ts:69-90. Legacy 2-arg callers (e.g.TimelineOverlays.tsx:110,115) still compile — new params default toundefinedand route through the fallback path. No dropped payload fields. - Lens B — semantic-vs-symptom. NOTED.
useTimelineEditCallbacks.ts:229introducesif (!anim.keyframes) return false;— flat boundary drags now silent-no-op instead of dispatching a boundary resize. Documented in the diff comment ("Boundary-to-clip resize wiring is intentionally deferred") and covered by the test "safely no-ops a boundary drag while the tween is still flat" (useTimelineEditCallbacks.test.tsx:167-177). Semantics shift is real but intentional. - Lens C — firing-order determinism. PASSED. The two
useEffecthooks inPropertyPanelFlat.tsx:239andAnimationCard.tsx:32run in child-mount order: parent opens Motion group first, child then clears the store token viaonFocusSegmentConsumed?.(). No callback-fire-order tests broken. - Lens D — React ref/handler stability. FIRED. See P2s below.
- Lens E — interaction with #2787. NOTED. #2787 hardens the exact surface this PR consolidates (deletion, retime, easing). The consolidate-then-harden order is defensible only because #2786 does not silently regress live behavior: the 8 new test cases in
useTimelineEditCallbacks.test.tsxpin the cross-element deletion contract that #2787 will build on. If any of those pins are missing, #2787's hardening becomes tautological. I read them as sufficient.
Findings
P2 — packages/studio/src/components/editor/AnimationCard.tsx:32-38, 40-47. Two new useEffect hooks synchronize state (setExpanded, setExpandedKfPct, imperative scrollIntoView) in response to the focusedSegment prop. This violates the repo golden rule "No useEffect for state syncing" (CLAUDE.md). Parents at GsapAnimationSection.tsx:79 and propertyPanelFlatMotionSection.tsx:161 pass a fresh inline onFocusSegmentConsumed={() => setFocusedEaseSegment(null)} each render → the effect's deps change every render → the effect fires every render (returns early once focusedSegment is null, but still burns work). Prefer a key={focusedSegment?.tweenPercentage ?? "none"} reset on the card, or memoize the consumer callback in the parents. The PR's deferred-findings list already parks this (🟢 AnimationCard.tsx:60) — flagging here because the golden rule marks it a blocker unless suppressed with cause.
P2 — packages/studio/src/components/editor/PropertyPanelFlat.tsx:239-245. Same golden-rule violation: useEffect fires setOpenGroupId("motion") in response to the focusedEaseSegment store subscription. The state-sync belongs either in the store selector (derive openGroupId) or in the click handler that sets focusedEaseSegment in the first place — the effect is a symptom of the write happening in a place that can't reach the setter. Not blocking on this stack tip, but should not proliferate.
P2 — packages/studio/src/components/nle/useTimelineEditCallbacks.ts:148-152 (fallback cache lookup). When callers pass only 2 args (TimelineOverlays.tsx:110 keyframe-diamond context menu, TimelineOverlays.tsx:115 move-to-playhead), resolveKeyframeTarget falls through to usePlayerStore.getState().keyframeCache.get(domEditSelection?.id ?? ""). That's the currently-selected element's cache — if the diamond context menu opens on a non-selected element's diamond, the fallback resolves against the wrong element's cache and returns null (or worse, a same-pct collision from a stale cache entry). The deferred-findings list acknowledges this (🟡 useTimelineEditCallbacks.ts:150); the tests pin the new-args path but do not pin the fallback path against a non-selected element. Not a regression from prior behavior (the old code was equally selection-bound), but the consolidation is now close to fixing this — a two-line change to route the fallback through resolveElementAnimations(elId)'s source-file resolution would close it. Worth resolving inside this PR since the plumbing is already here.
P3 — packages/studio/src/components/editor/GsapAnimationSection.tsx:37 + propertyPanelFlatMotionSection.tsx:139. withTrackedGsapAnimationCallbacks(callbacks, track) runs unconditionally each render and returns a fresh object with fresh function identities → every AnimationCard's memo() boundary is defeated. The prior inline-arrow code had the same defect, so this is not a regression, but the extraction is the natural place to wrap the whole return in useMemo(() => withTracked…, [callbacks, track]). Deferred-findings list has this at 🟢 already.
What's genuinely good
resolveTimelineKeyframeTargetatuseTimelineEditCallbacks.ts:33-51now preferskf.animationIdbefore falling back to property-group disambiguation. That closes the ambiguous same-group case that used to return the wrong tween. Pinned by two test cases inuseTimelineEditCallbacks.test.ts(same-group collision + stale-identity rejection). Solid correctness lift.- The cross-element deletion path (
onDeleteKeyframe→buildDomSelectionForTimelineElement(element).then(...)→removeKeyframeTarget(..., selection)) is the right shape and pinned by two symmetric tests (flat lane + authored endpoint on non-selected element). This is the load-bearing fix in the PR. GsapAddAnimationControlextraction from two callsites with two style variants is a clean dedupe.
— Via
james-russo-rames-d-jusso
left a comment
There was a problem hiding this comment.
Reviewed at 041ead68d1fa.
R2 adversarial pass on the editor-callback consolidation. R9 verdict: split_path_exists — the animationId propagation + onDeleteKeyframe per-element authority form a coherent core, but ~5 clusters (telemetry-wrapper DRY, GsapAddAnimationControl UI dedupe, shouldShowMotionPath, focusedEaseSegment auto-open flow, useStudioContextValue.shouldShowMotionPath decoupling) share no mutable authority with the core and could split cleanly. Two blockers in the mutation-authority story the PR claims to fix: (1) onDeleteKeyframe missing null-selection guard silently falls back to domEditSelection — the exact authority the comment on lines 216-218 claims to prevent; (2) onTogglePropertyGroupKeyframe looks up flat-tween identity in selectedGsapAnimations instead of the passed element's animations — a click on a non-selected element's diamond can strand the flat animation instead of removing it. Two orange items on the sibling callbacks that got the fix but weren't converted (onMoveKeyframeToPlayhead and onMoveKeyframe still _elId-discarding).
R9 exception verdict — split_path_exists
Cluster-by-cluster:
- A. animationId propagation + resolveTimelineKeyframeTarget: backward-compat superset with dedicated test — genuinely R9 core.
- B. onDeleteKeyframe per-element authority: independent — handleGsap* helpers already accepted selectionOverride pre-PR (
git diff pr-2785-r3...pr-2786-r3 -- packages/studio/src/hooks/useGsapSelectionHandlers.tsis empty). - C. onTogglePropertyGroupKeyframe: adds a NEW optional field to TimelineEditCallbacks — removing it leaves other callers unaffected (TimelineTrackHeader.tsx:444 uses
onTogglePropertyGroupKeyframe?.(...)). - D. withTrackedGsapAnimationCallbacks: pure telemetry-DRY refactor — pre-PR both sections had inline wrappers with identical semantics.
- E. GsapAddAnimationControl: pure UI dedupe with two style variants — no data-flow entanglement.
- F. shouldShowMotionPath: decouples motion-path visibility from shouldShowSelectedDomBounds (real behavior change: motion path now shows on Layers tab too) — orthogonal to timeline callbacks.
- G. focusedEaseSegment consumption + AnimationCard focus prop + auto-open: independent UI flow (producer in useTimelineKeyframeHandlers.ts, untouched here).
Removing D, E, F, or G individually keeps the remaining PR compiling and behavior-preserving. No dead-at-SHA code (all producers/consumers wired). No dual-authority intermediate contract created by extracting any of D/E/F/G.
🟢 Verified clean
- GsapAddAnimationControl.tsx variant/style split preserves pre-PR classes exactly (both variants' method/cancel/trigger classes match prior inline call sites)
- withTrackedGsapAnimationCallbacks preserves pre-PR tracked-event semantics (verified against gsapAnimationCallbacks.test.ts golden events list)
- resolveTimelineKeyframeTarget's rendered-identity branch guarded by tests 'uses the rendered animation identity to resolve a same-group collision' and 'rejects a rendered animation identity absent from the element' at useTimelineEditCallbacks.test.ts:43-70
- useInspectorState now takes domEditSelection and its four new tests cover the shouldShowMotionPath visibility matrix cleanly
Complements Via's parallel adversarial pass (Family B rollup). Where Via found 0 P1 / 17 P2 / 18 P3, the adversarial subagent pass here surfaced additional depth on the mutation-authority thread and the Promise chain.
| onMoveKeyframeToPlayhead: (_elId: string, pct: number) => { | ||
| const target = resolveKeyframeTarget(pct); | ||
| onMoveKeyframeToPlayhead: (_elId, pct, group, tweenPct, animationId) => { | ||
| const target = resolveKeyframeTarget(pct, group, tweenPct, animationId); |
There was a problem hiding this comment.
🔴 onDeleteKeyframe missing null-selection guard — silent-fallback deletes against the wrong element
void buildDomSelectionForTimelineElement(element).then((selection) => {
removeKeyframeTarget(target.animId, target.tweenPct, animations, selection);
});buildDomSelectionForTimelineElement is typed Promise<DomEditSelection | null> (useDomSelection.ts:94-96). When it resolves null, removeKeyframeTarget forwards null → handleGsapDeleteAnimation(id, null) or handleGsapRemoveKeyframe(id, pct, undefined, null). Both do selectionOverride ?? domEditSelection ?? lastSelectionRef.current (useGsapSelectionHandlers.ts:177,318), so explicit null falls through to the CURRENTLY-SELECTED element's DOM selection — precisely the authority the comment on lines 216-218 claims to prevent. onDeleteAllKeyframes in the same PR block (line 203) does guard with if (selection); onDeleteKeyframe does not. New tests only mockResolvedValue(mocks.selection); no null-resolution assertion.
Fix: Guard with if (!selection) return; before invoking removeKeyframeTarget in the then-callback, mirroring onDeleteAllKeyframes at line 203-205.
| target.properties, | ||
| undefined, | ||
| selection, | ||
| ); |
There was a problem hiding this comment.
🔴 onTogglePropertyGroupKeyframe looks up flat-tween identity in selectedGsapAnimations instead of the passed element's animations
if (target.remove) {
removeKeyframeTarget(
target.animationId,
target.tweenPercentage,
selectedGsapAnimations,
selection,
);
return;
}removeKeyframeTarget uses the passed animations array to decide flat-vs-keyframed: animations.find(candidate => candidate.id === animationId); if (animation && !animation.keyframes) handleGsapDeleteAnimation(...). When onTogglePropertyGroupKeyframe fires for a non-selected element (TimelineTrackHeader.tsx:444 invokes it on expandedElement, not the currently-selected one), target.animationId is unlikely to appear in selectedGsapAnimations — so animation is undefined, delete-animation branch is skipped, and it falls to handleGsapRemoveKeyframe for what is actually a flat tween. Strands the flat animation instead of removing it — the very footgun the flat-boundary comment at line 261-263 protects onMoveKeyframe against.
Fix: Pass resolveElementAnimations(getTimelineElementIdentity(element)) instead of selectedGsapAnimations, matching the pattern established by onDeleteKeyframe on line 208.
| // Retime the keyframe to the playhead, preserving its value + ease. | ||
| onMoveKeyframeToPlayhead: (_elId: string, pct: number) => { | ||
| const target = resolveKeyframeTarget(pct); | ||
| onMoveKeyframeToPlayhead: (_elId, pct, group, tweenPct, animationId) => { |
There was a problem hiding this comment.
🟠 onMoveKeyframeToPlayhead silent-fallback to domEditSelection (delete-authority fix not carried through)
onMoveKeyframeToPlayhead: (_elId, pct, group, tweenPct, animationId) => {
const target = resolveKeyframeTarget(pct, group, tweenPct, animationId);
if (target) handleGsapMoveKeyframeToPlayhead(target.animId, target.tweenPct);
},First arg is _elId — explicitly discarded. handleGsapMoveKeyframeToPlayhead does selectionOverride ?? domEditSelection ?? lastSelectionRef.current (useGsapSelectionHandlers.ts:330). Retime-to-playhead on a diamond belonging to a non-selected element (or one in a different source file) commits against the current domEditSelection — the exact authority bug onDeleteKeyframe fixes in this same PR. Inconsistent authority within one consolidation.
Fix: Route through buildDomSelectionForTimelineElement(element) like onDeleteKeyframe does; take the TimelineElement as the first arg or find it in usePlayerStore.getState().elements from elId.
| onMoveKeyframe: async (_elId: string, fromClipPct: number, toClipPct: number) => { | ||
| const target = resolveKeyframeTarget(fromClipPct); | ||
| onMoveKeyframe: async (_elId, fromClipPct, toClipPct, group, tweenPct, animationId) => { | ||
| const target = resolveKeyframeTarget(fromClipPct, group, tweenPct, animationId); |
There was a problem hiding this comment.
🟠 onMoveKeyframe silent-fallback to domEditSelection
onMoveKeyframe: async (_elId, fromClipPct, toClipPct, group, tweenPct, animationId) => {
const target = resolveKeyframeTarget(fromClipPct, group, tweenPct, animationId);
const sel = domEditSelection;Drag-retime uses const sel = domEditSelection — same authority-bug class as previous finding. handleGsapMoveKeyframe accepts selectionOverride (useGsapSelectionHandlers.ts:342-346) and would honor a per-element override, but this call site never supplies one. A drag on a non-selected diamond routes through the wrong element's DOM. Tests either exercise selected element or hit flat-tween early-return.
Fix: Take TimelineElement (or look up via elId), await buildDomSelectionForTimelineElement, pass resolved selection as 4th arg of handleGsapMoveKeyframe (and handleGsapResizeKeyframedTween / handleGsapUpdateMeta which also accept overrides).
| return resolveTimelineKeyframeTarget(pct, cached?.keyframes ?? [], selectedGsapAnimations); | ||
| return resolveTimelineKeyframeTarget( | ||
| pct, | ||
| explicitTarget ?? cached?.keyframes ?? [], |
There was a problem hiding this comment.
🟡 resolveKeyframeTarget cache key still hardcoded to domEditSelection?.id
const cached = usePlayerStore.getState().keyframeCache.get(domEditSelection?.id ?? "");
return resolveTimelineKeyframeTarget(
pct,
explicitTarget ?? cached?.keyframes ?? [],
animations,
);Fallback branch when caller omits propertyGroup/tweenPercentage/animationId reads the SELECTED element's keyframe cache — not the diamond's element. Inconsistent with the new resolveElementAnimations (line 118) which correctly uses the passed elementKey. For a diamond click on a non-selected element arriving without explicit-target fields (allowed by the optional signature in timelineCallbacks.ts:74-80), resolution consults the wrong cache. Current callers (KeyframeDiamondContextMenu.tsx:89) do pass all fields, so bug isn't triggered today — latent authority leak.
Fix: Thread the elementKey into resolveKeyframeTarget (as with resolveElementAnimations) and read keyframeCache.get(elementKey) ?? keyframeCache.get(elementId).
| } | ||
| : undefined | ||
| focusedSegment={ | ||
| focusedEaseSegment?.animationId === anim.id ? focusedEaseSegment : null |
There was a problem hiding this comment.
🟢 GsapAnimationSection focus-segment gate lacks the element-id scoping the flat panel has
focusedEaseSegment?.animationId === anim.id ? focusedEaseSegment : nullpropertyPanelFlatMotionSection.tsx:148 filters by BOTH animationId AND elementId (focusedEaseSegment.elementId === renderedElementId), with an explicit comment that a shared class-selector animation id could otherwise open the wrong element's editor. Classic GsapAnimationSection performs no element-id filter, so if two panels ever share an animation id (class-selector authoring) the classic path can auto-open the wrong card.
Fix: Add elementId filter to GsapAnimationSection matching the flat panel — accept element identity and gate focusedEaseSegment.elementId === renderedElementId before matching animationId.
| ? animations.find((animation) => animation.id === kf.animationId) | ||
| : undefined; | ||
| if (kf.animationId) { | ||
| return identifiedAnimation |
There was a problem hiding this comment.
🟢 resolveTimelineKeyframeTarget synthesized-target falls back tweenPct to clip-% when caller omits tweenPercentage
if (kf.animationId) {
return identifiedAnimation
? { animId: identifiedAnimation.id, tweenPct: kf.tweenPercentage ?? pct }
: null;
}When resolveKeyframeTarget is called with explicit animationId but tweenPercentage === undefined (allowed by the optional-arg signature), synthesized explicitTarget has tweenPercentage undefined, and this line collapses tweenPct to clip-%. Flat tweens don't sit at 0-100 clip-% (they occupy a sub-range), so downstream handleGsapRemoveKeyframe/handleGsapMoveKeyframeToPlayhead receive wrong tween percentage. All current diamond callers (KeyframeDiamondContextMenu.tsx:89-93) do pass state.tweenPercentage, so not triggered today — type-level footgun asymmetric with R9's locked-down contract claim.
Fix: Require tweenPercentage when animationId is provided (in the callback signature or by rejecting the target when tweenPercentage is undefined in the identified-animation branch).
|
|
||
| useEffect(() => { | ||
| if (!focusedSegment) return; | ||
| setExpanded(true); |
There was a problem hiding this comment.
🟢 AnimationCard focus-segment auto-scroll races the ease editor's mount
useEffect(() => {
if (!pendingAutoScrollRef.current || expandedKfPct === null) return;
const segment = cardRef.current?.querySelector<HTMLElement>(
`[data-ease-segment-pct="${expandedKfPct}"]`,
);
segment?.scrollIntoView({ block: "nearest", behavior: "smooth" });
pendingAutoScrollRef.current = false;
}, [expandedKfPct]);First effect sets both expanded=true and expandedKfPct. When focus arrives while card is collapsed, the [data-ease-segment-pct] DOM node only exists after ease editor renders — needs both flags to have committed AND child mounts to have run. This second effect fires on the expandedKfPct change; if ease editor renders in same commit, querySelector returns null and scrollIntoView silently no-ops, leaving pendingAutoScrollRef=false so no retry happens. Tests don't cover scroll path. onFocusSegmentConsumed is also a new function reference each parent render (inline arrow at GsapAnimationSection.tsx:60), so first effect re-runs on every parent re-render — guarded by if (!focusedSegment) return, so wasteful but not incorrect.
Fix: Defer scroll to a layoutEffect gated on the segment's mount (data-ease-segment-pct present) or use requestAnimationFrame + ref lookup with one retry. Memoize onFocusSegmentConsumed at the parent to avoid effect re-runs.
| STUDIO_INSPECTOR_PANELS_ENABLED && !rightCollapsed && inspectorPanelActive, | ||
| // Keep the selection box + motion path drawn even when the Inspector is | ||
| // collapsed — closing the panel shouldn't visually deselect the element. | ||
| shouldShowMotionPath: !!domEditSelection && !isPlaying && !isGestureRecording, |
There was a problem hiding this comment.
🟡 shouldShowMotionPath decoupling is a behavior change unrelated to 'consolidated timeline editor callbacks'
shouldShowMotionPath: !!domEditSelection && !isPlaying && !isGestureRecording,Pre-PR the motion path was drawn under the same condition as shouldShowSelectedDomBounds (Design/Variables tab open with a selection). Post-PR, motion path draws for ANY selection regardless of inspector tab. Grouped under a PR titled 'consolidated timeline editor callbacks', this is an orthogonal visibility change and a canonical R9 split candidate — removing this cluster leaves remaining PR compiling and behavior-preserving; consumers (App.tsx:397, EditorShell.tsx:94, PreviewOverlays.tsx:279) form a self-contained thread.
Fix: Extract shouldShowMotionPath work into its own PR with focused description of the intended visibility contract; leave this PR to the callback consolidation it advertises.
041ead6 to
bcc22d6
Compare
Three review follow-ups on the editor-callback consolidation. The keyframe-target resolve now takes the clicked element's key and reads that element's keyframe cache. The diamond context menu and move-to-playhead pass no explicit target, so they fell through to the cache of whatever element happened to be selected: opening the menu on a non-selected element's diamond resolved against the wrong keyframes. PropertyPanelFlat opens the Motion group by adjusting state during render instead of in an effect, so the AnimationCard mounts on the same commit the focus request arrives on rather than a frame later. Both animation sections pass a module-level focus consumer instead of a fresh inline arrow, so AnimationCard's focus effect stops re-running on every parent render.
The lane-header keyframe toggle fires on whichever element owns the lane, which need not be the selected one. The remove path looked the animation up in the selected element's animations, so a non-selected element's flat tween missed and silently took the remove-one-keyframe branch, stranding the tween instead of deleting it.
Drag-to-retime resolved the dragged diamond against the selected element's animations and committed through the selected element's DOM selection, so dragging a diamond on a non-selected clip retimed the wrong tween. It now resolves against the clicked element's animations and commits through that element's selection, matching the delete path. The three diamond callbacks also take the TimelineKeyframeTarget they already had instead of five positional fields, and the two copies of the sourceFile#domId split share splitTimelineElementKey.
0b9eff8 to
cf1cc8c
Compare
89eeddb to
783fa4a
Compare
What
Consolidates timeline keyframe editing callbacks behind one mutation boundary.
Why
Selection, add/remove, retime, and easing actions previously risked resolving element state through different paths, which can apply edits to stale or incorrect targets.
How
Moves editor mutations into one callback owner and passes explicit element/keyframe identity through callers. The cohesive 1,287-line delta is documented as an R9 exception because splitting it would create temporary dual mutation authority. This is B6 of the Family B draft Graphite stack.
Test plan
Exact Family B tip validation: 2,865 Studio tests, 67 Studio Server route tests, both package typechecks, formatting, lint, diff check, file-size gate, and Fallow audit passed.
Deferred review findings
Every blocker and high finding raised on this PR is fixed in the stack. The 4 remaining low/nit findings are parked, verbatim, in
.scratch/studio-timeline-family-b/issues/06-pr-2688-deferred-review-findings.md:packages/studio/src/components/nle/useTimelineEditCallbacks.ts:150— resolveKeyframeTarget fallback still uses domEditSelection?.id for keyframe-cache lookuppackages/studio/src/components/editor/GsapAnimationSection.tsx:30— withTrackedGsapAnimationCallbacks recomputed every render — defeats AnimationCard memoizationpackages/studio/src/components/nle/useTimelineEditCallbacks.ts:123— resolveElementAnimations hardcodes 'index.html' twice as default composition pathpackages/studio/src/components/editor/AnimationCard.tsx:60— onFocusSegmentConsumed inline-arrow deps make scroll effect fire every renderSupersedes #2688, which was closed when
mainwas rewritten to unwind an early landing of this stack. Same head commit, same review history.R1 review follow-ups
Fixed in this PR:
PropertyPanelFlatopens the Motion group by adjusting state during render instead of in an effect, so the card mounts on the same commit the focus request lands on.AnimationCard's focus effect stops re-running every parent render.The remaining low finding (
withTrackedGsapAnimationCallbacksdefeating theAnimationCardmemo boundary) is parked in.scratch/studio-timeline-family-b/issues/09-family-b-v2-r1-deferred.md. A plainuseMemodoes not close it:callbacksis a rest-spread object with a fresh identity each render, so the fix has to memoize at the source of each callback.R2 review follow-ups
Fixed in this PR:
useTimelineEditCallbacks.test.tsx.Verified already closed, no change needed:
handleGsapAddKeyframeBatchnull-selection guard:onTogglePropertyGroupKeyframereturns early whenbuildDomSelectionForTimelineElementresolves null.Promise<boolean>retime chain:moveKeyframeawaits the raw (non-swallowing) commit and returns itschangedflag,handleGsapMoveKeyframereturns that promise, and the diamond reverts its optimistic position on both afalseresult and a rejection.R2 review follow-ups
Fixed in this PR:
TimelineKeyframeTargetthey already had instead of five positional fields, and the two copies of thesourceFile#domIdsplit sharesplitTimelineElementKey.Deliberate, stated for the record:
GsapAddAnimationControl.tsxis new as a file, not as behaviour: the add-animation control moved out ofGsapAnimationSection.tsx, which shrinks by roughly the same amount in the same commit.