Skip to content

fix!: default overlay portal target to a document.body root - #2808

Merged
mariechatfield merged 7 commits into
mainfrom
fix/marie/popper-relative-positioning
Sep 21, 2026
Merged

mariechatfield merged 7 commits into
mainfrom
fix/marie/popper-relative-positioning

Conversation

@mariechatfield

@mariechatfield mariechatfield commented Sep 18, 2026

Copy link
Copy Markdown
Contributor

Why

React-aria's Popover positions itself with position: absolute. CSS's containing-block rule for an absolutely-positioned element is: the nearest ancestor with position != static, or the viewport if there is none — and that's a real walk up the actual DOM, through every ancestor, not just the ones inside this SDK's own component tree.

position: relative is an extremely ordinary thing to set on a layout wrapper — scroll containers, sticky headers, grid systems (Bootstrap's .col-* classes set it by default) all reach for it. We've hit this exact mechanism more than once now, including in our own SDK Dev App harness (main.main-content, set for the design-comments tool).

Any such ancestor between wherever a partner mounts GustoProvider and document.body puts react-aria's available-space math on a different code path — one that computes the trigger's offset relative to that ancestor instead of the viewport. This doesn't collapse every popover under a positioned ancestor identically: the resulting error scales with the specific geometry at the moment the popover opens (trigger position, container size, scroll offset), so some placements come out fine or nearly so while others land at max-height: 0px. It isn't flaky, though — for a given layout, scroll position, and trigger, the outcome is deterministic and reproducible, not random.

The first few rows in this table are always fine, and the last few rows show the popover beautifully... if there isn't any position: relative above the parent:
Screenshot 2026-09-18 at 3 48 38 PM

But with the position: relative breaking the link between the viewport and the ancestor, a table with enough rows will eventually math out to an "empty" menu:
Screenshot 2026-09-18 at 4 19 43 PM

We don't own the ancestor markup that triggers this — it's the host page's own layout, not ours to change. So there's no CSS property we can set on our own root to fix it from the inside.

What changed

The industry-standard fix for this exact class of bug: portal overlay content to document.body instead of rendering in place. This isn't unusual — it's what React Aria's own PortalProvider docs recommend ("portal to the root of the entire application... outside any possible overflow or stacking contexts"), and what Floating UI and Radix Primitives both default to for the same reason.

  • ThemeProvider now creates a themed portal root and appends it directly to document.body (lifecycle-managed, removed on unmount) instead of using the SDK's in-place root article as the default portal target. An explicit portalContainer prop still overrides this, unchanged.
  • MultiSelectComboBox gains a portalContainer prop for parity with Select/ComboBox/Menu/DatePicker, which already had it.

This does mean we break containment — here's what we did about it

Escaping into document.body isn't free, so this includes explicit safety nets, not just the mechanism change:

  • A distinguishing class, not just a shared one. The portal root and the in-place root both carry the GSDK class (required so overlay content still inherits SDK theming), but the portal root additionally carries GSDK-portal-root. Without this, a host page writing a plausible global .GSDK { overflow: hidden } (undocumented, generic-looking class name) would silently clip every overlay again — this time via partner CSS instead of host-page CSS.
  • Specificity-based defense, not just an opt-out. .GSDK.GSDK-portal-root ships a rule resetting position/overflow/transform/filter/contain to safe values. Two classes beat a plain .GSDK override on specificity, so the dangerous case is neutralized by default (verified live in Chrome, not just asserted) without reaching for !important.
  • Shadow DOM is a real, named gap. If a host renders GustoProvider inside a shadow root without an explicit portalContainer, the new default portal root — a document.body child — sits outside that shadow boundary, breaking style encapsulation. This is now called out explicitly as required, not optional, in both the portalContainer TSDoc and the theming guide.
  • Custom component implementations aren't silently exempt. portalContainer is part of the public prop contract for Select/ComboBox/Menu/DatePicker/MultiSelectComboBox — the SDK's own screens pass a live value at specific call sites (fields inside SDK-managed modals) regardless of whether the default or a partner-supplied component occupies that slot. Documented in the component-adapter guide, since nothing previously said a custom implementation should honor it.
  • Regression-tested the causal invariant, not the implementation detail. The new test asserts no ancestor between an opened overlay and document.body establishes a CSS containing block — this mirrors react-aria's own isContainingBlock check, holds in both jsdom and real Chrome, and survives future changes to the portal root's exact shape.
  • Scanned every existing usage before changing the default. Every current Select/ComboBox/Menu/DatePicker/MultiSelectComboBox render site inside an SDK-managed modal already passes an explicit portalContainer (or renders inside Dialog, which auto-scopes theming regardless of the global default) — zero at-risk usages found.

Verification

  • Full suite green: 374 files / 4113 tests, tsc --noEmit clean, eslint clean, npm run build clean, npm run docs:build/docs:lint clean.
  • Live-verified in the SDK Dev App: the exact repro from the investigation (scrolled employee list, row-action kebab menu) now renders at real size (max-height: 126px, not 0px) inside the new body-level portal root.
  • Verified live in Chrome (not just jsdom, which doesn't implement CSS specificity correctly for this case) that the GSDK.GSDK-portal-root defensive rule actually wins over a conflicting .GSDK override.

Test plan

  • npm run test -- --run
  • npm run build
  • npx tsc --noEmit
  • npm run docs:lint / npm run docs:build
  • Manual verification in SDK Dev App via live browser inspection

…ancestor

Asserts that no ancestor between an opened Select listbox and document.body
establishes a new CSS containing block (position/transform/filter/contain),
mirroring react-aria's own containing-block check. Currently red — the fix
(defaulting overlay portals to a document.body-appended container) lands in
a follow-up commit.
Select, ComboBox, Menu, and DatePicker all accept a per-instance
portalContainer override for their Popover; MultiSelectComboBox was
missing it, forcing it to always use the theme default.
react-aria's calculatePosition resolves an overlay's containing block by
walking up the DOM; when a host page sets position/transform/filter/contain
on any ancestor between the SDK root and the viewport, overlays collapse
(max-height: 0) instead of measuring available space correctly. Portalling
in-place into the SDK's own root article inherited whatever containing
block the host page happened to introduce.

ThemeProvider now creates a dedicated themed root and appends it directly
to document.body (lifecycle-managed, removed on unmount), and uses that as
the default portal container for every overlay instead of the in-place
article. An explicit portalContainer prop still overrides this.

BREAKING CHANGE: the default portal container for SDK overlays (Select,
ComboBox, Menu, DatePicker, MultiSelectComboBox) is no longer the SDK's
root article element. Consumers relying on overlays rendering in-place
inside the SDK's DOM subtree (e.g. via CSS descendant selectors scoped to
the SDK root) should pass an explicit portalContainer to GustoProvider.
The default portal root and the SDK's in-place root both carried the
same bare `GSDK` class, with nothing distinguishing them. A host page
writing a global `.GSDK { overflow: hidden }` or `.GSDK { position:
relative }` override — plausible, since `.GSDK` is undocumented and
looks like a generic top-level hook — would now also reach the portal
root, reintroducing this branch's exact containing-block bug via
partner CSS instead of host-page CSS.

Give the portal root an additional `GSDK-portal-root` class (matching
its existing data-testid) so it can be excluded from a `.GSDK`
override, and ship a defensive rule that resets position/overflow/
transform/filter/contain on the two-class selector — its specificity
beats a plain `.GSDK` override without needing `!important`.
…OM requirement

Nothing previously explained that overlay components portal outside
the SDK's in-place root, that the portal root shares the plain `GSDK`
class, or that a shadow-DOM host must pass an explicit portalContainer
(document.body sits outside every shadow boundary, so the SDK's
default portal root would otherwise escape shadow-scoped styling).

Also refreshes six portalContainer TSDoc comments that described the
old in-place default ("overrides the default SDK root container from
context") to describe the new document.body-appended default.
…tations

portalContainer isn't just internal plumbing for the SDK's own default
components — the SDK's own screens (e.g. fields rendered inside an
SDK-managed modal) pass a live value into whatever component occupies
the Select/ComboBox/Menu/DatePicker/MultiSelectComboBox slot, default
or partner-supplied. A custom implementation that ignores it falls
back to its own library's default portal target and may render outside
the modal it's meant to appear in. Nothing in the component-adapter
guide previously said so.
@mariechatfield mariechatfield changed the title fix!: default overlay portal target to a document.body-appended root fix!: default overlay portal target to a document.body root Sep 18, 2026
@mariechatfield
mariechatfield marked this pull request as ready for review September 19, 2026 00:01
@mariechatfield
mariechatfield requested a review from a team as a code owner September 19, 2026 00:01

@serikjensen serikjensen left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I worry a little bit about this approach and if it has the potential to cause regressions for partner builds. You might consider going this direction instead and just disabling relative positioning in sdk app when comments are not visible https://gustohq.atlassian.net/browse/SDK-1323

If we do go this direction, i think we need to

  • Thoroughly thoroughly test all overlaying elements across sdk app of various component types and with scroll positions
  • Give adopting partners a heads up about this change so they can be paying attention to the types of errors it might introduce

const defaultPortalRoot = document.createElement('div')
defaultPortalRoot.className = 'GSDK GSDK-portal-root'
defaultPortalRoot.setAttribute('data-testid', 'GSDK-portal-root')
document.body.appendChild(defaultPortalRoot)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i worry a little bit about the implications for partner implementations and incidentally causing regressions. This seems to make assumptions about how the document body and descendent SDK rendered DOM will behave with respect to each other. I've already proven that relative positioning in a container can mess with it

@mariechatfield
mariechatfield added this pull request to the merge queue Sep 21, 2026
Merged via the queue into main with commit 5cfee85 Sep 21, 2026
45 checks passed
@mariechatfield
mariechatfield deleted the fix/marie/popper-relative-positioning branch September 21, 2026 19:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants