Skip to content

Revert "Expose UNSTABLE container for RSP overlays" #5806

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Feb 5, 2024
Merged

Revert "Expose UNSTABLE container for RSP overlays" #5806

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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: 1 addition & 3 deletions packages/@react-spectrum/dialog/src/DialogContainer.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,7 @@ export function DialogContainer(props: SpectrumDialogContainerProps) {
type = 'modal',
onDismiss,
isDismissable,
isKeyboardDismissDisabled,
UNSTABLE_portalContainer
isKeyboardDismissDisabled
} = props;

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

return (
<Modal
container={UNSTABLE_portalContainer}
state={state}
type={type}
isDismissable={isDismissable}
Expand Down
7 changes: 1 addition & 6 deletions packages/@react-spectrum/dialog/src/DialogTrigger.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ function DialogTrigger(props: SpectrumDialogTriggerProps) {
targetRef,
isDismissable,
isKeyboardDismissDisabled,
UNSTABLE_portalContainer,
...positionProps
} = props;
if (!Array.isArray(children) || children.length > 2) {
Expand Down Expand Up @@ -72,7 +71,6 @@ function DialogTrigger(props: SpectrumDialogTriggerProps) {
return (
<PopoverTrigger
{...positionProps}
UNSTABLE_portalContainer={UNSTABLE_portalContainer}
state={state}
targetRef={targetRef}
trigger={trigger}
Expand All @@ -91,7 +89,6 @@ function DialogTrigger(props: SpectrumDialogTriggerProps) {
<Modal
state={state}
isDismissable={type === 'modal' ? isDismissable : false}
container={UNSTABLE_portalContainer}
type={type}
isKeyboardDismissDisabled={isKeyboardDismissDisabled}
onExiting={onExiting}
Expand All @@ -103,7 +100,6 @@ 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 @@ -147,7 +143,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, UNSTABLE_portalContainer, ...props}) {
function PopoverTrigger({state, targetRef, trigger, content, hideArrow, ...props}) {
let triggerRef = useRef<HTMLElement>(null);
let {triggerProps, overlayProps} = useOverlayTrigger({type: 'dialog'}, state, triggerRef);

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

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 {act, fireEvent, render, triggerPress, within} from '@react-spectrum/test-utils';
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, {useState} from 'react';
import React from 'react';
import {theme} from '@react-spectrum/theme-default';
import userEvent from '@testing-library/user-event';

describe('DialogContainer', function () {
beforeAll(() => {
Expand Down Expand Up @@ -214,57 +207,4 @@ 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: 0 additions & 51 deletions packages/@react-spectrum/dialog/test/DialogTrigger.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1026,55 +1026,4 @@ 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: 2 additions & 4 deletions packages/@react-spectrum/menu/src/MenuTrigger.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,7 @@ function MenuTrigger(props: SpectrumMenuTriggerProps, ref: DOMRef<HTMLElement>)
shouldFlip = true,
direction = 'bottom',
closeOnSelect,
trigger = 'press',
UNSTABLE_portalContainer
trigger = 'press'
} = props;

let [menuTrigger, menu] = React.Children.toArray(children);
Expand Down Expand Up @@ -75,7 +74,7 @@ function MenuTrigger(props: SpectrumMenuTriggerProps, ref: DOMRef<HTMLElement>)
let overlay;
if (isMobile) {
overlay = (
<Tray state={state} isFixedHeight container={UNSTABLE_portalContainer}>
<Tray state={state} isFixedHeight>
{menu}
</Tray>
);
Expand All @@ -84,7 +83,6 @@ 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: 0 additions & 52 deletions packages/@react-spectrum/menu/test/MenuTrigger.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,6 @@ 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 @@ -81,10 +80,6 @@ describe('MenuTrigger', function () {
jest.useFakeTimers();
});

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

afterEach(() => {
onOpenChange.mockClear();
onOpen.mockClear();
Expand Down Expand Up @@ -1230,51 +1225,4 @@ 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);
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: 1 addition & 2 deletions packages/@react-spectrum/overlays/src/Tray.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,7 @@ import {useViewportSize} from '@react-aria/utils';
interface TrayProps extends AriaModalOverlayProps, StyleProps, Omit<OverlayProps, 'nodeRef' | 'shouldContainFocus'> {
children: ReactNode,
state: OverlayTriggerState,
isFixedHeight?: boolean,
container?: Element
isFixedHeight?: boolean
}

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

let [trigger, tooltip] = React.Children.toArray(children) as [ReactElement, ReactElement];
Expand Down Expand Up @@ -89,7 +88,7 @@ function TooltipTrigger(props: SpectrumTooltipTriggerProps) {
arrowRef: arrowRef,
...tooltipProps
}}>
<Overlay isOpen={state.isOpen} nodeRef={overlayRef} container={UNSTABLE_portalContainer}>
<Overlay isOpen={state.isOpen} nodeRef={overlayRef}>
{tooltip}
</Overlay>
</TooltipContext.Provider>
Expand Down
42 changes: 0 additions & 42 deletions packages/@react-spectrum/tooltip/test/TooltipTrigger.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -961,46 +961,4 @@ 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