diff --git a/.changeset/perf-dialog-has-selector.md b/.changeset/perf-dialog-has-selector.md new file mode 100644 index 00000000000..bc6b83d2b5e --- /dev/null +++ b/.changeset/perf-dialog-has-selector.md @@ -0,0 +1,10 @@ +--- +'@primer/react': patch +--- + +perf(Dialog): Add feature flag for CSS :has() selector performance optimization + +- 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[data-dialog-scroll-disabled]` data attribute with ref counting +- 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 d750854c158..a81c278bdbe 100644 --- a/packages/react/src/Dialog/Dialog.module.css +++ b/packages/react/src/Dialog/Dialog.module.css @@ -229,7 +229,29 @@ } } -body:has(.Dialog.DisableScroll) { +/* 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 OPTIMIZATION: Direct attribute on body - O(1) lookup + * Active when primer_react_css_has_selector_perf flag is ON + */ +/* stylelint-disable-next-line selector-no-qualifying-type */ +body[data-dialog-scroll-disabled] { /* stylelint-disable-next-line primer/spacing */ padding-right: var(--prc-dialog-scrollgutter) !important; overflow: hidden !important; diff --git a/packages/react/src/Dialog/Dialog.test.tsx b/packages/react/src/Dialog/Dialog.test.tsx index fb693f07c15..3ce178375ab 100644 --- a/packages/react/src/Dialog/Dialog.test.tsx +++ b/packages/react/src/Dialog/Dialog.test.tsx @@ -1,13 +1,19 @@ import React from 'react' import {render, fireEvent, waitFor} from '@testing-library/react' -import {describe, expect, it, vi} from 'vitest' +import {describe, expect, it, vi, beforeEach} from 'vitest' import userEvent from '@testing-library/user-event' import {Dialog} from './Dialog' import {Button} from '../Button' import {implementsClassName} from '../utils/testing' import classes from './Dialog.module.css' +import {FeatureFlags} from '../FeatureFlags' +import {__resetDialogScrollOptimizedCount} from '../FeatureFlags/FeatureFlags' describe('Dialog', () => { + beforeEach(() => { + __resetDialogScrollOptimizedCount() + }) + implementsClassName(Dialog, classes.Dialog) it('renders with role "dialog" by default', () => { const {getByRole} = render( {}}>Pay attention to me) @@ -338,4 +344,128 @@ 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 or data-dialog-scroll-disabled when flag is OFF', () => { + const {unmount} = render( + + {}}>Dialog content + , + ) + + expect(document.body.hasAttribute('data-dialog-scroll-optimized')).toBe(false) + expect(document.body.hasAttribute('data-dialog-scroll-disabled')).toBe(false) + + unmount() + + expect(document.body.hasAttribute('data-dialog-scroll-optimized')).toBe(false) + expect(document.body.hasAttribute('data-dialog-scroll-disabled')).toBe(false) + }) + + it('adds data-dialog-scroll-optimized at provider level and data-dialog-scroll-disabled when dialog mounts', () => { + const {unmount} = render( + + {}}>Dialog content + , + ) + + // Provider sets data-dialog-scroll-optimized, Dialog sets data-dialog-scroll-disabled + expect(document.body.hasAttribute('data-dialog-scroll-optimized')).toBe(true) + expect(document.body.hasAttribute('data-dialog-scroll-disabled')).toBe(true) + + unmount() + + // Both should be removed on unmount + expect(document.body.hasAttribute('data-dialog-scroll-optimized')).toBe(false) + expect(document.body.hasAttribute('data-dialog-scroll-disabled')).toBe(false) + }) + + it('sets data-dialog-scroll-optimized even when no dialogs are open', () => { + const {unmount} = render( + +
No dialogs here
+ , + ) + + // Provider sets the attribute even without dialogs + expect(document.body.hasAttribute('data-dialog-scroll-optimized')).toBe(true) + expect(document.body.hasAttribute('data-dialog-scroll-disabled')).toBe(false) + + unmount() + + expect(document.body.hasAttribute('data-dialog-scroll-optimized')).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.hasAttribute('data-dialog-scroll-disabled')).toBe(true) + + // Render second dialog + const {unmount: unmount2} = render( + + {}}>Dialog 2 + , + ) + + // Attributes should still be present + expect(document.body.hasAttribute('data-dialog-scroll-optimized')).toBe(true) + expect(document.body.hasAttribute('data-dialog-scroll-disabled')).toBe(true) + + // Unmount first dialog + unmount1() + + // Attributes should still be present (second dialog and provider are still mounted) + expect(document.body.hasAttribute('data-dialog-scroll-optimized')).toBe(true) + expect(document.body.hasAttribute('data-dialog-scroll-disabled')).toBe(true) + + // Unmount second dialog + unmount2() + + // Now both should be removed + expect(document.body.hasAttribute('data-dialog-scroll-optimized')).toBe(false) + expect(document.body.hasAttribute('data-dialog-scroll-disabled')).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.hasAttribute('data-dialog-scroll-disabled')).toBe(false) + + // Render second dialog + const {unmount: unmount2} = render( + + {}}>Dialog 2 + , + ) + + // Attributes should not be present + expect(document.body.hasAttribute('data-dialog-scroll-optimized')).toBe(false) + expect(document.body.hasAttribute('data-dialog-scroll-disabled')).toBe(false) + + // Unmount first dialog + unmount1() + + // Attributes should still not be present + expect(document.body.hasAttribute('data-dialog-scroll-optimized')).toBe(false) + expect(document.body.hasAttribute('data-dialog-scroll-disabled')).toBe(false) + + // Unmount second dialog + unmount2() + + // Attributes should still not be present + expect(document.body.hasAttribute('data-dialog-scroll-optimized')).toBe(false) + expect(document.body.hasAttribute('data-dialog-scroll-disabled')).toBe(false) + }) + }) }) diff --git a/packages/react/src/Dialog/Dialog.tsx b/packages/react/src/Dialog/Dialog.tsx index 0381c445e58..41bfa481dd0 100644 --- a/packages/react/src/Dialog/Dialog.tsx +++ b/packages/react/src/Dialog/Dialog.tsx @@ -291,11 +291,31 @@ 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 + const usePerfOptimization = document.body.hasAttribute('data-dialog-scroll-optimized') + + // Add DisableScroll class to this dialog (for legacy :has() selector path) + dialog?.classList.add(classes.DisableScroll) document.body.style.setProperty('--prc-dialog-scrollgutter', `${scrollbarWidth}px`) + + if (usePerfOptimization) { + // Optimized path: set attribute on body for direct CSS targeting + document.body.setAttribute('data-dialog-scroll-disabled', '') + } + // Legacy path: no action needed - CSS :has(.Dialog.DisableScroll) handles it + + return () => { + dialog?.classList.remove(classes.DisableScroll) + + const remainingDialogs = document.querySelectorAll(`.${classes.DisableScroll}`) + + if (remainingDialogs.length === 0) { + document.body.style.removeProperty('--prc-dialog-scrollgutter') + if (usePerfOptimization) { + document.body.removeAttribute('data-dialog-scroll-disabled') + } + } + } }, []) const header = slots.header ?? (renderHeader ?? DefaultHeader)(defaultedProps) 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, diff --git a/packages/react/src/FeatureFlags/FeatureFlags.tsx b/packages/react/src/FeatureFlags/FeatureFlags.tsx index 2df681f5234..c0e2ac4798f 100644 --- a/packages/react/src/FeatureFlags/FeatureFlags.tsx +++ b/packages/react/src/FeatureFlags/FeatureFlags.tsx @@ -2,16 +2,57 @@ import type React from 'react' import {useContext, useMemo} from 'react' import {FeatureFlagContext} from './FeatureFlagContext' import {FeatureFlagScope, type FeatureFlags} from './FeatureFlagScope' +import useIsomorphicLayoutEffect from '../utils/useIsomorphicLayoutEffect' export type FeatureFlagsProps = React.PropsWithChildren<{ flags: FeatureFlags }> +/** + * Ref count for data-dialog-scroll-optimized attribute management. + * + * NOTE: This is temporary infrastructure while we feature flag the CSS :has() + * performance optimization (primer_react_css_has_selector_perf). Once the flag + * is removed and the optimization is the default behavior, this ref counting + * can be removed - the attribute can simply always be present. + * + * @internal - Not part of the public API + */ +let dialogScrollOptimizedCount = 0 + +/** + * Reset the ref count for testing purposes only. + * + * @internal - Not part of the public API. Only exported for test isolation. + */ +export function __resetDialogScrollOptimizedCount(): void { + dialogScrollOptimizedCount = 0 + document.body.removeAttribute('data-dialog-scroll-optimized') +} + export function FeatureFlags({children, flags}: FeatureFlagsProps) { const parentFeatureFlags = useContext(FeatureFlagContext) const value = useMemo(() => { const scope = FeatureFlagScope.merge(parentFeatureFlags, FeatureFlagScope.create(flags)) return scope }, [parentFeatureFlags, flags]) + + const isOptimizationEnabled = value.enabled('primer_react_css_has_selector_perf') + + // Set body attribute for CSS :has() optimization when flag is enabled + useIsomorphicLayoutEffect(() => { + if (isOptimizationEnabled) { + dialogScrollOptimizedCount++ + document.body.setAttribute('data-dialog-scroll-optimized', '') + + return () => { + dialogScrollOptimizedCount-- + if (dialogScrollOptimizedCount === 0) { + document.body.removeAttribute('data-dialog-scroll-optimized') + } + } + } + }, [isOptimizationEnabled]) + return {children} } diff --git a/packages/react/src/FeatureFlags/__tests__/FeatureFlags.test.tsx b/packages/react/src/FeatureFlags/__tests__/FeatureFlags.test.tsx index cc7b69e802f..035b43bf0eb 100644 --- a/packages/react/src/FeatureFlags/__tests__/FeatureFlags.test.tsx +++ b/packages/react/src/FeatureFlags/__tests__/FeatureFlags.test.tsx @@ -1,8 +1,14 @@ -import {describe, expect, it} from 'vitest' +import {describe, expect, it, beforeEach} from 'vitest' import {render} from '@testing-library/react' import {FeatureFlags, useFeatureFlag} from '../../FeatureFlags' +import {__resetDialogScrollOptimizedCount} from '../FeatureFlags' describe('FeatureFlags', () => { + beforeEach(() => { + // Reset module state between tests for isolation + __resetDialogScrollOptimizedCount() + }) + it('should allow a component to check if a feature flag is enabled', () => { const calls: Array = [] @@ -42,4 +48,188 @@ describe('FeatureFlags', () => { expect(calls).toEqual([false]) }) + + describe('data-dialog-scroll-optimized attribute management', () => { + it('should set data-dialog-scroll-optimized attribute when primer_react_css_has_selector_perf is enabled', () => { + expect(document.body.hasAttribute('data-dialog-scroll-optimized')).toBe(false) + + const {unmount} = render( + +
Content
+ , + ) + + expect(document.body.hasAttribute('data-dialog-scroll-optimized')).toBe(true) + + unmount() + + expect(document.body.hasAttribute('data-dialog-scroll-optimized')).toBe(false) + }) + + it('should not set data-dialog-scroll-optimized attribute when primer_react_css_has_selector_perf is disabled', () => { + expect(document.body.hasAttribute('data-dialog-scroll-optimized')).toBe(false) + + const {unmount} = render( + +
Content
+ , + ) + + expect(document.body.hasAttribute('data-dialog-scroll-optimized')).toBe(false) + + unmount() + + expect(document.body.hasAttribute('data-dialog-scroll-optimized')).toBe(false) + }) + + it('should handle ref counting correctly with multiple FeatureFlags providers', () => { + expect(document.body.hasAttribute('data-dialog-scroll-optimized')).toBe(false) + + // Mount first provider + const {unmount: unmount1} = render( + +
Provider 1
+ , + ) + + expect(document.body.hasAttribute('data-dialog-scroll-optimized')).toBe(true) + + // Mount second provider + const {unmount: unmount2} = render( + +
Provider 2
+ , + ) + + expect(document.body.hasAttribute('data-dialog-scroll-optimized')).toBe(true) + + // Unmount first provider - attribute should still be present + unmount1() + expect(document.body.hasAttribute('data-dialog-scroll-optimized')).toBe(true) + + // Unmount second provider - attribute should be removed + unmount2() + expect(document.body.hasAttribute('data-dialog-scroll-optimized')).toBe(false) + }) + + it('should handle nested providers with different flag values correctly', () => { + expect(document.body.hasAttribute('data-dialog-scroll-optimized')).toBe(false) + + const {unmount} = render( + +
Outer provider with flag enabled
+ +
Inner provider with flag disabled
+ + , + ) + + // Outer provider sets the attribute, inner provider inherits but doesn't override + // (inner provider flag is false, so it won't add to count) + expect(document.body.hasAttribute('data-dialog-scroll-optimized')).toBe(true) + + unmount() + + expect(document.body.hasAttribute('data-dialog-scroll-optimized')).toBe(false) + }) + + it('should handle nested providers where parent has flag disabled and child has flag enabled', () => { + expect(document.body.hasAttribute('data-dialog-scroll-optimized')).toBe(false) + + const {unmount} = render( + +
Outer provider with flag disabled
+ +
Inner provider with flag enabled
+ + , + ) + + // Inner provider enables the flag, so attribute should be set + expect(document.body.hasAttribute('data-dialog-scroll-optimized')).toBe(true) + + unmount() + + expect(document.body.hasAttribute('data-dialog-scroll-optimized')).toBe(false) + }) + + it('should only remove attribute when all providers with flag enabled have unmounted', () => { + expect(document.body.hasAttribute('data-dialog-scroll-optimized')).toBe(false) + + // Mount three providers with flag enabled + const {unmount: unmount1} = render( + +
Provider 1
+ , + ) + + const {unmount: unmount2} = render( + +
Provider 2
+ , + ) + + const {unmount: unmount3} = render( + +
Provider 3
+ , + ) + + expect(document.body.hasAttribute('data-dialog-scroll-optimized')).toBe(true) + + // Unmount first provider + unmount1() + expect(document.body.hasAttribute('data-dialog-scroll-optimized')).toBe(true) + + // Unmount second provider + unmount2() + expect(document.body.hasAttribute('data-dialog-scroll-optimized')).toBe(true) + + // Unmount third provider - now attribute should be removed + unmount3() + expect(document.body.hasAttribute('data-dialog-scroll-optimized')).toBe(false) + }) + + it('should handle flag value changing from false to true', () => { + const {rerender, unmount} = render( + +
Content
+ , + ) + + expect(document.body.hasAttribute('data-dialog-scroll-optimized')).toBe(false) + + rerender( + +
Content
+ , + ) + + expect(document.body.hasAttribute('data-dialog-scroll-optimized')).toBe(true) + + unmount() + expect(document.body.hasAttribute('data-dialog-scroll-optimized')).toBe(false) + }) + + it('should handle flag value changing from true to false', () => { + const {rerender, unmount} = render( + +
Content
+ , + ) + + expect(document.body.hasAttribute('data-dialog-scroll-optimized')).toBe(true) + + rerender( + +
Content
+ , + ) + + expect(document.body.hasAttribute('data-dialog-scroll-optimized')).toBe(false) + + unmount() + expect(document.body.hasAttribute('data-dialog-scroll-optimized')).toBe(false) + }) + }) })