fix(studio): show Layers full-height in the flat inspector, not split with Design#2497
Conversation
… with Design The flat inspector split Layers and Design into a vertically-resizable stacked pair whenever both panes were toggled on, mirroring the legacy panel's layout. For the flat redesign this reads as two competing panels crammed into one column; Layers should always render full-height by itself there instead. Gate the split-view branch behind !STUDIO_FLAT_INSPECTOR_ENABLED so it still applies to the legacy panel, and fall through to Layers rendering alone (the existing `layersPaneOpen` branch already does this — it just never got reached previously because the split check ran first). Also added setExclusiveRightInspectorPane (radio-style: selecting one pane turns the other off) and use it for the Design/Layers tab clicks under the flat flag, since leaving both panes independently toggleable would highlight both tabs as "active" while only one actually renders. New usePanelLayout.test.ts covers both the existing toggle behavior and the new exclusive variant. Full studio suite (2634 tests) green; typecheck/ oxlint/oxfmt clean.
76860dc to
79b688f
Compare
miga-heygen
left a comment
There was a problem hiding this comment.
Review — #2497: show Layers full-height in flat inspector
Invariant: in flat-inspector mode, exactly one right-inspector pane is visible (radio-style); in legacy mode, panes toggle independently with a last-pane guard.
Data carriers: RightInspectorPane (union "layers" | "design"), rightInspectorPanes state, setExclusiveRightInspectorPane callback, STUDIO_FLAT_INSPECTOR_ENABLED gate.
What I checked
| Area | Verdict |
|---|---|
setExclusiveRightInspectorPane exhaustive over RightInspectorPane union |
✅ — union is exactly "layers" | "design", setter covers both |
handleToggleInspectorPane routes correctly per flag |
✅ — early return under STUDIO_FLAT_INSPECTOR_ENABLED calls setExclusiveRightInspectorPane, legacy path falls through to toggleRightInspectorPane |
Render branch gated on !STUDIO_FLAT_INSPECTOR_ENABLED |
✅ — split-container only renders in legacy mode; flat-inspector falls through to single-pane branches |
Bypass risk (direct toggleRightInspectorPane call while flat inspector is on) |
Acceptable — last-pane guard prevents both-false; render branch prevents split view; worst case both show "active" in state but only one renders |
| Test coverage | ✅ — independent toggle, last-pane guard, and exclusive radio-style toggle all exercised |
PanelLayoutContext threading |
✅ — new setter threaded through provider value consistently |
| SSOT | ✅ — "flat = exclusive" decision lives in one place (handleToggleInspectorPane); render gate is a separate concern (layout, not decision) |
Disconfirming probe: can setExclusiveRightInspectorPane("layers") followed by toggleRightInspectorPane("design") leave both panes on? Yes in state — but only if someone bypasses handleToggleInspectorPane. The UI handler is the only call site, and it gates on the flag. Residual risk: negligible.
Verdict
Code: clean. CI: some checks still in-progress (typecheck, test, windows render/build). Merge gate: wait for CI.
Review by Miga
james-russo-rames-d-jusso
left a comment
There was a problem hiding this comment.
Reviewed at 79b688f2. Traced the exclusivity invariant across every writer of rightInspectorPanes and every caller of setRightPanelTab("design"|"layers").
Blockers
(none)
Concerns
🟠 trackedSetRightPanelTab bypasses the new exclusivity invariant — trivially reproducible on fresh boot.
The Design/Layers tab-click handler correctly calls setExclusiveRightInspectorPane when inspectorTabActive === true (line 247-249 in StudioRightPanel.tsx), but the entry path (!inspectorTabActive at line 240-242) falls back to setRightPanelTab(pane) — and that resolves to trackedSetRightPanelTab in usePanelLayout.ts:80-90, which does the additive setRightInspectorPanes((panes) => ({ ...panes, [tab]: true })) regardless of the flat flag. Three external callers (StudioHeader.tsx:344, App.tsx:523, useDomSelection.ts:216) also route through the same trackedSetRightPanelTab without any flat-flag exclusivity handling.
Concrete repro from STUDIO_FLAT_INSPECTOR_ENABLED=true fresh boot:
- Default state:
rightPanelTab="renders",rightInspectorPanes={design:true, layers:false}(pergetInitialRightInspectorPanes). - User clicks the Layers tab.
handleInspectorPaneButtonClick("layers")sees!inspectorTabActive === true→setRightPanelTab("layers")→trackedSetRightPanelTabruns the additive spread → new staterightInspectorPanes={design:true, layers:true}andrightPanelTab="layers". - Now
inspectorTabActive === true, bothdesignPaneOpenandlayersPaneOpenevaluatetrue(assumingdesignPanelActiveandSTUDIO_INSPECTOR_PANELS_ENABLED), so both tab-buttonactivestates light up. - Render tree falls to
layersPaneOpen ? <LayersPanel />(branch 2 in the ternary, since the split branch is gated!STUDIO_FLAT_INSPECTOR_ENABLED). Design pane never renders.
Result: both tabs highlight, only Layers renders — the exact "highlight both tabs as 'active' while only one actually shows" outcome the PR body says this change prevents. The same shape reproduces via any external caller that reaches setRightPanelTab("design") (element-select → useDomSelection.ts:216, closing block-params → App.tsx:523, the Inspector button in StudioHeader.tsx:344).
Two clean fix shapes:
-
In the hook (my preference — centralizes the invariant): make
trackedSetRightPanelTabflat-aware.if (tab === "design" || tab === "layers") { if (STUDIO_FLAT_INSPECTOR_ENABLED) { setRightInspectorPanes({ design: tab === "design", layers: tab === "layers" }); } else { setRightInspectorPanes((panes) => ({ ...panes, [tab]: true })); } }
Adds the flag import to
usePanelLayout.ts, one branch, closes the invariant across every current and future caller ofsetRightPanelTab("design"|"layers"). -
At each call site: pair every
setRightPanelTab("design"|"layers")with asetExclusiveRightInspectorPane(...)under the flat flag. Works, but four sites (this PR'shandleInspectorPaneButtonClickentry branch + the three external callers I traced) all need it individually — one gets forgotten, invariant leaks again.
Either fix should be paired with a new usePanelLayout.test.ts case pinning trackedSetRightPanelTab("layers") under flat=on from a state where design is already true — right now the tests only exercise the toggle/exclusive primitives directly, not the tab-switch path that hits trackedSetRightPanelTab.
Nits
(none)
Green notes
🟢 setExclusiveRightInspectorPane primitive itself is clean. Radio semantics correctly enforced ({ design: pane === "design", layers: pane === "layers" }), stable useCallback([], ...), test at :57-67 pins both directions plus the transition from both-true → exclusive. Threading through PanelLayoutContext.tsx is complete and consistent.
🟢 Render-branch gating is correct in isolation. layersPaneOpen && designPaneOpen && !STUDIO_FLAT_INSPECTOR_ENABLED short-circuits split view under flat, and the ordered fallthrough (layersPaneOpen before designPaneOpen) matches the PR body's "Layers renders full-height by itself" contract. The inspectorTabActive "unavailable" branch at line 583+ still catches the pathological {design:false, layers:false} state gracefully.
🟢 toggleRightInspectorPane's existing "refuses to turn off the last remaining pane" guard is preserved. No collateral change to that invariant.
🟢 Test file is well-scoped. New usePanelLayout.test.ts uses happy-dom + React.act, covers the two existing behaviors + the new exclusive primitive at the hook level. Adding one more case for the trackedSetRightPanelTab entry path under flat=on would fill the gap I flagged above.
Hostile counterexamples
- Layers-first initial state (
initialState.rightPanelTab === "layers"→getInitialRightInspectorPanesreturns{layers:true, design:false}): first click on Design tab under flat=on hits!inspectorTabActivebranch → same buggy state. Symmetric to the boot-fresh repro above. - Programmatic element-select landing on Renders/Slideshow/Variables: user is on the Variables tab, selects an element in preview →
useDomSelection.ts:216gates onrightPanelTabRef.current !== "variables"and no-ops. Safe. But from any other tab (renders,slideshow,block-params) the same select triggerssetRightPanelTab("design")→ buggy state. - Block-params close:
App.tsx:522-524onCloseBlockParams→setRightPanelTab("design"). If the user opened block-params from the Layers tab, closing block-params lands the buggy{design:true, layers:true}state.
What I didn't verify
- Didn't run the studio dev instance in flat mode to eye-check the actual DOM. My concern is a static-analysis walk of the exclusivity invariant across writers; a manual click-through would confirm the tab-highlight side of the bug. The state-shape divergence I traced is confident.
Verdict framing
Fix is directionally right but leaks the exclusivity invariant on every non-direct-click writer. Best addressed at the hook (one branch) rather than every call site. LGTM from my side once trackedSetRightPanelTab (or the equivalent choke-point) is flat-aware and a test pins the entry path.
…rect tab click Review feedback on #2497 (Rames D Jusso) found a real gap: the exclusivity this PR introduced only applied to the direct in-panel tab click, which calls setExclusiveRightInspectorPane. Every OTHER caller that reaches setRightPanelTab("design"|"layers") — element select (useDomSelection.ts), closing block-params (App.tsx), the header Inspector button (StudioHeader.tsx), and even this PR's own "!inspectorTabActive" entry branch in handleInspectorPaneButtonClick — went through trackedSetRightPanelTab's old unconditional additive `{...panes, [tab]: true}`, reproducing the exact "both tabs highlight, only one renders" bug this PR claims to fix. Confirmed via the reviewer's traced repro: fresh boot, click Layers tab while no inspector tab is yet active → rightInspectorPanes ends up {design:true, layers:true}. Fixed at the reviewer's preferred choke point: trackedSetRightPanelTab itself is now flat-aware, applying the same exclusive-radio update setExclusiveRightInspectorPane does whenever STUDIO_FLAT_INSPECTOR_ENABLED is on, falling back to the legacy additive update otherwise. This closes the gap for every current and future caller of setRightPanelTab, not just the one call site this PR touched. New usePanelLayout.test.ts cases pin both directions: setRightPanelTab stays additive under flat=off (legacy split-view behavior unchanged), and enforces exclusivity under flat=on even when called directly (not through the tab-click handler) — using the vi.doMock(manualEditingAvailability) pattern already established in PropertyPanel.test.tsx for flag-dependent module state. Full studio suite (2643 tests) green; typecheck/oxlint/oxfmt clean.
|
Fixed per your traced repro — thanks for tracking it through every writer.
Added Full studio suite green, typecheck/oxlint/oxfmt clean. Pushed as |
miga-heygen
left a comment
There was a problem hiding this comment.
Re-review — #2497 v2: flat-inspector exclusivity in setRightPanelTab
The v2 closes the bypass path I flagged in round 1. Previously, only the in-panel tab click (handleToggleInspectorPane → setExclusiveRightInspectorPane) enforced radio-style exclusivity — every other caller reaching trackedSetRightPanelTab directly (element select, closing block-params, header Inspector button) would still additively open both panes, causing the "both tabs highlight, only one renders" bug.
What changed
usePanelLayout.ts: trackedSetRightPanelTab now gates on STUDIO_FLAT_INSPECTOR_ENABLED:
- Flat mode → exclusive:
{ design: tab === "design", layers: tab === "layers" } - Legacy mode → additive:
(panes) => ({ ...panes, [tab]: true })
Tests: Two new cases with vi.doMock to toggle the flag per test:
- Legacy (
STUDIO_FLAT_INSPECTOR_ENABLED: false):setRightPanelTab("layers")leaves both panes on — additive, as expected. - Flat (
STUDIO_FLAT_INSPECTOR_ENABLED: true):setRightPanelTab("layers")sets layers-only — exclusive, closing the bypass.
renderPanelLayoutWith refactored to accept the hook as a parameter for module-mocked variants. Clean.
Verification
The exclusivity decision now lives in two places: setExclusiveRightInspectorPane (direct tab click) and trackedSetRightPanelTab (all other callers). Both use the same { design: tab === "design", layers: tab === "layers" } expression. This is NOT a SSOT violation — they're different entry points for the same invariant, and the expression is trivial enough that extracting it would add indirection without value.
Disconfirming probe (resolved): Can any caller still additively open both panes under flat mode? No — setRightPanelTab covers the indirect paths, setExclusiveRightInspectorPane covers the direct tab click. The raw toggleRightInspectorPane is still independently toggleable, but it's not called under the flat flag (gated in handleToggleInspectorPane).
Verdict
Code: clean. Probe gap from v1 is closed. LGTM.
Review by Miga
james-russo-rames-d-jusso
left a comment
There was a problem hiding this comment.
Re-verified at fb24ecac. Δ from 79b688f2: 2 files / +66 / -4 — one new fix commit landing at the reviewer-preferred choke point.
Blockers
(none)
Concerns
(none)
Nits
(none)
Green notes
🟢 R1 concern closed at the hook layer, as suggested. usePanelLayout.ts:81-97 now makes trackedSetRightPanelTab itself flat-aware — the ternary
setRightInspectorPanes(
STUDIO_FLAT_INSPECTOR_ENABLED
? { design: tab === "design", layers: tab === "layers" }
: (panes) => ({ ...panes, [tab]: true }),
);matches the exclusive-radio shape setExclusiveRightInspectorPane uses, so every caller reaching setRightPanelTab("design"|"layers") — the !inspectorTabActive entry branch in handleInspectorPaneButtonClick, StudioHeader.tsx:344, App.tsx:523, useDomSelection.ts:216, and any future caller — now enforces exclusivity under the flat flag without needing per-site coordination. Under STUDIO_FLAT_INSPECTOR_ENABLED=false, the legacy additive spread runs unchanged, so split-view behavior is preserved.
🟢 New tests pin both directions of the invariant. usePanelLayout.test.ts:76-92 covers the flat-off legacy additive path ({design:true, layers:false} → setRightPanelTab("layers") → {design:true, layers:true}), and :94-116 covers the flat-on exclusive path (same starting state → {design:false, layers:true}). The vi.doMock(manualEditingAvailability) + vi.resetModules() + re-import pattern is the right shape for a module-load-time flag constant — and the vi.doUnmock + vi.resetModules() in afterEach cleanly restores between cases so the earlier renderPanelLayout()-based tests aren't affected.
🟢 Commit message credits the R1 walkthrough and reproduces the failure mode. Every concrete flow I traced (element-select via useDomSelection, block-params close via App.tsx, header Inspector button via StudioHeader, and this PR's own !inspectorTabActive entry branch) is named explicitly — future readers can see WHY the choke point moved.
🟢 No collateral change. setExclusiveRightInspectorPane, toggleRightInspectorPane, and the render-tree gating in StudioRightPanel are byte-identical to R1. Delta is limited to usePanelLayout.ts + its test file.
Verdict framing
Concern closed at the choke point I preferred, with test coverage for both flag states. Clean second round. LGTM from my side.

Summary
!STUDIO_FLAT_INSPECTOR_ENABLEDso it still applies to the legacy panel, and falls through to rendering Layers alone under the flat flag.setExclusiveRightInspectorPane(radio-style: selecting one pane turns the other off) and used it for the Design/Layers tab clicks under the flat flag — leaving both panes independently toggleable would highlight both tabs as "active" while only one actually renders.Test plan
usePanelLayout.test.tscovers both the existing toggle behavior and the new exclusive variant🤖 Generated with Claude Code