fix(playground-ui): make popup portal container resolution null-safe#17560
Conversation
…r null
Base UI's FloatingPortal treats container={null} as "not ready, render
nothing", so Combobox/Select/Popover/DropdownMenu popups opened outside a
SideDialog mounted to no DOM node — the model and provider pickers in the
chat composer opened in state but rendered nowhere.
Move the null->undefined coercion into usePortalContainer so the resolver
can never emit null, and drop the per-consumer `?? undefined` band-aids.
Add regression tests for the resolver contract and the combobox portal
target. Follow-up hardening to #17556.
🦋 Changeset detectedLatest commit: 2468b9e The changes in this PR will be included in the next version bump. This PR includes changesets to release 4 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (8)
WalkthroughPortal container resolution is refactored to normalize ChangesPortal Container and Component Portal Resolution
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning Tools execution failed with the following error: Failed to run tools: 13 INTERNAL: Received RST_STREAM with code 2 (Internal server error) Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
PR triageLinked issue check skipped for core contributor @damien-schneider. PR complexity score
Applied label: Changed test gateChanged Test Gate is pending. The |
dane-ai-mastra
Bot
added
the
tests: green ✅
|
Some context on the root cause: base-ui's We were the ones emitting This fixes it at the source: the sentinel is |
2 participants
Summary
Follow-up hardening to #17556. Base UI's
FloatingPortaltreatscontainer={null}as "not ready — render nothing", whileundefinedmeans "portal todocument.body". OurPortalContainerContextdefaulted tonull, so popups opened outside aSideDialog(the model & provider pickers live in the chat composer, which has noPortalContainerProviderancestor) resolved their portal container tonulland mounted to no DOM node — they opened in state but rendered nowhere.Change
usePortalContainer(container?)now resolvesexplicit → provider → undefinedand never returnsnull; the "no container" sentinel isundefinedto match Base UI's contract.?? undefinedband-aids inCombobox,Select,Popover, andDropdownMenu— they now just callusePortalContainer(container).Tests
portal-container.test.tsxguards the resolver contract (nevernull; normalizes anullprovider; explicit wins).combobox.test.tsxasserts the popup portals intodocument.body.null-leaking behavior (combobox open/select tests go red) and pass with the fix.ELI5
Imagine a magical doorway that decides where popup menus (like dropdowns and options) should appear on the screen. This PR fixes a broken doorway that prevented popups from showing up when opened outside certain panels—now the doorway always works and shows popups in the right place (the main document area) as a fallback.
Problem
Base UI's FloatingPortal treats
container={null}as "not ready — render nothing" whileundefinedmeans "portal to document.body". The PortalContainerContext was defaulting tonull, which caused popups opened outside a SideDialog (such as model and provider pickers in the chat composer) to fail rendering because there was no valid DOM node to attach to.Solution
The
usePortalContainerhook was redesigned to:containerparameter that can explicitly override the default behaviorundefinednull; always returnundefinedas the "no container" sentinel to match Base UI's expectationsThe PortalContainerContext now treats
undefinedas the default (fall back to document.body) and normalizes any incomingnullvalues toundefined.Changes Made
PortalContainertype, updatedusePortalContainerto accept an optional container override and exclude null from its return type, and normalized null values in the provider contextcontainer ?? portalContainer ?? undefined) with direct calls tousePortalContainer(container)document.bodywhen no portal container provider is presentTesting
Tests were added to verify the contract of the portal-container resolver and confirm that popups now mount correctly when opened outside a SideDialog. Tests were verified to fail under the previous null-leaking behavior and pass with the fix.