The lane-header keyframe toggle fires on whichever element owns the lane,
which need not be the selected one. The remove path looked the animation up
in the selected element's animations, so a non-selected element's flat tween
missed and silently took the remove-one-keyframe branch, stranding the tween
instead of deleting it.
Three review follow-ups on the editor-callback consolidation.
The keyframe-target resolve now takes the clicked element's key and reads
that element's keyframe cache. The diamond context menu and move-to-playhead
pass no explicit target, so they fell through to the cache of whatever
element happened to be selected: opening the menu on a non-selected
element's diamond resolved against the wrong keyframes.
PropertyPanelFlat opens the Motion group by adjusting state during render
instead of in an effect, so the AnimationCard mounts on the same commit the
focus request arrives on rather than a frame later.
Both animation sections pass a module-level focus consumer instead of a
fresh inline arrow, so AnimationCard's focus effect stops re-running on
every parent render.
LegacyTrackHeader reads as deprecated code. It is the live path for every
track that has no keyframe clip to disclose, so call it PlainTrackHeader and
say so in a comment.
The header file owned value sampling, readout formatting, lane-state
resolution and the JSX at once, so a formatting change and a layout change
edited the same file. Sampling and formatting now live in
trackHeaderLaneValues, lane-state resolution in trackHeaderLaneState, and
resolveLaneHeaderState returns only the four fields its caller reads.
Also shows the track's clip count next to the track identity, which the
header promised but never rendered.
The prev/next keyframe chevrons and the group toggle diamond let their
click bubble to the ancestor track row, so seeking to a keyframe also
reselected the track. The disclosure caret and the eye already stop it;
these now match.
Truncated labels (layer name, track label, group label, value readout)
also carry a title so the full text is reachable on hover.
CONTRIBUTING.md asks for a guard clause rather than a non-null assertion outside
an already-checked path. The index check and the lookup are now the same guard.
Escape now ends an in-flight diamond drag the way it already ends clip
and element drags: the armed gesture is marked cancelled, the preview is
dropped, and the pointerup that follows is swallowed instead of falling
through to the click branch.
The preview also flushes once per animation frame instead of once per
pointermove, so a high-rate trackpad no longer re-renders every diamond
in the row several times a frame. Single-diamond retime stays the
documented scope; multi-select drag needs a batched mutation the script
ops do not express yet.
Rows stopped sharing one pixel height when lanes gained expansion, so the only
production caller was passing cumulative row coordinates with trackHeight 1 and
both scrollTops zeroed. The parameter names described units the values no longer
carried. The vertical axis is now a row index and the caller keeps ownership of
folding scroll and per-row heights into it.
The local extractIdFromSelector duplicated the `#id`-only regex that
idFromSelector replaced, so both DOM-less paths in
resolveSelectorElementIds (no-iframe fallback and querySelectorAll-throw
recovery) read no id at all for the bracketed `[id="..."]` form writers
emit for CSS-unsafe ids. Deleted the duplicate and imported the shared
reader; both forms now resolve.
Dragging the playhead to the start of the composition needed a very slow
drag. The scrub surface begins GUTTER + TRACKS_LEFT_PAD px right of the
viewport edge, and both scrub paths bailed out when the pointer sat left of
that origin rather than clamping. So the last 80px of the drag toward zero
silently did nothing: the playhead stuck at whatever the last in-range sample
reported, and only a drag slow enough to sample inside the thin sliver before
the origin ever reached 0.
Both paths now share getTimelineScrubTime, which clamps to [0, duration]. One
owner, so the live-feedback path and the committed-seek path cannot disagree
about the edge again.
The R1/R3 residuals on this PR were fixed at the top of the stack, so they
only cleared once every branch above landed. They belong here, next to the
code they correct:
- `idFromSelector` inverts `idSelector` for both regex readers, so the
post-commit cache refresh stops skipping the CSS-unsafe ids `idSelector`
exists to support.
- `deduplicateKeyframes` drops `ease` when it is ambiguous; the flag was the
only honest answer and the last-writer-wins curve belonged to an arbitrary
colliding tween.
- `isStaticPositionHold` is now the single owner of the hold skip. The
`sourceAnimations` filter and the `allKeyframes` filter had diverged on
whether `immediateRender` counts as a property.
- The keyframe-cache setters no-op when the write changes nothing, instead of
handing every subscriber a fresh Map.
- `reset()` clears `focusedEaseSegment`.
- The test hook `delete`s its window key rather than setting it to undefined,
so feature detection still works.
- The `toClipKeyframes` fixture uses `as unknown as T` with the justification
CONTRIBUTING.md asks for.
R3 review follow-ups on the keyframe cache:
- clearKeyframeCacheForFile collected ids from the index.html alias prefix
too, so a re-scan of one composition file wiped rows a sibling file had
just written (several files re-scan concurrently). Only the file's own
prefixed keys name the ids now; clearKeyframeCacheForElement still takes
the alias and bare key with them.
- toClipKeyframes fell back to a fixed 1s tween duration, which put a
duration-less tween's keyframes at a percentage no edit path agreed with.
It now spans the clip, matching resolveEditableTweenDuration.
- collectAnimatableKeyframeProperties takes `object` so call sites drop
their `as Record<string, unknown>` casts.
Regression tests cover both fixes.
Each keyframe-cache writer re-derived a clip-relative percentage inline, and the
post-commit writer rounded to 0.1% while the others used 0.001%. Selection keys
embed that number, so a commit-time rewrite could orphan a live key.
toClipPercentage owns the rounding, toClipKeyframes owns the whole row (percentage
plus the tween percentage and animation identity the lanes read), and the parsed
write reuses elementCacheKeys instead of open-coding the three key variants.
An ungrouped tween (mixed property groups classify to propertyGroup
undefined) fed keyframeCache but was skipped by every gsapAnimations
writer, so the collapsed row drew diamonds the expanded lanes had no
source animation to render. Drop the property-group gate at all three
writers; lane consumers already filter by group.
Also route the same-percentage merge in updateKeyframeCacheFromParsed
through deduplicateKeyframes so the easeAmbiguous rule has one owner.
* fix(studio): play preview audio at speeds above 1x
The player force-muted preview audio whenever playback rate exceeded 1x
and disabled the mute button (and the M shortcut) in that state. Users
changing speed to 2x heard nothing, and clicking unmute did nothing.
Media elements play audio fine at any rate (pitch preserved by default),
so remove the special case: audio now follows only the user's mute
toggle at every speed. Drops the shouldMutePreviewAudio helper and the
audioAutoMuted UI/keyboard gating that existed solely for it.
* test(studio): update seek audio test for unmuted-above-1x behavior
useTimelinePlayer.seek.test.ts still asserted the old force-mute: at 2x
with the user unmuted it expected a set-muted{muted:true} message. With
audio now following only the user's toggle, the preview receives
set-muted{muted:false}. Update the assertion and title to match.
slideshowIslandRegex scanned the full file content on every editingFile
change even for the common non-slideshow case. Gate it behind a plain
substring check on SLIDESHOW_ISLAND_TYPE first — cheap, and avoids the
full-content RegExp pass for files that plainly have no island.
Added a test for the still-open behavior this preserves: a malformed
island (invalid JSON) still trips the substring check and the regex,
so the tab stays discoverable rather than silently disappearing.
Flip STUDIO_FLAT_INSPECTOR_ENABLED's default from false to true — the
bug-fix pass on the flat inspector is complete (right-aligned values,
Stroke width/style split, promote-badge overlap, Layout/Style section
gating for non-visual elements like audio). VITE_STUDIO_FLAT_INSPECTOR_ENABLED=false
still opts back into the legacy panel.
Updates the two tests that asserted the old false default: the flag's
own default test, and the "classic PropertyPanel input coverage" suite,
which relied on that default to reach the legacy panel and now mocks it
explicitly (mirroring the adjacent "flat" suite's existing pattern).
The Slideshow tab rendered unconditionally, showing the branching editor
for any composition regardless of whether it was actually a slideshow —
a plain video comp offered a tab with nothing meaningful to edit.
Gate it on the composition carrying the slideshow JSON island
(<script type="application/hyperframes-slideshow+json">), the same
definitive marker the CLI's `present` command already requires (it
refuses to run without one). Presence-only, not full manifest
validation, so a malformed island still surfaces the tab rather than
disappearing entirely. Also bounce rightPanelTab off "slideshow" to
"renders" if the active composition stops being a slideshow while that
tab is open (e.g. switching files), since its button would otherwise
vanish with no way back to it.
Extracted the gating + scene-list derivation into useSlideshowTabState
to keep StudioRightPanel.tsx under the 600-LOC gate.
Both panels showed Layout (X/Y/W/H/Rotation/Z-index) unconditionally —
no gate existed for it at all — and Style was gated only on
canEditStyles (a permission check), never on the element's tag. Neither
gate accounted for `<audio>`, which never paints a visual frame, so a
music track's inspector showed a full set of position/size/fill/shadow
controls with zero visual effect.
Add `layout`/`style` applicability to resolveEditingSections (core),
keyed on tag !== "audio", and gate both panels' Layout section and the
existing Style gate on it. Media/Motion/Grade/Text were already
correctly gated (verified via a research pass across both panels) and
are untouched.
6px between rows left little clearance above a row's value for the
promote-to-variable badge (now positioned above the row). Widen the
row gap to 10px.
Shrinking the wrapped control's width to make room for the badge (previous
commit) fixed the overlap but pushed the value left unnecessarily. Move the
badge to sit above the row instead, clearing the value without touching its
layout.
PromotableControl absolutely-positions its "◇ var" / "◆ {id}" badge over
the wrapped control without reserving any space, so on rows where the
value renders flush to the right edge (flat Font/Color rows) the badge
sat directly on top of the value text instead of beside it. Add a
right-padding gutter on the wrapper sized to each badge state, and cap
the bound chip to a fixed max-width so it always fits inside its gutter.
Stroke width committed border-width and border-style together from one
free-text field, so setting a style meant typing an exact CSS keyword
(e.g. "dashed") with no indication of which ones were valid — the row
also duplicated the discoverable Stroke style select directly below it.
Stroke width now only commits border-width; style changes go exclusively
through the existing select.
FlatRow lays out label…gap…value across a `justify-between` row, but the
shared CommitField input it wraps had no text-align, so its text hugged
the LEFT edge of the value's own (often much wider) right-hand box —
looking left-aligned relative to the row, out of step with FlatSelectRow
and FlatSlider, which already right-align.
Added an optional `align` prop to CommitField (default "left", preserving
the legacy panel's MetricField/DetailField layouts where label-then-value
sits inline and left reads naturally) and pass `align="right"` from
FlatRow. Left the Motion Timing row's Start/End/Duration cells alone —
those stack label-above-value in a grid, a different pattern from the
inline label…value row this fix targets.
New tests: FlatRow's input has `text-right` (not `text-left`); the legacy
MetricField's input keeps `text-left` (not `text-right`), pinning
CommitField's default so the shared component doesn't drift for the
panel that didn't ask for this.
Full studio suite (2645 tests) green; typecheck/oxlint/oxfmt clean.
Reported as "template variables are broken": binding an element's field to a
variable via the flat inspector's "◇ var" promote chip (or editing an
already-bound field's value) wrote the correct bytes to disk, but the
Variables tab kept showing the pre-edit value until the whole Studio page
was hard-reloaded.
Root cause: DesignPanelPromoteProvider deliberately opens its OWN SDK
session (`useSdkSession(projectId, selection.sourceFile ?? activeCompPath)`)
so that promoting inside a sub-composition binds the variable in the
sub-comp's own file, not the host's. For the common case — a top-level
element, same file as `activeCompPath` — this session is a SEPARATE
in-memory `Composition` instance from the shared one `VariablesPanel`
(Variables tab, Slideshow, etc.) reads. A persist through the promote
provider's session never fires the shared session's own "change" event.
Worse, the shared session's file-change listener runs
`isSelfWriteEcho(path, content)` to decide whether to reload — but
`sdkSelfWriteRegistry` is keyed by file path only, not by session instance
(its own doc comment assumes "the studio process has a single SDK session
lifecycle at a time"). It sees the promote provider's write registered
under the same path and concludes it's its own echo, permanently
suppressing the reload it actually needs.
Threaded `forceReloadSdkSession` (the same mechanism every other
server-side-write path in Studio already uses for exactly this "resync
after a write I didn't make myself" case) from App.tsx through
StudioRightPanel into DesignPanelPromoteProvider, and call it after every
successful promote/setDefault persist — unconditionally, not gated on the
promote target matching activeCompPath, since re-opening a file that
didn't change is a harmless no-op re-parse and a path-equality guard here
already produced one subtly wrong comparison (activeCompPath can be null
while the shared session still defaults to "index.html") before landing on
this simpler version. Verified live: editing a variable-bound field's
value now updates the Variables tab immediately, no reload required.
App.tsx crossed the 600-line file-size gate after threading the new prop;
extracted the tiny handleAddAssetAtPlayhead wrapper into its own
useAddAssetAtPlayhead hook (with a regression test) to bring it back under.
Full studio suite (2639 tests) green against a fresh main; typecheck/
oxlint/oxfmt clean.
Miguel Angel Simon Sierra