diff --git a/.changeset/soft-pianos-carry.md b/.changeset/soft-pianos-carry.md new file mode 100644 index 00000000000..bebeac93431 --- /dev/null +++ b/.changeset/soft-pianos-carry.md @@ -0,0 +1,5 @@ +--- +'@primer/react': minor +--- + +AnchoredOverlay: Add Popover API to AnchoredOverlay (behind `primer_react_css_anchor_positioning` feature flag) diff --git a/e2e/components/AnchoredOverlay.test.ts b/e2e/components/AnchoredOverlay.test.ts index 3cff067da0c..37bf305df1a 100644 --- a/e2e/components/AnchoredOverlay.test.ts +++ b/e2e/components/AnchoredOverlay.test.ts @@ -9,6 +9,7 @@ const stories: Array<{ viewport?: keyof typeof viewports waitForText?: string buttonName?: string + buttonNames?: string[] openDialog?: boolean openNestedDialog?: boolean }> = [ @@ -97,6 +98,11 @@ const stories: Array<{ // buttonName: 'Open Overlay', // openDialog: true, // }, + { + title: 'Multiple Overlays', + id: 'components-anchoredoverlay-features--multiple-overlays', + buttonNames: ['renderAnchor 1', 'External anchor 1', 'renderAnchor 2', 'External anchor 2'], + }, { title: 'Within Sticky Element', id: 'components-anchoredoverlay-features--within-sticky-element', @@ -153,19 +159,36 @@ test.describe('AnchoredOverlay', () => { await page.getByRole('button', {name: 'Open Inner Dialog'}).click() } - // Open the overlay - const buttonName = story.buttonName ?? 'Button' - await page.locator('button', {hasText: buttonName}).first().waitFor() - const overlayButton = page.getByRole('button', {name: buttonName}).first() - await overlayButton.click() + // If the story has multiple overlays, screenshot each one individually + if (story.buttonNames) { + for (const name of story.buttonNames) { + await page.locator('button', {hasText: name}).first().waitFor() + const btn = page.getByRole('button', {name}).first() + await btn.click() + await waitForImages(page) + + expect(await page.screenshot({animations: 'disabled'})).toMatchSnapshot( + `AnchoredOverlay.${story.title}.${name}.${theme}${namePostfix}.png`, + ) - // for the dev stories, we intentionally change the content after the overlay is open to test that it repositions correctly - if (story.waitForText) await page.getByText(story.waitForText).waitFor() - await waitForImages(page) + // Close the overlay before opening the next one + await btn.click() + } + } else { + // Open the overlay + const buttonName = story.buttonName ?? 'Button' + await page.locator('button', {hasText: buttonName}).first().waitFor() + const overlayButton = page.getByRole('button', {name: buttonName}).first() + await overlayButton.click() - expect(await page.screenshot({animations: 'disabled'})).toMatchSnapshot( - `AnchoredOverlay.${story.title}.${theme}${namePostfix}.png`, - ) + // for the dev stories, we intentionally change the content after the overlay is open to test that it repositions correctly + if (story.waitForText) await page.getByText(story.waitForText).waitFor() + await waitForImages(page) + + expect(await page.screenshot({animations: 'disabled'})).toMatchSnapshot( + `AnchoredOverlay.${story.title}.${theme}${namePostfix}.png`, + ) + } }) } }) diff --git a/packages/react/src/ActionMenu/ActionMenu.test.tsx b/packages/react/src/ActionMenu/ActionMenu.test.tsx index ffdec1402a6..26bd8df5096 100644 --- a/packages/react/src/ActionMenu/ActionMenu.test.tsx +++ b/packages/react/src/ActionMenu/ActionMenu.test.tsx @@ -9,7 +9,6 @@ import {Tooltip as TooltipV2} from '../TooltipV2/Tooltip' import {SingleSelect} from '../ActionMenu/ActionMenu.features.stories' import {MixedSelection} from '../ActionMenu/ActionMenu.examples.stories' import {SearchIcon, KebabHorizontalIcon} from '@primer/octicons-react' -import anchoredOverlayClasses from '../AnchoredOverlay/AnchoredOverlay.module.css' import {getAnchoredPosition} from '@primer/behaviors' import type {AnchorPosition} from '@primer/behaviors' @@ -651,7 +650,6 @@ describe('ActionMenu', () => { ) const anchor = component.getByRole('button', {name: 'Toggle Menu'}) expect(anchor).toHaveClass('test-class') - expect(anchor).toHaveClass(anchoredOverlayClasses.Anchor) }) it('supports className prop on ActionMenu.Button with css anchor positioning flag', async () => { @@ -680,7 +678,6 @@ describe('ActionMenu', () => { ) const button = component.getByRole('button', {name: 'Toggle Menu'}) expect(button).toHaveClass('test-class') - expect(button).toHaveClass(anchoredOverlayClasses.Anchor) }) it('supports className prop on ActionMenu.Anchor', async () => { @@ -711,7 +708,6 @@ describe('ActionMenu', () => { ) const anchor = component.getByRole('button', {name: 'Toggle Menu'}) expect(anchor).toHaveClass('test-class') - expect(anchor).not.toHaveClass(anchoredOverlayClasses.Anchor) }) it('supports className prop on ActionMenu.Button', async () => { @@ -740,7 +736,6 @@ describe('ActionMenu', () => { ) const button = component.getByRole('button', {name: 'Toggle Menu'}) expect(button).toHaveClass('test-class') - expect(button).not.toHaveClass(anchoredOverlayClasses.Anchor) }) }) diff --git a/packages/react/src/AnchoredOverlay/AnchoredOverlay.features.stories.tsx b/packages/react/src/AnchoredOverlay/AnchoredOverlay.features.stories.tsx index d7602f5c616..9656519d5a8 100644 --- a/packages/react/src/AnchoredOverlay/AnchoredOverlay.features.stories.tsx +++ b/packages/react/src/AnchoredOverlay/AnchoredOverlay.features.stories.tsx @@ -575,6 +575,95 @@ export const WithinDialogOverflowing = () => { ) } +export const MultipleOverlays = () => { + const [openOverlay, setOpenOverlay] = useState(null) + const externalAnchorRefA = useRef(null) + const externalAnchorRefB = useRef(null) + + const open = (key: string) => () => setOpenOverlay(key) + const close = () => setOpenOverlay(null) + + return ( + + } + overlayProps={{ + role: 'dialog', + 'aria-modal': true, + 'aria-label': 'Overlay 1', + }} + focusZoneSettings={{disabled: true}} + preventOverflow={false} + > +
{hoverCard}
+
+ + + +
{hoverCard}
+
+ + } + overlayProps={{ + role: 'dialog', + 'aria-modal': true, + 'aria-label': 'Overlay 3', + }} + focusZoneSettings={{disabled: true}} + preventOverflow={false} + > +
{hoverCard}
+
+ + + +
{hoverCard}
+
+
+ ) +} + export const WithinStickyElement = () => { return (
diff --git a/packages/react/src/AnchoredOverlay/AnchoredOverlay.module.css b/packages/react/src/AnchoredOverlay/AnchoredOverlay.module.css index 72485a8d665..5edbdb0a043 100644 --- a/packages/react/src/AnchoredOverlay/AnchoredOverlay.module.css +++ b/packages/react/src/AnchoredOverlay/AnchoredOverlay.module.css @@ -13,25 +13,23 @@ } } -.Wrapper { - anchor-scope: --anchored-overlay-anchor; -} - -.Anchor { - /* Anchor name, this is currently tied to `renderAnchor` */ - anchor-name: --anchored-overlay-anchor; -} - .AnchoredOverlay { - /* Anchor position, this is currently tied to `` */ - position-anchor: --anchored-overlay-anchor; position-try-fallbacks: flip-block, flip-inline, flip-block flip-inline; position-visibility: anchors-visible; z-index: 100; - position: fixed; + position: fixed !important; + + &[popover] { + inset: auto; + margin: 0; + padding: 0; + border: 0; + max-height: none; + max-width: none; + } &[data-side='outside-bottom'] { /* stylelint-disable primer/spacing */ diff --git a/packages/react/src/AnchoredOverlay/AnchoredOverlay.test.tsx b/packages/react/src/AnchoredOverlay/AnchoredOverlay.test.tsx index 8bcb8d46284..76b079c38d1 100644 --- a/packages/react/src/AnchoredOverlay/AnchoredOverlay.test.tsx +++ b/packages/react/src/AnchoredOverlay/AnchoredOverlay.test.tsx @@ -11,6 +11,7 @@ import {FeatureFlags} from '../FeatureFlags' import overlayClasses from '../Overlay/Overlay.module.css' import anchoredOverlayClasses from './AnchoredOverlay.module.css' +import type {OverlayProps} from '../Overlay' type TestComponentSettings = { initiallyOpen?: boolean @@ -19,6 +20,7 @@ type TestComponentSettings = { onPositionChange?: ({position}: {position: AnchorPosition}) => void className?: string withCSSAnchorPositioningFeatureFlag?: boolean + overlayProps?: Pick } const AnchoredOverlayTestComponent = ({ @@ -28,6 +30,7 @@ const AnchoredOverlayTestComponent = ({ onPositionChange, className, withCSSAnchorPositioningFeatureFlag, + overlayProps, }: TestComponentSettings = {}) => { const [open, setOpen] = useState(initiallyOpen) const onOpen = useCallback( @@ -54,6 +57,7 @@ const AnchoredOverlayTestComponent = ({ renderAnchor={props => } onPositionChange={onPositionChange} className={className} + {...overlayProps} > @@ -202,7 +206,6 @@ describe.each([true, false])( { @@ -221,62 +224,62 @@ describe.each([true, false])( render() - expect(document.getElementById('overlay')).toBe(ref.current) + expect(ref.current).toBeInstanceOf(HTMLDivElement) + expect(ref.current).toHaveAttribute('data-component', 'AnchoredOverlay') }) }, ) describe('AnchoredOverlay feature flag specific behavior', () => { describe('with primer_react_css_anchor_positioning feature flag enabled', () => { - it('should render wrapper div when flag is enabled', () => { - const {container} = render( - - - , - ) - - const wrapper = container.querySelector(`.${anchoredOverlayClasses.Wrapper}`) - expect(wrapper).toBeInTheDocument() - }) - - it('should apply Anchor class to anchor element when flag is enabled', () => { - const {container} = render( + it('should render overlay as visible immediately when flag is enabled', () => { + const {baseElement} = render( , ) - const anchor = container.querySelector('[aria-haspopup="true"]') - expect(anchor).toHaveClass(anchoredOverlayClasses.Anchor) + const overlay = baseElement.querySelector('[data-component="AnchoredOverlay"]') + expect(overlay).toHaveAttribute('data-visibility-visible', '') }) - it('should render overlay as visible immediately when flag is enabled', () => { + it('should use portal when flag is enabled', () => { const {baseElement} = render( - + , ) - const overlay = baseElement.querySelector('[data-component="AnchoredOverlay"]') - expect(overlay).toHaveAttribute('data-visibility-visible', '') + const portalRoot = baseElement.querySelector('#__primerPortalRoot__') + const overlayInPortal = portalRoot?.querySelector('[data-component="AnchoredOverlay"]') + expect(overlayInPortal).toBeInTheDocument() }) - it('should not use portal when flag is enabled', () => { + it('should not use portal when disablePortal is passed via overlayProps', () => { const {baseElement, container} = render( - + + {}} + onClose={() => {}} + renderAnchor={props => } + overlayProps={{disablePortal: true}} + > + + + , ) - // The overlay should be inside the component tree, not in the portal root + // The overlay should not be inside the portal root const portalRoot = baseElement.querySelector('#__primerPortalRoot__') const overlayInPortal = portalRoot?.querySelector('[data-component="AnchoredOverlay"]') expect(overlayInPortal).toBeNull() - // The overlay should be inside the wrapper - const wrapper = container.querySelector(`.${anchoredOverlayClasses.Wrapper}`) - const overlayInWrapper = wrapper?.querySelector('[data-component="AnchoredOverlay"]') - expect(overlayInWrapper).toBeInTheDocument() + // The overlay should be inside the container + const overlayInContainer = container.querySelector('[data-component="AnchoredOverlay"]') + expect(overlayInContainer).toBeInTheDocument() }) it('should apply AnchoredOverlay class to overlay when flag is enabled', () => { @@ -303,28 +306,6 @@ describe('AnchoredOverlay feature flag specific behavior', () => { }) describe('with primer_react_css_anchor_positioning feature flag disabled', () => { - it('should not render wrapper div when flag is disabled', () => { - const {container} = render( - - - , - ) - - const wrapper = container.querySelector(`.${anchoredOverlayClasses.Wrapper}`) - expect(wrapper).not.toBeInTheDocument() - }) - - it('should not apply Anchor class to anchor element when flag is disabled', () => { - const {container} = render( - - - , - ) - - const anchor = container.querySelector('[aria-haspopup="true"]') - expect(anchor).not.toHaveClass(anchoredOverlayClasses.Anchor) - }) - it('should use portal when flag is disabled', () => { const {baseElement} = render( diff --git a/packages/react/src/AnchoredOverlay/AnchoredOverlay.tsx b/packages/react/src/AnchoredOverlay/AnchoredOverlay.tsx index 276c1d4eba1..dd000c6531a 100644 --- a/packages/react/src/AnchoredOverlay/AnchoredOverlay.tsx +++ b/packages/react/src/AnchoredOverlay/AnchoredOverlay.tsx @@ -1,5 +1,6 @@ import type React from 'react' import {useCallback, useEffect, useRef, type JSX} from 'react' +import useLayoutEffect from '../utils/useIsomorphicLayoutEffect' import type {OverlayProps} from '../Overlay' import Overlay from '../Overlay' import type {FocusTrapHookSettings} from '../hooks/useFocusTrap' @@ -196,13 +197,18 @@ export const AnchoredOverlay: React.FC { @@ -247,13 +253,51 @@ export const AnchoredOverlay: React.FC { + if (!cssAnchorPositioning || !anchorRef.current) return + + const anchor = anchorRef.current + const overlay = overlayRef.current + anchor.style.setProperty('anchor-name', `--anchored-overlay-anchor-${id}`) + + return () => { + anchor.style.removeProperty('anchor-name') + if (overlay) { + overlay.style.removeProperty('position-anchor') + } + } + }, [cssAnchorPositioning, anchorRef, overlayRef, id]) + + // Track the overlay element so we can re-run the effect when it changes. + // This is necessary when using a Portal, as React re-renders can replace + // the DOM node, causing the popover to lose its :popover-open state. + const overlayElement = overlayRef.current + + useLayoutEffect(() => { + // Read ref inside effect to get the value after child refs are attached + const currentOverlay = overlayRef.current + + if (!cssAnchorPositioning || !open || !currentOverlay) return + currentOverlay.style.setProperty('position-anchor', `--anchored-overlay-anchor-${id}`) + try { + if (!currentOverlay.matches(':popover-open')) { + currentOverlay.showPopover() + } + } catch { + // Ignore if popover is already showing or not supported + } + }, [cssAnchorPositioning, open, overlayElement, id, overlayRef]) + const showXIcon = onClose && variant.narrow === 'fullscreen' && displayCloseButton const XButtonAriaLabelledBy = closeButtonProps['aria-labelledby'] const XButtonAriaLabel = closeButtonProps['aria-label'] - const {className: overlayClassName, ...restOverlayProps} = overlayProps || {} + const {className: overlayClassName, disablePortal, ...restOverlayProps} = overlayProps || {} - const innerContent = ( + return ( <> {renderAnchor && renderAnchor({ @@ -264,7 +308,7 @@ export const AnchoredOverlay: React.FC { if (overlayProps?.ref) { assignRef(overlayProps.ref, node) @@ -316,12 +363,6 @@ export const AnchoredOverlay: React.FC ) - - if (cssAnchorPositioning) { - return
{innerContent}
- } - - return innerContent } function assignRef( diff --git a/packages/react/src/Overlay/Overlay.tsx b/packages/react/src/Overlay/Overlay.tsx index ae39f51ceec..ad992c0c06b 100644 --- a/packages/react/src/Overlay/Overlay.tsx +++ b/packages/react/src/Overlay/Overlay.tsx @@ -138,6 +138,7 @@ export const BaseOverlay = React.forwardRef( type ContainerProps = { anchorSide?: AnchorSide + disablePortal?: boolean ignoreClickRefs?: React.RefObject[] initialFocusRef?: React.RefObject onClickOutside: (e: TouchOrMouseEvent) => void @@ -152,6 +153,7 @@ type internalOverlayProps = Merge /** * @param anchorSide If provided, the Overlay will slide into position from the side of the anchor with a brief animation + * @param disablePortal Optional. If true, the Overlay will not be rendered in a Portal. Defaults to false. When using CSS anchor positioning, popovers render in the browser's top layer which already escapes stacking contexts, so a Portal is not strictly necessary but still useful for style isolation. * @param height Sets the height of the `Overlay`, pick from our set list of heights, or pass `auto` to automatically set the height based on the content of the `Overlay`, or pass `initial` to set the height based on the initial content of the `Overlay` (i.e. ignoring content changes). `xsmall` corresponds to `192px`, `small` corresponds to `256px`, `medium` corresponds to `320px`, `large` corresponds to `432px`, `xlarge` corresponds to `600px`. * @param ignoreClickRefs Optional. An array of ref objects to ignore clicks on in the `onOutsideClick` behavior. This is often used to ignore clicking on the element that toggles the open/closed state for the `Overlay` to prevent the `Overlay` from being toggled twice. * @param initialFocusRef Optional. Ref for the element to focus when the `Overlay` is opened. If nothing is provided, the first focusable element in the `Overlay` body is focused. @@ -170,6 +172,7 @@ const Overlay = React.forwardRef( ( { anchorSide, + disablePortal, height = 'auto', ignoreClickRefs, initialFocusRef, @@ -190,7 +193,6 @@ const Overlay = React.forwardRef( forwardedRef, // eslint-disable-next-line @typescript-eslint/no-explicit-any ): ReactElement => { - const cssAnchorPositioning = useFeatureFlag('primer_react_css_anchor_positioning') const featureFlagMaxHeightClampToViewport = useFeatureFlag('primer_react_overlay_max_height_clamp_to_viewport') const overlayRef = useRef(null) useRefObjectAsForwardedRef(forwardedRef, overlayRef) @@ -248,7 +250,13 @@ const Overlay = React.forwardRef( /> ) - if (cssAnchorPositioning) { + // disablePortal can be used to render the overlay without a Portal. + // When using CSS anchor positioning, popovers render in the browser's + // top layer which already escapes stacking contexts, so a Portal is + // not strictly necessary. However, Portal can still be useful for + // style isolation. Defaults to false (Portal enabled) for backwards + // compatibility. + if (disablePortal) { return overlayContent }