Skip to content
Closed
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
1 change: 1 addition & 0 deletions packages/@react-spectrum/actionbar/src/ActionBar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,7 @@ function ActionBarInner<T>(props: ActionBarInnerProps<T>, ref: Ref<HTMLDivElemen
overflowMode="collapse"
buttonLabelBehavior="collapse"
onAction={onAction}
UNSTABLE_portalContainer={props.UNSTABLE_portalContainer}
UNSAFE_className={classNames(styles, 'react-spectrum-ActionBar-actionGroup')}>
{children}
</ActionGroup>
Expand Down
21 changes: 13 additions & 8 deletions packages/@react-spectrum/actiongroup/src/ActionGroup.tsx
Copy link
Member

Choose a reason for hiding this comment

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

I see this is the main offender for how many levels the portal container prop would need to go through, I think the idea of doing this via context that you suggested makes sense. Maybe we could specify the portal container override somewhere high level (Provider level?) and have component level overrides for specific cases where it may differ? I would assume most of the time there will only be one place a user would want to portal stuff to typically.

On the flip side, do we have to support this prop to such a degree across all the components or should we only add it when there is a large enough use case/ask for it?

Copy link
Member Author

@snowystinger snowystinger Feb 28, 2024

Choose a reason for hiding this comment

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

The demos I've seen have already made use of a few, menu/popover/tooltip. But they're also usually in fullscreen situations, which could easily be open to everything because it's basically just another app inside the fullscreen dialog situation.

Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ function ActionGroup<T extends object>(props: SpectrumActionGroupProps<T>, ref:
onAction,
buttonLabelBehavior,
summaryIcon,
UNSTABLE_portalContainer,
...otherProps
} = props;

Expand Down Expand Up @@ -205,7 +206,8 @@ function ActionGroup<T extends object>(props: SpectrumActionGroupProps<T>, ref:
summaryIcon={summaryIcon}
hideButtonText={hideButtonText}
isOnlyItem={visibleItems === 0}
orientation={orientation} />
orientation={orientation}
UNSTABLE_portalContainer={UNSTABLE_portalContainer} />
);
}

Expand Down Expand Up @@ -247,7 +249,8 @@ function ActionGroup<T extends object>(props: SpectrumActionGroupProps<T>, ref:
item={item}
state={state}
hideButtonText={hideButtonText}
orientation={orientation} />
orientation={orientation}
UNSTABLE_portalContainer={UNSTABLE_portalContainer} />
))}
{menuItem}
</Provider>
Expand All @@ -271,10 +274,11 @@ interface ActionGroupItemProps<T> extends DOMProps, StyleProps {
staticColor?: 'white' | 'black',
hideButtonText?: boolean,
orientation?: 'horizontal' | 'vertical',
onAction?: (key: Key) => void
onAction?: (key: Key) => void,
UNSTABLE_portalContainer?: HTMLElement
}

function ActionGroupItem<T>({item, state, isDisabled, isEmphasized, staticColor, onAction, hideButtonText, orientation}: ActionGroupItemProps<T>) {
function ActionGroupItem<T>({item, state, isDisabled, isEmphasized, staticColor, onAction, hideButtonText, orientation, UNSTABLE_portalContainer}: ActionGroupItemProps<T>) {
let ref = useRef(null);
let {buttonProps} = useActionGroupItem({key: item.key}, state);
isDisabled = isDisabled || state.disabledKeys.has(item.key);
Expand Down Expand Up @@ -347,7 +351,7 @@ function ActionGroupItem<T>({item, state, isDisabled, isEmphasized, staticColor,

if (hideButtonText && textContent) {
button = (
<TooltipTrigger placement={orientation === 'vertical' ? 'end' : 'top'}>
<TooltipTrigger placement={orientation === 'vertical' ? 'end' : 'top'} UNSTABLE_portalContainer={UNSTABLE_portalContainer}>
{button}
<Tooltip>{textContent}</Tooltip>
</TooltipTrigger>
Expand All @@ -371,10 +375,11 @@ interface ActionGroupMenuProps<T> extends AriaLabelingProps {
summaryIcon?: ReactNode,
isOnlyItem?: boolean,
orientation?: 'horizontal' | 'vertical',
onAction?: (key: Key) => void
onAction?: (key: Key) => void,
UNSTABLE_portalContainer?: HTMLElement
}

function ActionGroupMenu<T>({state, isDisabled, isEmphasized, staticColor, items, onAction, summaryIcon, hideButtonText, isOnlyItem, orientation, ...otherProps}: ActionGroupMenuProps<T>) {
function ActionGroupMenu<T>({state, isDisabled, isEmphasized, staticColor, items, onAction, summaryIcon, hideButtonText, isOnlyItem, orientation, UNSTABLE_portalContainer, ...otherProps}: ActionGroupMenuProps<T>) {
// Use the key of the first item within the menu as the key of the button.
// The key must actually exist in the collection for focus to work correctly.
let key = items[0].key;
Expand Down Expand Up @@ -430,7 +435,7 @@ function ActionGroupMenu<T>({state, isDisabled, isEmphasized, staticColor, items

return (
// Use a PressResponder to send DOM props through.
<MenuTrigger align={isOnlyItem ? 'start' : 'end'} direction={orientation === 'vertical' ? 'end' : 'bottom'}>
<MenuTrigger align={isOnlyItem ? 'start' : 'end'} direction={orientation === 'vertical' ? 'end' : 'bottom'} UNSTABLE_portalContainer={UNSTABLE_portalContainer}>
<SlotProvider
slots={{
text: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,8 @@ function _MobileSearchAutocomplete<T extends object>(props: SpectrumSearchAutoco
validate,
name,
isReadOnly,
onSubmit = () => {}
onSubmit = () => {},
UNSTABLE_portalContainer
} = props;

let {contains} = useFilter({sensitivity: 'base'});
Expand Down Expand Up @@ -156,7 +157,7 @@ function _MobileSearchAutocomplete<T extends object>(props: SpectrumSearchAutoco
</SearchAutocompleteButton>
</Field>
<input {...inputProps} ref={inputRef} />
<Tray state={state} isFixedHeight {...overlayProps}>
<Tray state={state} isFixedHeight {...overlayProps} container={UNSTABLE_portalContainer}>
<SearchAutocompleteTray
{...props}
onClose={state.close}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,8 @@ function _SearchAutocompleteBase<T extends object>(props: SpectrumSearchAutocomp
loadingState,
onLoadMore,
onSubmit = () => {},
validate
validate,
UNSTABLE_portalContainer
} = props;

let stringFormatter = useLocalizedStringFormatter(intlMessages, '@react-spectrum/autocomplete');
Expand Down Expand Up @@ -167,7 +168,8 @@ function _SearchAutocompleteBase<T extends object>(props: SpectrumSearchAutocomp
placement={`${direction} ${align}`}
hideArrow
isNonModal
shouldFlip={shouldFlip}>
shouldFlip={shouldFlip}
container={UNSTABLE_portalContainer}>
<ListBoxBase
{...listBoxProps}
ref={listBoxRef}
Expand Down
6 changes: 4 additions & 2 deletions packages/@react-spectrum/combobox/src/ComboBox.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,8 @@ const ComboBoxBase = React.forwardRef(function ComboBoxBase<T extends object>(pr
allowsCustomValue,
menuWidth: customMenuWidth,
name,
formValue = 'text'
formValue = 'text',
UNSTABLE_portalContainer
} = props;
if (allowsCustomValue) {
formValue = 'text';
Expand Down Expand Up @@ -181,7 +182,8 @@ const ComboBoxBase = React.forwardRef(function ComboBoxBase<T extends object>(pr
placement={`${direction} ${align}`}
hideArrow
isNonModal
shouldFlip={shouldFlip}>
shouldFlip={shouldFlip}
container={UNSTABLE_portalContainer}>
<ListBoxBase
{...listBoxProps}
ref={listBoxRef}
Expand Down
5 changes: 3 additions & 2 deletions packages/@react-spectrum/combobox/src/MobileComboBox.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,8 @@ export const MobileComboBox = React.forwardRef(function MobileComboBox<T extends
validationBehavior,
name,
formValue = 'text',
allowsCustomValue
allowsCustomValue,
UNSTABLE_portalContainer
} = props;
if (allowsCustomValue) {
formValue = 'text';
Expand Down Expand Up @@ -145,7 +146,7 @@ export const MobileComboBox = React.forwardRef(function MobileComboBox<T extends
</ComboBoxButton>
</Field>
<input {...inputProps} ref={inputRef} />
<Tray state={state} isFixedHeight {...overlayProps}>
<Tray state={state} isFixedHeight {...overlayProps} container={UNSTABLE_portalContainer}>
<ComboBoxTray
{...props}
onClose={state.close}
Expand Down
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'));
});
});
});
3 changes: 2 additions & 1 deletion packages/@react-spectrum/menu/src/ActionMenu.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,8 @@ function ActionMenu<T extends object>(props: SpectrumActionMenuProps<T>, ref: Fo
onOpenChange={props.onOpenChange}
align={props.align}
direction={props.direction}
shouldFlip={props.shouldFlip}>
shouldFlip={props.shouldFlip}
UNSTABLE_portalContainer={props.UNSTABLE_portalContainer}>
<ActionButton
ref={ref}
{...props}
Expand Down
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
Loading