From e04ba702f291b7157f890166ead23866c8efea7a Mon Sep 17 00:00:00 2001 From: Matthew Costabile Date: Mon, 22 Dec 2025 03:33:06 +0000 Subject: [PATCH 1/6] Add primer_react_css_contain_portal feature flag When enabled, applies `contain: layout style` to Portal elements to improve rendering performance by isolating layout and style calculations. The containment does not clip overflow, so dropdowns/tooltips continue to work correctly. - Added useFeatureFlag check in Portal component - Added useLayoutEffect to apply/remove containment based on flag - Added comprehensive tests for the feature flag behavior --- .changeset/portal-css-containment.md | 5 ++ packages/react/src/Portal/Portal.test.tsx | 71 +++++++++++++++++++++++ packages/react/src/Portal/Portal.tsx | 13 +++++ 3 files changed, 89 insertions(+) create mode 100644 .changeset/portal-css-containment.md diff --git a/.changeset/portal-css-containment.md b/.changeset/portal-css-containment.md new file mode 100644 index 00000000000..c11b9f7be0c --- /dev/null +++ b/.changeset/portal-css-containment.md @@ -0,0 +1,5 @@ +--- +"@primer/react": minor +--- + +Added `primer_react_css_contain_portal` feature flag that applies CSS containment (`contain: layout style`) to Portal elements when enabled. This can improve rendering performance by isolating layout and style calculations within portals without clipping overflow content. diff --git a/packages/react/src/Portal/Portal.test.tsx b/packages/react/src/Portal/Portal.test.tsx index 7b4975b4616..10ed8638901 100644 --- a/packages/react/src/Portal/Portal.test.tsx +++ b/packages/react/src/Portal/Portal.test.tsx @@ -4,6 +4,7 @@ import Portal, {registerPortalRoot, PortalContext} from '../Portal/index' import {render} from '@testing-library/react' import BaseStyles from '../BaseStyles' import React from 'react' +import {FeatureFlags} from '../FeatureFlags' describe('Portal', () => { it('renders a default portal into document.body (no BaseStyles present)', () => { @@ -183,4 +184,74 @@ describe('Portal', () => { document.body.removeChild(contextPortalRoot) document.body.removeChild(propPortalRoot) }) + + describe('CSS containment feature flag', () => { + it('does not apply CSS containment by default', () => { + const {baseElement} = render(test-content) + const generatedRoot = baseElement.querySelector('#__primerPortalRoot__') + const portalElement = generatedRoot?.firstElementChild as HTMLElement + + expect(portalElement).toBeInstanceOf(HTMLElement) + expect(portalElement.style.contain).toBe('') + + baseElement.innerHTML = '' + }) + + it('applies CSS containment when primer_react_css_contain_portal flag is enabled', () => { + const toRender = ( + + contained-content + + ) + + const {baseElement} = render(toRender) + const generatedRoot = baseElement.querySelector('#__primerPortalRoot__') + const portalElement = generatedRoot?.firstElementChild as HTMLElement + + expect(portalElement).toBeInstanceOf(HTMLElement) + expect(portalElement.style.contain).toBe('layout style') + + baseElement.innerHTML = '' + }) + + it('does not apply CSS containment when flag is explicitly disabled', () => { + const toRender = ( + + uncontained-content + + ) + + const {baseElement} = render(toRender) + const generatedRoot = baseElement.querySelector('#__primerPortalRoot__') + const portalElement = generatedRoot?.firstElementChild as HTMLElement + + expect(portalElement).toBeInstanceOf(HTMLElement) + expect(portalElement.style.contain).toBe('') + + baseElement.innerHTML = '' + }) + + it('removes CSS containment when flag changes from enabled to disabled', () => { + const {baseElement, rerender} = render( + + toggle-content + , + ) + + const generatedRoot = baseElement.querySelector('#__primerPortalRoot__') + const portalElement = generatedRoot?.firstElementChild as HTMLElement + + expect(portalElement.style.contain).toBe('layout style') + + rerender( + + toggle-content + , + ) + + expect(portalElement.style.contain).toBe('') + + baseElement.innerHTML = '' + }) + }) }) diff --git a/packages/react/src/Portal/Portal.tsx b/packages/react/src/Portal/Portal.tsx index 68797db5a15..69fe06ebe2e 100644 --- a/packages/react/src/Portal/Portal.tsx +++ b/packages/react/src/Portal/Portal.tsx @@ -1,6 +1,7 @@ import React, {useContext} from 'react' import {createPortal} from 'react-dom' import useLayoutEffect from '../utils/useIsomorphicLayoutEffect' +import {useFeatureFlag} from '../FeatureFlags' const PRIMER_PORTAL_ROOT_ID = '__primerPortalRoot__' const DEFAULT_PORTAL_CONTAINER_NAME = '__default__' @@ -75,6 +76,7 @@ export const Portal: React.FC> = ({ containerName: _containerName, }) => { const {portalContainerName} = useContext(PortalContext) + const enableCSSContainment = useFeatureFlag('primer_react_css_contain_portal') const elementRef = React.useRef(null) if (!elementRef.current) { const div = document.createElement('div') @@ -88,6 +90,17 @@ export const Portal: React.FC> = ({ const element = elementRef.current + // Apply CSS containment when feature flag is enabled. + // `contain: layout style` isolates layout and style calculations without + // clipping overflow (which would break dropdowns/tooltips). + useLayoutEffect(() => { + if (enableCSSContainment) { + element.style.contain = 'layout style' + } else { + element.style.contain = '' + } + }, [element, enableCSSContainment]) + useLayoutEffect(() => { let containerName = _containerName ?? portalContainerName if (containerName === undefined) { From d580ff394489bdbb83494acd2e7c69f6c15f5a6d Mon Sep 17 00:00:00 2001 From: Matthew Costabile Date: Mon, 22 Dec 2025 03:36:02 +0000 Subject: [PATCH 2/6] optimize oimpl --- packages/react/src/Portal/Portal.test.tsx | 23 ----------------------- packages/react/src/Portal/Portal.tsx | 14 +++----------- 2 files changed, 3 insertions(+), 34 deletions(-) diff --git a/packages/react/src/Portal/Portal.test.tsx b/packages/react/src/Portal/Portal.test.tsx index 10ed8638901..9f0d486b53f 100644 --- a/packages/react/src/Portal/Portal.test.tsx +++ b/packages/react/src/Portal/Portal.test.tsx @@ -230,28 +230,5 @@ describe('Portal', () => { baseElement.innerHTML = '' }) - - it('removes CSS containment when flag changes from enabled to disabled', () => { - const {baseElement, rerender} = render( - - toggle-content - , - ) - - const generatedRoot = baseElement.querySelector('#__primerPortalRoot__') - const portalElement = generatedRoot?.firstElementChild as HTMLElement - - expect(portalElement.style.contain).toBe('layout style') - - rerender( - - toggle-content - , - ) - - expect(portalElement.style.contain).toBe('') - - baseElement.innerHTML = '' - }) }) }) diff --git a/packages/react/src/Portal/Portal.tsx b/packages/react/src/Portal/Portal.tsx index 69fe06ebe2e..304d261949d 100644 --- a/packages/react/src/Portal/Portal.tsx +++ b/packages/react/src/Portal/Portal.tsx @@ -85,22 +85,14 @@ export const Portal: React.FC> = ({ // to change the zIndex to a value other than "1". div.style.position = 'relative' div.style.zIndex = '1' + if (enableCSSContainment) { + div.style.contain = 'layout style' + } elementRef.current = div } const element = elementRef.current - // Apply CSS containment when feature flag is enabled. - // `contain: layout style` isolates layout and style calculations without - // clipping overflow (which would break dropdowns/tooltips). - useLayoutEffect(() => { - if (enableCSSContainment) { - element.style.contain = 'layout style' - } else { - element.style.contain = '' - } - }, [element, enableCSSContainment]) - useLayoutEffect(() => { let containerName = _containerName ?? portalContainerName if (containerName === undefined) { From 636c750f21abf0967012a9139cf4b81dd4dfc4f3 Mon Sep 17 00:00:00 2001 From: Matthew Costabile Date: Mon, 22 Dec 2025 03:41:00 +0000 Subject: [PATCH 3/6] Apply CSS containment to shared __primerPortalRoot__ instead of individual portals --- packages/react/src/Portal/Portal.test.tsx | 21 +++++++++------------ packages/react/src/Portal/Portal.tsx | 10 +++++----- 2 files changed, 14 insertions(+), 17 deletions(-) diff --git a/packages/react/src/Portal/Portal.test.tsx b/packages/react/src/Portal/Portal.test.tsx index 9f0d486b53f..a2f6ea96d4a 100644 --- a/packages/react/src/Portal/Portal.test.tsx +++ b/packages/react/src/Portal/Portal.test.tsx @@ -188,11 +188,10 @@ describe('Portal', () => { describe('CSS containment feature flag', () => { it('does not apply CSS containment by default', () => { const {baseElement} = render(test-content) - const generatedRoot = baseElement.querySelector('#__primerPortalRoot__') - const portalElement = generatedRoot?.firstElementChild as HTMLElement + const generatedRoot = baseElement.querySelector('#__primerPortalRoot__') as HTMLElement - expect(portalElement).toBeInstanceOf(HTMLElement) - expect(portalElement.style.contain).toBe('') + expect(generatedRoot).toBeInstanceOf(HTMLElement) + expect(generatedRoot.style.contain).toBe('') baseElement.innerHTML = '' }) @@ -205,11 +204,10 @@ describe('Portal', () => { ) const {baseElement} = render(toRender) - const generatedRoot = baseElement.querySelector('#__primerPortalRoot__') - const portalElement = generatedRoot?.firstElementChild as HTMLElement + const generatedRoot = baseElement.querySelector('#__primerPortalRoot__') as HTMLElement - expect(portalElement).toBeInstanceOf(HTMLElement) - expect(portalElement.style.contain).toBe('layout style') + expect(generatedRoot).toBeInstanceOf(HTMLElement) + expect(generatedRoot.style.contain).toBe('layout style') baseElement.innerHTML = '' }) @@ -222,11 +220,10 @@ describe('Portal', () => { ) const {baseElement} = render(toRender) - const generatedRoot = baseElement.querySelector('#__primerPortalRoot__') - const portalElement = generatedRoot?.firstElementChild as HTMLElement + const generatedRoot = baseElement.querySelector('#__primerPortalRoot__') as HTMLElement - expect(portalElement).toBeInstanceOf(HTMLElement) - expect(portalElement.style.contain).toBe('') + expect(generatedRoot).toBeInstanceOf(HTMLElement) + expect(generatedRoot.style.contain).toBe('') baseElement.innerHTML = '' }) diff --git a/packages/react/src/Portal/Portal.tsx b/packages/react/src/Portal/Portal.tsx index 304d261949d..48030e551ff 100644 --- a/packages/react/src/Portal/Portal.tsx +++ b/packages/react/src/Portal/Portal.tsx @@ -21,7 +21,7 @@ export function registerPortalRoot(root: Element, name = DEFAULT_PORTAL_CONTAINE // Ensures that a default portal root exists and is registered. If a DOM element exists // with id __primerPortalRoot__, allow that element to serve as the default portal root. // Otherwise, create that element and attach it to the end of document.body. -function ensureDefaultPortal() { +function ensureDefaultPortal(enableCSSContainment = false) { const existingDefaultPortalContainer = portalRootRegistry[DEFAULT_PORTAL_CONTAINER_NAME] if (!existingDefaultPortalContainer || !document.body.contains(existingDefaultPortalContainer)) { let defaultPortalContainer = document.getElementById(PRIMER_PORTAL_ROOT_ID) @@ -32,6 +32,9 @@ function ensureDefaultPortal() { defaultPortalContainer.style.top = '0' defaultPortalContainer.style.left = '0' defaultPortalContainer.style.width = '100%' + if (enableCSSContainment) { + defaultPortalContainer.style.contain = 'layout style' + } const suitablePortalRoot = document.querySelector('[data-portal-root]') if (suitablePortalRoot) { suitablePortalRoot.appendChild(defaultPortalContainer) @@ -85,9 +88,6 @@ export const Portal: React.FC> = ({ // to change the zIndex to a value other than "1". div.style.position = 'relative' div.style.zIndex = '1' - if (enableCSSContainment) { - div.style.contain = 'layout style' - } elementRef.current = div } @@ -97,7 +97,7 @@ export const Portal: React.FC> = ({ let containerName = _containerName ?? portalContainerName if (containerName === undefined) { containerName = DEFAULT_PORTAL_CONTAINER_NAME - ensureDefaultPortal() + ensureDefaultPortal(enableCSSContainment) } const parentElement = portalRootRegistry[containerName] From 859d5093bc20da163bb5abd8114528e8993341c2 Mon Sep 17 00:00:00 2001 From: Matthew Costabile Date: Mon, 22 Dec 2025 03:42:45 +0000 Subject: [PATCH 4/6] Apply containment to existing portal roots and warn on override --- packages/react/src/Portal/Portal.test.tsx | 49 ++++++++++++++++++++++- packages/react/src/Portal/Portal.tsx | 18 +++++++-- 2 files changed, 63 insertions(+), 4 deletions(-) diff --git a/packages/react/src/Portal/Portal.test.tsx b/packages/react/src/Portal/Portal.test.tsx index a2f6ea96d4a..b0f61bd1b92 100644 --- a/packages/react/src/Portal/Portal.test.tsx +++ b/packages/react/src/Portal/Portal.test.tsx @@ -1,4 +1,4 @@ -import {describe, expect, it} from 'vitest' +import {describe, expect, it, vi} from 'vitest' import Portal, {registerPortalRoot, PortalContext} from '../Portal/index' import {render} from '@testing-library/react' @@ -227,5 +227,52 @@ describe('Portal', () => { baseElement.innerHTML = '' }) + + it('applies CSS containment to pre-existing portal root', () => { + // Create a pre-existing portal root declaratively + const existingRoot = document.createElement('div') + existingRoot.id = '__primerPortalRoot__' + document.body.appendChild(existingRoot) + + const toRender = ( + + content-in-existing-root + + ) + + const {baseElement} = render(toRender) + const generatedRoot = baseElement.querySelector('#__primerPortalRoot__') as HTMLElement + + expect(generatedRoot).toBeInstanceOf(HTMLElement) + expect(generatedRoot.style.contain).toBe('layout style') + + baseElement.innerHTML = '' + document.body.removeChild(existingRoot) + }) + + it('warns when overriding existing contain value', () => { + const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}) + + // Create a pre-existing portal root with a different contain value + const existingRoot = document.createElement('div') + existingRoot.id = '__primerPortalRoot__' + existingRoot.style.contain = 'size' + document.body.appendChild(existingRoot) + + const toRender = ( + + content-with-override + + ) + + render(toRender) + + expect(warnSpy).toHaveBeenCalledWith( + 'Portal root already has contain: "size". Overriding with "layout style" due to primer_react_css_contain_portal flag.', + ) + + warnSpy.mockRestore() + document.body.removeChild(existingRoot) + }) }) }) diff --git a/packages/react/src/Portal/Portal.tsx b/packages/react/src/Portal/Portal.tsx index 48030e551ff..b00c2f6da81 100644 --- a/packages/react/src/Portal/Portal.tsx +++ b/packages/react/src/Portal/Portal.tsx @@ -32,9 +32,6 @@ function ensureDefaultPortal(enableCSSContainment = false) { defaultPortalContainer.style.top = '0' defaultPortalContainer.style.left = '0' defaultPortalContainer.style.width = '100%' - if (enableCSSContainment) { - defaultPortalContainer.style.contain = 'layout style' - } const suitablePortalRoot = document.querySelector('[data-portal-root]') if (suitablePortalRoot) { suitablePortalRoot.appendChild(defaultPortalContainer) @@ -45,6 +42,21 @@ function ensureDefaultPortal(enableCSSContainment = false) { registerPortalRoot(defaultPortalContainer) } + + // Apply CSS containment to the portal root if enabled + if (enableCSSContainment) { + const portalRoot = portalRootRegistry[DEFAULT_PORTAL_CONTAINER_NAME] + if (portalRoot instanceof HTMLElement) { + const existingContain = portalRoot.style.contain + if (existingContain && existingContain !== 'layout style') { + // eslint-disable-next-line no-console + console.warn( + `Portal root already has contain: "${existingContain}". Overriding with "layout style" due to primer_react_css_contain_portal flag.`, + ) + } + portalRoot.style.contain = 'layout style' + } + } } /** From b0ad5121cc067d71c652fa5a907a0be65e6c1a0f Mon Sep 17 00:00:00 2001 From: Matthew Costabile Date: Mon, 22 Dec 2025 03:44:06 +0000 Subject: [PATCH 5/6] Optimize: only apply CSS containment once --- packages/react/src/Portal/Portal.tsx | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/packages/react/src/Portal/Portal.tsx b/packages/react/src/Portal/Portal.tsx index b00c2f6da81..70284132910 100644 --- a/packages/react/src/Portal/Portal.tsx +++ b/packages/react/src/Portal/Portal.tsx @@ -8,6 +8,9 @@ const DEFAULT_PORTAL_CONTAINER_NAME = '__default__' const portalRootRegistry: Partial> = {} +// Track whether CSS containment has been applied to avoid repeated work +let cssContainmentApplied = false + /** * Register a container to serve as a portal root. * @param root The element that will be the root for portals created in this container @@ -43,8 +46,8 @@ function ensureDefaultPortal(enableCSSContainment = false) { registerPortalRoot(defaultPortalContainer) } - // Apply CSS containment to the portal root if enabled - if (enableCSSContainment) { + // Apply CSS containment to the portal root if enabled (only once) + if (enableCSSContainment && !cssContainmentApplied) { const portalRoot = portalRootRegistry[DEFAULT_PORTAL_CONTAINER_NAME] if (portalRoot instanceof HTMLElement) { const existingContain = portalRoot.style.contain @@ -55,6 +58,7 @@ function ensureDefaultPortal(enableCSSContainment = false) { ) } portalRoot.style.contain = 'layout style' + cssContainmentApplied = true } } } From eb4533536288d5cfdf44493360e24d518245a06d Mon Sep 17 00:00:00 2001 From: Matthew Costabile Date: Mon, 22 Dec 2025 03:47:25 +0000 Subject: [PATCH 6/6] Use WeakMap for containment tracking, add reset for tests --- packages/react/src/Portal/Portal.test.tsx | 9 +++++++-- packages/react/src/Portal/Portal.tsx | 21 +++++++++++++++------ packages/react/src/Portal/index.ts | 4 ++-- 3 files changed, 24 insertions(+), 10 deletions(-) diff --git a/packages/react/src/Portal/Portal.test.tsx b/packages/react/src/Portal/Portal.test.tsx index b0f61bd1b92..05635e12396 100644 --- a/packages/react/src/Portal/Portal.test.tsx +++ b/packages/react/src/Portal/Portal.test.tsx @@ -1,5 +1,5 @@ -import {describe, expect, it, vi} from 'vitest' -import Portal, {registerPortalRoot, PortalContext} from '../Portal/index' +import {describe, expect, it, vi, beforeEach} from 'vitest' +import Portal, {registerPortalRoot, PortalContext, resetCSSContainmentTracking} from '../Portal/index' import {render} from '@testing-library/react' import BaseStyles from '../BaseStyles' @@ -186,6 +186,11 @@ describe('Portal', () => { }) describe('CSS containment feature flag', () => { + beforeEach(() => { + // Reset containment tracking between tests + resetCSSContainmentTracking() + }) + it('does not apply CSS containment by default', () => { const {baseElement} = render(test-content) const generatedRoot = baseElement.querySelector('#__primerPortalRoot__') as HTMLElement diff --git a/packages/react/src/Portal/Portal.tsx b/packages/react/src/Portal/Portal.tsx index 70284132910..c142ad21794 100644 --- a/packages/react/src/Portal/Portal.tsx +++ b/packages/react/src/Portal/Portal.tsx @@ -8,8 +8,17 @@ const DEFAULT_PORTAL_CONTAINER_NAME = '__default__' const portalRootRegistry: Partial> = {} -// Track whether CSS containment has been applied to avoid repeated work -let cssContainmentApplied = false +// Track which portal roots have had CSS containment applied (auto-cleans when element is GC'd) +const cssContainmentApplied = new WeakMap() + +// Reset containment tracking (exported for testing) +export function resetCSSContainmentTracking(): void { + // WeakMap doesn't have a clear method, but we can reset by deleting the current portal root entry + const portalRoot = portalRootRegistry[DEFAULT_PORTAL_CONTAINER_NAME] + if (portalRoot) { + cssContainmentApplied.delete(portalRoot) + } +} /** * Register a container to serve as a portal root. @@ -46,10 +55,10 @@ function ensureDefaultPortal(enableCSSContainment = false) { registerPortalRoot(defaultPortalContainer) } - // Apply CSS containment to the portal root if enabled (only once) - if (enableCSSContainment && !cssContainmentApplied) { + // Apply CSS containment to the portal root if enabled (only once per root) + if (enableCSSContainment) { const portalRoot = portalRootRegistry[DEFAULT_PORTAL_CONTAINER_NAME] - if (portalRoot instanceof HTMLElement) { + if (portalRoot instanceof HTMLElement && !cssContainmentApplied.has(portalRoot)) { const existingContain = portalRoot.style.contain if (existingContain && existingContain !== 'layout style') { // eslint-disable-next-line no-console @@ -58,7 +67,7 @@ function ensureDefaultPortal(enableCSSContainment = false) { ) } portalRoot.style.contain = 'layout style' - cssContainmentApplied = true + cssContainmentApplied.set(portalRoot, true) } } } diff --git a/packages/react/src/Portal/index.ts b/packages/react/src/Portal/index.ts index 375dce81355..493f74ac3b5 100644 --- a/packages/react/src/Portal/index.ts +++ b/packages/react/src/Portal/index.ts @@ -1,7 +1,7 @@ import type {PortalProps} from './Portal' -import {Portal, registerPortalRoot, PortalContext} from './Portal' +import {Portal, registerPortalRoot, PortalContext, resetCSSContainmentTracking} from './Portal' export default Portal -export {registerPortalRoot} +export {registerPortalRoot, resetCSSContainmentTracking} export type {PortalProps} export {PortalContext}