fix(store): portal install drawer above global header#406
Open
sneumannb5 wants to merge 1 commit into
Open
Conversation
ConnysCode
approved these changes
Jul 2, 2026
ConnysCode
left a comment
ConnysCode
left a comment
Contributor
There was a problem hiding this comment.
Review: Approve
Nothing to change before merge. The diagnosis is right and the fix is minimal; I verified the parts that usually break a portal-to-body change and they hold.
Verified
- Theming survives the portal.
--paper,--ink,--rule*etc. are declared on:root(web-ui/app/_lib/theme.css:37,147,151), so they cascade todocument.bodyand the portaled subtree — the drawer keeps its styling and theme/palette overrides (:root[data-theme],:root[data-palette]). - Stacking fix is sound. The header is
relative z-40under body-level context providers (web-ui/app/layout.tsx:96); the portaled drawer is afixed z-50child of<body>, so both resolve at the root stacking context andz-50>z-40wins. (The header'sbackdrop-bluris itself abackdrop-filter→ own stacking context, which is exactly why escaping via portal — not a z-index bump — is the correct fix.) - SSR-safe.
InstallDraweris gated by{drawerOpen ? … : null}anddrawerOpenis false for the initialphase.kind === 'idle'(InstallButton.tsx:90-94,173), socreatePortal(…, document.body)never runs server-side — same shape as the siblingCreateIssueButton({open && createPortal(dialog, document.body)}). z-10removal is safe. Inside the portal<div>, the backdrop<button>(absolute, auto z) precedes the<aside>(relative, auto z) in DOM order, so the aside still paints on top — the explicitz-10was redundant.
Good, accurate code comment too — it captures the why (sticky always creates a stacking context) rather than just the what.
PR description — factcheck
| Claim | Reality |
|---|---|
Test plan lists npm run lint and npm run test as [X] checked, but the same lines read "— pending" |
Contradictory — the boxes are ticked while the text says pending. Non-blocking, but please either run them and drop "pending", or uncheck the boxes so the record is honest. I couldn't run them here (no node_modules in the clone) so I did not independently confirm lint/test. |
"Portals ... matches existing createPortal usage (CreateIssueButton, StreamToasts)" |
Confirmed — StreamToasts.tsx:278 and CreateIssueButton.tsx:597 both portal to document.body. |
Nits (non-blocking)
InstallButton.tsx:596-598— now that the drawer is a proper body-levelrole="dialog" aria-modal="true", it still has no focus trap, ESC-to-close, or focus-restore-on-close. This is pre-existing (the fix neither adds nor removes it) and out of scope here, so purely optional — worth a follow-up ticket. The siblingCreateIssueButtonwires ESC via auseEffectkeydown listener if you want a reference.StreamToasts.tsxadditionally guards withif (typeof document === 'undefined') return nullbefore portaling. You don't need it here (thedrawerOpenconditional already prevents any server render), just noting the two patterns differ.
(Reviewed against head a0aadc7.)
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
2 participants
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.
What
Portal the plugin install/config drawer to document.body so it renders above the global navigation header. Closes #399
Why
The drawer mounts inside the store sidebar's position: sticky element, which always creates a stacking context —
trapping the drawer's z-50 below the header's z-40. At scroll-top the header covered the drawer's heading and Close
button. Portaling escapes that context; matches existing createPortal usage (CreateIssueButton, StreamToasts).
Test plan
Risk / blast radius
Low. Single client component; no schema/API/env/CI changes. Drawer only mounts on interaction (phase.kind !== 'idle')
→ SSR-safe.