Skip to content

fix(studio): the properties panel no longer hands GSAP an element it does not position - #4830

Merged
miguel-heygen merged 4 commits into
mainfrom
fix/studio-move-route-position-channels
Oct 1, 2026
Merged

miguel-heygen merged 4 commits into
mainfrom
fix/studio-move-route-position-channels

Conversation

@miguel-heygen

@miguel-heygen miguel-heygen commented Oct 1, 2026 •

Copy link
Copy Markdown
Collaborator

What changes

Selecting a layer that GSAP only fades, colours or otherwise animates without moving no longer hands that layer's position to GSAP. Its next drag or nudge then moves it by its own CSS translate, the same way #4799 moves a layer GSAP does not animate at all.

Before, the Properties panel read GSAP's x, y, rotation, scale and depth values for any layer with a tween, on every frame while it was selected. Any GSAP read of a transform value makes GSAP's CSS plugin fold the layer's CSS translate, rotate and scale into GSAP's own transform. From then on GSAP counted as owning the layer's position, so selecting a fading title was enough to send its next move through gsap.set.

How

readGsapRuntimeValuesForPanel (propertyPanelHelpers.ts) still reads every non-transform value it read before, such as opacity and border radius. It reads GSAP's transform values only for a layer GSAP positions (gsapWritesPosition, the same check the move uses). For any other layer the panel falls back to the values it already shows when GSAP has none: the layer's CSS translate for X/Y, Studio's own rotation value (--hf-studio-rotation) for rotation, and identity in the 3D section.

The list of GSAP transform channels is GSAP_TRANSFORM_KEYS in gsapRuntimeKeyframes.ts. It is GSAP 3.15's CSS plugin transform list, including transformOrigin, svgOrigin, force3D and smoothOrigin and the aliases, minus transform itself, which GSAP reads without parsing. GSAP doesn't expose that list at runtime, so it is copied, and a test pins the origin case.

Test

gsapLivePreview.test.ts: "the panel reads GSAP's transform only off an element GSAP positions". For a fade-only tween the panel reads only opacity; for a tween that moves the layer it reads the full transform set, as before. A fade with transformOrigin: "0 0" also reads only opacity. With the new guard removed, the fade case fails because the panel reads the nine transform channels too. With the four origin keys missing from the list, the origin case fails.

Before

Main, a fixture with one box placed by its CSS translate: 40px 30px and a GSAP tween that only fades it (opacity: 0.4). Selecting the box with the Design panel open and dropping a +90/+60 move: the panel shows X 130 / Y 90, and the file gains gsap.set("#target", { x: 130, y: 90 }) in the timeline script, with no change to the box's own translate.

Before: the drop on main, Design panel open

Before: the saved source gains gsap.set on line 19

After

Same fixture and gesture on this branch. The panel shows the same X 130 / Y 90, and the only change to the file is style="translate: 130px 90px" on the box, with no gsap.set. The source view is shown after reloading Studio.

After: the drop on this branch, Design panel open

After: the saved source changes only the box's inline translate

@github-actions

github-actions Bot commented Oct 1, 2026

Copy link
Copy Markdown

Edit accuracy: 494 passing here, 494 on the base branch

The gate passes.
Smoothness is reported in the artifact, not gated. A case fails only if it fails 2 of 3 runs.

@miguel-heygen
miguel-heygen marked this pull request as ready for review October 1, 2026 05:39
@miguel-heygen
miguel-heygen merged commit 4c76818 into main Oct 1, 2026
82 checks passed
@miguel-heygen
miguel-heygen deleted the fix/studio-move-route-position-channels branch October 1, 2026 05:52

@somanshreddy somanshreddy left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict at 0657fe27: no blockers. This is a post-merge review: the PR merged at 05:52 UTC while I was still reviewing, with no GitHub review on it. I reviewed in two independent passes, one by Codex on the raw PR and one of my own, and checked every finding below against the source at this head.

What I verified

  • The panel guard does what it says. For an element that gsapWritesPosition reports as not positioned by GSAP, readGsapRuntimeValuesForPanel skips every key in GSAP_TRANSFORM_KEYS, so no transform getter runs. Move and nudge already route on the same predicate (createManualOffsetDragMember → plainTranslate; elementOffsetStager.ts:71). So a fade-only layer's next drag now writes its CSS translate.
  • GSAP_TRANSFORM_KEYS matches GSAP 3.15.0 exactly. I checked it against CSSPlugin.js: the 19 entries of _transformProps minus transform, plus the 8 aliases (translateX/Y/Z, rotate, rotationZ, rotateZ/X/Y). Leaving out transform is right, because _get skips both alias resolution and _parseTransform for that key.
  • Tests. The full studio suite passes at head (561 files, 6202 tests, NODE_ENV=test). Each of these mutations made the new test fail: dropping the guard, forcing readsTransform to false or to true, and removing the four origin keys.
  • Deleting gsapAnimatesTransform is safe. It had no consumers.
  • CI is green: 76 pass, 7 skipped.

Should-fix / follow-ups (non-blocking)

  1. The Properties panel's X/Y (and rotation) fields still write through GSAP for a fade-only layer. propertyPanelTransformCommit.ts:50 routes through onCommitAnimatedProperty whenever hasGsapAnimation, which is true for any tween (PropertyPanel.tsx:222). Typing X=130 into the field still adds GSAP x to the file, while dragging the same layer now writes its CSS translate. That isn't a regression, since this was already the behaviour on main. But the two edits to the same field now land in different channels. The field should route on gsapWritesPosition like the drag does.
  2. The 494 = 494 gate doesn't exercise this route at all. grid.mjs only has gsap: none | tween | hold, and both tween and hold animate x/y. No case has a non-positioning tween, so the gate can't move either way. Adding a fade value to the gsap axis (an opacity-only tl.to) would make the before/after in the description a banked regression case.
  3. The tests prove the call list, not the folding. getProperty is a mock, so the test pins which keys get read. That's the right unit property, but it doesn't pin the list itself: removing svgOrigin, skewX or rotateZ from GSAP_TRANSFORM_KEYS each leaves the suite green (measured). A test that loads real GSAP in happy-dom could assert the property directly: for every key, getProperty sets _gsap.renderTransform, and for no other panel key does it. That would also catch a future GSAP adding a transform alias.
  4. The origin test case only holds before the tween first renders. A real fade tween with transformOrigin parses the transform at init (CSSPlugin.js:1059, where isTransformRelated is true for transformOrigin), so it sets renderTransform itself. From then on the panel correctly reads all channels. The exclusion still matters before the tween's first render, which is what the test covers in effect. The test name could say so.
  5. Fallback display for authored CSS transforms. For a fade-only layer with an authored rotate: 20deg / scale: 2 (or transform: rotate() scale()), rotation now falls back to --hf-studio-rotation (0 at this head) and scale/depth to identity. This matches how a fully un-animated layer already displays on main. #4802's readShownRotation fixes the rotation half once it lands, but scale/3D would still show identity.

Other routes that still read GSAP at press (outside this PR, noted for the union)

At this head (base = main), two press paths still call GSAP's transform getters on a fade-only layer, and either one alone makes its next move take the GSAP route:

  • Rotate press: domEditOverlayStartGesture.ts:235 calls readGsapRotation unless a plain-translate member exists, and rotate never creates one. #4802 gates this on gsapWritesRotation.
  • Resize press: domEditOverlayStartGesture.ts:180-181 calls readElementGsapNumber(x/y) on every resize. #4805 removes this read.

#4830 and #4802 test-merge cleanly onto main, and so do #4830 and #4805. #4802 and #4805 conflict with each other in useDomGeometryCommit.ts and baseline.json (already flagged on those PRs). I haven't re-verified #4805's resize commit path here.

Pre-existing, in the shared predicate (from #4799, not this PR)

  • matchesElement (gsapRuntimeKeyframes.ts:162) matches tweens by id across every timeline in the window. A root #card that only fades counts as "GSAP-positioned" if a sub-composition's separate #card has an x tween. The panel now follows the move router, so the two stay consistent, but both are wrong in that case.
  • MOVE_CHANNELS includes left/top, which CSSPlugin doesn't parse as transforms. A left-only tween therefore routes moves through GSAP and lets the panel read the transform. That's consistent with the router, but it's broader than "GSAP owns the transform".

— Somu

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants