fix(editor): raise floating panels above the sidebars - #343
Open
lovepixel-git wants to merge 1 commit into
Open
Conversation
Opening the AI assistant while a docked properties panel was showing made it vanish: the floating shell sat at z-index 50, the sidebars sit at 85, so the sidebar won and the panel was unreachable without closing or undocking the sidebar first. The convention already existed. The shared FloatingWindow primitive is z-index 90, deliberately above the 85 sidebar tier, and floating windows built on it layer correctly. AgentPanel and PropertiesPanel each hand-roll their own floating shell rather than using that primitive, and both were left at 50, so neither followed it. Both move to 90. PropertiesPanel had the same bug unreported: undock it, open any docked sidebar panel, and it disappears the same way. Only the floating rules change. Both docked variants already reset z-index to auto, so docked layout is untouched. Nothing that must stay on top is affected: the code editor (95), the inspector's own StyleSurface (99), step-up and toolbar dialogs (200/201), the preview overlay (400/401), spotlight (9000) and tooltips (10001) all still win. Fixes CoreBunch#332
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #332.
Root cause
The floating AI assistant shell is
z-index: 50. The sidebars are85. So a docked properties panel wins and the assistant is unreachable without closing or undocking the sidebar first, exactly as reported.The interesting part is that the convention already exists and these panels just are not on it. The shared
FloatingWindowprimitive (src/admin/shared/FloatingWindow/FloatingWindow.module.css) isz-index: 90, deliberately above the 85 sidebar tier, and every floating window built on it layers correctly.AgentPanelandPropertiesPaneleach hand-roll their own floating shell instead of using that primitive, and both were left at 50.So this is not "bump the number until the symptom goes away", it is putting the two hand-rolled shells on the layer the shared primitive already defines.
Same bug, second site
PropertiesPanelhas it too and nobody has reported it yet: undock the inspector, then open any docked sidebar panel, and it disappears the same way. Fixed here since it is one root cause rather than two problems. Happy to split it out if you would rather keep this to the reported path.Blast radius
Only the floating rules change. Both docked variants (
.floatPanelDocked,.panelDocked) already resetz-index: auto, so docked layout is untouched.I checked what sits above 90 to be sure nothing important is now covered:
StyleSurfaceTies at exactly 90 are the other floating surfaces (
FloatingWindow,MediaViewerWindow, the left sidebar's floating rule), which is the correct peer group. Order among them falls to DOM order, which is the same rule that already governs them today.Two things worth your call
Which behavior you wanted. The issue proposed either "float over" or "split the right sidebar". This does the first, because it is the smaller change and it matches the existing floating tier. Splitting the sidebar is a real layout feature and a different PR.
A token, if you want one.
90is now written in three places. Extracting something like--z-floating-panelintoglobals.cssnext to the existing--z-dropdown: 20would pin the convention properly. I left it out because that is a broader call about your z-index system than this bug needs, and the values run 1, 2, 30, 50, 85, 90, 95, 99, 200, 400 and one2147483647, so a real scale is more than a bug fix should decide. Say the word and I will add it.Tests
No new test. This changes a layer, not behavior, and the assertion that would matter (a floating panel is clickable over a docked sidebar) needs a real browser, so it belongs in the Playwright suite rather than
bun test. I did not want to assert a z-index policy you have not stated, but if you would like the convention pinned there is a precedent for a source-scanning gate insrc/__tests__/architecture/and I am glad to add one.bun run lintcleanbunx tsc -bexit 0bun run buildcleanbun test6571 pass, 1 failThe failure is pre-existing
Bundle size budgets > ContentPage-*.js, 91206 B against a 90000 B budget. Not from this change, and it cannot be: this PR touches two CSS declarations in the site editor, not the content route. I stashed the diff, rebuilt cleanmainand got the same failure. Worth flagging that it is drifting rather than static, it was 90043 B onmaina few days ago and is 91206 B now.