perf(Dialog): Replace body:has() with direct class and scope footer selector #7329

mattcosta7 · 2025-12-15T14:54:08Z

Summary

Performance optimizations for Dialog to improve INP via a feature flag (primer_react_css_has_selector_perf).

Changes

Feature flag for body:has() optimization - Add primer_react_css_has_selector_perf flag to toggle between legacy :has() selector and direct data attribute on body
Dual CSS path - Legacy path uses body:not([data-dialog-scroll-optimized]):has(.Dialog.DisableScroll), optimized path uses body[data-dialog-scroll-disabled]
Provider-level optimization marker - data-dialog-scroll-optimized set at FeatureFlags provider level to short-circuit :has() evaluation with O(1) negation check
Dialog-level scroll disabled marker - data-dialog-scroll-disabled set on body when a dialog mounts (optimized path only)

Expected INP Impact

Scenario	Before	After	Improvement
Worst case (Dialog open on large page, 10k+ nodes)	~100-200ms style recalc	<5ms	95-98% reduction
Average case (Dialog on typical page)	~30-60ms style recalc	<3ms	90-95% reduction
Best case (Dialog on small page)	~10-20ms style recalc	<2ms	80-90% reduction

Why this matters

body:has(.Dialog.DisableScroll) is extremely expensive:

Fires on every style recalculation across the entire page
Forces full DOM traversal from body element
Impacts all interactions while dialog is open, not just dialog interactions

By using body[data-dialog-scroll-disabled] (a direct attribute selector), we eliminate all traversal cost.

Why we didn't scope :has() to portal roots

An alternative considered was scoping the :has() selector to portal root containers (e.g., [data-portal-root]:has(.Dialog.DisableScroll)). This was rejected because:

Still O(n) complexity - registerPortalRoot sets data-portal-root on portal containers, but scoping :has() to these elements still requires traversing all descendants to find matching elements
No browser optimization - Browsers cannot optimize :has() traversal regardless of the scoping ancestor; the selector must evaluate all descendants
Direct attribute is always O(1) - Using body[data-dialog-scroll-disabled] is a simple attribute lookup with constant time complexity, regardless of DOM size

The only way to achieve true O(1) performance is to avoid :has() entirely and use a direct attribute on body.

Why we kept :has() for footer detection

The .Dialog:has(.Footer) selector was considered for optimization but left unchanged because:

Dialogs are small - The :has() scope is limited to the dialog element itself, which contains a small, bounded number of children
Acceptable cost - Traversing dialog children is fast enough that optimization provides negligible benefit
Simpler implementation - Avoiding additional data attributes for footer detection keeps the code cleaner

Implementation Details

Flag OFF (default): Uses legacy body:not([data-dialog-scroll-optimized]):has(.Dialog.DisableScroll) selector
Flag ON: Uses body[data-dialog-scroll-disabled] direct attribute selector
Short-circuit mechanism: When flag is ON, data-dialog-scroll-optimized is set on body by the FeatureFlags provider, causing the :not() check to fail first (O(1)), preventing the expensive :has() evaluation
Ref counting: Multiple dialogs and providers are handled via ref counting to ensure proper cleanup
.

…elector - Replace body:has(.Dialog.DisableScroll) with direct body.DialogScrollDisabled class - Scope :has(.Footer) to direct child with > combinator for O(1) lookup Part of #7312

changeset-bot · 2025-12-15T14:54:12Z

🦋 Changeset detected

Latest commit: 1f1be39

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@primer/react	Patch

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

github-actions · 2025-12-15T14:54:41Z

👋 Hi, this pull request contains changes to the source code that github/github-ui depends on. If you are GitHub staff, test these changes with github/github-ui using the integration workflow. Or, apply the integration-tests: skipped manually label to skip these checks.

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

Copilot

Pull request overview

This PR optimizes Dialog component performance by replacing expensive CSS selectors with more efficient alternatives. The main change replaces body:has(.Dialog.DisableScroll) with a direct class DialogScrollDisabled on the body element, and scopes the footer selector using the direct child combinator (>). These optimizations target INP (Interaction to Next Paint) metrics by reducing style recalculation costs, especially on large pages with thousands of DOM nodes.

Key Changes:

Add/remove DialogScrollDisabled class directly to body element via JavaScript instead of relying on CSS :has() selector
Scope footer selector from :has(.Footer) to :has(> .Footer) for O(1) lookup instead of full subtree search

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File	Description
packages/react/src/Dialog/Dialog.tsx	Adds JavaScript logic to add/remove `DialogScrollDisabled` class to body element in useEffect with cleanup function
packages/react/src/Dialog/Dialog.module.css	Replaces expensive `body:has(.Dialog.DisableScroll)` selector with `body:global(.DialogScrollDisabled)` and scopes footer selector to direct child
.changeset/perf-dialog-has-selector.md	Documents the performance optimization changes in changeset for release notes

packages/react/src/Dialog/Dialog.tsx

siddharthkp

This seems good on the surface, but because it's visual changes, can you please verify this has no accidental side effects in dotcom?

Orrrr, we can feature flag the change to be safe

mattcosta7 · 2025-12-19T22:43:55Z

This seems good on the surface, but because it's visual changes, can you please verify this has no accidental side effects in dotcom?

Orrrr, we can feature flag the change to be safe

@siddharthkp totally onboard with flagging! Is there a good pattern for that when the flag relies on changes to global styles and an eager matching selector?

if the old :has stype exists in the document at all, we take the cost of matching it - but i'm not sure how we'd actively flag that here! Any thoughts?

Otherwise i'll pick this up in Jan and do some thorough testing too

siddharthkp · 2025-12-23T20:02:42Z

Is there a good pattern for that when the flag relies on changes to global styles...

Not yet. But adding a data-attribute is probably the way to go. However, in this case, because you are adding the className targeted by the global style styles, you can check for the feature flag in javascript.

...and an eager matching selector?
if the old :has stype exists in the document at all, we take the cost of matching it - but i'm not sure how we'd actively flag that here! Any thoughts?

Yeah. That's tricky.

Does doing something like this help? I'm guessing if the data-optimised doesn't match, it's not expensive?

/* we need to add both of these */

/* equivalent to old selector with default value of flag */
.Dialog[data-optimised="false"]:has(.Footer) {

/* optimised behavior when feature flag is true */
.Dialog[data-optimised="true"]:has(> .Footer) {

) Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: mattcosta7 <8616962+mattcosta7@users.noreply.github.com>

Copilot

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.

packages/react/src/Dialog/Dialog.tsx

Copilot · 2025-12-29T16:51:40Z

packages/react/src/Dialog/Dialog.tsx

+          document.body.removeAttribute('data-dialog-scroll-optimized')
+          document.body.removeAttribute('data-dialog-scroll-disabled')
+        } else {
+          document.body.classList.remove('DialogScrollDisabled')


Since the DialogScrollDisabled class added on line 309 has no effect (it's not used in any CSS rules), this removal is also unnecessary and should be deleted along with the addition.

packages/react/src/Dialog/Dialog.tsx

.changeset/perf-dialog-has-selector.md

…der level (#7393) Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: mattcosta7 <8616962+mattcosta7@users.noreply.github.com>

mattcosta7 · 2025-12-30T04:59:49Z

Is there a good pattern for that when the flag relies on changes to global styles...

Not yet. But adding a data-attribute is probably the way to go. However, in this case, because you are adding the className targeted by the global style styles, you can check for the feature flag in javascript.

...and an eager matching selector?
if the old :has stype exists in the document at all, we take the cost of matching it - but i'm not sure how we'd actively flag that here! Any thoughts?

Yeah. That's tricky.

Does doing something like this help? I'm guessing if the data-optimised doesn't match, it's not expensive?
/* we need to add both of these */

/* equivalent to old selector with default value of flag */
.Dialog[data-optimised="false"]:has(.Footer) {

/* optimised behavior when feature flag is true */
.Dialog[data-optimised="true"]:has(> .Footer) {

Ok, I think this handles it

The flagging is a bit rough, since we need to add an attribute to the body when the flag is enabled (and increment/decrement a count when doing so), but I think that handles it well enough - and expect we should be quickly able to remove that afterwards

I removed the footer has query changes for now as well, the actual dialog content should be small enough that has is well enough scoped there as is - really just the body unbounded has is the main issue

…#7398) Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: mattcosta7 <8616962+mattcosta7@users.noreply.github.com>

primer-integration · 2025-12-30T16:33:09Z

👋 Hi from github/github-ui! Your integration PR is ready: https://github.com/github/github-ui/pull/9550

primer-integration · 2025-12-30T16:52:32Z

🔬 github-ui Integration Test Results

Check	Status	Details
CI	✅ Passed	View run
Projects (Memex)	✅ Passed	View run
VRT	✅ Passed	View run

✨ All checks passed! Your integration PR is ready for review.

perf(Dialog): Replace body:has() with direct class and scope footer s…

4580808

…elector - Replace body:has(.Dialog.DisableScroll) with direct body.DialogScrollDisabled class - Scope :has(.Footer) to direct child with > combinator for O(1) lookup Part of #7312

github-actions bot added the integration-tests: recommended This change needs to be tested for breaking changes. See https://arc.net/l/quote/tdmpakpm label Dec 15, 2025

mattcosta7 deployed to github-pages December 15, 2025 15:04 — with GitHub Actions View deployment

github-actions bot temporarily deployed to storybook-preview-7329 December 15, 2025 15:05 Inactive

Merge branch 'main' into perf/dialog-has-selector

6a8cfc6

mattcosta7 marked this pull request as ready for review December 16, 2025 04:45

mattcosta7 requested a review from a team as a code owner December 16, 2025 04:45

mattcosta7 requested review from Copilot and siddharthkp December 16, 2025 04:45

mattcosta7 self-assigned this Dec 16, 2025

Copilot started reviewing on behalf of mattcosta7 December 16, 2025 04:46 View session

mattcosta7 commented Dec 16, 2025

View reviewed changes

packages/react/src/Dialog/Dialog.module.css Outdated Show resolved Hide resolved

mattcosta7 commented Dec 16, 2025

View reviewed changes

packages/react/src/Dialog/Dialog.module.css Show resolved Hide resolved

Copilot AI reviewed Dec 16, 2025

View reviewed changes

packages/react/src/Dialog/Dialog.tsx Outdated Show resolved Hide resolved

packages/react/src/Dialog/Dialog.tsx Outdated Show resolved Hide resolved

mattcosta7 deployed to github-pages December 16, 2025 04:54 — with GitHub Actions View deployment

github-actions bot temporarily deployed to storybook-preview-7329 December 16, 2025 04:54 Inactive

siddharthkp reviewed Dec 16, 2025

View reviewed changes

Merge branch 'main' into perf/dialog-has-selector

be7ab8e

mattcosta7 deployed to github-pages December 22, 2025 17:27 — with GitHub Actions View deployment

github-actions bot temporarily deployed to storybook-preview-7329 December 22, 2025 17:28 Inactive

Copilot AI mentioned this pull request Dec 25, 2025

feat: Add feature flag for Dialog scroll performance optimization #7366

Merged

13 tasks

feat: Add feature flag for Dialog scroll performance optimization (#7366

a305fed

) Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: mattcosta7 <8616962+mattcosta7@users.noreply.github.com>

mattcosta7 deployed to github-pages December 25, 2025 21:18 — with GitHub Actions View deployment

github-actions bot temporarily deployed to storybook-preview-7329 December 25, 2025 21:18 Inactive

Merge branch 'main' into perf/dialog-has-selector

a5f932b

mattcosta7 deployed to github-pages December 26, 2025 17:50 — with GitHub Actions View deployment

mattcosta7 deployed to github-pages December 27, 2025 16:23 — with GitHub Actions View deployment

github-actions bot temporarily deployed to storybook-preview-7329 December 27, 2025 16:23 Inactive

mattcosta7 requested a review from siddharthkp December 27, 2025 16:25

Merge branch 'main' into perf/dialog-has-selector

7944449

mattcosta7 requested a review from Copilot December 29, 2025 16:47

Copilot started reviewing on behalf of mattcosta7 December 29, 2025 16:47 View session

Copilot AI reviewed Dec 29, 2025

View reviewed changes

mattcosta7 deployed to github-pages December 29, 2025 16:57 — with GitHub Actions View deployment

github-actions bot temporarily deployed to storybook-preview-7329 December 29, 2025 16:57 Inactive

Copilot AI mentioned this pull request Dec 29, 2025

perf(Dialog): Move data-dialog-scroll-optimized to FeatureFlags provider level #7393

Merged

13 tasks

perf(Dialog): Move data-dialog-scroll-optimized to FeatureFlags provi…

7d01d84

…der level (#7393) Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: mattcosta7 <8616962+mattcosta7@users.noreply.github.com>

mattcosta7 deployed to github-pages December 30, 2025 05:00 — with GitHub Actions View deployment

github-actions bot temporarily deployed to storybook-preview-7329 December 30, 2025 05:00 Inactive

Merge branch 'main' into perf/dialog-has-selector

eaf4653

mattcosta7 deployed to github-pages December 30, 2025 05:43 — with GitHub Actions View deployment

github-actions bot temporarily deployed to storybook-preview-7329 December 30, 2025 05:43 Inactive

siddharthkp approved these changes Dec 30, 2025

View reviewed changes

Copilot AI and others added 2 commits December 30, 2025 11:29

Refactor Dialog to check DOM attribute instead of useFeatureFlag hook (…

36ecba6

…#7398) Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: mattcosta7 <8616962+mattcosta7@users.noreply.github.com>

Merge branch 'main' into perf/dialog-has-selector

1f1be39

github-actions bot requested a deployment to storybook-preview-7329 December 30, 2025 16:33 Abandoned

mattcosta7 deployed to github-pages December 30, 2025 16:42 — with GitHub Actions View deployment

github-actions bot deployed to storybook-preview-7329 December 30, 2025 16:42 View deployment

mattcosta7 added this pull request to the merge queue Dec 30, 2025

Merged via the queue into main with commit 501a41f Dec 30, 2025
52 checks passed

mattcosta7 deleted the perf/dialog-has-selector branch December 30, 2025 17:31

primer bot mentioned this pull request Dec 30, 2025

Release tracking #7365

Open

perf(Dialog): Replace body:has() with direct class and scope footer selector #7329

perf(Dialog): Replace body:has() with direct class and scope footer selector #7329

Uh oh!

Conversation

mattcosta7 commented Dec 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Changes

Expected INP Impact

Why this matters

Why we didn't scope :has() to portal roots

Why we kept :has() for footer detection

Implementation Details

Uh oh!

changeset-bot bot commented Dec 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🦋 Changeset detected

Uh oh!

github-actions bot commented Dec 15, 2025

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

siddharthkp left a comment

Choose a reason for hiding this comment

Uh oh!

mattcosta7 commented Dec 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

siddharthkp commented Dec 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI Dec 29, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

mattcosta7 commented Dec 30, 2025

Uh oh!

primer-integration bot commented Dec 30, 2025

Uh oh!

primer-integration bot commented Dec 30, 2025

🔬 github-ui Integration Test Results

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

mattcosta7 commented Dec 15, 2025 •

edited

Loading

changeset-bot bot commented Dec 15, 2025 •

edited

Loading

mattcosta7 commented Dec 19, 2025 •

edited

Loading

siddharthkp commented Dec 23, 2025 •

edited

Loading