Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Dec 30, 2025

Thanks for asking me to work on this. I will get started on it and keep this PR's description up to date as I form a plan and make progress.

Original prompt

Summary

Optimize the body:has(.Dialog.DisableScroll) selector by scoping it to portal root containers, significantly reducing the DOM search space.

Problem

The current selector on line 232 of Dialog.module.css:

body:has(.Dialog.DisableScroll) {
  padding-right: var(--prc-dialog-scrollgutter) !important;
  overflow: hidden !important;
}

This scans the entire document on every DOM mutation to check if any .Dialog.DisableScroll element exists. This is O(n) where n = all DOM nodes.

Solution

Dialogs are always rendered inside portal roots:

  1. Default portal root: #__primerPortalRoot__ (created automatically by Portal.tsx)
  2. Custom portal roots: Elements with [data-portal-root] attribute (used when registerPortalRoot() is called or when a suitable root exists)

We can scope the :has() selector to only search within these containers:

body:has(:is(#__primerPortalRoot__, [data-portal-root]) .Dialog.DisableScroll) {
  padding-right: var(--prc-dialog-scrollgutter) !important;
  overflow: hidden !important;
}

How This Works

  1. Browser finds #__primerPortalRoot__ via O(1) ID lookup
  2. Browser finds any [data-portal-root] elements
  3. Only scans descendants of those elements for .Dialog.DisableScroll

Since portals typically contain only overlays/dialogs/tooltips, the search space is dramatically smaller than the full document.

Changes Required

packages/react/src/Dialog/Dialog.module.css

Change line 232-236 from:

body:has(.Dialog.DisableScroll) {
  /* stylelint-disable-next-line primer/spacing */
  padding-right: var(--prc-dialog-scrollgutter) !important;
  overflow: hidden !important;
}

To:

/*
 * PERFORMANCE: Scope :has() to portal roots to avoid scanning entire document.
 * Dialogs are always rendered inside Portal, which uses either:
 * - #__primerPortalRoot__ (default, created automatically)
 * - [data-portal-root] (custom portal roots via registerPortalRoot())
 * This reduces the :has() search space from O(document) to O(portal subtree).
 */
body:has(:is(#__primerPortalRoot__, [data-portal-root]) .Dialog.DisableScroll) {
  /* stylelint-disable-next-line primer/spacing */
  padding-right: var(--prc-dialog-scrollgutter) !important;
  overflow: hidden !important;
}

Testing

  1. Verify dialogs still lock body scroll when opened (default portal)
  2. Verify dialogs still lock body scroll when using custom portal roots via registerPortalRoot()
  3. Verify dialogs still lock body scroll when using [data-portal-root] attribute on a container
  4. Verify scroll lock is removed when all dialogs close
  5. Performance testing: measure style recalc time before/after on a large DOM

Benefits

  • No feature flag needed - This is a drop-in CSS optimization
  • No JS changes - Pure CSS change
  • Maintains ergonomics - Still declarative, no manual attribute management
  • Backwards compatible - Works with all existing portal usage patterns

Related

  • Portal implementation: packages/react/src/Portal/Portal.tsx
  • Default portal root ID: PRIMER_PORTAL_ROOT_ID = '__primerPortalRoot__'
  • Custom portal support: registerPortalRoot() function and [data-portal-root] attribute

This pull request was created from Copilot chat.


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

@changeset-bot
Copy link

changeset-bot bot commented Dec 30, 2025

⚠️ No Changeset found

Latest commit: 6f8674e

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

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