fix(studio): overlay jump, delete-all, split polish [9/10]#1191
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. |
6e391b2 to
eca150f
Compare
82a5989 to
6c8acf6
Compare
eca150f to
dcbe8da
Compare
790ef3f to
99fc9ee
Compare
3a8e22f to
0eb2b93
Compare
1937dd3 to
c048e70
Compare
0eb2b93 to
626983d
Compare
jrusso1020
left a comment
There was a problem hiding this comment.
Polish + bugfix PR. +175/-40. Five concerns named in the body — let me verify each is correctly scoped:
1. gsapAnimatesProperty guard preventing reapplyBoxSizes overwrite
The mechanism is sound: when a GSAP tween interpolates width/height, the bounding-box reapply path must skip those properties. Two checks:
- Guard granularity: per-property, not per-element. If GSAP animates
widthbut notheight,reapplyBoxSizesshould still setheight. Verify the guard doesn't blanket-skip. - Guard timing: must fire before the reapply, not after. A late guard reverts the GSAP-interpolated value.
2. Delete All Keyframes → handleGsapDeleteAnimation fallback
When "delete all keyframes" runs removeAllKeyframesFromScript from #1167, the function collapses to flat tween with the last keyframe's properties. So "delete all" actually means "convert to flat tween" — not "remove the entire animation".
The fallback to handleGsapDeleteAnimation extends this: if the user truly wants to remove the whole tween, the fallback path catches that intent. Verify:
- Trigger condition: when does
handleGsapDeleteAnimationfire vs the keyframe-collapse path? Worth a comment in the code explaining the discriminator. - Confirmation prompt: "delete all" is destructive at two different scales (drop keyframes vs drop the whole tween). User should know which one they're getting.
3. Wire split through component chain
App → StudioPreviewArea → NLELayout → Timeline. Verify prop-drilling didn't break a single intermediate (a missing prop at any layer is silent — TypeScript catches Required props, but optional ones can no-op).
4. Split restricted to media (video/audio/img) — hidden for divs/compositions
This is the client-side gate from my #1189 comment. Both PRs should be hardening together — confirm splitElementInHtml server-side also gates on the same element-type check.
5. Clip context menu onContextMenu prop
Verify it doesn't double-fire if a parent (Timeline / NLELayout) also has an onContextMenu handler. React events bubble; if the parent has a non-e.stopPropagation() handler, both menus could render.
Architecture is fine — five independent fixes, each bounded. None of them should affect the main keyframe flow. Test coverage would benefit from at least one "split on a tween-targeted div" test (should be hidden, but the negative-case test is good defense).
Review by Jerrai (hyperframes specialist)
c048e70 to
aae066c
Compare
7149806 to
9ee5a4d
Compare
aae066c to
0845d38
Compare
Revert totalTime nudge that caused black first frames in from() tweens. Keep stale CSS offset cleanup. Regenerate baselines for offset cleanup.
…yBoxSizes guard - Fix overlay bounding box jump: reapplyBoxSizes now skips elements whose width/height are animated by GSAP (gsapAnimatesProperty check prevents studio CSS from overwriting GSAP interpolated values) - Delete All Keyframes removes entire animation (handleGsapDeleteAnimation) with fallback to first animation when no keyframed anim exists - Wire split clip through App → StudioPreviewArea → NLELayout → Timeline (onSplitElement prop, toolbar button, S hotkey, clip context menu) - Add onContextMenu to TimelineClip for right-click clip context menu
Sub-compositions (data-composition-src) cannot be meaningfully split — the clone would load the same source and fight for the same timeline. Block with a specific toast message and disable in the context menu.
9ee5a4d to
f7407bd
Compare
0845d38 to
7b29dd3
Compare
1a4e04b
into
feat/keyframes-6-dynamic-keyframes

Summary
gsapAnimatesPropertyguard preventsreapplyBoxSizesfrom overwriting GSAP-interpolated width/heighthandleGsapDeleteAnimationwith fallback)+175 LOC