fix(studio): resize an element without GSAP by its own width, height and translate - #4805
Conversation
9ee9849 to
7ba40b3
Compare
2a84c10 to
2ea7bab
Compare
7ba40b3 to
e504f16
Compare
e504f16 to
979e4fc
Compare
Edit accuracy: 530 passing here, 494 on the base branchThe gate passes. Newly passing (36)
Unstable (1)
|
somanshreddy
left a comment
somanshreddy
left a comment
There was a problem hiding this comment.
Review at 39ac0ebf: no blockers; one cheap should-fix, not stamping yet
Verdict: the core fix holds up. A box GSAP doesn't own now goes to the CSS writer before any animation read. The anchor writes a plain px translate, and size, crop and translate land in one commit. I found one gap in the ownership predicate that I'd close in this PR (it's one line). Nothing blocking.
| # | Finding | Severity | Source |
|---|---|---|---|
| 1 | gsapWritesBox checks fewer channels than the CSS writer overwrites |
should-fix (medium) | Codex, confirmed at source |
| 2 | Panel W/H and agent resize also switch to the CSS route (not mentioned or benched) | non-blocking (blast radius) | mine |
| 3 | No host-level test covers a GSAP-free commitBoxSize |
non-blocking (test gap) | mine |
| 4 | Resize freezes a % translate to px, so a later panel size edit no longer re-centres |
question | mine |
| 5 | Routing depends on _gsap.renderTransform existing |
question, unverified | mine |
| 6 | A 3-value translate loses its z on a committed resize |
nit (low) | Codex, confirmed at source |
1. Should-fix: gsapWritesBox is narrower than what the CSS writer writes
gsapRuntimeKeyframes.ts:444:
return gsapWritesPosition(el) || gsapWritesChannels(el, ["width", "height"]);applyStudioBoxSizeDimensions (manualEditsDom.ts:424) writes more than width and height. It sets inline min-width: 0px, min-height: 0px, max-width: none, max-height: none, box-sizing, and in a flex parent flex-basis, flex-grow and flex-shrink. BOX_SIZE_STYLE_PROPS (manualEditsDomPatches.ts:121) persists all of these. So if a GSAP tween owns maxWidth, minWidth or flexBasis on the element, the box is classed as CSS-owned. The resize then saves max-width: none into the source, and that becomes the tween's start value. Before this PR, that element could never reach the CSS writer: the GSAP route always ran, and the writer's rejectGsapCssFallback guard rejected anything GSAP-targeted. This PR makes it reachable.
Scope: I grepped registry/, examples/ and skills/ and found no GSAP tween on these channels, so nothing shipped hits this today. It's a hole in the invariant, not a live bug. Fix: derive the channel list from the writer's property set (width, height, minWidth, maxWidth, minHeight, maxHeight, flexBasis, flexGrow, flexShrink). Add a manualOffsetDrag.test.ts row like the existing { width: 300 } one, for example { maxWidth: 300 } → plainTranslate === false.
2. Non-blocking: blast radius beyond the drag
propertyPanelTransformCommit.ts:108 (the panel's W/H inputs) and StudioAgentTools.resizeSelectionFromAgent both call handleDomBoxSizeCommit from context, which is handleGsapAwareBoxSizeCommit (useDomEditSession.ts:538). So for GSAP-free elements, panel size edits and agent resizes also move from the GSAP writer to plain CSS. That's the right direction, but the bench only covers drag cases. One line in the PR body would help.
3. Non-blocking: the host seam is unpinned
useDomGeometryCommit now passes the real handleDomBoxSizeCommit instead of the noDomBoxSizeRoute stub. Both commitBoxSize tests in useDomGeometryCommit.test.tsx (:214, :248) make the element GSAP-owned (the new _gsap.renderTransform stub), so as far as I can tell, putting the stub back would leave that file green. The routing line itself is pinned in useGsapAwareEditing.test.tsx. A host-level "GSAP-free resize saves width/height/translate and no GSAP script" test, mirroring the existing GSAP-free move test, would close this. (I reached this by reading the tests. I couldn't mutation-test it because hook tests don't run on my box; see Verification.)
4. Question: freezing a % translate
Take an element placed with left:50%; top:50%; translate:-50% -50%. The press converts the translate to px, and the drop saves it as px. A later panel width edit passes no offset, so it no longer keeps the element centred, whereas the original % would have. This matches how #4799 handles moves, so it may be intended. Flagging it because the resize does this even though the user never moved the element.
5. Question (unverified): routing depends on GSAP having touched the transform
gsapWritesPosition is true when _gsap.renderTransform exists. As far as I know, GSAP creates that only once CSSPlugin parses the element's transform, typically when a transform tween first renders. If the only GSAP ownership is a scale or rotation tween the playhead hasn't reached yet, a resize may now take the CSS route, where it used to take the GSAP scale route. I didn't check this against the GSAP runtime, so treat it as a question.
6. Nit: 3-value translate
formatTranslatePx writes x and y only, and the press-time %→px hold (domEditOverlayStartGesture.ts:213) means a committed resize also drops the z of a translate: 10px 20px 30px. A tap or cancel is fine, because restore puts back the pre-press snapshot. The existing ponytail comment already notes this for moves, and it's rare in compositions.
Verified at source / by running
- Gate claim. I recomputed with
ratchet.mjs's ownaccurate()against bothbaseline.jsonfiles: 494 → 530 passing. The 36 newly passing cases are allresize-none-*, and no case flipped from passing to failing in the banked file. The PR's bot comment agrees. - CI is green at
39ac0ebf. That includesTest (studio), whose log showsuseGsapAwareEditing.test.tsx(23),useDomGeometryCommits.test.tsx(11) anduseDomGeometryCommit.test.tsx(12) passing, plus all 8 edit-accuracy shards and the gate. - Local tests.
manualOffsetDrag,domEditOverlayStartGestureandanchoredResizeCommitFeedsOffsetpass (38/38). I ran three mutations, and each turned the suite red:gsapOwns→gsapWritesPosition, dropping the press-time%hold, and droppinggesture: "resize". The three hook test files can't run on my box (act is not a function, which also hits untouched files there). For those I rely on the CI log above. - No GSAP read at press. With a plain translate,
startGestureskipsreadGsapRotation, andcreateManualOffsetDragMemberskipsgsap.getProperty. The removedresizeAnchorhad no other readers (grepped). - Rollback. On a failed save, the release
restoreclosure callsrestoreStudioPathOffsetwith the pre-press snapshot, andcaptureStudioPathOffsetincludes the inlinetranslate. So size and translate both return to their pre-gesture values, after the writer's own revert. - Raw writer exposure.
useDomEditCommitsexposes the raw writer, which no longer has its GSAP guard. ButuseDomEditSessionre-exports it only ashandleGsapAwareBoxSizeCommit, so every caller (overlay, panel, agent, host) goes through thegsapWritesBoxgate first.
How this was done: an independent Codex pass on the raw PR plus my own pass, written separately and reconciled at source. Findings 1 and 6 came from Codex and I confirmed both. Codex didn't raise 2–5.
— Somu
…and translate A corner resize of an element GSAP does not size or position now saves plain CSS: its inline width and height, the scaled crop, and the translate that keeps its centre where it was, in one save and one undo step, with no animation read and no preview reload. It used to write a GSAP script into a file that had none, and the box snapped back to its old size on release. The anchor that keeps the centre planted is the same plain px translate a move writes, kept to a thousandth of a pixel, and a percent translate is held as px from the press so a growing box cannot drag it along on the first frame.
…the CSS box writer sets gsapWritesBox now reads its channels from BOX_SIZE_STYLE_PROPS, the writer's own property set, instead of width and height alone. A host-level test pins the GSAP-free resize route through useDomGeometryCommit.
… GSAP route They are GSAP's names for parts of the one CSS scale the box writer sets.
e2f3c41 to
5ae51f1
Compare
baseline.json is the edit-accuracy-gate artifact of the CI run at 5ae51f1, copied byte for byte: 494 to 530 passing, the 36 resize cases without GSAP, none regressed.
2 participants
Follows #4799 (move and nudge by the element's own CSS translate), now on main.
What changes
A corner resize of an element that GSAP neither sizes nor positions now saves plain CSS: the element's inline
widthandheight, the crop scaled with the box, and thetranslatethat keeps its centre where it was. That is one save and one undo step, with no animation read and no preview reload. The live element already shows the result on the frame the pointer is released.Why it was broken
gsap.setfor the position,tl.setfor the size). On release the box snapped back to its old size (or lost its size and moved, for px and % placements). The CSS size writer could not be reached.translatein percent follows the element's size. The first frame of a resize measured the centre while the stylesheet percent was still live, then wrote px over it, so the first frame was off by a quarter or half of the size change.What this does
gsapWritesBox(next togsapWritesPosition): GSAP owns the box when it positions the element or tweens its width or height. The resize anchor member and the resize commit both route on it, the same way fix(studio): move and nudge an element without GSAP by its own CSS translate #4799 routes moves ongsapWritesPosition.translatePatch, the same plain px literal a move writes, kept to a thousandth of a pixel. It still owns the crop rescale (fix(studio): resizing a cropped element keeps the same part of it in view #4743).useDomGeometryCommit) gets the real CSS size writer instead of a stub that rejected every such resize, and reuses that hook's offset stager instead of its own copy.resizeAnchor) was never read, and on a page that loads GSAP the read itself made GSAP bake the CSS translate into its transform, so the box looked GSAP-owned and took the GSAP route.Edit accuracy bench,
^resize-none-(36 cases)Same machine, same main, the bench on main.
Nested undo can still miss under heavy load (an earlier run lost 1 nested cell: the file reverted, the live element did not); that flake sits in the nested undo path shared by move, resize and rotate and is tracked separately. Smoothness is owned by separate work and this PR adds no per-frame work.
Tests that fail without the fix
useGsapAwareEditing.test.tsx: "resizes a box GSAP does not own through the CSS writer, with no GSAP write or fetch" fails with the routing line removed.useDomGeometryCommits.test.tsx: "saves the anchor as the element's own plain px translate, sub-pixel, with the size" fails with the old legacy-offset patch, and "rolls the whole gesture back once when the resize save fails" fails without the writer calling the gesture's restore.manualOffsetDrag.test.ts: "keeps the centre with the element's own plain translate when GSAP does not own the box" fails when the resize anchor keeps the legacy channel.anchoredResizeCommitFeedsOffset.test.ts: "keeps the centre on the first frame when the authored translate is a percent" fails without the press-time px hold.domEditOverlayStartGesture.test.ts: the resize row of "the press never asks GSAP about the element" fails if the press reads GSAP x/y.useGsapAwareEditing.test.tsx: "fails a GSAP-owned resize loudly when there is no GSAP writer, never writing CSS" fails with the old CSS fallback.Not checked
--hf-studio-widthand the original-size attributes) are still written by the size writer, since "Reset layer edits" restores from them. Whether a resize should write onlywidth/heightis left open.Before
A 240x160 box in a GSAP-free bench project, dragged from its corner to about 340x227 and released: the box is back at its old size.
After
The same gesture: the box keeps 340x227 with its centre where it was.