Skip to content

fix(studio): resizing a cropped element keeps the same part of it in view - #4743

Merged
miguel-heygen merged 4 commits into
mainfrom
fix/studio-resize-scales-crop
Sep 30, 2026
Merged

miguel-heygen merged 4 commits into
mainfrom
fix/studio-resize-scales-crop

Conversation

@miguel-heygen

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

Copy link
Copy Markdown
Collaborator

What

Resizing a cropped element now scales its crop with it, so the same part of the element stays in view, just larger or smaller. Before, the crop kept its pixel insets, so growing a cropped element showed more of it than was cropped (a 300 px wide element cropped to 81% showed 88% after being resized to 473 px).

This holds for every size write:

  • Resize commit (handleGsapAwareBoxSizeCommit, with its CSS fallback): a corner drag, the design panel's W/H fields on an element with no animation, the agent resize tool, and a host's geometry commit.
  • Animated property commit (commitAnimatedProperties): the W/H fields on an element that has any GSAP animation, and any other width/height edit made through the animated property path.

Both routes use the same pair, prepareCropResize then saveCropResize, from one module.

Why

A crop is clip-path: inset(...) in pixels, measured against the element's box. Nothing on the resize path touched it, so every resize silently changed what the crop showed. Design tools treat a crop as part of the element: resizing it scales the crop too.

Related work

Rebased onto #4745 and #4730. The crop release keeps #4745's rule that a drag ending where it started saves nothing (it compares against the press-time insets). This PR adds two things to it: the press-time insets are read from the element when you press, and the radius rides on the gesture.

How

  • One owner for the crop: the element's own clip-path. While an element is selected, the crop UI shows it uncropped so it can dim what is cut away. That used to be done by writing clip-path: none onto the element and keeping the real crop in component refs. Undo, redo and panel edits rewrite the element's style in place, so that held copy went stale.
    • The lift is now a clip-path: none !important rule keyed on the element's data-hf-id (or id), added while it is selected. The element's inline clip-path stays the one record of the crop, and everything reads it.
    • Deselect just removes the rule; there is nothing to restore.
    • The selection box and hover ring still show the full box while the element is lifted, as before.
  • The resize commit rescales that crop. It scales per axis (left/right by the width ratio, top/bottom by the height ratio) and keeps the radius. It writes the result onto the element and saves it in the same undo step as the size:
    • CSS route: the clip-path patch rides in the same patch commit as the size.
    • GSAP route: the patch is saved after the script writes land, under the resize transaction's undo key, the same way the anchor move already is. Only then is the new size live for every caller, including the W/H fields, which apply no live draft.
    • Animated property route: the property writes are keyed to one undo step, and the crop patch is saved into that step once they land.
  • The ratio's starting box is the element's box before Studio's resize draft first changed it, recorded by the draft writer (used by the corner drag and the agent tool). With no draft (the W/H fields), it is the box when the commit starts. The end box is measured, not requested, so a resize saved as a GSAP scale leaves the crop alone; it already scales with the transform.
  • During a drag, the dimmed crop area follows the box only while a resize draft is live. So an element whose width is animated keeps its crop pixels when the playhead moves.
  • A crop drag reads the element when you press, not the last render, so a drag right after a resize starts from the scaled crop.
  • The crop is left alone when a GSAP tween drives clip-path (a percent crop already scales with the box), or when the crop was edited while the resize was still saving; that edit was drawn in the new box already. An inline !important crop keeps its priority.
  • Per axis, an animated size keeps its crop. When GSAP tweens or keyframes the width, the left and right insets stay as authored, because one inline crop cannot match every time of an animated size. The same goes for height with top and bottom. The other axis still scales. An instant hold (set) does not count as animated. The drag preview and the save ask the same question, through the existing live-tween check (hasNonHoldTweenForElement), which now also takes an element and understands the keyframes: { width: [...] } form. The answer is taken before the write lands, because a W edit can add a width keyframe of its own.
  • The live-tween check reads every timeline. It used to read only the first one registered. A soft reload re-adds the rebuilt composition's timeline last, so after any edit that soft-reloaded the main composition, a tween on main was missed. That affected the crop, and also the existing resize, drag and rotation preflights that share the check.

Known limits

  • If the crop's save fails after the size saved, the file keeps the new size with the old crop, while the preview puts back the old size and crop. This is the same order the anchor move already has.
  • A sub-composition mounted twice: selecting one copy also shows the other copy uncropped while selected. The rule is keyed on the element's id, which the copies share. It is only visual, and nothing wrong is saved.
  • A crop on an element whose size is animated is left as authored on that axis (a tweened or keyframed width keeps its left and right insets, a height its top and bottom), for the corner drag and the W/H fields alike.
  • Follow-up: undo on four write routes has no test of its own yet (keyframe at the playhead, the set-props merge, the whole-tween offset and add-with-keyframes). Their code shares the keyed write that is tested, and add-with-keyframes was walked: one undo restores the file.
  • An element whose inline clip-path is !important is not shown uncropped while selected, because an inline important declaration beats the lift rule. Its crop still scales.

Test plan

  • Unit tests added/updated. Each of these fails without its fix:
    • Undo while selected, then a crop drag: the drag starts from the undone crop.
    • W/H-style resize (size lands only when saved): the crop scales, under the resize's undo key.
    • A caller that applied a draft first (agent tool): the crop scales from the pre-draft box.
    • Deselect before the save lands: the preview shows the scaled crop.
    • A W field on an animated element: the crop scales, in the same undo step as the size.
    • A crop drag right after a resize, with no render in between: it starts from the scaled crop.
    • Per axis, while dragging and once saved: a height tween still lets a W edit scale the left/right insets; a width tween (percent or { width: [...] } keyframes) keeps them; an instant set still scales.
    • The decision is made before the write lands, so a W edit that adds its own width tween still scales the crop.
    • The live-tween check finds a tween in a later timeline, as after a soft reload (the check itself, and the crop test with its tween in the second timeline).
    • The W field's real route (the hook's static set, alone and batched with another property): the size write and the crop patch share one undo key.
    • Also each of these: the lift rule (it hides the crop over the inline clip, and deselect removes it), a stylesheet crop read while lifted, the hover hug ignoring a lifted crop, the crop UI drawing the element's current crop, a failed save putting the crop back, the stage ending the draft, the kept !important, a crop edited during the save, and a tweened crop.
    • Also covered: the CSS route commit, a scale resize leaving the crop alone, and the lift never rewriting the element's clip.
  • Manual testing performed: headless Chrome walks on the fixture (below).
  • Documentation updated (if applicable)
  • Comments follow CONTRIBUTING.md "Comments"

Walks: headless Chrome, edit fixture, #medium 300x200. "Share" is the visible width fraction.

walk main this branch
Crop right edge, then corner resize to 473x316 (mid-drag / after release / deselected) 0.8779 / 0.8779 / 0.8779, inset(0 57.75px 0 0) 0.8075 / 0.8075 / 0.8075, inset(0 91.05px 0 0)
One undo after that file back to the cropped 300x200 box same; preview back to 300x200 at 57.75px
Undo, redo, undo while still selected, then deselect, 10px crop drag, reload not walked crop UI 0.8075 at every step, element stays uncropped while selected; deselect shows 57.75px; drag saves inset(0 86.63px 0 0); same after reload
W field 300 -> 600, no animation not walked on main inset(0 115.5px 0 0), share 0.8075, in the file and the preview
W field 300 -> 600 on an element with an opacity tween crop stays 57.75px, share 0.9038 (at the previous head) inset(0 115.5px 0 0), share 0.8075; one undo puts back both the width and 57.75px
H field 200 -> 400 on the same element, bottom crop crop stays 57.75px, vertical share 0.8556 (at the previous head) inset(0 0 115.5px 0), vertical share 0.7113
Inline clip-path: … !important, corner resize not walked saved as inset(0px 94.6px 0px 0px) !important
px clip-path tween, corner resize not walked authored crop left as it was
Resize, Escape in the same tick as release (#cropped 400x300) not walked preview and file both inset(28.33px 42.45px 56.67px 70.75px) at 566x425, share 0.8
W field 450 -> 900 at t=2 on an element with a keyframed width 300 -> 600 and a 60 px crop crop doubled to 120 px for every time, so after reload t=0 shows 0.6 instead of 0.8 (at the previous head) crop left at 60 px; after reload t=0 0.8 and t=4 0.9 as before, t=2 now 900 px wide at 0.9333
W field 300 -> 600 at t=2 on an element whose height is tweened, 60 px right crop crop stays 60 px, share 0.9 at every time after reload (at the previous head) inset(0 120px 0 0), share 0.8 at t=0, 2 and 4 after reload
W field -> 600 at t=2 on an element with keyframes: { width: [300, 600] }, 60 px crop crop rescaled to 80 px for every time, after reload t=0 shows 0.7333 (at the previous head) crop stays 60 px; after reload t=0 0.8
Corner drag at t=2 on the keyframed-width element dimmed crop 0.8667 while dragging, 0.9276 after release (at the previous head) 0.9276 while dragging and after release
W on a keyframed-opacity element (soft-reloads main, moving its timeline last), then W 450 -> 900 at t=2 on the keyframed-width element with a 60 px crop crop doubled to 120 px, after reload t=0 shows 0.6 (at the previous head) crop stays 60 px; after reload t=0 0.8, t=4 0.9
Resize saved as scale (element with a scale tween) not walked box stays 300 px, crop left alone, share 0.8161 before and after

Before

On main, the right edge of the red box is cropped, then a corner resize makes it larger. Mid-drag, the dimmed cropped strip stays thin while the box grows:

Before: mid-drag

After deselecting, the crop kept its old pixel width, so more of the box shows than before the resize:

Before: deselected

After

With the fix, the dimmed strip grows with the box during the drag:

After: mid-drag

After deselecting, the crop scaled with the box, so the same part of it stays in view:

After: deselected

…view

Rebased onto main as one commit (was a0a86cc..080dfec). Merged with the crop
release rule from main: a drag that ends where it started saves nothing.
@miguel-heygen
miguel-heygen force-pushed the fix/studio-resize-scales-crop branch from 080dfec to 48ee967 Compare September 30, 2026 12:45

@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.

Approving at 4175ab48. I read the diff against the full files. I did not run the tests.

What checks out

  • One source of truth for the crop. liftElementCrop (domEditOverlayCrop.ts:47) hides the crop with a clip-path:none!important rule keyed on the element. The element's inline clip-path is no longer touched while it is selected. That removes the whole class of stale-ref bugs the old preLiftInlineClipRef / committedClipRef pair had with undo and redo. Deselect just removes the rule (DomEditCropHandles.tsx:140).
  • readElementClipPath while lifted takes the rule out, reads the stylesheet crop once, and puts the rule back where it was (domEditOverlayCrop.ts:76-79). Both the hover hug and useCropOverlay skip a lifted element, so the selection box keeps the full box as before.
  • Every size write route stages the crop.
    • CSS route: useDomGeometryCommits.ts:107-128. The patch goes into the same commit as the size, and crop?.revert() runs on failure.
    • GSAP route: useGsapAwareEditing.ts:358,388,432-437. The crop is saved only after the script writes land, under the resize's coalesceKey. The DOM fallback leaves cropUndoKey null, so the crop is not scaled twice.
    • Animated property route: writeSizeWithCrop (cropResize.ts:125-142) keys both the single and the batched mutation to one undo step. commitAnimatedProperty was already a one-prop wrapper around commitAnimatedProperties (useAnimatedPropertyCommit.ts:589), so routing it through the wrapped version changes nothing else.
  • prepareCropResize measures the end box instead of trusting the request (cropResize.ts:71), and returns null when the inset string is unchanged. So a resize saved as a scale really does leave the crop alone, as the PR body says. A crop edited mid-save (untouched) and a GSAP-driven clipPath are skipped too.
  • The draft base is cleared on every exit: stage(), restoreStudioBoxSize and clearStudioBoxSize (manualEditsSnapshot.ts:118,295). So a failed or cancelled resize does not leave a stale base for the next one.
  • The hasNonHoldTween change is a real fix for the existing preflights too. Before, it read only the first registered timeline, so after a soft reload re-added main last, main's tweens were missed. The resize, position and rotation preflights (gsapResizePreflight.ts:46, gsapRuntimeBridge.ts:190,312,397) all benefit. The { width: [...] } keyframe form is matched by key, and the object form's keys are percentages, so there is no false positive there. gsapRuntimeKeyframes.test.ts:170 pins the later-timeline case.

Nits (not blocking)

  1. Toast and telemetry say "position" for a failed crop save. The crop patch goes through commitPositionPatchToHtml, so a failed crop save toasts "Failed to save position" and records mutationType: "position" (useDomEditPositionPatchCommit.ts:44,50). This is the same path the anchor move uses, so it is consistent, but the wording is slightly off for a crop.
  2. Cost of readCropFollowingResize while dragging. It runs on every render of DomEditCropHandles, and while a resize draft is live cropFollows walks every timeline's children twice (cropResize.ts:26-27). It is fine at normal sizes. If a large composition ever shows resize-drag jank, this is the first thing to memoise.

CI

The one red job was attempt 1 of run 36727661720, "Studio: timeline viewport gate". The default arm passed 5/5 at about 33 ms. The virtualization-off control arm (1000 mounted rows) passed 3 of 5 measured runs, and it needs 4. The two misses were interaction p95 80.8 ms and 82.7 ms against the 75 ms budget. The rerun (attempt 2) passed the off arm 5/5 at about 47-49 ms, and the job is green at this head. The "Test" failure in that run was the studio shard being cancelled, not a test failure. Nothing in this diff touches the timeline rows, so this reads as runner timing noise on the unvirtualized arm.

Verdict: APPROVE
Reasoning: Every size write route now scales the crop in the same undo step and puts it back on failure, and moving the crop's source of truth onto the element's own clip-path removes the stale-crop bugs instead of patching them. The only remaining points are wording and performance nits.

— Rames Jusso

@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.

Re-approving at f75c3f19. My earlier approval was at 4175ab48.

This head is only a merge of main, with parents 4175ab48 and 5b79038c. I checked that it adds nothing else:

  • 5b79038c is on main.
  • git diff 5b79038c f75c3f19 has the same stable patch-id (c43a95b6) as the patch I reviewed (git diff ae8d1780 4175ab48). Both show 23 files, +838/-209. So the merge resolved nothing by hand and the PR's own change is byte-identical.
  • The commits it brings in from main (#4784, #4770, #4769, #4760, #4782, #4759, #4775, #4756, #4768) touch none of the PR's 23 files. They add no calls to what this PR changes: hasNonHoldTweenForElement, the crop readers, commitAnimatedProperties, applyStudioBoxSizeDraft, handleDomBoxSizeCommit, commitPositionPatchToHtml, or any clip-path writes.

My earlier findings and the two non-blocking nits still apply as written.

Verdict: APPROVE
Reasoning: The new head is a clean merge of main with an identical PR patch, and nothing on main newly interacts with the code this PR changes.

— Rames Jusso

@miguel-heygen
miguel-heygen added this pull request to the merge queue Sep 30, 2026
Merged via the queue into main with commit 76c634d Sep 30, 2026
56 checks passed
@miguel-heygen
miguel-heygen deleted the fix/studio-resize-scales-crop branch September 30, 2026 18:56
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