Skip to content

fix(studio): undo takes back the last edit, even right after a resize or a nudge - #5048

Merged
miguel-heygen merged 14 commits into
mainfrom
fix/studio-undo-order-pending-saves
Oct 5, 2026
Merged

miguel-heygen merged 14 commits into
mainfrom
fix/studio-undo-order-pending-saves

Conversation

@miguel-heygen

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

Copy link
Copy Markdown
Collaborator

What

Three cases where undo took back the wrong thing, or showed it wrong, are fixed. Each Cmd+Z now takes back exactly the last edit you made.

  1. Resize, then type a W. Resize an element on the canvas, then type a new W in the Design panel right after. One Cmd+Z now takes back only the W edit. Before, it took back the resize too, and every later undo landed one step early.
  2. Nudge, then edit in the panel. Nudge an element with the arrow keys, then edit a Design-panel field before the nudge has saved. The panel edit is now recorded after the nudge, so the first Cmd+Z takes back the panel edit. Before, the panel edit was recorded first, and the first Cmd+Z took back the nudge (36 px in the bench), showing a state that never existed on screen. This now holds on GSAP-animated elements too.
  3. Undo a W on an animated element. On a GSAP-animated box, W 300 then Cmd+Z now puts the box and the Design panel back at the old width. Before, the file went back but the preview stayed at 300.

Why

Resize, then W. For an element without an animation, a canvas resize and a W or H edit in the Design panel save through the same commit (handleDomBoxSizeCommit). Both recorded their history entry under one undo key per element, box-size:<element>, with the default 300 ms merge window. A size edit that reached history within 300 ms of the resize was added to the resize's entry.

Nudge, then panel edit. A burst of arrow-key nudges saves once, after a short pause (CANVAS_NUDGE_COMMIT_DEBOUNCE_MS). A pointer gesture and undo both made a still-waiting burst save first. A Design-panel edit did not, so its save, and its history entry, landed ahead of the nudge's.

How

Resize, then W. Each box-size commit gets its own undo key with no time limit, the way a plain rotate already does (plainRotation.ts, #4802). A commit's own writes stay one step, and a later edit never merges into it. Gestures that pass their own key (GSAP resizes, crop resizes) are unchanged.

Nudge, then panel edit. The pending-edits registry (studioPendingEdits.ts) is where every edit starts: trackedStudioEdit wraps every Design-panel action, and beginStudioPendingEdit starts every canvas gesture. Starting an edit there now raises the existing flush event that undo already raises. A debounced edit, such as a nudge burst or a color-grading save, therefore commits before the new edit begins, in the order the user made them.

Starting the flushed save first is not enough on an animated element: its save reads and rewrites the GSAP script asynchronously, and a panel edit started right after could still write first. So a Design-panel action or a canvas gesture that starts while a flushed save is still writing waits for that save, then runs. Two such edits keep the order they were made in. Studio's own internal saves (the GSAP script commit, timeline edits) never wait, so a save never waits on itself, including a save made from inside a waiting edit or a flushed one. An edit that runs inside a canvas gesture's own commit, such as a second nudge burst's save, already waited when the gesture started and does not wait again.

The wait. It only happens in the window between a nudge burst's last key and its save landing, and only for an edit made in that window. Measured from W + Enter to the new width on screen, median of 3 runs per row:

Case main this branch
GSAP-animated element, no nudge before the edit 159.6 ms
GSAP-animated element, right after a nudge burst 184.9 ms, wrong undo order 149.2 ms
Plain element, right after a nudge burst wrong undo order 68.8 ms

Undoing a gsap.set on an animated element. On a GSAP-animated box, W 300 writes gsap.set("#target", { width: 300 }). Cmd+Z took it out of the file, but the preview and the Design panel stayed at 300. The panel applies W to the live element and writes the set only to the file, so the preview's own script never contains it. Undo re-runs the restored script in place, but that reload only reset what the preview's own script had written, so the inline width: 300px stayed. The same was true for any later edit that removes the set, such as deleting the element's animations.

A live patch of a set value goes through one place, applyGlobalSet. It now records the element and the property names it applied, with no parsing. The next in-place reload of that element's composition, from undo or any other edit, resets exactly those properties to their authored value. The re-run script then sets again whatever it still sets, in the same synchronous step, and the record is dropped. The record covers only what a live patch applied. A set that the preview's own script ran is read from that script instead: after a page load, such as undoing a W from an earlier session, or after an in-place reload re-ran a script that still had the set. That is at most one parse per reload, and only when the script contains gsap.set(. gsapSoftReload.test.ts "clears what a standalone gsap.set wrote once the new script no longer sets it" is the test that fails without it.

Before

Main. Each video ends on the state after one Cmd+Z.

  • Nudge, then W, on a GSAP-animated box (f01ae78): the undo takes back the nudge instead of the W, and the box stays 300 wide.
gsap-nudge-main.mp4
  • W 300, then Cmd+Z, on a GSAP-animated box (f01ae78): the file goes back, but the box and the W field stay at 300.
before-gsapset-main.mp4
  • Nudge, then W, on a plain box (aa7f228): the undo takes back the nudge.
before-nudge-main.mp4
  • Resize, then W (aa7f228): the undo takes back both, back to 240×160.
before-resize-main.mp4

After

This branch, the same steps.

  • Nudge, then W, on a GSAP-animated box: only the W is undone. The nudge stays, and the box is back to 240 wide in GSAP, the W field and on screen.
gsap-nudge-branch.mp4
  • W 300, then Cmd+Z, on a GSAP-animated box: the box and the W field read 240 again, with no preview reload. Redo brings back 300.
after-gsapset-branch.mp4
  • Nudge, then W, on a plain box: only the W is undone, and the nudge stays.
after-nudge-branch.mp4
  • Resize, then W: only the W is undone, and the resize stays at 480×320.
after-resize-branch.mp4

Test plan

  • Red first, nudge then panel edit.
    • In useDomEditNudge.test.tsx: an arrow-key burst still inside its pause, then a Design-panel edit. The nudge commits first.
    • In studioPendingEdits.test.ts: a debounced edit commits before a panel edit and before a canvas gesture begins.
    • Both fail on main and pass with the change. Removing the flush fails both.
  • Red first, the wait.
    • In studioPendingEdits.test.ts, a panel edit starts only after a flushed save that was still writing has written. Later edits keep their order behind it.
    • In useDomEditNudge.test.tsx, a Design-panel edit made during a burst lands after the nudge.
    • Making edits never wait fails the ordering tests.
  • Never waits on itself. One test covers a waiting panel edit, a canvas gesture and a flushed save that each call one of Studio's internal saves. All of them finish and nothing is left saving. Letting internal saves wait too makes that test hang.
  • Red first, gsap.set undo. In gsapUndoRestore.test.ts, undoing a W whose set the preview never ran clears the width and keeps the rest of the element's style. It fails at 300px without the change. gsapSoftReload.test.ts covers the same reset when the preview's script did run the set. In gsapSoftReload.test.ts, W 300 then H 200 applied live, then a reload whose script sets only the width, clears the height and keeps the rest; a later reload leaves a height the new script set. Removing the record, the reset or the forget each fails it.
  • Red first, a second nudge burst. In studioPendingEdits.test.ts, a burst flushed while the first burst is still saving saves after it, and later panel edits land in order. It hangs without the change.
  • 3 runs. The pending-edits, nudge and soft-reload files pass three runs in a row.
  • Studio's unit suite. 7,566 tests pass with the change.
  • Red first, resize then W. In useDomGeometryCommits.test.tsx, a resize and the next size edit of the same element record under different undo keys. The test fails on main with the shared key box-size:index.html||box|#box| and passes with the change.
  • Mutation. Putting the per-element key back fails the test.
  • 3 runs. The file passes 18/18 three runs in a row.
  • End to end. The edit-accuracy bench's 20-step undo chain (seqdeep-none-px-r0-root-z100, from the parked undo-chain bench work, which a W edit right after a resize fails), on a Linux dev machine with Chrome for Testing:
    • main: 2 of 2 runs fail undo and reload. One undo and one redo leave different bytes, and one undo write never lands.
    • this branch: 3 of 3 runs have no undo or redo with different bytes and no lost write. Only the ungated smoothness check fails, from machine load.

Trade-offs

  • The wait above. On an animated element the repaint after W takes about 150 ms with or without a nudge burst before it, so the wait was not measurable there. On a plain element, an edit right after a burst repaints in about 69 ms. The other order would record the edits in the wrong order.
  • Selection is read when a waiting edit runs. A group, ungroup or set-all-eases that waits behind a nudge save acts on the selection as it is when it runs, not when it was clicked. The window is the one in the table.
  • What a set reset does not reach. An element the preview cannot find in its file keeps the old behaviour. A gsap.set selector that also matches an element of another inlined composition resets that element too, and only its own script's next run sets it again.
  • Legacy W/H scrubbing. In the legacy Design panel (off by default), each scrub step is its own undo step, now that a size edit never merges into an earlier one.

Also: Studio's published types name cn by its package

Carried here from a separate small fix, so it lands with this PR.

  • The bug. Main's @hyperframes/studio types import cn's type from 'node_modules/cn/dist/types', which does not resolve for a consumer. On some runners the type build refused it (TS2742 on cn.ts), so Studio's build failed intermittently in the Windows render check.
  • The fixes.
    • cn is annotated with CnFunction from the cn package's public entry. This fixes the Windows failure.
    • Studio's tsconfig drops "baseUrl": ".", whose only use was a path alias that resolves without it. This stops such a path shipping silently on Linux.
  • Checked. The declaration output is byte-identical with and without baseUrl once cn is annotated, and Studio's typecheck and build pass.

@github-actions

github-actions Bot commented Oct 5, 2026 •

Copy link
Copy Markdown

Edit accuracy: accurate 2040 (base branch 2040), smooth 1548 of those

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

Quarantined, measured but not gated (0)

@miguel-heygen miguel-heygen changed the title fix(studio): a size edit right after a resize is its own undo step fix(studio): undo takes back the last edit, even right after a resize or a nudge Oct 5, 2026
@miguel-heygen
miguel-heygen force-pushed the fix/studio-undo-order-pending-saves branch from a253a4a to 38beaf0 Compare October 5, 2026 09:13
@miguel-heygen
miguel-heygen force-pushed the fix/studio-undo-order-pending-saves branch from 2431f74 to 191e331 Compare October 5, 2026 15:06
@miguel-heygen
miguel-heygen marked this pull request as ready for review October 5, 2026 15:58

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

No blocker at 191e331c34716a390d6d53d2ec842bcdb32efd28. Approving; the CI jobs were all green when I stamped (66 pass, 13 skipping).

What I checked (local, NODE_ENV=test)

  • Changed files, three runs each, all green: studioPendingEdits.test 28, useDomEditNudge.test 15, useDomGeometryCommits.test 18, gsapSoftReload.test 55, gsapUndoRestore.test 40. Neighbours (one file at a time) pass: useEditHistoryActions 13 + paint 10, useTrackPendingTimelineEdit 3, useGsapScriptCommits 37, DomEditContext 2, dragUndoPaint 7, gsapRuntimePatch 20, gsapResizeElementSize 3, groupPlan 6, useTimelineDeleteOps 10, timelineClipDragCommit 60, usePersistentEditHistory 24, usePreviewPersistence.history 7, useRazorSplit.history 5. tsc --noEmit in packages/studio is clean.
  • Mutations (16, each reverted), 12 caught:
    • Caught: per-element box-size key back; no flush in trackedStudioEdit; no flush in beginStudioPendingEdit; panel edits never wait; gesture adopt never waits; internal saves allowed to wait (drops the adopting guard, hangs/fails 10 tests); flush while adopting; never clearing the flushed-saves chain; no recordLiveSet; no live-set targets; no standalone gsap.set targets; no forgetLiveSets.
    • Survived: see non-blocking 1 to 3.
  • Call-site census: trackedStudioEdit( has five production callers (DomEditContext, App.tsx z-reorder/delete, useGsapScriptCommits x2, useTrackPendingTimelineEdit); only DomEditContext passes afterOlderSaves, so Studio's internal commit paths still never wait. Edits that run inside a gesture commit are covered by the adopting flag.
  • The box-size coalesce key is now unique per commit, so a resize and the next size edit cannot merge; the legacy per-step undo for W/H scrubbing is the trade-off the PR names.
  • Overlap with #5045: it also edits studioPendingEdits.ts and gsapUndoRestore.test.ts; a merge of the two heads is conflict-free (git merge-tree).
  • No secrets in the diff.

Non-blocking

  1. DomEditContext.tsx passing { afterOlderSaves: true } is the production hook-up of the wait, and removing it keeps every test green (useDomEditNudge.test calls trackedStudioEdit with the option directly). A small DomEditContext test (nudge burst pending, then a wrapped action) would pin it.
  2. Dropping the previous chain entry (Promise.allSettled([flushedSavesStillWriting, ...])) stays green: a second flush while the first save is still writing is not pinned to wait for it.
  3. coalesceMs: Number.POSITIVE_INFINITY in useDomGeometryCommits is redundant with the unique key (dropping it stays green); harmless.
  4. A deferred edit returns a Promise cast as R. The wrapped actions I looked at (handleGroupSelection, handleSetArcPath, handleUnroll) return void and are not awaited, so nothing breaks today, but an edit that throws synchronously inside the deferred path now rejects an unhandled promise instead of throwing.
  5. I did not run the edit-accuracy browser bench (the 20-step undo chain, the 150 ms / 69 ms timings); those numbers are the author's.

Reviewed on 191e331c34716a390d6d53d2ec842bcdb32efd28. This is a review verdict, not authorization to merge.

— Review by tai (pr-review)

@miguel-heygen
miguel-heygen added this pull request to the merge queue Oct 5, 2026
Merged via the queue into main with commit 98fe011 Oct 5, 2026
129 of 130 checks passed
@miguel-heygen
miguel-heygen deleted the fix/studio-undo-order-pending-saves branch October 5, 2026 17:04
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