From d56213021bcba44505fbda094220f687653ec1b5 Mon Sep 17 00:00:00 2001 From: Marie Lucca Date: Tue, 9 Sep 2025 14:26:44 -0400 Subject: [PATCH 01/13] chore: add portal context --- packages/react/src/Portal/Portal.tsx | 9 +++++++-- packages/react/src/Portal/index.ts | 3 ++- packages/react/src/index.ts | 2 +- 3 files changed, 10 insertions(+), 4 deletions(-) diff --git a/packages/react/src/Portal/Portal.tsx b/packages/react/src/Portal/Portal.tsx index 896281a80dd..02ea26b1170 100644 --- a/packages/react/src/Portal/Portal.tsx +++ b/packages/react/src/Portal/Portal.tsx @@ -1,4 +1,4 @@ -import React from 'react' +import React, {useContext} from 'react' import {createPortal} from 'react-dom' import useLayoutEffect from '../utils/useIsomorphicLayoutEffect' @@ -43,6 +43,10 @@ function ensureDefaultPortal() { } } +export const PortalContext = React.createContext<{ + portalContainerName?: string +}>({portalContainerName: DEFAULT_PORTAL_CONTAINER_NAME}) + export interface PortalProps { /** * Called when this portal is added to the DOM @@ -80,7 +84,8 @@ export const Portal: React.FC> = ({ const element = elementRef.current useLayoutEffect(() => { - let containerName = _containerName + const {portalContainerName} = useContext(PortalContext) + let containerName = _containerName ?? portalContainerName if (containerName === undefined) { containerName = DEFAULT_PORTAL_CONTAINER_NAME ensureDefaultPortal() diff --git a/packages/react/src/Portal/index.ts b/packages/react/src/Portal/index.ts index 485af9814ba..375dce81355 100644 --- a/packages/react/src/Portal/index.ts +++ b/packages/react/src/Portal/index.ts @@ -1,6 +1,7 @@ import type {PortalProps} from './Portal' -import {Portal, registerPortalRoot} from './Portal' +import {Portal, registerPortalRoot, PortalContext} from './Portal' export default Portal export {registerPortalRoot} export type {PortalProps} +export {PortalContext} diff --git a/packages/react/src/index.ts b/packages/react/src/index.ts index 2d2c5365d04..9242236b52d 100644 --- a/packages/react/src/index.ts +++ b/packages/react/src/index.ts @@ -126,7 +126,7 @@ export {default as PointerBox} from './PointerBox' export type {PointerBoxProps} from './PointerBox' export {default as Popover} from './Popover' export type {PopoverProps, PopoverContentProps} from './Popover' -export {default as Portal, registerPortalRoot} from './Portal' +export {default as Portal, registerPortalRoot, PortalContext} from './Portal' export type {PortalProps} from './Portal' export {ProgressBar} from './ProgressBar' export type {ProgressBarProps, ProgressBarItemProps} from './ProgressBar' From 9f4bc0e3ffc183b40f7bc45144227ec04eee7232 Mon Sep 17 00:00:00 2001 From: Marie Lucca Date: Tue, 9 Sep 2025 14:32:39 -0400 Subject: [PATCH 02/13] fix rule of hooks --- packages/react/src/Portal/Portal.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/react/src/Portal/Portal.tsx b/packages/react/src/Portal/Portal.tsx index 02ea26b1170..94ad46d61d2 100644 --- a/packages/react/src/Portal/Portal.tsx +++ b/packages/react/src/Portal/Portal.tsx @@ -71,6 +71,7 @@ export const Portal: React.FC> = ({ containerName: _containerName, }) => { const elementRef = React.useRef(null) + const {portalContainerName} = useContext(PortalContext) if (!elementRef.current) { const div = document.createElement('div') // Portaled content should get their own stacking context so they don't interfere @@ -84,7 +85,6 @@ export const Portal: React.FC> = ({ const element = elementRef.current useLayoutEffect(() => { - const {portalContainerName} = useContext(PortalContext) let containerName = _containerName ?? portalContainerName if (containerName === undefined) { containerName = DEFAULT_PORTAL_CONTAINER_NAME From b221f8467fab1d0a210e9fa39a48968e03541ac3 Mon Sep 17 00:00:00 2001 From: Copilot <198982749+Copilot@users.noreply.github.com> Date: Tue, 9 Sep 2025 19:22:18 -0400 Subject: [PATCH 03/13] chore: add portal context with comprehensive tests (#6816) Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: francinelucca <40550942+francinelucca@users.noreply.github.com> --- packages/react/src/Portal/Portal.test.tsx | 85 ++++++++++++++++++++++- packages/react/src/Portal/Portal.tsx | 7 +- 2 files changed, 88 insertions(+), 4 deletions(-) diff --git a/packages/react/src/Portal/Portal.test.tsx b/packages/react/src/Portal/Portal.test.tsx index 7e6d64c3d05..7b4975b4616 100644 --- a/packages/react/src/Portal/Portal.test.tsx +++ b/packages/react/src/Portal/Portal.test.tsx @@ -1,8 +1,9 @@ import {describe, expect, it} from 'vitest' -import Portal, {registerPortalRoot} from '../Portal/index' +import Portal, {registerPortalRoot, PortalContext} from '../Portal/index' import {render} from '@testing-library/react' import BaseStyles from '../BaseStyles' +import React from 'react' describe('Portal', () => { it('renders a default portal into document.body (no BaseStyles present)', () => { @@ -100,4 +101,86 @@ describe('Portal', () => { baseElement.innerHTML = '' }) + + it('renders into custom portal when PortalContext is supplied with portalContainerName', () => { + // Create and register a custom portal root + const customPortalRoot = document.createElement('div') + customPortalRoot.id = 'customContextPortal' + document.body.appendChild(customPortalRoot) + registerPortalRoot(customPortalRoot, 'customContext') + + const toRender = ( + + context-portal-content + + ) + + render(toRender) + + expect(customPortalRoot.textContent.trim()).toEqual('context-portal-content') + + // Cleanup + document.body.removeChild(customPortalRoot) + }) + + it('renders into default portal when PortalContext does not specify portalContainerName', () => { + const toRender = ( + + default-portal-content + + ) + + const {baseElement} = render(toRender) + const generatedRoot = baseElement.querySelector('#__primerPortalRoot__') + + expect(generatedRoot).toBeInstanceOf(HTMLElement) + expect(generatedRoot?.textContent.trim()).toEqual('default-portal-content') + + baseElement.innerHTML = '' + }) + + it('renders into default portal when PortalContext portalContainerName is undefined', () => { + const toRender = ( + + undefined-context-content + + ) + + const {baseElement} = render(toRender) + const generatedRoot = baseElement.querySelector('#__primerPortalRoot__') + + expect(generatedRoot).toBeInstanceOf(HTMLElement) + expect(generatedRoot?.textContent.trim()).toEqual('undefined-context-content') + + baseElement.innerHTML = '' + }) + + it('containerName prop overrides PortalContext portalContainerName', () => { + // Create and register custom portal roots + const contextPortalRoot = document.createElement('div') + contextPortalRoot.id = 'contextPortal' + document.body.appendChild(contextPortalRoot) + registerPortalRoot(contextPortalRoot, 'contextPortal') + + const propPortalRoot = document.createElement('div') + propPortalRoot.id = 'propPortal' + document.body.appendChild(propPortalRoot) + registerPortalRoot(propPortalRoot, 'propPortal') + + const toRender = ( + + prop-overrides-context + + ) + + render(toRender) + + // Should render in the portal specified by the prop, not the context + expect(propPortalRoot.textContent.trim()).toEqual('prop-overrides-context') + expect(contextPortalRoot.textContent.trim()).toEqual('') + + // Cleanup + document.body.removeChild(contextPortalRoot) + document.body.removeChild(propPortalRoot) + }) }) diff --git a/packages/react/src/Portal/Portal.tsx b/packages/react/src/Portal/Portal.tsx index 94ad46d61d2..2e262db7aa0 100644 --- a/packages/react/src/Portal/Portal.tsx +++ b/packages/react/src/Portal/Portal.tsx @@ -70,6 +70,7 @@ export const Portal: React.FC> = ({ onMount, containerName: _containerName, }) => { + const {portalContainerName} = useContext(PortalContext) const elementRef = React.useRef(null) const {portalContainerName} = useContext(PortalContext) if (!elementRef.current) { @@ -86,7 +87,7 @@ export const Portal: React.FC> = ({ useLayoutEffect(() => { let containerName = _containerName ?? portalContainerName - if (containerName === undefined) { + if (containerName === undefined || containerName === DEFAULT_PORTAL_CONTAINER_NAME) { containerName = DEFAULT_PORTAL_CONTAINER_NAME ensureDefaultPortal() } @@ -94,7 +95,7 @@ export const Portal: React.FC> = ({ if (!parentElement) { throw new Error( - `Portal container '${_containerName}' is not yet registered. Container must be registered with registerPortal before use.`, + `Portal container '${containerName}' is not yet registered. Container must be registered with registerPortal before use.`, ) } parentElement.appendChild(element) @@ -105,7 +106,7 @@ export const Portal: React.FC> = ({ } // eslint-disable-next-line react-compiler/react-compiler // eslint-disable-next-line react-hooks/exhaustive-deps - }, [element]) + }, [element, _containerName, portalContainerName]) return createPortal(children, element) } From c19577c07f4e990d2a760e0bf06a7ba1f03006d9 Mon Sep 17 00:00:00 2001 From: Copilot <198982749+Copilot@users.noreply.github.com> Date: Wed, 10 Sep 2025 10:14:51 -0400 Subject: [PATCH 04/13] chore: add portal context with comprehensive tests and storybook demo (#6821) Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: francinelucca <40550942+francinelucca@users.noreply.github.com> Co-authored-by: Marie Lucca --- .../src/Portal/Portal.features.stories.tsx | 108 +++++++++++++++++- packages/react/src/Portal/Portal.tsx | 1 - 2 files changed, 106 insertions(+), 3 deletions(-) diff --git a/packages/react/src/Portal/Portal.features.stories.tsx b/packages/react/src/Portal/Portal.features.stories.tsx index 0dfb807cc7a..46321c7e96e 100644 --- a/packages/react/src/Portal/Portal.features.stories.tsx +++ b/packages/react/src/Portal/Portal.features.stories.tsx @@ -1,6 +1,6 @@ -import React from 'react' +import React, {useEffect} from 'react' import type {Meta} from '@storybook/react-vite' -import {Portal, registerPortalRoot} from './Portal' +import {Portal, PortalContext, registerPortalRoot} from './Portal' import classes from './Portal.stories.module.css' import {clsx} from 'clsx' @@ -81,3 +81,107 @@ export const MultiplePortalRoots: React.FC ) } + +export const WithPortalContext = () => { + const customContainerRef = React.useRef(null) + const overrideContainerRef = React.useRef(null) + const [mounted, setMounted] = React.useState(false) + useEffect(() => { + if (customContainerRef.current instanceof HTMLElement && overrideContainerRef.current instanceof HTMLElement) { + registerPortalRoot(customContainerRef.current, 'custom-portal') + registerPortalRoot(overrideContainerRef.current, 'override-portal') + setMounted(true) + } + }, []) + + return ( + <> +
+

Using PortalContext

+

This story demonstrates how to use PortalContext to control where Portal content is rendered.

+ + {/* Default Portal */} +
+ Default Portal (no context): + {mounted ? ( + +
+ Content in default portal +
+
+ ) : null} +
+ + {/* Portal with Context */} +
+ Portal with PortalContext: + + {mounted ? ( + +
+ Content in custom portal (via PortalContext) +
+
+ ) : null} +
+
+ + {/* Override context with containerName prop */} +
+ Context + containerName prop override: + + {mounted ? ( + +
+ Content overriding context with containerName prop +
+
+ ) : null} +
+
+
+ + {/* Custom portal containers */} +
+ Custom Portal Container: +
+
+ +
+ Override Portal Container: +
+
+ + ) +} diff --git a/packages/react/src/Portal/Portal.tsx b/packages/react/src/Portal/Portal.tsx index 2e262db7aa0..490b901c62a 100644 --- a/packages/react/src/Portal/Portal.tsx +++ b/packages/react/src/Portal/Portal.tsx @@ -72,7 +72,6 @@ export const Portal: React.FC> = ({ }) => { const {portalContainerName} = useContext(PortalContext) const elementRef = React.useRef(null) - const {portalContainerName} = useContext(PortalContext) if (!elementRef.current) { const div = document.createElement('div') // Portaled content should get their own stacking context so they don't interfere From ebfc64845dcefe8cce629e0401c3d8345c1c44a3 Mon Sep 17 00:00:00 2001 From: Marie Lucca <40550942+francinelucca@users.noreply.github.com> Date: Wed, 10 Sep 2025 10:16:00 -0400 Subject: [PATCH 05/13] Update version for @primer/react and rename context --- .changeset/cuddly-ads-behave.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/cuddly-ads-behave.md diff --git a/.changeset/cuddly-ads-behave.md b/.changeset/cuddly-ads-behave.md new file mode 100644 index 00000000000..5c3b6f07ecf --- /dev/null +++ b/.changeset/cuddly-ads-behave.md @@ -0,0 +1,5 @@ +--- +"@primer/react": minor +--- + +chore: add PortalContext From de1e26e4b51d86f5d3f65908fc5f4d112482c1e6 Mon Sep 17 00:00:00 2001 From: Marie Lucca <40550942+francinelucca@users.noreply.github.com> Date: Wed, 10 Sep 2025 10:19:49 -0400 Subject: [PATCH 06/13] Update packages/react/src/Portal/Portal.tsx Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- packages/react/src/Portal/Portal.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/react/src/Portal/Portal.tsx b/packages/react/src/Portal/Portal.tsx index 490b901c62a..88e218d9093 100644 --- a/packages/react/src/Portal/Portal.tsx +++ b/packages/react/src/Portal/Portal.tsx @@ -86,7 +86,7 @@ export const Portal: React.FC> = ({ useLayoutEffect(() => { let containerName = _containerName ?? portalContainerName - if (containerName === undefined || containerName === DEFAULT_PORTAL_CONTAINER_NAME) { + if (containerName === undefined) { containerName = DEFAULT_PORTAL_CONTAINER_NAME ensureDefaultPortal() } From c21c23416ef6b26209e9bb1f973973f5e3feaeda Mon Sep 17 00:00:00 2001 From: Marie Lucca <40550942+francinelucca@users.noreply.github.com> Date: Wed, 10 Sep 2025 10:20:31 -0400 Subject: [PATCH 07/13] Update packages/react/src/Portal/Portal.tsx Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- packages/react/src/Portal/Portal.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/react/src/Portal/Portal.tsx b/packages/react/src/Portal/Portal.tsx index 88e218d9093..50d368c6c28 100644 --- a/packages/react/src/Portal/Portal.tsx +++ b/packages/react/src/Portal/Portal.tsx @@ -45,7 +45,7 @@ function ensureDefaultPortal() { export const PortalContext = React.createContext<{ portalContainerName?: string -}>({portalContainerName: DEFAULT_PORTAL_CONTAINER_NAME}) +}>({}) export interface PortalProps { /** From f31ce63a2660b20850a862ab841c5e62584c63d1 Mon Sep 17 00:00:00 2001 From: Marie Lucca <40550942+francinelucca@users.noreply.github.com> Date: Wed, 10 Sep 2025 10:21:40 -0400 Subject: [PATCH 08/13] Update packages/react/src/Portal/Portal.tsx Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- packages/react/src/Portal/Portal.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/react/src/Portal/Portal.tsx b/packages/react/src/Portal/Portal.tsx index 50d368c6c28..1eb57434d40 100644 --- a/packages/react/src/Portal/Portal.tsx +++ b/packages/react/src/Portal/Portal.tsx @@ -94,7 +94,7 @@ export const Portal: React.FC> = ({ if (!parentElement) { throw new Error( - `Portal container '${containerName}' is not yet registered. Container must be registered with registerPortal before use.`, + `Portal container '${containerName}' is not yet registered. Container must be registered with registerPortalRoot before use.`, ) } parentElement.appendChild(element) From fd01e9c263fcda5d378e1a20130e8510e5d3efb4 Mon Sep 17 00:00:00 2001 From: Marie Lucca <40550942+francinelucca@users.noreply.github.com> Date: Thu, 11 Sep 2025 10:04:26 -0400 Subject: [PATCH 09/13] Update packages/react/src/Portal/Portal.tsx Co-authored-by: Adam Dierkens --- packages/react/src/Portal/Portal.tsx | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/packages/react/src/Portal/Portal.tsx b/packages/react/src/Portal/Portal.tsx index 1eb57434d40..74159a198b9 100644 --- a/packages/react/src/Portal/Portal.tsx +++ b/packages/react/src/Portal/Portal.tsx @@ -43,6 +43,10 @@ function ensureDefaultPortal() { } } +/** + * Provides the ability for component trees to override the portal root container for a sub-set of the experience. + * The portal will prioritize the context value unless overridden by their own `containerName` prop, and fallback to the default root if neither are specified + */ export const PortalContext = React.createContext<{ portalContainerName?: string }>({}) From ed0f8896906fef20660523c3a657a93f391b847e Mon Sep 17 00:00:00 2001 From: Marie Lucca Date: Thu, 11 Sep 2025 10:25:09 -0400 Subject: [PATCH 10/13] lint --- packages/react/src/Portal/Portal.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/react/src/Portal/Portal.tsx b/packages/react/src/Portal/Portal.tsx index 74159a198b9..816b4c8437d 100644 --- a/packages/react/src/Portal/Portal.tsx +++ b/packages/react/src/Portal/Portal.tsx @@ -44,8 +44,8 @@ function ensureDefaultPortal() { } /** - * Provides the ability for component trees to override the portal root container for a sub-set of the experience. - * The portal will prioritize the context value unless overridden by their own `containerName` prop, and fallback to the default root if neither are specified + * Provides the ability for component trees to override the portal root container for a sub-set of the experience. + * The portal will prioritize the context value unless overridden by their own `containerName` prop, and fallback to the default root if neither are specified */ export const PortalContext = React.createContext<{ portalContainerName?: string From 2b0433962928a4c157afcb643c3143026908bdd5 Mon Sep 17 00:00:00 2001 From: Marie Lucca Date: Thu, 11 Sep 2025 10:25:50 -0400 Subject: [PATCH 11/13] update snapshot --- packages/react/src/__tests__/__snapshots__/exports.test.ts.snap | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/react/src/__tests__/__snapshots__/exports.test.ts.snap b/packages/react/src/__tests__/__snapshots__/exports.test.ts.snap index 0c462053e33..7bfd870d729 100644 --- a/packages/react/src/__tests__/__snapshots__/exports.test.ts.snap +++ b/packages/react/src/__tests__/__snapshots__/exports.test.ts.snap @@ -118,6 +118,7 @@ exports[`@primer/react > should not update exports without a semver change 1`] = "type PopoverContentProps", "type PopoverProps", "Portal", + "PortalContext", "type PortalProps", "ProgressBar", "type ProgressBarItemProps", From 34f076467bb06a503137860792ffead92d1cda8e Mon Sep 17 00:00:00 2001 From: Marie Lucca Date: Thu, 11 Sep 2025 11:00:19 -0400 Subject: [PATCH 12/13] update story position --- packages/react/src/Portal/Portal.features.stories.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/react/src/Portal/Portal.features.stories.tsx b/packages/react/src/Portal/Portal.features.stories.tsx index 46321c7e96e..6e5cab74bd3 100644 --- a/packages/react/src/Portal/Portal.features.stories.tsx +++ b/packages/react/src/Portal/Portal.features.stories.tsx @@ -154,8 +154,8 @@ export const WithPortalContext = () => {
Date: Fri, 12 Sep 2025 14:23:21 -0400 Subject: [PATCH 13/13] [WIP] fix: address accessibility issue with dark theme using token background colors (#6847) Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: francinelucca <40550942+francinelucca@users.noreply.github.com> --- .../src/Portal/Portal.features.stories.tsx | 53 +++------------- .../src/Portal/Portal.stories.module.css | 62 +++++++++++++++++++ 2 files changed, 70 insertions(+), 45 deletions(-) diff --git a/packages/react/src/Portal/Portal.features.stories.tsx b/packages/react/src/Portal/Portal.features.stories.tsx index 6e5cab74bd3..4a91b0cad4d 100644 --- a/packages/react/src/Portal/Portal.features.stories.tsx +++ b/packages/react/src/Portal/Portal.features.stories.tsx @@ -101,49 +101,34 @@ export const WithPortalContext = () => {

This story demonstrates how to use PortalContext to control where Portal content is rendered.

{/* Default Portal */} -
+
Default Portal (no context): {mounted ? ( -
- Content in default portal -
+
Content in default portal
) : null}
{/* Portal with Context */} -
+
Portal with PortalContext: {mounted ? ( -
- Content in custom portal (via PortalContext) -
+
Content in custom portal (via PortalContext)
) : null}
{/* Override context with containerName prop */} -
+
Context + containerName prop override: {mounted ? ( -
- Content overriding context with containerName prop -
+
Content overriding context with containerName prop
) : null}
@@ -151,34 +136,12 @@ export const WithPortalContext = () => {
{/* Custom portal containers */} -
+
Custom Portal Container:
-
+
Override Portal Container:
diff --git a/packages/react/src/Portal/Portal.stories.module.css b/packages/react/src/Portal/Portal.stories.module.css index 73850efb469..36163852772 100644 --- a/packages/react/src/Portal/Portal.stories.module.css +++ b/packages/react/src/Portal/Portal.stories.module.css @@ -9,3 +9,65 @@ .InnerContainer { background-color: var(--bgColor-success-muted); } + +/* Portal Context Story Styles */ +.DefaultSection { + background-color: var(--bgColor-accent-muted); + margin: var(--base-size-8); + padding: var(--base-size-8); +} + +.ContextSection { + background-color: var(--bgColor-attention-muted); + margin: var(--base-size-8); + padding: var(--base-size-8); +} + +.OverrideSection { + background-color: var(--bgColor-success-muted); + margin: var(--base-size-8); + padding: var(--base-size-8); +} + +.DefaultPortalContent { + background-color: var(--bgColor-accent-emphasis); + padding: var(--base-size-4); + border: var(--borderWidth-thin) solid var(--borderColor-accent-emphasis); + color: var(--fgColor-onEmphasis); +} + +.ContextPortalContent { + background-color: var(--bgColor-attention-emphasis); + padding: var(--base-size-4); + border: var(--borderWidth-thin) solid var(--borderColor-attention-emphasis); + color: var(--fgColor-onEmphasis); +} + +.OverridePortalContent { + background-color: var(--bgColor-success-emphasis); + padding: var(--base-size-4); + border: var(--borderWidth-thin) solid var(--borderColor-success-emphasis); + color: var(--fgColor-onEmphasis); +} + +.CustomPortalContainer { + position: fixed; + bottom: var(--base-size-8); + left: var(--base-size-8); + background-color: var(--bgColor-neutral-muted); + padding: var(--base-size-8); + border: var(--borderWidth-thick) solid var(--borderColor-attention-emphasis); + border-radius: var(--borderRadius-medium); + max-width: 200px; +} + +.OverridePortalContainer { + position: fixed; + bottom: var(--base-size-8); + right: var(--base-size-8); + background-color: var(--bgColor-neutral-muted); + padding: var(--base-size-8); + border: var(--borderWidth-thick) solid var(--borderColor-success-emphasis); + border-radius: var(--borderRadius-medium); + max-width: 200px; +}