Repository navigation
fix(studio): playhead keyframe edits keep their ease, undo names its edit and never waits on videos - #4889
Conversation
Edit accuracy: accurate 1301 (base branch 1217), smooth 1141 of thoseThe gate passes. Newly passing (84)
Quarantined, measured but not gated (0) |
ba48107 to
ac7e288
Compare
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Automations to automatically generate PRs for you. |
ac7e288 to
cf082c4
Compare
…keyframes (#4866) * test(studio): score edits on GSAP-animated size, scale, rotation, keyframes and from/fromTo * test(studio): settle the edit bench only once no shadow preview is waiting to be swapped in * test(studio): end a drag at its recorded pointer-up, even when no frame painted while held * ci(studio): run the keyframed edit accuracy cases in CI across 20 shards * refactor(studio): split the keyframe render and stray-css checks into small helpers * test(studio): bank the edit bench's smooth verdict as smooth, judged on the worst frame's work * style(studio): name the edit gate's smooth count helper * test(studio): count smooth edit cases only among the accurate ones * test(studio): only keyframed cases wait for the preview swap, so others keep main's undo timing * refactor(studio): keep the edit bench's keyframe check apart from the hidden preview scan * test(studio): check the keyframed cases are in the full grid without pinning its size * test(studio): quarantine the nudge sequence whose redo waits behind preview videos (#4889) * test(studio): bank the 255 keyframed edit cases that pass
cf082c4 to
5b5a027
Compare
… playhead into the file's tween
…ne started after the key
handleDomGroupMoveBy(moves) on useDomEditSession and DomEditValue moves each
selection by its delta in composition px, for Desktop's align and distribute.
It reuses the group drag's own path: a drag member per element (press-time
route, measured offset map), the delta carried to screen px through the
preview frames, then handleGsapAwareGroupPathOffsetCommit, so one shared
coalesce key gives one undo entry and GSAP members take the GSAP write.
A member that can't move refuses the call before anything is written; the
refusal rejects with its reason and shows no Studio toast (the group commit
takes refusalToast: false; telemetry stays). Zero deltas are skipped.
Tests: a pre-offset element and one under a half-scale parent move by exactly
{10,0} and {0,-5} under one group key; a half-scale parent with a 30 deg
rotated ancestor lands within the 0.001 px a translate is written with; a
GSAP member goes through the GSAP write; three refusal kinds write nothing,
leave the boxes where they were, show no toast and leave no gesture mark,
while a canvas group drag still toasts; the session hands the call to the
group commit with refusalToast: false. Each mutation (no frame mapping, no
zero skip, no restore, no teardown, no capability check, toast on refusal)
fails at least one of them.
While a gesture mark is live in the preview, refreshPlayer takes the shadow load instead of
swapping scenes in place. A swap replaced the node under the pointer mid-drag; the shadow load
is then held by the existing gesture hold and, if the drop saved, reloaded fresh by the stale
rule, so this adds no second "released" signal.
Test: an outside change during a live drag on a preview that can swap scenes leaves the dragged
node in place, shows nothing under the pointer, and after the drop loads fresh with the dropped
position. It fails on the parent without the condition ("swapped under the pointer").
Cost case, measured in a real headless preview on the edit-accuracy bench's per-frame teleport
check. Every GSAP instant patch was forced to miss and fall back to reloadPreview during its own
commit; move and rotate of a held element (gsap.set and tl.set holds), px and xPercent
placement, r0 and r30, root and nested, at 100%:
- The gesture mark was live when the fallback asked for the reload, but had always ended by the
time refreshPlayer ran (refreshKey is consumed in an effect after the commit settles). So the
new condition never fired there: root previews offer no scene swap and load shadow on both
builds, and nested previews swap on both builds.
- Worst post-release frame offset, 32 cases on this build and 32 on the parent: 0.00 px over
76-106 frames each (release to settle). No frame was off by more than 0.5 px.
- Sensitivity: a 3 px nudge injected 50 ms after the fallback reads back as 3.00 px.
…humbnails ask one at a time
…edentials opt-out
…n place, never beside it
…emetry posts through studioApiFetch
…studioApiFetch too
…skipping nested files
…ement sweep covers it
|
Re: tai's review at 375d1ca (resize-fromto-px-r0-root-z50-on, ~90 px after pointer-up). Agreed on the cause, and your forward/backward replay of the saved HTML matches mine: main's backward rerender (#4911) at the Dry run of this PR's next head (this branch + current main + #4942), on the bench:
When #4942 merges I merge main here, push that test with the regression pinned through the built save and reload path for drop and reload, run it three times, and ask you to re-review with the numbers from this branch's own CI run. |
|
Re: tai's review at 375d1ca, with the numbers from this head. At 93a9d43 (main merged, including #4942's core seek fix), from this PR's own CI edit-accuracy run (37144840600):
Regression through the built runtime, in this head: |
What
Edits on GSAP-animated layers now write their value at the playhead into the tween the file already has. Several undo, history, host and connection fixes ride along. The edit accuracy bank comes last.
Behaviour change
Studio's own API requests now carry no cookies, so they no longer queue behind the preview's paused videos. A host whose mounted Studio API authenticates with same-origin cookies sets VITE_STUDIO_API_SAME_ORIGIN_CREDENTIALS=true.
This applies when Studio is served from a loopback host (
localhost,127.0.0.1,[::1]). Served from any other host, Studio's requests keep same-origin credentials as before.An edit inside a tween keeps every other tween's time. Extending a tween changes its span, so tweens placed relative to it move with it: after an edit past its end, a following call with no position,
>,+=or a chained.to(); after an edit before its start (which writes its own position as a number), a following<call. These shifts are smaller than main's, which moved the edited tween to the end of the script.Item 1: one writer for an edit at the playhead
A move, nudge, resize (size or scale) or rotate on a property a GSAP tween animates now follows one rule: on a keyframe, change it; between two keyframes, add one at the playhead; outside the tween, extend it to the playhead and keep its authored ends. Other keyframes keep their values and times. Values come from the file's tween, or from GSAP's own parse of it where the tween leaves an end implicit (a
from()end, ato()start, a CSStranslateGSAP folds intox), never from the dragged element. GSAP parses ato()tween only once the playhead has passed its start, and every soft reload resets that; such a tween's timeline is played from the tween's start through its end and back, with the tween's channels cleared, before it is read. Starting at the tween's start lets an earlier tween on the same layer set its start value, as playback does. The gesture's live value is never read as its start, and afterwards the timeline's time and every layer's live value are put back. The edit is refused with "Edit this animation in the Code tab" only if that parse still leaves the end unknown.Replaced writers: the size keyframe tween a resize used to add beside the existing one, the convert-then-add-keyframe and outside-range rewrites in the move, resize and rotate paths.
Root fixes on the way:
.to(),<,>,+=, a label) moved.addLabel, so a tween placed at a label gets its real start there too. It read 0, which also misplaced the hold set kept before a first position keyframe.rotationY,rotationX) holds the new angle across the whole tween, instead of turning from 0 at the tween's start. The writer now refuses a value that no keyframe carries and nothing holds, so no caller can leave a channel un-held.x/yinto a tween that only animates size: the position finder fell back to any tween. It now returns only tweens that write position.delayis matched at its real start, and the edit is written so GSAP shows the new value at the playhead, notdelaylater.power1.out, by name. GSAP holds a resolved ease as a function, which used to read as unknown and refuse the edit. A built-in ease is named the same way; a custom ease function is still refused.repeat,yoyo,repeatDelay,snap,overwriteorimmediateRenderis edited as on main: the keyframes are written and those settings are kept.easeEach. A tween-leveleasedoes not ease percentage keyframes, so a linear tween re-written that way eased each segmentpower1.inOutand drifted off its path mid-segment. This covers the extend-to-playhead path, and the auto-keyframe-off whole-path and whole-property shifts when the tween authors its ease.resolveGroupTweenno longer splits a tween that mixes several property groups into one tween per group. It returns the mixed tween and the edit goes into it in place (the writer leaves its other channels alone). Why: inside one gesture the writes are buffered, so the split's re-read saw the pre-split file, and later writes targeted ids the split had already removed and were appended beside the old tweens as duplicates. A resize whose tween also animates position now writes the size and the anchor move in that one write.Item 2: undo pressed while an edit saves undoes that edit
Built on the pending-edit registry: an undo pressed while an edit is still saving waits for that edit and undoes it, not an edit started after the key.
revertNewestStudioPendingEditkeeps returning the function that shows the edit again.backgroundColor,WebkitX) saves under its CSS name.Item 3: an edit stays saved when the history reply after it cannot be read
A history reply that is a 200 but not JSON used to reject inside the history refresh. The edit is on disk and in history either way; the refresh now logs "The history's reply was unreadable." and keeps the last history it read.
Item 4: style fields for hosts
ColorFieldandGradientFieldare exported, and a session style commit takes a map of properties.Item 5: a host can move layers by exact film px
handleDomGroupMoveBy(moves)moves each selection by its delta in composition px, as one undo step, through the group drag's own path (for align and distribute in a host app). A member that cannot move refuses the call before anything is written. A group move or drag whose animated layers live in different files is refused too, since the group's save writes one file. Which file a layer writes to is decided by the same rule the save uses, so a layer that names the open file and one that leaves it implied count as one file. Each member is planned at its real drop before the first write.Item 6: an outside change during a drag waits for the drop
Item 7: connections
studioApiFetchowns Studio's API requests (every call in the package moves to it, including the freeze-frame, loudness and peak-map requests that landed on main since), with a lint rule against a barefetch. Requests carry no cookies, so Chrome pools them apart from the preview's media; paused videos could hold all six sockets a host gets and leave an undo waiting. The two telemetry calls go through it too: they post to PostHog's own host with the API key in the body, and a cross-origin request carries no cookies under the default either, so omitting credentials changes nothing for them.Since the last review (707e3d6)
transform, anattrtween's value) and GSAP's cache, including the depth origin. Targets that are not styled elements (the runtime's own{}duration filler, an XML-namespace element) are skipped; before, the filler made the read throw. The filter is one shared helper,elementTargets, inutils/elementGsap.ts.studioApiFetch.Earlier rounds (358a94b)
useGsapAwareGroupMove) and added keyframe usage tracking. This PR's group changes (no refusal toast for host calls, planning each member at its real drop, the one-file rule) now live in that hook. An edit at the playhead reports a keyframe add only when it adds one, as main's replaced writers did.Test plan
gsapValueAtPlayhead.test.ts; the 10 that go through the move, resize and rotate entry points fail on the base code (from() edit, to() edit before its first keyframe with a folded translate, refusal on an unparsed tween, parsed end beats the dragged value, array steps, nox/yinto a size tween, two tweens meeting at the playhead, rotation at a boundary, size and scale resize).easeEach: four tests fail on the base code (whole-property shift of a linear flat tween and of a keyframed tween, auto-keyframe-off drag, extend-to-playhead).xandwidthmakes one write on the file's own id.<with+=,>, a label, afrom()call, a chained.to().to(), and the edited tween's own<, label and number position). Each compares every other call's source text and resolved start before and after. All 9 fail on the base route code.to()tween the playhead has not reached, in real GSAP with a dragged value set (reads its authored start, keeps the live value and the playhead); a flat edit next to a keyframed tween at the same start; a trailing-comma call given a position (the old output does not parse); two undo presses started together during one save (they undo both edits: each press picks its edit at the press, and in turn skips it if another press already undid it; a failed undo changes nothing, so the next press targets that edit again; and a press whose edit was already undone, with a newer edit made since the press, says "Can't undo" as main does instead of undoing that newer edit). The abort slot release and the shared-tween refusal each fail when their line is removed.ease: "none"show the dragged value at the playhead (the old code refused, then showed 150 for 200); a tween with no ease writeseaseEach: "power1.out";expo.inis named and a custom function refused; arepeat: 1, yoyo: truetween is written and keeps both. A group with members in two files writes nothing, and the group preflight plans at the member's real drop. Each fails with its fix removed.attrvalue survives a read beside the runtime's{}filler (fails on 13d7b8d with a TypeError, and with a style-only restore as 50 for 77); the shared target filter drops a plain object and an XML-namespace element. The undo and GSAP test files pass three runs in a row (413 tests).Known follow-ups
delayis copied into a keyframe's properties. Older than this PR (add-keyframe's array conversion does the same).easeEach, so its segments easepower1.inOutrather than GSAP's defaultpower1.out. No worse than main.delay, so a call with no position after a delayed tween is placeddelaytoo early. Next PR.gsapDragPositionCommit.ts:197-198on main: clear the dragged channels, seek to the tween's start). Pre-existing on main, not introduced here. Two cases still differ from plain playback, both as on main: a later tween withoverwrite: "auto"takes its property from the earlier tween before playback reaches it, and a starting value set outside the timeline (agsap.setin the script) is read as 0 because the tween's channels are cleared first. The fix is to read the start without setting the tween up, and to clear only a live gesture's channels. Next PR, each case with a test that fails on main first.attrvalue drafted by hand on an XML-namespace element (not something Studio writes) comes back at its timeline value. Fixed in the next head, which merges main again for the core seek fix.repeat: -1loop stretches every loop. Both noted in review; next PR.Before
Main at b5e3ffa, Studio on generated fixture projects, headless Chrome.
xtween (5 s to 10 s,ease: "none") is Alt-dragged 200 px at 8 s, which shifts its whole path. The saved keyframes have noeaseEach, so each segment easespower1.inOut. At 6.17 s the box is 174 px behind where the linear path puts it (dashed outline). The dots, sampled every 0.25 s, bunch up at the ends.translate: 300px 0px).before-undo-while-saving.mp4
before-undo-paused-videos.mp4
After
This PR at cf082c4, with the same fixtures and steps.
easeEach: "none". At 6.17 s the box sits on the linear path (0 px off) and the dots are evenly spaced.after-undo-while-saving.mp4
after-undo-paused-videos.mp4