Skip to content

fix(studio): move and nudge an element without GSAP by its own CSS translate - #4799

Merged
miguel-heygen merged 9 commits into
mainfrom
fix/studio-move-plain-translate
Oct 1, 2026
Merged

miguel-heygen merged 9 commits into
mainfrom
fix/studio-move-plain-translate

Conversation

@miguel-heygen

@miguel-heygen miguel-heygen commented Sep 30, 2026 •

Copy link
Copy Markdown
Collaborator

What changes

When GSAP does not position an element, dragging it or nudging it with the arrow keys now moves it by its own CSS translate. The drag preview, the drop and the saved file all use the same literal: inline translate: 130px 90px in plain px. Nothing else is written: no gsap.set, no timeline, and no GSAP script from the CDN is added to a GSAP-free file. The save goes through the existing inline-style patch, with no preview reload and no read of the file's animations.

Before this change, every move went through the GSAP writer:

  • It added a GSAP script and a gsap.set to a CSS-only composition. The first save took 2-3 s and failed offline.
  • GSAP folded the stylesheet translate into x/y, then the set overwrote them, so the element landed short of the drop by its own translate (50 px on the fixture below).
  • A sub-composition file got the script after </template>, where it never runs, so a nested move or nudge was lost.

How

  • One decision, gsapWritesPosition(el) (gsapRuntimeKeyframes.ts): GSAP positions the element if a live timeline tween or hold writes x/y/xPercent/yPercent/left/top/motionPath on it, or if GSAP already renders its transform. It is synchronous and does no fetch. The press, the drop and the save all ask it. A GSAP-positioned element keeps today's route unchanged.
  • One reader, readTranslatePx (plainTranslate.ts): the element's translate in px as Chrome resolves it. %, calc(), min()/max()/clamp(), var() and em are resolved by Chrome itself, not by parsing.
  • One writer on the element (elementOffsetStager.ts): a GSAP-free move is saved on the element itself, as a shared-tween word already was, and puts the live value back if the save fails. It serves single moves and group members.
  • The drag member keeps its start translate in JS. A drop computes the absolute translate right away, so a second gesture that starts before the first save lands starts where the first dropped and cannot double-count.
  • The press no longer calls gsap.getProperty on these elements, because that makes GSAP bake the CSS translate into its transform.
  • The Properties panel's X/Y show and edit that same translate for these elements.
  • Plain px only, because GSAP's CSSPlugin splits translate on spaces and loses a calc().
  • Accepted cost: a layer centred with translate: -50% -50% is stored in px at its current size once moved.

Before

Main, edit accuracy fixture move-none-px-r0-root-z100 (translate: 40px 30px, no GSAP), after dropping a +90/+60 move. The box lands 50 px short of the drop, and the file gains a CDN GSAP script plus gsap.set("#target", { x: 90, y: 60 }).

Before: the box lands 50 px short of the drop

After

Same fixture and gesture on this branch. The box stays where it was dropped, and the only change to the file is style="translate: 129.998px 90px" on the element. The Lint badge counts the fixture's own two findings (no __timelines registration), which the GSAP bootstrap used to hide.

After: the box stays where it was dropped

Edit accuracy slice ^(move|nudge)-none- (72 cases)

Before: main 299670e, devbox.

Gesture Cases tracking press drop reload undo
move 36 2 36 7 35 35
nudge 36 36 36 6 36 36

After: this branch merged with main, CI edit accuracy gate (8 shards, each case 2 of 3 runs).

Gesture Cases tracking press drop reload render undo
move 36 36 36 36 36 36 36
nudge 36 36 36 36 36 36 36

All 72 pass every metric except smoothness; 59 of 72 pass smoothness too, which its own track scores. Across the whole 660-case grid the gate counts 494 passing (main: 434), 60 newly passing and none regressed. baseline.json banks them byte for byte from that run's artifact.

The press reads a % or calc() translate arithmetically against the border box and writes nothing to the element. An earlier head resolved it through a temporary inline transform, and that showed for the first frame after the press: CI saw a press jump equal to the element's own translate on every percent case.

Tests

  • A GSAP-free move saves one inline translate patch and makes no GSAP request. A failed save puts the live translate back (useDomGeometryCommit.test.tsx).
  • The drag and drop draw plain px and never call GSAP. A second move started before the first save lands starts where the first dropped. A nudge adds to the translate. A GSAP-rendered element and a timeline hold keep the GSAP writer (manualOffsetDrag.test.ts).
  • readTranslatePx resolves px, %, calc(P% ± Lpx) and padding plus border without writing to the element, and hands min()/max()/clamp() to Chrome with the style put back (plainTranslate.test.ts). A drag press on a page that loads GSAP but does not position the element (nothing animated, or only its parent) never asks GSAP and keeps the CSS route. A centred element starts from its -50% translate in px (domEditOverlayStartGesture.test.ts).
  • A single GSAP-free move saves on the element with no animation read. A group saves GSAP-free and shared-tween members on themselves under the group's undo key (useGsapAwareEditing.test.tsx).

Not in this PR

Resize and rotate routing for GSAP-free elements, the nudge's layout read, and deleting the left/top and legacy offset channels come in follow-ups.

@somanshreddy
somanshreddy enabled auto-merge October 1, 2026 01:00
@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 force-pushed the fix/studio-move-plain-translate branch from 2a84c10 to 2ea7bab Compare October 1, 2026 02:26

@jrusso1020 jrusso1020 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Reviewed at 2ea7bab8. Read the full changed files plus the callers of the new gate (drag press, nudge, single and group commit, stager, panel X/Y), and the GSAP 3.15 CSSPlugin code the design depends on.

Verdict: APPROVE. No blockers. The routing decision fails safe in the cases that matter, and the edit accuracy numbers check out. Nits below. None of them needs to block tonight.

Is "un-animated" detection right?

gsapWritesPosition (gsapRuntimeKeyframes.ts:438-440) says yes if el._gsap.renderTransform is set, or if any child of any window.__timelines entry targets the element and writes x/y/xPercent/yPercent/left/top/translateX/translateY/motionPath (:420, :422-433).

  • GSAP loaded but the element is not tweened: no matching tween and no renderTransform, so it takes the CSS route. Covered by the new domEditOverlayStartGesture tests.
  • Tween targets the element through a selector or class, or a stagger: GSAP resolves targets when the tween is created, and matchesElement compares them by identity (:165). So .box, [data-x] and gsap.utils.toArray all match. A shared stagger is still detected, which keeps the shared-tween left/top path intact (elementOffsetStager.ts:71 only branches off when GSAP does not position the element).
  • Ancestor or descendant tweens: these don't match, and they shouldn't. A parent transform composes with the child's own translate, so moving the child by translate is correct. Covered for the parent case.
  • Nested compositions: detection reads el.ownerDocument.defaultView.__timelines, the same source elementHasNonHoldTween already uses. getChildren(true) is deep, so a sub-timeline added to a parent is included. The id fallback in matchesElement can only produce a false "animated" (two instances sharing an authored id), and that falls back to the old path. The *-nested move/nudge cases are among the 60 newly passing.
  • Later in the timeline: getChildren(true) returns every child regardless of the playhead, so a position tween starting at 5s is detected at t=0.

I found no false "un-animated" for a position tween registered on a timeline. See nit 1 for the remaining gaps.

Can a GSAP-animated element get a translate that fights the tween?

  • Position tween anywhere on a registered timeline: no. The press (manualOffsetDrag.ts:342), commit (useGsapAwareEditing.ts:156, :249) and stager (elementOffsetStager.ts:71) all keep the GSAP writer.
  • Opacity-only or other non-transform tweens: these get the CSS route. GSAP never touches transform/translate there, so nothing competes.
  • A scale/rotation tween that has not rendered yet (it starts after the playhead): this gets the CSS route. At that tween's init, CSSPlugin's _parseTransform folds the computed translate into transform and sets translate: none (CSSPlugin.js 859-865), so x/y start at the moved value and the position holds. It is folded once, not fought. One side effect: whether the next move routes CSS or GSAP depends on whether that tween has rendered at the current playhead. Both routes land in the right place.
  • A position tween added after a move: the same fold applies at init. It matches main's semantics, where the move had become gsap.set x/y. The next move routes through GSAP.
  • Existing transform: the writer touches only translate. Individual translate composes ahead of transform, so rotate/scale in transform are not overwritten. The r30 fixtures cover this.
  • Runtime not booted: detection reads the live runtime only, so an empty __timelines reads as un-animated. In practice the commit path refuses before the preview has booted (useDomGeometryCommit.ts:149), and the press needs a rendered overlay.

Press-jump fix

An earlier head resolved %/calc() by writing a temporary inline transform, and that showed for a frame. readTranslatePx (plainTranslate.ts:59-67) now resolves px, %, and Chrome's calc(P% ± Lpx) arithmetically against the border box, which is the reference box for HTML elements. It only falls back to the transform probe for min()/max()/clamp(). That probe is set and restored synchronously, so nothing paints in between. The rotation read was moved after member creation and is skipped for plain moves (domEditOverlayStartGesture.ts:235). That is needed, because getProperty would run _parseTransform and bake the translate. Looks right.

baseline.json and the 494/434 claim

I recomputed this with the gate's own accurate() from ratchet.mjs against main's baseline: 434 → 494 accurate, 0 regressed, 60 newly passing, all move-none-*/nudge-none-*. That matches the body. At this head, Studio: edit accuracy gate is success and the sticky comment reads the same. A second run of the shards was still in progress when I checked.

The composite pass field (which includes smoothness) flips true→false on 58 non-"none" cases. All of those flips are smoothness only: work is null in 88 entries vs 16 before, and no gated metric got worse by more than 0.5px. The ratchet does not gate on smoothness, so this is harmless. It does make the raw pass column read worse than main.

Tests

  • bun install --frozen-lockfile, then built parsers, lint, core and studio-server.
  • vitest run src/components/editor src/hooks: 254 files, 2700 tests, all pass.
  • tsc --noEmit -p packages/studio: clean.
  • Mutations on gsapWritesPosition:
    • Dropping the timeline channel scan fails "keeps the GSAP writer for an element a timeline hold positions".
    • Dropping the renderTransform check fails 10 tests.
    • Dropping motionPath from MOVE_CHANNELS passes everything. So does dropping the keyframeVarsCarryChannel branch (nit 2).

Nits

  1. Detection gaps that land in the dangerous direction (all edge cases):

    • A tween outside window.__timelines that hasn't rendered yet (a standalone gsap.to(el, {x, delay})).
    • A tween written as transform: "translateX(..)" or translate: "...", which aren't in MOVE_CHANNELS.
    • A not-yet-rendered transform tween with clearProps: "transform"|"all". CSSPlugin.js:598 sets translate: none at completion, which would wipe a saved move.

    Adding "transform" and "translate" to MOVE_CHANNELS (gsapRuntimeKeyframes.ts:420) is cheap, and it only fails toward the old path.

  2. No test pins the keyframeVarsCarryChannel or motionPath branches of gsapWritesChannels. A keyframes: { x: [...] } tween starting after the playhead is the case that would quietly get a plain translate if either branch regressed. Worth one test each.

  3. The route is decided three times, independently: at press (manualOffsetDrag.ts:342), at commit (useGsapAwareEditing.ts:156) and in the stager (elementOffsetStager.ts:71). In the plain route, next is absolute translate px. In the GSAP route it is a legacy offset. I could not find a way for the answers to diverge mid-gesture, because renderTransform is sticky and the panel's getProperty reads run at selection, before the press. Still, carrying member.plainTranslate through to the commit would make that true by construction rather than by sequencing.

  4. The group commit returns early when gsapCommitMutation is absent (useGsapAwareEditing.ts:194), before the plain-member check. The single-move path checks plain first (:156). If gsapCommitMutation can ever be absent, a group of GSAP-free members would silently not save while a single move would. Moving the plain partition above that guard would make the two paths consistent.

  5. readTranslatePx prefers the inline value over the computed one (plainTranslate.ts:61). A stylesheet translate: ... !important would make both the read and the write ineffective. That's rare, so a comment is enough.

— Rames

@miguel-heygen
miguel-heygen merged commit c8f848f into main Oct 1, 2026
118 checks passed
@miguel-heygen
miguel-heygen deleted the fix/studio-move-plain-translate branch October 1, 2026 03:09
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.

3 participants