diff --git a/.changeset/perf-dialog-has-selector.md b/.changeset/perf-dialog-has-selector.md index c90404ca952..631826c682e 100644 --- a/.changeset/perf-dialog-has-selector.md +++ b/.changeset/perf-dialog-has-selector.md @@ -2,7 +2,10 @@ '@primer/react': patch --- -perf(Dialog): Replace body:has() with direct class and scope footer selector +perf(Dialog): Add feature flag for CSS :has() selector performance optimization -- Replace `body:has(.Dialog.DisableScroll)` with direct `body.DialogScrollDisabled` class -- Scope `:has(.Footer)` to direct child with `>` combinator for O(1) lookup +- Add `primer_react_css_has_selector_perf` feature flag (default: false) +- When flag is OFF: uses legacy `body:has(.Dialog.DisableScroll)` selector +- When flag is ON: uses optimized direct `body.DialogScrollDisabled` class with ref counting +- Scope footer `:has(.Footer)` to direct child with `>` combinator for O(1) lookup +- Enables gradual rollout and easy rollback of performance optimization diff --git a/packages/react/src/Dialog/Dialog.module.css b/packages/react/src/Dialog/Dialog.module.css index 65bc9b6516e..dbf6df6f12f 100644 --- a/packages/react/src/Dialog/Dialog.module.css +++ b/packages/react/src/Dialog/Dialog.module.css @@ -229,10 +229,26 @@ } } +/* DisableScroll class is added to Dialog when scroll should be disabled on body */ +.DisableScroll { + /* This class is used as a selector target for the legacy :has() CSS selector */ +} + +/* + * LEGACY: Scoped :has() selector with negation guard + * Only evaluates when data-dialog-scroll-optimized is NOT present on body. + * When the attribute IS present (flag ON), browser skips :has() evaluation + * because the :not() check fails first (O(1) attribute lookup). + */ +body:not([data-dialog-scroll-optimized]):has(.Dialog.DisableScroll) { + /* stylelint-disable-next-line primer/spacing */ + padding-right: var(--prc-dialog-scrollgutter) !important; + overflow: hidden !important; +} + /* - * PERFORMANCE: Using a direct class on body instead of body:has(.Dialog.DisableScroll) - * The :has() selector forces the browser to scan the entire DOM on every style recalc, - * which is O(n) where n is the number of DOM nodes. This is expensive on large pages. + * PERFORMANCE OPTIMIZATION: Direct class on body - O(1) lookup + * Active when primer_react_css_has_selector_perf flag is ON */ body:global(.DialogScrollDisabled) { /* stylelint-disable-next-line primer/spacing */ diff --git a/packages/react/src/Dialog/Dialog.test.tsx b/packages/react/src/Dialog/Dialog.test.tsx index fb693f07c15..2d41fe267b4 100644 --- a/packages/react/src/Dialog/Dialog.test.tsx +++ b/packages/react/src/Dialog/Dialog.test.tsx @@ -6,6 +6,7 @@ import {Dialog} from './Dialog' import {Button} from '../Button' import {implementsClassName} from '../utils/testing' import classes from './Dialog.module.css' +import {FeatureFlags} from '../FeatureFlags' describe('Dialog', () => { implementsClassName(Dialog, classes.Dialog) @@ -338,4 +339,107 @@ describe('Footer button loading states', () => { expect(publishButton).toHaveAttribute('data-loading', 'true') expect(deleteButton).not.toHaveAttribute('data-loading', 'true') }) + + describe('primer_react_css_has_selector_perf feature flag', () => { + it('does not add data-dialog-scroll-optimized attribute when flag is OFF', () => { + const {unmount} = render( + + {}}>Dialog content + , + ) + + expect(document.body.hasAttribute('data-dialog-scroll-optimized')).toBe(false) + expect(document.body.classList.contains('DialogScrollDisabled')).toBe(true) + + unmount() + + expect(document.body.classList.contains('DialogScrollDisabled')).toBe(false) + }) + + it('adds data-dialog-scroll-optimized attribute when flag is ON', () => { + const {unmount} = render( + + {}}>Dialog content + , + ) + + expect(document.body.hasAttribute('data-dialog-scroll-optimized')).toBe(true) + expect(document.body.classList.contains('DialogScrollDisabled')).toBe(true) + + unmount() + + expect(document.body.hasAttribute('data-dialog-scroll-optimized')).toBe(false) + expect(document.body.classList.contains('DialogScrollDisabled')).toBe(false) + }) + + it('handles multiple dialogs with ref counting when flag is ON', () => { + const {unmount: unmount1} = render( + + {}}>Dialog 1 + , + ) + + expect(document.body.hasAttribute('data-dialog-scroll-optimized')).toBe(true) + expect(document.body.classList.contains('DialogScrollDisabled')).toBe(true) + + // Render second dialog + const {unmount: unmount2} = render( + + {}}>Dialog 2 + , + ) + + // Attribute should still be present + expect(document.body.hasAttribute('data-dialog-scroll-optimized')).toBe(true) + expect(document.body.classList.contains('DialogScrollDisabled')).toBe(true) + + // Unmount first dialog + unmount1() + + // Attribute and class should still be present (second dialog is still open) + expect(document.body.hasAttribute('data-dialog-scroll-optimized')).toBe(true) + expect(document.body.classList.contains('DialogScrollDisabled')).toBe(true) + + // Unmount second dialog + unmount2() + + // Now both should be removed + expect(document.body.hasAttribute('data-dialog-scroll-optimized')).toBe(false) + expect(document.body.classList.contains('DialogScrollDisabled')).toBe(false) + }) + + it('handles multiple dialogs correctly when flag is OFF', () => { + const {unmount: unmount1} = render( + + {}}>Dialog 1 + , + ) + + expect(document.body.hasAttribute('data-dialog-scroll-optimized')).toBe(false) + expect(document.body.classList.contains('DialogScrollDisabled')).toBe(true) + + // Render second dialog + const {unmount: unmount2} = render( + + {}}>Dialog 2 + , + ) + + // Attribute should not be present, class should be present + expect(document.body.hasAttribute('data-dialog-scroll-optimized')).toBe(false) + expect(document.body.classList.contains('DialogScrollDisabled')).toBe(true) + + // Unmount first dialog + unmount1() + + // Class should still be present (second dialog is still open) + expect(document.body.classList.contains('DialogScrollDisabled')).toBe(true) + + // Unmount second dialog + unmount2() + + // Class should be removed + expect(document.body.classList.contains('DialogScrollDisabled')).toBe(false) + }) + }) }) diff --git a/packages/react/src/Dialog/Dialog.tsx b/packages/react/src/Dialog/Dialog.tsx index 2fe0aac7dc3..39d1a58bb9d 100644 --- a/packages/react/src/Dialog/Dialog.tsx +++ b/packages/react/src/Dialog/Dialog.tsx @@ -16,6 +16,7 @@ import type {ForwardRefComponent as PolymorphicForwardRefComponent} from '../uti import classes from './Dialog.module.css' import {clsx} from 'clsx' import {useSlots} from '../hooks/useSlots' +import {useFeatureFlag} from '../FeatureFlags' /* Dialog Version 2 */ @@ -289,22 +290,42 @@ const _Dialog = React.forwardRef { const scrollbarWidth = window.innerWidth - document.body.clientWidth - // If the dialog is rendered, we add a class to the dialog element to disable - dialogRef.current?.classList.add(classes.DisableScroll) - // and set a CSS variable to the scrollbar width so that the dialog can - // account for the scrollbar width when calculating its width. + const dialog = dialogRef.current + + // Add DisableScroll class to this dialog + dialog?.classList.add(classes.DisableScroll) document.body.style.setProperty('--prc-dialog-scrollgutter', `${scrollbarWidth}px`) - // Add class directly to body for scroll disabling (instead of using CSS :has() selector) - // This avoids expensive DOM-wide style recalcs on every interaction - document.body.classList.add('DialogScrollDisabled') + if (usePerfOptimization) { + // Optimized path: set attribute and class on body + document.body.setAttribute('data-dialog-scroll-optimized', '') + document.body.classList.add('DialogScrollDisabled') + } else { + // Legacy path: only add class (CSS :has() selector handles the rest) + document.body.classList.add('DialogScrollDisabled') + } + return () => { - document.body.classList.remove('DialogScrollDisabled') - document.body.style.removeProperty('--prc-dialog-scrollgutter') + // Remove DisableScroll class from this dialog + dialog?.classList.remove(classes.DisableScroll) + + // Query DOM to check if any other dialogs with DisableScroll remain + const remainingDialogs = document.querySelectorAll(`.${classes.DisableScroll}`) + + if (remainingDialogs.length === 0) { + // No more dialogs open, clean up body + document.body.style.removeProperty('--prc-dialog-scrollgutter') + document.body.classList.remove('DialogScrollDisabled') + if (usePerfOptimization) { + document.body.removeAttribute('data-dialog-scroll-optimized') + } + } } - }, []) + }, [usePerfOptimization]) const header = slots.header ?? (renderHeader ?? DefaultHeader)(defaultedProps) const body = slots.body ?? (renderBody ?? DefaultBody)({...defaultedProps, children: childrenWithoutSlots}) diff --git a/packages/react/src/FeatureFlags/DefaultFeatureFlags.ts b/packages/react/src/FeatureFlags/DefaultFeatureFlags.ts index d5c1cf6ab92..4a2c0896f50 100644 --- a/packages/react/src/FeatureFlags/DefaultFeatureFlags.ts +++ b/packages/react/src/FeatureFlags/DefaultFeatureFlags.ts @@ -3,6 +3,7 @@ import {FeatureFlagScope} from './FeatureFlagScope' export const DefaultFeatureFlags = FeatureFlagScope.create({ primer_react_action_list_item_as_button: false, primer_react_breadcrumbs_overflow_menu: false, + primer_react_css_has_selector_perf: false, primer_react_overlay_overflow: false, primer_react_select_panel_fullscreen_on_narrow: false, primer_react_select_panel_order_selected_at_top: false,