fix(studio): surface fromTo from-state in GSAP design panel#1122
Conversation
Closes #1121. The core already parsed, serialized, and mutated fromProperties end to end (gsapParser.ts, applyUpdatesToCall, buildTweenStatementCode). The panel never wired it in — AnimationCard only read animation.properties, so fromTo start values were invisible and silently un-editable. Changes: - files.ts: add update-from-property / add-from-property / remove-from-property mutation types; pass fromProperties through the add case; add fromTo to the method union - useGsapScriptCommits: updateGsapFromProperty, addGsapFromProperty, removeGsapFromProperty; addGsapAnimation extended to fromTo with { opacity:0 } → { opacity:1 } defaults - gsapAnimationConstants: fromTo added to ADD_METHODS / ADD_METHOD_LABELS ("From → To") so it can be authored from the panel - AnimationCard: From section with per-row edit/remove and + From property picker (orange accent to distinguish from To section); buildTweenSummary includes from-state description for fromTo; PropertyRow and AddPropertyTrigger extracted to eliminate the structural duplication between From and To rows - GsapAnimationSection / PropertyPanel / useDomEditSession / DomEditContext / StudioRightPanel: thread the three new callbacks through the full prop/context chain
jrusso1020
left a comment
There was a problem hiding this comment.
Reviewed against issue #1121. Verifying each item from the reporter's "not wired in the panel" list against the diff:
| Issue item | PR fix | Status |
|---|---|---|
AnimationCard.tsx only reads animation.properties, never fromProperties (start state invisible) |
New From section renders Object.entries(animation.fromProperties ?? {}).map(...) above the To section for fromTo animations, with orange accent vs emerald (AnimationCard.tsx:377-405) |
✅ |
ADD_METHODS = ["to", "from", "set"] omits fromTo — can't author from panel |
ADD_METHODS = ["to", "from", "fromTo", "set"] + ADD_METHOD_LABELS.fromTo = "From → To" (gsapAnimationConstants.ts:124-128) |
✅ |
add mutation's method union omits fromTo — can't add through API |
Mutation union widened to "to" | "from" | "set" | "fromTo"; fromProperties passed through to addAnimationToScript (files.ts:556 and :638) |
✅ |
No path to edit fromProperties (update/add/remove only address properties) |
Three new mutation types: update-from-property, add-from-property, remove-from-property, with corresponding hook helpers and full prop-chain wiring through useDomEditSession → DomEditContext → StudioRightPanel → PropertyPanel → GsapAnimationSection → AnimationCard |
✅ |
Four-for-four against the issue's stated gaps. Root cause, not symptom-mitigation: the issue notes core already models fromProperties end-to-end (parser L502-524, applyUpdatesToCall L702, buildTweenStatementCode L741) — this PR just wires the panel through to the existing core primitive rather than patching around it.
Server-side validation
Each of the three new mutation handlers checks if (anim.method !== "fromTo") and returns 400 "animation is not a fromTo":
case "update-from-property": {
const parsed = parseGsapScript(block.scriptText);
const anim = parsed.animations.find((a) => a.id === body.animationId);
if (!anim) return c.json({ error: "animation not found" }, 404);
if (anim.method !== "fromTo") return c.json({ error: "animation is not a fromTo" }, 400);
// ...
}Same defense in add-from-property and remove-from-property. A frontend bug that calls the mutation on a non-fromTo animation gets a clear error rather than silent corruption of unrelated state. Good shape.
Refactor before adding
AnimationCard.tsx extracts PropertyRow and AddPropertyTrigger helpers BEFORE the From section gets added. The existing To-property rendering is rewritten to use these helpers, so the From and To sections share a single implementation. Means any bug in the helper affects both — but that's the right trade for eliminating structural duplication. The MetricField semantics (suffix, tooltip, scrub, liveCommit, onCommit) are preserved verbatim. ✓
New-fromTo defaults
addGsapAnimation seeds new fromTo animations with properties: { opacity: 1 } and fromProperties: { opacity: 0 }:
const toDefaults: Record<string, Record<string, number>> = {
from: { opacity: 0 },
to: { opacity: 1 },
set: { opacity: 1 },
fromTo: { opacity: 1 },
};
// ...
properties: toDefaults[method] ?? { opacity: 1 },
fromProperties: method === "fromTo" ? { opacity: 0 } : undefined,Reasonable fade-in default. Worth noting from's default is also { opacity: 0 } — the difference is that from-only tweens animate AWAY from this value back to whatever the element's static style is, while fromTo animates explicitly to properties. The defaults make both shapes feel consistent.
Test coverage — one ask
The new mutation route handlers in files.ts:606-682 (the three *-from-property cases) don't have direct tests in this diff. The issue notes core's gsapParser.test.ts has fromProperties round-trip coverage already (so the in-place mutation is exercised), but the API-level handlers — including the anim.method !== "fromTo" validation and the merge semantics for adding to an existing fromProperties object — aren't tested.
Suggested follow-up (not blocking): a minimal route test that:
- Sends
update-from-propertyagainst a freshfromToanimation — asserts the response is 200 and the script contains the updated value - Sends
update-from-propertyagainst atl.to(...)animation — asserts the 400 "animation is not a fromTo" error - Sends
add-from-propertyfor a key already present — asserts the new value overwrites
This is a known pattern from Miguel's earlier review (he's flagged the same shape — missing-test-for-new-mutation-path — on other PRs today), so calling it out for symmetry.
Other observations
- PR is
mergeable_state: blocked— likely needs a rebase against main or a check that hasn't run yet fallow-ignore-next-line complexityadded tobuildTweenSummary(newfromTobranch) — fine- Visual UX: orange (
text-orange-400/70) for From, emerald (text-emerald-400/70) for To — readable color-coding; consistent with how most NLE-style tools distinguish start vs end states. The+ From propertybutton uses the orange accent too, which reinforces the section ownership
Verdict
Materially LGTM. Fix is root-cause, every issue item addressed, server-side validation is defensive, refactor-before-add eliminates duplication. The one ask is route-level tests for the three new mutation handlers — known Miguel pattern, suggested follow-up.
Holding the stamp — same protocol. James gates.
— Rames Jusso (hyperframes)
Covers the three new mutation types introduced in the fromTo panel fix: - update-from-property: asserts value written and sibling keys preserved - update-from-property: asserts 400 for non-fromTo animation - add-from-property: asserts new key merged without clobbering existing keys - remove-from-property: asserts targeted key removed, others intact - remove-from-property: asserts 400 for non-fromTo animation - add with method "fromTo": asserts fromProperties written to source All exercised at the HTTP route layer via the same Hono app harness as the existing gsap-mutations tests.
vanceingalls
left a comment
There was a problem hiding this comment.
Review
Architecture
The design is correct. The core (gsapParser.ts, applyUpdatesToCall, buildTweenStatementCode) already had full fromProperty support — this PR wires the missing UI layer and plugs the API gap. The scope is well-contained: new mutation types in files.ts, mirror hooks in useGsapScriptCommits, prop-chain threading through GsapAnimationSection → PropertyPanel → StudioRightPanel, and the rendering change in AnimationCard.
PropertyRow and AddPropertyTrigger extraction is the right call. Without it the From/To sections would have been ~80 lines of duplication. Pulling them out at the same time as the feature is the correct moment.
The 400 guard (animation is not a fromTo) on update-from-property and remove-from-property is correct defense-in-depth — the client enforces the same invariant, but the server can't trust the client.
Important
The three new mutation cases in the switch (update-from-property, add-from-property, remove-from-property) each inline the same "parse → find → check method === fromTo" sequence, which is why fallow-ignore-next-line complexity had to be suppressed. A requireFromToAnimation(parsed, animationId) helper returning GsapAnimation | Response would eliminate all three duplications and the bypass. Not blocking given the PR is merged, but worth a follow-up before this switch grows more cases.
Nits
addmutation acceptsfromPropertiesregardless ofmethod— if a client posts{method: "to", fromProperties: {...}}, the field is silently dropped byaddAnimationToScript. Consider either validating at the boundary or documenting the no-op contract.buildTweenSummaryforfromTohas no test coverage. The presentation logic is straightforward but isolated unit coverage would catch regressions as more methods land.
Verdict
Correct, well-scoped, good test coverage on the new mutation API surface. The complexity suppression is the only debt worth tracking.
— Vai
Fixes #1121.
Root cause
The core already handled
fromPropertiesend to end —gsapParser.tsparsed them (L502–524),applyUpdatesToCallmutated them (L702), andbuildTweenStatementCodeserialized them (L741). The panel never wired any of it in:AnimationCardread onlyanimation.properties, so selecting an element animated withtl.fromTo(...)showed a card whose start values were invisible and silently dropped on any edit.Changes
packages/core/src/studio-api/routes/files.tsupdate-from-property,add-from-property,remove-from-propertyaddmutation:methodunion extended to include"fromTo";fromPropertiespassed through toaddAnimationToScriptpackages/studio/src/hooks/useGsapScriptCommits.tsupdateGsapFromProperty,addGsapFromProperty,removeGsapFromProperty— mirror the existing to-property helpersaddGsapAnimationaccepts"fromTo"and seeds it with{ opacity: 0 } → { opacity: 1 }defaultspackages/studio/src/components/editor/gsapAnimationConstants.ts"fromTo"added toADD_METHODSandADD_METHOD_LABELS("From → To") so it can be authored from the add-effect menupackages/studio/src/components/editor/AnimationCard.tsxPropertyRowandAddPropertyTriggerextracted — eliminates the structural duplication that the From/To sections would otherwise createfromToanimations, orange accent to distinguish start-state from end-statebuildTweenSummarydescribes the from→to transition forfromToonUpdateFromProperty,onAddFromProperty,onRemoveFromPropertyGsapAnimationSection/PropertyPanel/useDomEditSession/DomEditContext/StudioRightPanel— the three new callbacks threaded through the full prop/context chain.Test plan
tl.fromTo(...)— the From section appears above the To section, showing the start-state values with orange labels+ From property— picker shows available props, selection adds it tofromProperties+ Add effect → From → Toto author a newfromTofrom scratch — starts withopacity:0 → opacity:1to,from,setanimations unchanged — no From section appears