Skip to content
9 changes: 5 additions & 4 deletions packages/@react-aria/focus/src/FocusScope.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import {
isChrome,
isFocusable,
isTabbable,
nodeContains,
ShadowTreeWalker,
useLayoutEffect
} from '@react-aria/utils';
Expand Down Expand Up @@ -440,7 +441,7 @@ function isElementInScope(element?: Element | null, scope?: Element[] | null) {
if (!scope) {
return false;
}
return scope.some(node => node.contains(element));
return scope.some(node => nodeContains(node, element));
}

function isElementInChildScope(element: Element, scope: ScopeRef = null) {
Expand Down Expand Up @@ -771,7 +772,7 @@ export function getFocusableTreeWalker(root: Element, opts?: FocusManagerOptions
{
acceptNode(node) {
// Skip nodes inside the starting node.
if (opts?.from?.contains(node)) {
if (opts?.from && nodeContains(opts.from, node)) {
return NodeFilter.FILTER_REJECT;
}

Expand Down Expand Up @@ -822,7 +823,7 @@ export function createFocusManager(ref: RefObject<Element | null>, defaultOption
let {from, tabbable = defaultOptions.tabbable, wrap = defaultOptions.wrap, accept = defaultOptions.accept} = opts;
let node = from || getActiveElement(getOwnerDocument(root));
let walker = getFocusableTreeWalker(root, {tabbable, accept});
if (root.contains(node)) {
if (nodeContains(root, node)) {
walker.currentNode = node!;
}
let nextNode = walker.nextNode() as FocusableElement;
Expand All @@ -843,7 +844,7 @@ export function createFocusManager(ref: RefObject<Element | null>, defaultOption
let {from, tabbable = defaultOptions.tabbable, wrap = defaultOptions.wrap, accept = defaultOptions.accept} = opts;
let node = from || getActiveElement(getOwnerDocument(root));
let walker = getFocusableTreeWalker(root, {tabbable, accept});
if (root.contains(node)) {
if (nodeContains(root, node)) {
walker.currentNode = node!;
} else {
let next = last(walker);
Expand Down
203 changes: 203 additions & 0 deletions packages/@react-aria/focus/test/FocusScope.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import {Provider} from '@react-spectrum/provider';
import React, {useEffect, useState} from 'react';
import ReactDOM from 'react-dom';
import {Example as StorybookExample} from '../stories/FocusScope.stories';
import {UNSAFE_PortalProvider} from '@react-aria/overlays';
import {useEvent} from '@react-aria/utils';
import userEvent from '@testing-library/user-event';

Expand Down Expand Up @@ -2150,6 +2151,208 @@ describe('FocusScope with Shadow DOM', function () {
unmount();
document.body.removeChild(shadowHost);
});

it('should reproduce the specific issue #8675: Menu items in popover close immediately with UNSAFE_PortalProvider', async function () {
const {shadowRoot, cleanup} = createShadowRoot();
let actionExecuted = false;
let menuClosed = false;

// Create portal container within the shadow DOM for the popover
const popoverPortal = document.createElement('div');
popoverPortal.setAttribute('data-testid', 'popover-portal');
shadowRoot.appendChild(popoverPortal);

// This reproduces the exact scenario described in the issue
function WebComponentWithReactApp() {
const [isPopoverOpen, setIsPopoverOpen] = React.useState(true);

const handleMenuAction = key => {
actionExecuted = true;
// In the original issue, this never executes because the popover closes first
console.log('Menu action executed:', key);
};

return (
<UNSAFE_PortalProvider getContainer={() => shadowRoot}>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same

<div data-testid="web-component-root">
<button
data-testid="close-popover"
onClick={() => {
setIsPopoverOpen(false);
menuClosed = true;
}}
style={{position: 'absolute', top: 0, right: 0}}>
Close
</button>
{/* Portal the popover overlay to simulate real-world usage */}
{isPopoverOpen &&
ReactDOM.createPortal(
<FocusScope contain restoreFocus>
<div data-testid="popover-overlay">
<FocusScope contain>
<div role="menu" data-testid="menu-container">
<button role="menuitem" data-testid="menu-item-save" onClick={() => handleMenuAction('save')}>
Save Document
</button>
<button
role="menuitem"
data-testid="menu-item-export"
onClick={() => handleMenuAction('export')}>
Export Document
</button>
</div>
</FocusScope>
</div>
</FocusScope>,
popoverPortal
)}
</div>
</UNSAFE_PortalProvider>
);
}

const {unmount} = render(<WebComponentWithReactApp />);

// Wait for rendering
act(() => {
jest.runAllTimers();
});

// Query elements from shadow DOM
const saveMenuItem = shadowRoot.querySelector('[data-testid="menu-item-save"]');
const exportMenuItem = shadowRoot.querySelector('[data-testid="menu-item-export"]');
const menuContainer = shadowRoot.querySelector('[data-testid="menu-container"]');
const popoverOverlay = shadowRoot.querySelector('[data-testid="popover-overlay"]');
// const closeButton = shadowRoot.querySelector('[data-testid="close-popover"]');

// Verify the menu is initially visible in shadow DOM
expect(popoverOverlay).not.toBeNull();
expect(menuContainer).not.toBeNull();
expect(saveMenuItem).not.toBeNull();
expect(exportMenuItem).not.toBeNull();

// Focus the first menu item
act(() => {
saveMenuItem.focus();
});
expect(shadowRoot.activeElement).toBe(saveMenuItem);

// Click the menu item - this should execute the onAction handler, NOT close the menu
await user.click(saveMenuItem);

// The action should have been executed (this would fail in the buggy version)
expect(actionExecuted).toBe(true);

// The menu should still be open (this would fail in the buggy version where it closes immediately)
expect(menuClosed).toBe(false);
expect(shadowRoot.querySelector('[data-testid="menu-container"]')).not.toBeNull();

// Test focus containment within the menu
act(() => {
saveMenuItem.focus();
});
await user.tab();
expect(shadowRoot.activeElement).toBe(exportMenuItem);

await user.tab();
// Focus should wrap back to first item due to containment
expect(shadowRoot.activeElement).toBe(saveMenuItem);

// Cleanup
unmount();
cleanup();
});

it('should handle web component scenario with multiple nested portals and UNSAFE_PortalProvider', async function () {
const {shadowRoot, cleanup} = createShadowRoot();

// Create nested portal containers within the shadow DOM
const modalPortal = document.createElement('div');
modalPortal.setAttribute('data-testid', 'modal-portal');
shadowRoot.appendChild(modalPortal);

const tooltipPortal = document.createElement('div');
tooltipPortal.setAttribute('data-testid', 'tooltip-portal');
shadowRoot.appendChild(tooltipPortal);

function ComplexWebComponent() {
const [showModal, setShowModal] = React.useState(true);
const [showTooltip] = React.useState(true);

return (
<UNSAFE_PortalProvider getContainer={() => shadowRoot}>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same question about the context here too. I'm guessing the AI got confused, the consumer is inside our hooks/components which call createPortal, it won't just work on every instance of ReactDOM.createPortal unfortunately.

https://react-spectrum.adobe.com/react-aria/PortalProvider.html#contexts

<div data-testid="main-app">
<button data-testid="main-button">Main Button</button>

{/* Modal with its own focus scope */}
{showModal &&
ReactDOM.createPortal(
<FocusScope contain restoreFocus autoFocus>
<div data-testid="modal" role="dialog">
<button data-testid="modal-button-1">Modal Button 1</button>
<button data-testid="modal-button-2">Modal Button 2</button>
<button data-testid="close-modal" onClick={() => setShowModal(false)}>
Close Modal
</button>
</div>
</FocusScope>,
modalPortal
)}

{/* Tooltip with nested focus scope */}
{showTooltip &&
ReactDOM.createPortal(
<FocusScope>
<div data-testid="tooltip" role="tooltip">
<button data-testid="tooltip-action">Tooltip Action</button>
</div>
</FocusScope>,
tooltipPortal
)}
</div>
</UNSAFE_PortalProvider>
);
}

const {unmount} = render(<ComplexWebComponent />);

const modalButton1 = shadowRoot.querySelector('[data-testid="modal-button-1"]');
const modalButton2 = shadowRoot.querySelector('[data-testid="modal-button-2"]');
const tooltipAction = shadowRoot.querySelector('[data-testid="tooltip-action"]');

// Due to autoFocus, the first modal button should be focused
act(() => {
jest.runAllTimers();
});
expect(shadowRoot.activeElement).toBe(modalButton1);

// Tab navigation should work within the modal
await user.tab();
expect(shadowRoot.activeElement).toBe(modalButton2);

// Focus should be contained within the modal due to the contain prop
await user.tab();
// Should cycle to the close button
expect(shadowRoot.activeElement.getAttribute('data-testid')).toBe('close-modal');

await user.tab();
// Should wrap back to first modal button
expect(shadowRoot.activeElement).toBe(modalButton1);

// The tooltip button should be focusable when we explicitly focus it
act(() => {
tooltipAction.focus();
});
act(() => {
jest.runAllTimers();
});
// But due to modal containment, focus should be restored back to modal
expect(shadowRoot.activeElement).toBe(modalButton1);

// Cleanup
unmount();
cleanup();
});
});

describe('Unmounting cleanup', () => {
Expand Down
6 changes: 3 additions & 3 deletions packages/@react-aria/interactions/src/useFocusWithin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,14 +54,14 @@ export function useFocusWithin(props: FocusWithinProps): FocusWithinResult {

let onBlur = useCallback((e: FocusEvent) => {
// Ignore events bubbling through portals.
if (!e.currentTarget.contains(e.target)) {
if (!nodeContains(e.currentTarget as Element, e.target as Element)) {
return;
}

// We don't want to trigger onBlurWithin and then immediately onFocusWithin again
// when moving focus inside the element. Only trigger if the currentTarget doesn't
// include the relatedTarget (where focus is moving).
if (state.current.isFocusWithin && !(e.currentTarget as Element).contains(e.relatedTarget as Element)) {
if (state.current.isFocusWithin && !nodeContains(e.currentTarget as Element, e.relatedTarget as Element)) {
state.current.isFocusWithin = false;
removeAllGlobalListeners();

Expand All @@ -78,7 +78,7 @@ export function useFocusWithin(props: FocusWithinProps): FocusWithinResult {
let onSyntheticFocus = useSyntheticBlurEvent(onBlur);
let onFocus = useCallback((e: FocusEvent) => {
// Ignore events bubbling through portals.
if (!e.currentTarget.contains(e.target)) {
if (!nodeContains(e.currentTarget as Element, e.target as Element)) {
return;
}

Expand Down
4 changes: 2 additions & 2 deletions packages/@react-aria/interactions/src/useInteractOutside.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
// NOTICE file in the root directory of this source tree.
// See https://github.com/facebook/react/tree/cc7c1aece46a6b69b41958d731e0fd27c94bfc6c/packages/react-interactions

import {getOwnerDocument, useEffectEvent} from '@react-aria/utils';
import {getOwnerDocument, nodeContains, useEffectEvent} from '@react-aria/utils';
import {RefObject} from '@react-types/shared';
import {useEffect, useRef} from 'react';

Expand Down Expand Up @@ -121,7 +121,7 @@ function isValidEvent(event, ref) {
if (event.target) {
// if the event target is no longer in the document, ignore
const ownerDocument = event.target.ownerDocument;
if (!ownerDocument || !ownerDocument.documentElement.contains(event.target)) {
if (!ownerDocument || !nodeContains(ownerDocument.documentElement, event.target)) {
return false;
}
// If the target is within a top layer element (e.g. toasts), ignore.
Expand Down
Loading