From 023b52f1d9c749ceedd44ba2f8847960b0bf11ac Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 29 Dec 2025 22:17:19 +0000 Subject: [PATCH 1/7] Initial plan From 901507e05e65d762f856046af71bc1fc45787e80 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 29 Dec 2025 22:25:04 +0000 Subject: [PATCH 2/7] Implement Dialog scroll optimization improvements - Move data-dialog-scroll-optimized management to FeatureFlags provider - Simplify Dialog to only manage data-dialog-scroll-disabled - Remove unused DialogScrollDisabled class logic - Revert :has(> .Footer) back to :has(.Footer) - Update changeset to correctly describe data attribute usage - Update tests to reflect provider-level attribute behavior Co-authored-by: mattcosta7 <8616962+mattcosta7@users.noreply.github.com> --- .changeset/perf-dialog-has-selector.md | 3 +- packages/react/src/Dialog/Dialog.module.css | 3 +- packages/react/src/Dialog/Dialog.test.tsx | 47 ++++++++++++++----- packages/react/src/Dialog/Dialog.tsx | 15 ++---- .../react/src/FeatureFlags/FeatureFlags.tsx | 13 ++++- 5 files changed, 51 insertions(+), 30 deletions(-) diff --git a/.changeset/perf-dialog-has-selector.md b/.changeset/perf-dialog-has-selector.md index 631826c682e..bc6b83d2b5e 100644 --- a/.changeset/perf-dialog-has-selector.md +++ b/.changeset/perf-dialog-has-selector.md @@ -6,6 +6,5 @@ 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.DialogScrollDisabled` class with ref counting -- Scope footer `:has(.Footer)` to direct child with `>` combinator for O(1) lookup +- 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 c67177f1f5c..a81c278bdbe 100644 --- a/packages/react/src/Dialog/Dialog.module.css +++ b/packages/react/src/Dialog/Dialog.module.css @@ -266,9 +266,8 @@ Add a border between the body and footer if: - the dialog has a footer - the dialog has a body that can scroll - the browser supports the `animation-timeline` property and its `scroll()` function -PERFORMANCE: Footer is a direct child of Dialog, scope with > for O(1) lookup */ -.Dialog:has(> .Footer) { +.Dialog:has(.Footer) { --can-scroll: 0; .DialogOverflowWrapper { diff --git a/packages/react/src/Dialog/Dialog.test.tsx b/packages/react/src/Dialog/Dialog.test.tsx index b6d14ca9809..ad1a6477b20 100644 --- a/packages/react/src/Dialog/Dialog.test.tsx +++ b/packages/react/src/Dialog/Dialog.test.tsx @@ -341,7 +341,7 @@ describe('Footer button loading states', () => { }) describe('primer_react_css_has_selector_perf feature flag', () => { - it('does not add data-dialog-scroll-optimized attribute when flag is OFF', () => { + it('does not add data-dialog-scroll-optimized or data-dialog-scroll-disabled when flag is OFF', () => { const {unmount} = render( {}}>Dialog content @@ -349,29 +349,48 @@ describe('Footer button loading states', () => { ) expect(document.body.hasAttribute('data-dialog-scroll-optimized')).toBe(false) - expect(document.body.classList.contains('DialogScrollDisabled')).toBe(true) + expect(document.body.hasAttribute('data-dialog-scroll-disabled')).toBe(false) unmount() - expect(document.body.classList.contains('DialogScrollDisabled')).toBe(false) + 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 attribute when flag is ON', () => { + 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( @@ -389,14 +408,14 @@ describe('Footer button loading states', () => { , ) - // Attribute should still be present + // 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() - // Attribute and class should still be present (second dialog is still open) + // 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) @@ -416,7 +435,7 @@ describe('Footer button loading states', () => { ) expect(document.body.hasAttribute('data-dialog-scroll-optimized')).toBe(false) - expect(document.body.classList.contains('DialogScrollDisabled')).toBe(true) + expect(document.body.hasAttribute('data-dialog-scroll-disabled')).toBe(false) // Render second dialog const {unmount: unmount2} = render( @@ -425,21 +444,23 @@ describe('Footer button loading states', () => { , ) - // Attribute should not be present, class should be present + // Attributes should not be present expect(document.body.hasAttribute('data-dialog-scroll-optimized')).toBe(false) - expect(document.body.classList.contains('DialogScrollDisabled')).toBe(true) + expect(document.body.hasAttribute('data-dialog-scroll-disabled')).toBe(false) // Unmount first dialog unmount1() - // Class should still be present (second dialog is still open) - expect(document.body.classList.contains('DialogScrollDisabled')).toBe(true) + // 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() - // Class should be removed - expect(document.body.classList.contains('DialogScrollDisabled')).toBe(false) + // 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 630e5b60a28..0f86f6b51de 100644 --- a/packages/react/src/Dialog/Dialog.tsx +++ b/packages/react/src/Dialog/Dialog.tsx @@ -296,34 +296,25 @@ const _Dialog = React.forwardRef { - // 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') if (usePerfOptimization) { - document.body.removeAttribute('data-dialog-scroll-optimized') document.body.removeAttribute('data-dialog-scroll-disabled') - } else { - document.body.classList.remove('DialogScrollDisabled') } } } diff --git a/packages/react/src/FeatureFlags/FeatureFlags.tsx b/packages/react/src/FeatureFlags/FeatureFlags.tsx index 2df681f5234..110d396923b 100644 --- a/packages/react/src/FeatureFlags/FeatureFlags.tsx +++ b/packages/react/src/FeatureFlags/FeatureFlags.tsx @@ -1,5 +1,5 @@ import type React from 'react' -import {useContext, useMemo} from 'react' +import {useContext, useMemo, useEffect} from 'react' import {FeatureFlagContext} from './FeatureFlagContext' import {FeatureFlagScope, type FeatureFlags} from './FeatureFlagScope' @@ -13,5 +13,16 @@ export function FeatureFlags({children, flags}: FeatureFlagsProps) { const scope = FeatureFlagScope.merge(parentFeatureFlags, FeatureFlagScope.create(flags)) return scope }, [parentFeatureFlags, flags]) + + // Set body attribute for CSS :has() optimization when flag is enabled + useEffect(() => { + if (flags.primer_react_css_has_selector_perf) { + document.body.setAttribute('data-dialog-scroll-optimized', '') + return () => { + document.body.removeAttribute('data-dialog-scroll-optimized') + } + } + }, [flags.primer_react_css_has_selector_perf]) + return {children} } From c5eb1bf9c8021755f6ad0033d5d5bc3bb923ef94 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 29 Dec 2025 22:31:50 +0000 Subject: [PATCH 3/7] Add ref counting for data-dialog-scroll-optimized attribute Implement global ref counter to handle multiple FeatureFlags providers setting/removing the attribute concurrently. Co-authored-by: mattcosta7 <8616962+mattcosta7@users.noreply.github.com> --- packages/react/src/FeatureFlags/FeatureFlags.tsx | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/packages/react/src/FeatureFlags/FeatureFlags.tsx b/packages/react/src/FeatureFlags/FeatureFlags.tsx index 110d396923b..2a8b73d35f6 100644 --- a/packages/react/src/FeatureFlags/FeatureFlags.tsx +++ b/packages/react/src/FeatureFlags/FeatureFlags.tsx @@ -7,6 +7,9 @@ export type FeatureFlagsProps = React.PropsWithChildren<{ flags: FeatureFlags }> +// Global ref counter for data-dialog-scroll-optimized attribute +let dialogScrollOptimizedCount = 0 + export function FeatureFlags({children, flags}: FeatureFlagsProps) { const parentFeatureFlags = useContext(FeatureFlagContext) const value = useMemo(() => { @@ -17,9 +20,13 @@ export function FeatureFlags({children, flags}: FeatureFlagsProps) { // Set body attribute for CSS :has() optimization when flag is enabled useEffect(() => { if (flags.primer_react_css_has_selector_perf) { + dialogScrollOptimizedCount++ document.body.setAttribute('data-dialog-scroll-optimized', '') return () => { - document.body.removeAttribute('data-dialog-scroll-optimized') + dialogScrollOptimizedCount-- + if (dialogScrollOptimizedCount === 0) { + document.body.removeAttribute('data-dialog-scroll-optimized') + } } } }, [flags.primer_react_css_has_selector_perf]) From 495091b42dc1c1306510bb3e6667dcdf0627ec48 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 29 Dec 2025 22:44:42 +0000 Subject: [PATCH 4/7] Use merged flag value from value.enabled() for optimization check Read value.enabled('primer_react_css_has_selector_perf') in component scope so the effect depends on the resolved boolean value instead of the raw prop. Co-authored-by: mattcosta7 <8616962+mattcosta7@users.noreply.github.com> --- packages/react/src/FeatureFlags/FeatureFlags.tsx | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/react/src/FeatureFlags/FeatureFlags.tsx b/packages/react/src/FeatureFlags/FeatureFlags.tsx index 2a8b73d35f6..0f004b341d8 100644 --- a/packages/react/src/FeatureFlags/FeatureFlags.tsx +++ b/packages/react/src/FeatureFlags/FeatureFlags.tsx @@ -17,9 +17,11 @@ export function FeatureFlags({children, flags}: FeatureFlagsProps) { 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 useEffect(() => { - if (flags.primer_react_css_has_selector_perf) { + if (isOptimizationEnabled) { dialogScrollOptimizedCount++ document.body.setAttribute('data-dialog-scroll-optimized', '') return () => { @@ -29,7 +31,7 @@ export function FeatureFlags({children, flags}: FeatureFlagsProps) { } } } - }, [flags.primer_react_css_has_selector_perf]) + }, [isOptimizationEnabled]) return {children} } From 06aefc91d82b62575249b6d68598a73989b66619 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 29 Dec 2025 23:49:55 +0000 Subject: [PATCH 5/7] Implement WeakMap-based ref counting and add comprehensive tests - Replace global counter with WeakMap to track individual component instances - Add comprehensive test coverage for body attribute management - Test ref counting with multiple providers and nested scenarios - Ensures proper test isolation without exposing internal state Co-authored-by: mattcosta7 <8616962+mattcosta7@users.noreply.github.com> --- .../react/src/FeatureFlags/FeatureFlags.tsx | 24 ++- .../__tests__/FeatureFlags.test.tsx | 149 +++++++++++++++++- 2 files changed, 165 insertions(+), 8 deletions(-) diff --git a/packages/react/src/FeatureFlags/FeatureFlags.tsx b/packages/react/src/FeatureFlags/FeatureFlags.tsx index 0f004b341d8..7ea32fcea3d 100644 --- a/packages/react/src/FeatureFlags/FeatureFlags.tsx +++ b/packages/react/src/FeatureFlags/FeatureFlags.tsx @@ -1,5 +1,5 @@ import type React from 'react' -import {useContext, useMemo, useEffect} from 'react' +import {useContext, useMemo, useEffect, useRef} from 'react' import {FeatureFlagContext} from './FeatureFlagContext' import {FeatureFlagScope, type FeatureFlags} from './FeatureFlagScope' @@ -7,7 +7,9 @@ export type FeatureFlagsProps = React.PropsWithChildren<{ flags: FeatureFlags }> -// Global ref counter for data-dialog-scroll-optimized attribute +// WeakMap-based ref counting for data-dialog-scroll-optimized attribute +// Keys are component instances, values track if the instance has contributed to the count +const dialogScrollOptimizedInstances = new WeakMap() let dialogScrollOptimizedCount = 0 export function FeatureFlags({children, flags}: FeatureFlagsProps) { @@ -18,16 +20,24 @@ export function FeatureFlags({children, flags}: FeatureFlagsProps) { }, [parentFeatureFlags, flags]) const isOptimizationEnabled = value.enabled('primer_react_css_has_selector_perf') + const instanceRef = useRef({}) // Set body attribute for CSS :has() optimization when flag is enabled useEffect(() => { if (isOptimizationEnabled) { - dialogScrollOptimizedCount++ - document.body.setAttribute('data-dialog-scroll-optimized', '') + const instance = instanceRef.current + if (!dialogScrollOptimizedInstances.get(instance)) { + dialogScrollOptimizedInstances.set(instance, true) + dialogScrollOptimizedCount++ + document.body.setAttribute('data-dialog-scroll-optimized', '') + } return () => { - dialogScrollOptimizedCount-- - if (dialogScrollOptimizedCount === 0) { - document.body.removeAttribute('data-dialog-scroll-optimized') + if (dialogScrollOptimizedInstances.get(instance)) { + dialogScrollOptimizedInstances.delete(instance) + dialogScrollOptimizedCount-- + if (dialogScrollOptimizedCount === 0) { + document.body.removeAttribute('data-dialog-scroll-optimized') + } } } } diff --git a/packages/react/src/FeatureFlags/__tests__/FeatureFlags.test.tsx b/packages/react/src/FeatureFlags/__tests__/FeatureFlags.test.tsx index cc7b69e802f..de99183b739 100644 --- a/packages/react/src/FeatureFlags/__tests__/FeatureFlags.test.tsx +++ b/packages/react/src/FeatureFlags/__tests__/FeatureFlags.test.tsx @@ -1,8 +1,13 @@ -import {describe, expect, it} from 'vitest' +import {describe, expect, it, beforeEach} from 'vitest' import {render} from '@testing-library/react' import {FeatureFlags, useFeatureFlag} from '../../FeatureFlags' describe('FeatureFlags', () => { + beforeEach(() => { + // Clean up body attributes between tests + document.body.removeAttribute('data-dialog-scroll-optimized') + }) + it('should allow a component to check if a feature flag is enabled', () => { const calls: Array = [] @@ -42,4 +47,146 @@ 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) + }) + }) }) From 0163141d19791e2d6a52137720b0e12c9d04ce86 Mon Sep 17 00:00:00 2001 From: Copilot <198982749+Copilot@users.noreply.github.com> Date: Mon, 29 Dec 2025 23:22:51 -0500 Subject: [PATCH 6/7] Simplify FeatureFlags ref counting by removing WeakMap (#7394) Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: mattcosta7 <8616962+mattcosta7@users.noreply.github.com> --- .../react/src/FeatureFlags/FeatureFlags.tsx | 44 ++++++++++++------- .../__tests__/FeatureFlags.test.tsx | 5 ++- 2 files changed, 30 insertions(+), 19 deletions(-) diff --git a/packages/react/src/FeatureFlags/FeatureFlags.tsx b/packages/react/src/FeatureFlags/FeatureFlags.tsx index 7ea32fcea3d..3cf0806cb92 100644 --- a/packages/react/src/FeatureFlags/FeatureFlags.tsx +++ b/packages/react/src/FeatureFlags/FeatureFlags.tsx @@ -1,5 +1,5 @@ import type React from 'react' -import {useContext, useMemo, useEffect, useRef} from 'react' +import {useContext, useMemo, useEffect} from 'react' import {FeatureFlagContext} from './FeatureFlagContext' import {FeatureFlagScope, type FeatureFlags} from './FeatureFlagScope' @@ -7,11 +7,28 @@ export type FeatureFlagsProps = React.PropsWithChildren<{ flags: FeatureFlags }> -// WeakMap-based ref counting for data-dialog-scroll-optimized attribute -// Keys are component instances, values track if the instance has contributed to the count -const dialogScrollOptimizedInstances = new WeakMap() +/** + * 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(() => { @@ -20,24 +37,17 @@ export function FeatureFlags({children, flags}: FeatureFlagsProps) { }, [parentFeatureFlags, flags]) const isOptimizationEnabled = value.enabled('primer_react_css_has_selector_perf') - const instanceRef = useRef({}) // Set body attribute for CSS :has() optimization when flag is enabled useEffect(() => { if (isOptimizationEnabled) { - const instance = instanceRef.current - if (!dialogScrollOptimizedInstances.get(instance)) { - dialogScrollOptimizedInstances.set(instance, true) - dialogScrollOptimizedCount++ - document.body.setAttribute('data-dialog-scroll-optimized', '') - } + dialogScrollOptimizedCount++ + document.body.setAttribute('data-dialog-scroll-optimized', '') + return () => { - if (dialogScrollOptimizedInstances.get(instance)) { - dialogScrollOptimizedInstances.delete(instance) - dialogScrollOptimizedCount-- - if (dialogScrollOptimizedCount === 0) { - document.body.removeAttribute('data-dialog-scroll-optimized') - } + dialogScrollOptimizedCount-- + if (dialogScrollOptimizedCount === 0) { + document.body.removeAttribute('data-dialog-scroll-optimized') } } } diff --git a/packages/react/src/FeatureFlags/__tests__/FeatureFlags.test.tsx b/packages/react/src/FeatureFlags/__tests__/FeatureFlags.test.tsx index de99183b739..f09f5b4dd4e 100644 --- a/packages/react/src/FeatureFlags/__tests__/FeatureFlags.test.tsx +++ b/packages/react/src/FeatureFlags/__tests__/FeatureFlags.test.tsx @@ -1,11 +1,12 @@ 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(() => { - // Clean up body attributes between tests - document.body.removeAttribute('data-dialog-scroll-optimized') + // Reset module state between tests for isolation + __resetDialogScrollOptimizedCount() }) it('should allow a component to check if a feature flag is enabled', () => { From 19f89cdf800d5c86dfcaa51bffe4901c22873f5c Mon Sep 17 00:00:00 2001 From: Copilot <198982749+Copilot@users.noreply.github.com> Date: Mon, 29 Dec 2025 23:46:23 -0500 Subject: [PATCH 7/7] Fix test isolation and add flag transition coverage for dialog scroll optimization (#7396) Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: mattcosta7 <8616962+mattcosta7@users.noreply.github.com> --- packages/react/src/Dialog/Dialog.test.tsx | 7 +++- .../__tests__/FeatureFlags.test.tsx | 42 +++++++++++++++++++ 2 files changed, 48 insertions(+), 1 deletion(-) diff --git a/packages/react/src/Dialog/Dialog.test.tsx b/packages/react/src/Dialog/Dialog.test.tsx index ad1a6477b20..3ce178375ab 100644 --- a/packages/react/src/Dialog/Dialog.test.tsx +++ b/packages/react/src/Dialog/Dialog.test.tsx @@ -1,14 +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) diff --git a/packages/react/src/FeatureFlags/__tests__/FeatureFlags.test.tsx b/packages/react/src/FeatureFlags/__tests__/FeatureFlags.test.tsx index f09f5b4dd4e..035b43bf0eb 100644 --- a/packages/react/src/FeatureFlags/__tests__/FeatureFlags.test.tsx +++ b/packages/react/src/FeatureFlags/__tests__/FeatureFlags.test.tsx @@ -189,5 +189,47 @@ describe('FeatureFlags', () => { 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) + }) }) })