feat(studio): media properties panel for video/audio elements#941
Conversation
Adds a new Media section to the Design panel that appears when a <video> or <audio> element is selected. Controls include volume (slider), playback rate, media start offset, loop/muted toggles, and for video: object-fit, object-position, poster, and has-audio-track toggle. Extends the source patcher with an "html-attribute" operation type for native HTML attributes (loop, muted, poster) that don't use the data- prefix. Adds coalesceKey to attribute commits so rapid slider/scrub edits merge into a single undo entry.
jrusso1020
left a comment
There was a problem hiding this comment.
APPROVE — Media section is well-shaped. New html-attribute op type cleanly separates native HTML attrs from the existing data-* path. Three non-blocking flags.
Audited
PatchOperation widening (Rule-2 dispatch coverage):
- Type extended
"inline-style" | "attribute" | "text-content"→ adds"html-attribute". Both dispatch switches updated:applyPatch(line ~522) andapplyPatchByTarget(line ~544). No third dispatch site — both switches are exhaustive. ✓
Boolean vs valued attribute patching in patchHtmlAttributeInTag:
HTML_BOOLEAN_ATTRIBUTESset covers the right HTML5 boolean attrs (loop,muted,autoplay,playsinline,controls,disabled,hidden,readonly,required, etc.). Missingasync,checked,multiple,novalidate, but those aren't relevant to media — future widening can add them.- Boolean detection:
(?:^|\s)${attr}(?:\s|=|$)correctly identifies bare boolean attrs ANDattr=""form. - Boolean removal:
\s+${attr}(?:=(["'])[^"']*\1)?requires preceding whitespace, which is always present since HTML tags start with<tagand attrs follow with a space. - Boolean insertion: appends
${attr}to the inner-tag slice (the captured group fromidPatternexcludes the closing>), so the result is<video src="..." id="foo" loopand the surrounding>is preserved by the outerhtml.replace(tag, newTag). ✓ Verified by tracing the call chain throughpatchHtmlAttribute→patchHtmlAttributeInTag. - Valued removal:
\s+${attr}="..."symmetric with boolean. Replace: in-place value swap viaattrPattern.test(tag) → tag.replace(attrPattern, ...). Insert: append${attr}="${escaped}". ✓
html-attribute removal semantics:
value === null || value === "" || value === "false"removes the attribute. Matches both the patcher AND the iframe-side optimistic update inhandleDomHtmlAttributeCommit. Consistent across layers — no drift between what the preview shows and what gets persisted. ✓
coalesceKey for rapid edits:
attr:${attr}:${getDomEditTargetKey(...)}andhtml-attr:${attr}:${getDomEditTargetKey(...)}— rapid slider/scrub edits on the same attribute+target merge into one undo entry. The labels (Edit volume,Edit playback rate) are user-readable.
Within-file consistency in propertyPanelMediaSection.tsx:
data-*attributes viaonSetAttribute:volume,playback-rate,media-start,has-audio✓- Native HTML attributes via
onSetHtmlAttribute:loop,muted,poster✓ - CSS via
onSetStyle:object-fit,object-position✓ - Each commit path matches the attribute's storage semantics — no confusion between paths.
isMediaElement gating:
MEDIA_TAGS = new Set(["video", "audio"])— strict check by tagName. Only video/audio see the Media section. Audio gets a subset (no Object Fit / Position / Poster). ✓
CSS additions: object-fit and object-position added to CURATED_STYLE_PROPERTIES (domEditingTypes.ts). Right place — these are real CSS properties that the existing inline-style patching path handles natively.
Non-blocking flags
1. "Has audio track" toggle creates two separate undo entries.
onChange={(next) => {
if (next === "yes") {
void onSetAttribute("has-audio", "true"); // coalesceKey: attr:has-audio:...
void onSetHtmlAttribute("muted", null); // coalesceKey: html-attr:muted:...
} else {
void onSetAttribute("has-audio", "");
void onSetHtmlAttribute("muted", "true");
}
}}The two commits use different coalesceKeys (attr:has-audio:* vs html-attr:muted:*), so undo splits them: a single toggle requires two undo presses to fully reverse. Worth either:
- Sharing a coalesceKey across the pair (e.g.
composite:has-audio-toggle:targetKey), or - Wrapping the two ops into a single
persistDomEditOperationscall with a multi-op array (the persist API already accepts an array — see existing call sites inuseDomEditTextCommits).
The second approach matches the way the existing code handles compound edits and gives you one undo entry by construction.
2. Rate and Media start MetricFields silently reject out-of-range values.
// Rate
if (!Number.isFinite(parsed) || parsed < 0.1 || parsed > 5) return;
// Media start
const parsed = parseTimingValue(next);
if (parsed == null) return;User types 10 in Rate, hits commit, nothing happens — no toast, no clamp, no error styling. Same for negative media-start. UX-wise, three options:
- Clamp silently to the nearest valid value (best for sliders, worst for explicit numeric input)
- Show a flash error/red border (most discoverable)
- Reset the field to the previous valid value (current behavior plus a visual cue)
Whichever picks up the lowest-friction shape for your users; the current silent reject is the worst of the three for discoverability.
3. patchHtmlAttributeByTarget self-referential call is mildly confusing.
function patchHtmlAttributeByTarget(html, target, attr, value) {
const match = findTagByTarget(html, target);
if (!match) return html;
const newTag = patchHtmlAttributeInTag(match.tag, match.tag, attr, value); // ← both args same
return replaceTagAtMatch(html, match, newTag);
}Passing (match.tag, match.tag) as (html, tag) works because patchHtmlAttributeInTag operates locally inside html. The actual splice into the full document happens via replaceTagAtMatch. Correct, but a comment would help future readers:
// patchHtmlAttributeInTag is normally called with full HTML + the tag
// substring; here we pass the tag as both because we'll splice the
// mutated tag back via replaceTagAtMatch after.CI
Format, Lint, Build, Test, Typecheck, Preflight, Test: runtime contract — all ✓ at the latest sha. Smoke: global install, CLI smoke (required), Render on windows-latest, Tests on windows-latest — in_progress. No failures.
— Rames
- Move Timing + Media sections above Layout in the Design panel - Remove LayerTree from Design panel (redundant with Layers tab) - Replace Rate and Media Start with sliders matching Volume's UX - Replace Position DetailField with SelectField to match Fit height - Remove Poster field (not useful for HyperFrames compositions) - Show absolute filesystem path for Source (resolves symlinks) - Add Copy button for source path with checkmark feedback - Preserve element selection on undo/redo instead of clearing it
Summary
<video>and<audio>elementsmutedper lint rules)html-attributeoperation type for native boolean/string attributes (loop,muted,poster) without thedata-prefixcoalesceKeyto attribute commits so rapid slider/scrub edits merge into a single undo entry instead of flooding the history stackTest plan
<audio>elements — select the audio clip in the timeline, verify the Audio section appears in the Design panel with Volume, Rate, Media Start, Loop, Muted<video>elements — verify the Video section shows all audio controls plus Object Fit, Object Position, Poster, Has Audio Trackdata-volumeattribute updates in the source HTMLloopattribute is added to the source HTML (notdata-loop)mutedattribute is toggled correctlydata-has-audio="true"is set andmutedis removed