Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion packages/@react-spectrum/dialog/src/DialogContainer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,8 @@ export function DialogContainer(props: SpectrumDialogContainerProps) {
type = 'modal',
onDismiss,
isDismissable,
isKeyboardDismissDisabled
isKeyboardDismissDisabled,
UNSTABLE_portalContainer
} = props;

let childArray = React.Children.toArray(children);
Expand Down Expand Up @@ -67,6 +68,7 @@ export function DialogContainer(props: SpectrumDialogContainerProps) {

return (
<Modal
container={UNSTABLE_portalContainer}
state={state}
type={type}
isDismissable={isDismissable}
Expand Down
7 changes: 6 additions & 1 deletion packages/@react-spectrum/dialog/src/DialogTrigger.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ function DialogTrigger(props: SpectrumDialogTriggerProps) {
targetRef,
isDismissable,
isKeyboardDismissDisabled,
UNSTABLE_portalContainer,
...positionProps
} = props;
if (!Array.isArray(children) || children.length > 2) {
Expand Down Expand Up @@ -71,6 +72,7 @@ function DialogTrigger(props: SpectrumDialogTriggerProps) {
return (
<PopoverTrigger
{...positionProps}
UNSTABLE_portalContainer={UNSTABLE_portalContainer}
state={state}
targetRef={targetRef}
trigger={trigger}
Expand All @@ -89,6 +91,7 @@ function DialogTrigger(props: SpectrumDialogTriggerProps) {
<Modal
state={state}
isDismissable={type === 'modal' ? isDismissable : false}
container={UNSTABLE_portalContainer}
type={type}
isKeyboardDismissDisabled={isKeyboardDismissDisabled}
onExiting={onExiting}
Expand All @@ -100,6 +103,7 @@ function DialogTrigger(props: SpectrumDialogTriggerProps) {
return (
<Tray
state={state}
container={UNSTABLE_portalContainer}
isKeyboardDismissDisabled={isKeyboardDismissDisabled}>
{typeof content === 'function' ? content(state.close) : content}
</Tray>
Expand Down Expand Up @@ -143,7 +147,7 @@ DialogTrigger.getCollectionNode = function* (props: SpectrumDialogTriggerProps)
let _DialogTrigger = DialogTrigger as (props: SpectrumDialogTriggerProps) => JSX.Element;
export {_DialogTrigger as DialogTrigger};

function PopoverTrigger({state, targetRef, trigger, content, hideArrow, ...props}) {
function PopoverTrigger({state, targetRef, trigger, content, hideArrow, UNSTABLE_portalContainer, ...props}) {
let triggerRef = useRef<HTMLElement>(null);
let {triggerProps, overlayProps} = useOverlayTrigger({type: 'dialog'}, state, triggerRef);

Expand All @@ -155,6 +159,7 @@ function PopoverTrigger({state, targetRef, trigger, content, hideArrow, ...props
let overlay = (
<Popover
{...props}
container={UNSTABLE_portalContainer}
hideArrow={hideArrow}
triggerRef={targetRef || triggerRef}
state={state}>
Expand Down
64 changes: 62 additions & 2 deletions packages/@react-spectrum/dialog/test/DialogContainer.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,18 @@
* governing permissions and limitations under the License.
*/

import {act, fireEvent, render, triggerPress, within} from '@react-spectrum/test-utils';
import {act, fireEvent, pointerMap, render, triggerPress, within} from '@react-spectrum/test-utils';
import {ActionButton, Button} from '@react-spectrum/button';
import {ButtonGroup} from '@react-spectrum/buttongroup';
import {Content, Header} from '@react-spectrum/view';
import {Dialog, DialogContainer, useDialogContainer} from '../src';
import {DialogContainerExample, MenuExample, NestedDialogContainerExample} from '../stories/DialogContainerExamples';
import {Divider} from '@react-spectrum/divider';
import {Heading, Text} from '@react-spectrum/text';
import {Provider} from '@react-spectrum/provider';
import React from 'react';
import React, {useState} from 'react';
import {theme} from '@react-spectrum/theme-default';
import userEvent from '@testing-library/user-event';

describe('DialogContainer', function () {
beforeAll(() => {
Expand Down Expand Up @@ -207,4 +214,57 @@ describe('DialogContainer', function () {

expect(document.activeElement).toBe(button);
});

describe('portalContainer', () => {
let user;
beforeAll(() => {
user = userEvent.setup({delay: null, pointerMap});
jest.useFakeTimers();
});
function ExampleDialog(props) {
let container = useDialogContainer();

return (
<Dialog>
<Heading>The Heading</Heading>
<Header>The Header</Header>
<Divider />
<Content><Text>Lorem ipsum dolor sit amet, consectetur adipiscing elit. Proin sit amet tristique risus. In sit amet suscipit lorem. Orci varius natoque penatibus et magnis dis parturient montes, nascetur ridiculus mus. In condimentum imperdiet metus non condimentum. Duis eu velit et quam accumsan tempus at id velit. Duis elementum elementum purus, id tempus mauris posuere a. Nunc vestibulum sapien pellentesque lectus commodo ornare.</Text></Content>
{!props.isDismissable &&
<ButtonGroup>
<Button variant="secondary" onPress={container.dismiss}>Cancel</Button>
<Button variant="cta" onPress={container.dismiss}>Confirm</Button>
</ButtonGroup>
}
</Dialog>
);
}
function App(props) {
let [container, setContainer] = useState();
let [isOpen, setOpen] = useState(false);

return (
<Provider theme={theme}>
<ActionButton onPress={() => setOpen(true)}>Open dialog</ActionButton>
<DialogContainer onDismiss={() => setOpen(false)} UNSTABLE_portalContainer={container} {...props}>
{isOpen &&
<ExampleDialog {...props} />
}
</DialogContainer>
<div ref={setContainer} data-testid="custom-container" />
</Provider>
);
}

it('should render the dialog in the portal container', async () => {
let {getByRole, getByTestId} = render(
<App />
);

let button = getByRole('button');
await user.click(button);

expect(getByRole('dialog').closest('[data-testid="custom-container"]')).toBe(getByTestId('custom-container'));
});
});
});
51 changes: 51 additions & 0 deletions packages/@react-spectrum/dialog/test/DialogTrigger.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -1026,4 +1026,55 @@ describe('DialogTrigger', function () {
expect(document.activeElement).toBe(innerButton);
});

describe('portalContainer', () => {
function InfoDialog(props) {
return (
<Provider theme={theme}>
<DialogTrigger type={props.type} UNSTABLE_portalContainer={props.container}>
<ActionButton>Trigger</ActionButton>
<Dialog>contents</Dialog>
</DialogTrigger>
</Provider>
);
}
function App(props) {
let [container, setContainer] = React.useState();
return (
<>
<InfoDialog type={props.type} container={container} />
<div ref={setContainer} data-testid="custom-container" />
</>
);
}
it('should render the dialog in the portal container', async () => {
let {getByRole, getByTestId} = render(
<App />
);

let button = getByRole('button');
await user.click(button);

expect(getByRole('dialog').closest('[data-testid="custom-container"]')).toBe(getByTestId('custom-container'));
});
it('should render the tray in the portal container', async () => {
let {getByRole, getByTestId} = render(
<App type="tray" />
);

let button = getByRole('button');
await user.click(button);

expect(getByRole('dialog').closest('[data-testid="custom-container"]')).toBe(getByTestId('custom-container'));
});
it('should render the popover in the portal container', async () => {
let {getByRole, getByTestId} = render(
<App type="popover" />
);

let button = getByRole('button');
await user.click(button);

expect(getByRole('dialog').closest('[data-testid="custom-container"]')).toBe(getByTestId('custom-container'));
});
});
});
6 changes: 4 additions & 2 deletions packages/@react-spectrum/menu/src/MenuTrigger.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,8 @@ function MenuTrigger(props: SpectrumMenuTriggerProps, ref: DOMRef<HTMLElement>)
shouldFlip = true,
direction = 'bottom',
closeOnSelect,
trigger = 'press'
trigger = 'press',
UNSTABLE_portalContainer
} = props;

let [menuTrigger, menu] = React.Children.toArray(children);
Expand Down Expand Up @@ -74,7 +75,7 @@ function MenuTrigger(props: SpectrumMenuTriggerProps, ref: DOMRef<HTMLElement>)
let overlay;
if (isMobile) {
overlay = (
<Tray state={state} isFixedHeight>
<Tray state={state} isFixedHeight container={UNSTABLE_portalContainer}>
{menu}
</Tray>
);
Expand All @@ -83,6 +84,7 @@ function MenuTrigger(props: SpectrumMenuTriggerProps, ref: DOMRef<HTMLElement>)
<Popover
UNSAFE_style={{clipPath: 'unset', overflow: 'visible', filter: 'unset', borderWidth: '0px'}}
state={state}
container={UNSTABLE_portalContainer}
triggerRef={menuTriggerRef}
scrollRef={menuRef}
placement={initialPlacement}
Expand Down
52 changes: 52 additions & 0 deletions packages/@react-spectrum/menu/test/MenuTrigger.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ describe('MenuTrigger', function () {
let onSelect = jest.fn();
let onSelectionChange = jest.fn();
let user;
let windowSpy;

beforeAll(function () {
user = userEvent.setup({delay: null, pointerMap});
Expand All @@ -80,6 +81,10 @@ describe('MenuTrigger', function () {
jest.useFakeTimers();
});

beforeEach(() => {
windowSpy = jest.spyOn(window.screen, 'width', 'get').mockImplementation(() => 1024);
});

afterEach(() => {
onOpenChange.mockClear();
onOpen.mockClear();
Expand Down Expand Up @@ -1225,4 +1230,51 @@ describe('MenuTrigger', function () {
});
});
});

describe('portalContainer', () => {
function InfoMenu(props) {
return (
<Provider theme={theme}>
<MenuTrigger UNSTABLE_portalContainer={props.container}>
<ActionButton aria-label="trigger" />
<Menu>
<Item key="1">One</Item>
<Item key="">Two</Item>
<Item key="3">Three</Item>
</Menu>
</MenuTrigger>
</Provider>
);
}
function App() {
let [container, setContainer] = React.useState();
return (
<>
<InfoMenu container={container} />
<div ref={setContainer} data-testid="custom-container" />
</>
);
}
it('should render the menu in the portal container', async () => {
let {getByRole, getByTestId} = render(
<App />
);

let button = getByRole('button');
await user.click(button);

expect(getByRole('menu').closest('[data-testid="custom-container"]')).toBe(getByTestId('custom-container'));
});
it('should render the menu tray in the portal container', async () => {
windowSpy.mockImplementation(() => 700);
Copy link
Member

Choose a reason for hiding this comment

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

Seeing this test made me realize we didn't have a test for Menu loading as a tray on mobile. Submenu does have one. :)

let {getByRole, getByTestId} = render(
<App />
);

let button = getByRole('button');
await user.click(button);

expect(getByRole('menu').closest('[data-testid="custom-container"]')).toBe(getByTestId('custom-container'));
});
});
});
3 changes: 2 additions & 1 deletion packages/@react-spectrum/overlays/src/Tray.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,8 @@ import {useViewportSize} from '@react-aria/utils';
interface TrayProps extends AriaModalOverlayProps, StyleProps, Omit<OverlayProps, 'nodeRef' | 'shouldContainFocus'> {
children: ReactNode,
state: OverlayTriggerState,
isFixedHeight?: boolean
isFixedHeight?: boolean,
container?: Element
}

interface TrayWrapperProps extends TrayProps {
Expand Down
5 changes: 3 additions & 2 deletions packages/@react-spectrum/tooltip/src/TooltipTrigger.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,8 @@ function TooltipTrigger(props: SpectrumTooltipTriggerProps) {
crossOffset = DEFAULT_CROSS_OFFSET,
isDisabled,
offset = DEFAULT_OFFSET,
trigger: triggerAction
trigger: triggerAction,
UNSTABLE_portalContainer
} = props;

let [trigger, tooltip] = React.Children.toArray(children) as [ReactElement, ReactElement];
Expand Down Expand Up @@ -88,7 +89,7 @@ function TooltipTrigger(props: SpectrumTooltipTriggerProps) {
arrowRef: arrowRef,
...tooltipProps
}}>
<Overlay isOpen={state.isOpen} nodeRef={overlayRef}>
<Overlay isOpen={state.isOpen} nodeRef={overlayRef} container={UNSTABLE_portalContainer}>
Copy link
Member

Choose a reason for hiding this comment

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

Was this part of the request? It previously didn't support container. I'm trying to think of the case for this because Tooltip is usually the text label for an icon button.

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't understand the question

Copy link
Member

Choose a reason for hiding this comment

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

The container prop here is on RSP Overlay which we don't expose so it should be fine (said container gets set as the portal container internally). Users can pass UNSTABLE_portalContainer to TooltipTrigger though which falls in line with the intended change

{tooltip}
</Overlay>
</TooltipContext.Provider>
Expand Down
42 changes: 42 additions & 0 deletions packages/@react-spectrum/tooltip/test/TooltipTrigger.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -961,4 +961,46 @@ describe('TooltipTrigger', function () {
expect(queryByRole('tooltip')).toBeNull();
});
});

describe('portalContainer', () => {
function InfoTooltip(props) {
return (
<TooltipTrigger UNSTABLE_portalContainer={props.container}>
<ActionButton aria-label="trigger" />
<Tooltip>
<div data-testid="content">hello</div>
</Tooltip>
</TooltipTrigger>
);
}
function App() {
let [container, setContainer] = React.useState();
return (
<>
<InfoTooltip container={container} />
<div ref={setContainer} data-testid="custom-container" />
</>
);
}
it('should render the tooltip in the portal container', async () => {
let {getByRole, getByTestId} = render(
<Provider theme={theme}>
<App />
</Provider>
);

let button = getByRole('button');
act(() => {
button.focus();
});

expect(getByTestId('content').closest('[data-testid="custom-container"]')).toBe(getByTestId('custom-container'));
act(() => {
button.blur();
});
act(() => {
jest.advanceTimersByTime(CLOSE_TIME);
});
});
});
});
Loading