Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions .changeset/perf-dialog-has-selector.md
Original file line number Diff line number Diff line change
@@ -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
24 changes: 23 additions & 1 deletion packages/react/src/Dialog/Dialog.module.css
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
132 changes: 131 additions & 1 deletion packages/react/src/Dialog/Dialog.test.tsx
Original file line number Diff line number Diff line change
@@ -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(<Dialog onClose={() => {}}>Pay attention to me</Dialog>)
Expand Down Expand Up @@ -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(
<FeatureFlags flags={{primer_react_css_has_selector_perf: false}}>
<Dialog onClose={() => {}}>Dialog content</Dialog>
</FeatureFlags>,
)

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(
<FeatureFlags flags={{primer_react_css_has_selector_perf: true}}>
<Dialog onClose={() => {}}>Dialog content</Dialog>
</FeatureFlags>,
)

// 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(
<FeatureFlags flags={{primer_react_css_has_selector_perf: true}}>
<div>No dialogs here</div>
</FeatureFlags>,
)

// 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(
<FeatureFlags flags={{primer_react_css_has_selector_perf: true}}>
<Dialog onClose={() => {}}>Dialog 1</Dialog>
</FeatureFlags>,
)

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(
<FeatureFlags flags={{primer_react_css_has_selector_perf: true}}>
<Dialog onClose={() => {}}>Dialog 2</Dialog>
</FeatureFlags>,
)

// 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(
<FeatureFlags flags={{primer_react_css_has_selector_perf: false}}>
<Dialog onClose={() => {}}>Dialog 1</Dialog>
</FeatureFlags>,
)

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(
<FeatureFlags flags={{primer_react_css_has_selector_perf: false}}>
<Dialog onClose={() => {}}>Dialog 2</Dialog>
</FeatureFlags>,
)

// 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)
})
})
})
28 changes: 24 additions & 4 deletions packages/react/src/Dialog/Dialog.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -291,11 +291,31 @@ const _Dialog = React.forwardRef<HTMLDivElement, React.PropsWithChildren<DialogP

React.useEffect(() => {
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)
Expand Down
1 change: 1 addition & 0 deletions packages/react/src/FeatureFlags/DefaultFeatureFlags.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
41 changes: 41 additions & 0 deletions packages/react/src/FeatureFlags/FeatureFlags.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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 <FeatureFlagContext.Provider value={value}>{children}</FeatureFlagContext.Provider>
}
Loading
Loading