fix(studio): gate Layout and Style panel sections on the element having a rendered box#2534
Conversation
…ng a rendered box 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.
miga-heygen
left a comment
miga-heygen
left a comment
There was a problem hiding this comment.
Review — #2534: gate Layout and Style sections on rendered box
Adds layout and style booleans to EditingSectionApplicability. Gate: hasVisualBox = facts.tag !== "audio" — audio never paints a visual frame, so position/size/rotation/stacking and fill/radius/stroke/shadow are all inert on it.
Verified: classic PropertyPanel wraps Layout section in {sections.layout && (...)}, showEditableSections now also gates on sections.style. Flat PropertyPanelFlat wraps layout group push in if (sections.layout). Test coverage: audio → {layout: false, style: false}, div/img/video → both true.
The diff is +220/-191 but most of it is mechanical re-indentation from wrapping the Layout section in a conditional. The actual logic change is ~10 lines in affordances.ts + the two gating conditions in the panel components.
Code: clean.
Review by Miga
james-russo-rames-d-jusso
left a comment
james-russo-rames-d-jusso
left a comment
There was a problem hiding this comment.
Reviewed at 8a793ba2. Traced the "has rendered box" gate end-to-end (origin → carrier → both classic and flat panel consumers), verified <audio> is the only trigger, and cross-checked that no sibling section falls through the gate accidentally.
Blockers
(none)
Concerns
(none)
Nits
🟡 sections.style and sections.layout are always equal in this PR (both key on hasVisualBox = facts.tag !== "audio"). The doc-comment on EditingSectionApplicability justifies the split ("kept separate since a future tag could need one without the other"), which is fine speculative flex, but it means today the two fields are structurally the same signal wearing two hats. If they stay coupled indefinitely, hasVisualBox would read cleaner as a single derived boolean the panel reads directly, with the comment explaining the future split. Non-blocker either way — the added indirection is cheap and the doc-comment is honest about it.
Questions
(none)
Green notes
🟢 Envelope-carrier trace is clean end-to-end. resolveEditingSections(facts) computes hasVisualBox = facts.tag !== "audio" and stamps layout/style from it. PropertyPanel.tsx:200 binds sections = resolveEditingSections(...) once, then feeds BOTH the classic surface AND <PropertyPanelFlat sections={sections} showEditableSections={...} /> at line 271. Every downstream consumer reads the same object — no divergent recomputes, no stale prop drift.
🟢 Both panel surfaces are gated symmetrically for audio. Classic PropertyPanel.tsx:
- Layout section wrapped by
{sections.layout && (<Section title="Layout">...)}at line 388. - Style-bearing content wrapped by
{showEditableSections && (...)}at line 573, whereshowEditableSections = canEditStyles && sections.style(line 204).
Flat PropertyPanelFlat.tsx:
- Layout group:
if (sections.layout) { groups.push(...) }at line 391. - Style group:
if (showEditableSections) { groups.push({ id: "style", ... }) }at line 369, receivingshowEditableSectionsas a prop fromPropertyPanel.tsx(the ONLY caller —<PropertyPanelFlatappears exactly once in prod code, atPropertyPanel.tsx:271).
So for <audio>: classic hides Layout + Style, flat hides Layout group + Style group. Symmetric.
🟢 No sibling section falls through the gate unintentionally. Enumerated the other section-applicability fields (text, media, colorGrading, timing, animation) — none key on hasVisualBox, and each has its own targeted predicate: media on video|audio|img, colorGrading on video|img, etc. Audio correctly keeps media: true (so the Media group still renders with source URL / volume / duration controls) but loses Layout + Style. Exactly the desired shape.
🟢 Test matrix pins the invariant for both directions. affordances.test.ts:147-148 asserts audio: { layout: false, style: false }; :151-155 iterates div/img/video and asserts { layout: true, style: true }. Symmetric positive-and-negative coverage. Would also pass for svg/canvas/span/p etc. (all non-"audio" tags) though those aren't explicitly asserted.
🟢 Diff-line count is mostly indent-only re-wraps. Of the +220/-191, the substantive delta is the affordances.ts change (+7/-0), two 3-line test additions (+11/-0), the PropertyPanel.tsx showEditableSections derivation change (+1/-1), and the two guard-wrappers around the Layout section/group in classic and flat. The rest is JSX indentation cascading from the added {sections.layout && (...)} wrapper. git diff -w on the tip would probably collapse to under 30 semantic lines.
🟢 PropertyPanel3dTransform, KeyframeNavigation, Z-index/Stacking, and every keyframe hook all move inside the guarded Layout section unchanged. No accidental scope escape — every child under the Layout <Section> is inside the {sections.layout && (...)} wrapper.
What I didn't verify
- Didn't spin up Studio and select an
<audio>element to visually confirm the Layout + Style panels are absent. Structural trace of the gate wiring is my ground; live UI verification would strengthen this. - Didn't add an E2E-level test asserting "select audio → Layout + Style groups not in DOM" — the affordance-level unit test at
affordances.test.tsis the only new coverage. If audio-selection routing changes later and starts leaking a differenttag(e.g. wrapped in a shadowed host), the pure-fn test wouldn't catch it. Consider aPropertyPanel.test.tsxcase wrapping the panel with an<audio>selection to lock in the panel-level gating; not scope-required for this PR.
Verdict framing
Tight, correct fix at the right layer. <audio> no longer surfaces meaningless Layout/Style controls, and the gate is threaded through both classic and flat panel surfaces with equal rigor. LGTM from my side.
3 participants
Summary
Audited every panel section (Text/Style/Layout/Motion/Media/Grade, both the legacy and flat inspectors) against every element tag Studio recognizes (div, img, video, audio, sub-composition host, composition root) to find sections shown for an element type they don't apply to — the reported case was music tracks (
<audio>) showing the Layout menu.<audio>, which has no rendered box.canEditStyles(a permission check), never by tag — same audio gap.Added
layout/stylefields toresolveEditingSectionsin@hyperframes/core/editing, keyed ontag !== "audio", and gated both panels' Layout section and the Style-section entry point on it.Test plan
affordances.test.ts: audio →layout: false, style: false; div/img/video → bothtruebunx vitest run, 90 files / 1271 passed) and full studio suite (231 files / 2643 passed)tsc --noEmit,oxlint,oxfmt --checkclean across core + studio<audio>element — Design panel now shows only Motion + Media (Volume/Rate/Media start/Loop/Muted), no Layout/Style/Text/Grade; a<div>(Card) and text element (Title) are unaffected, still showing Style + Layout + Motion as before