diff --git a/packages/@react-aria/overlays/src/useModal.tsx b/packages/@react-aria/overlays/src/useModal.tsx index a56a37c725a..9062c0737fa 100644 --- a/packages/@react-aria/overlays/src/useModal.tsx +++ b/packages/@react-aria/overlays/src/useModal.tsx @@ -37,7 +37,7 @@ const Context = React.createContext(null); export function ModalProvider(props: ModalProviderProps) { let {children} = props; let parent = useContext(Context); - let [modalCount, setModalCount] = useState(parent ? parent.modalCount : 0); + let [modalCount, setModalCount] = useState(0); let context = useMemo(() => ({ parent, modalCount, diff --git a/packages/@react-spectrum/actiongroup/test/ActionGroup.test.js b/packages/@react-spectrum/actiongroup/test/ActionGroup.test.js index eb3be05a586..b83fcf9dd5f 100644 --- a/packages/@react-spectrum/actiongroup/test/ActionGroup.test.js +++ b/packages/@react-spectrum/actiongroup/test/ActionGroup.test.js @@ -130,6 +130,10 @@ function renderComponentWithExtraInputs(props) { } describe('ActionGroup', function () { + beforeAll(function () { + jest.useFakeTimers(); + }); + afterEach(() => { btnBehavior.reset(); }); @@ -600,10 +604,22 @@ describe('ActionGroup', function () { ); let button = tree.getByRole('button'); - triggerPress(button); + + act(() => { + triggerPress(button); + jest.runAllTimers(); + }); let dialog = tree.getByRole('dialog'); expect(dialog).toBeVisible(); + + act(() => { + fireEvent.keyDown(dialog, {key: 'Escape'}); + fireEvent.keyUp(dialog, {key: 'Escape'}); + jest.runAllTimers(); + }); + + expect(() => tree.getByRole('dialog')).toThrow(); }); it('supports TooltipTrigger as a wrapper around items', function () { diff --git a/packages/@react-spectrum/dialog/docs/DialogContainer.mdx b/packages/@react-spectrum/dialog/docs/DialogContainer.mdx new file mode 100644 index 00000000000..2fe1e855af9 --- /dev/null +++ b/packages/@react-spectrum/dialog/docs/DialogContainer.mdx @@ -0,0 +1,233 @@ + + +import {Layout} from '@react-spectrum/docs'; +export default Layout; + +import docs from 'docs:@react-spectrum/dialog'; +import {ExampleImage, HeaderInfo, PropTable, FunctionAPI} from '@react-spectrum/docs'; +import packageData from '@react-spectrum/dialog/package.json'; + +```jsx import +import {Content, Footer, Header} from '@react-spectrum/view'; +import {Form} from '@react-spectrum/form'; +import {Heading, Text} from '@react-spectrum/text'; +import {TextField} from '@react-spectrum/textfield'; +import {Divider} from '@react-spectrum/divider'; +import {DialogContainer, Dialog, AlertDialog} from '@react-spectrum/dialog'; +import {ActionButton, Button} from '@react-spectrum/button'; +import {ButtonGroup} from '@react-spectrum/buttongroup'; +import {MenuTrigger, Menu, Item} from '@react-spectrum/menu'; +import More from '@spectrum-icons/workflow/More'; +import Delete from '@spectrum-icons/workflow/Delete'; +import Edit from '@spectrum-icons/workflow/Edit'; +``` + +--- +category: Overlays +keywords: [dialog container, dialog, modal] +after_version: 3.3.0 +--- + +# DialogContainer + +

{docs.exports.DialogContainer.description}

+ + + +## Example + +```tsx example +function Example(props) { + let [isOpen, setOpen] = React.useState(false); + + return ( + <> + setOpen(true)}> + + Delete + + setOpen(false)} {...props}> + {isOpen && + + Are you sure you want to delete this item? + + } + + + ); +} +``` + +## Dialog triggered by a menu item + +DialogContainer is useful over a [DialogTrigger](DialogTrigger.html) when your have a trigger that can unmount while the dialog is +open. For example, placing a DialogTrigger around a menu item would not work because the menu closes when pressing an item, thereby +unmounting the DialogTrigger. When the trigger unmounts, so does the Dialog. In these cases, it is useful to place the dialog *outside* +the tree that unmounts, so that the dialog is not also removed. + +The following example shows a [MenuTrigger](MenuTrigger.html) containing a [Menu](Menu.html) with two actions: "edit" and "delete". +Each menu item opens a different dialog. This is implemented by using a DialogContainer that displays the edit dialog, +delete dialog, or no dialog depending on the current value stored in local state. Pressing a menu item triggers the menu's +`onAction` prop, which sets the state to the type of dialog to display, based on the menu item's `key`. This causes the associated +dialog to be rendered within the DialogContainer. + +This example also demonstrates the use of the `useDialogContainer` hook, which allows the dialog to dismiss itself when a user +presses one of the buttons inside it. This triggers the DialogContainer's `onDismiss` event, which resets the state storing the +open dialog back to `null`. In addition, the user can close the dialog using the Escape key (unless the +`isKeyboardDismissDisabled` prop is set), or by clicking outside (if the `isDismissable` prop is set). + +```tsx example +import {useDialogContainer} from '@react-spectrum/dialog'; + +function Example() { + let [dialog, setDialog] = React.useState(); + + return ( + <> + + + + Edit... + Delete... + + + setDialog(null)}> + {dialog === 'edit' && + + } + {dialog === 'delete' && + + Are you sure you want to delete this item? + + } + + + ); +} + +function EditDialog() { + // This hook allows us to dismiss the dialog when the user + // presses one of the buttons (below). + let dialog = useDialogContainer(); + + return ( + + Edit + + +
+ + + +
+ + + + +
+ ); +} +``` + +## Props + + + +## useDialogContainer + +The `useDialogContainer` hook can be used to allow a custom dialog component to access the `type` of container +the dialog is rendered in (e.g. `modal`, `popover`, `fullscreen`, etc.), and also to dismiss the dialog +programmatically. It works with both `DialogContainer` and [DialogTrigger](DialogTrigger.html). + + + +## Visual options + +### Full screen + +The `type` prop allows setting the type of modal to display. Set it to `"fullscreen"` to display a full screen dialog, which +only reveals a small portion of the page behind the underlay. Use this variant for more complex workflows that do not fit in +the available modal dialog [sizes](Dialog.html#size). + +```tsx example +function Example(props) { + let [isOpen, setOpen] = React.useState(false); + + return ( + <> + setOpen(true)}> + + Edit + + setOpen(false)} {...props}> + {isOpen && + + } + + + ); +} +``` + +### Full screen takeover + +Fullscreen takeover dialogs are similar to the fullscreen variant except that the dialog covers the entire screen. + +```tsx example +function Example(props) { + let [isOpen, setOpen] = React.useState(false); + + return ( + <> + setOpen(true)}> + + Edit + + setOpen(false)} {...props}> + {isOpen && + + } + + + ); +} +``` + +```tsx import +// Duplicated from above so we can access in other examples... +function EditDialog() { + let dialog = useDialogContainer(); + + return ( + + Edit + + +
+ + + +
+ + + + +
+ ); +} +``` diff --git a/packages/@react-spectrum/dialog/src/DialogContainer.tsx b/packages/@react-spectrum/dialog/src/DialogContainer.tsx new file mode 100644 index 00000000000..a4cbb26ecfb --- /dev/null +++ b/packages/@react-spectrum/dialog/src/DialogContainer.tsx @@ -0,0 +1,61 @@ +/* + * Copyright 2020 Adobe. All rights reserved. + * This file is licensed to you under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. You may obtain a copy + * of the License at http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under + * the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS + * OF ANY KIND, either express or implied. See the License for the specific language + * governing permissions and limitations under the License. + */ + +import {DialogContext} from './context'; +import {Modal} from '@react-spectrum/overlays'; +import React, {ReactElement, useRef} from 'react'; +import {SpectrumDialogContainerProps} from '@react-types/dialog'; + +/** + * A DialogContainer accepts a single Dialog as a child, and manages showing and hiding + * it in a modal. Useful in cases where there is no trigger element + * or when the trigger unmounts while the dialog is open. + */ +export function DialogContainer(props: SpectrumDialogContainerProps) { + let { + children, + type = 'modal', + onDismiss, + isDismissable, + isKeyboardDismissDisabled + } = props; + + let childArray = React.Children.toArray(children); + if (childArray.length > 1) { + throw new Error('Only a single child can be passed to DialogContainer.'); + } + + let lastChild = useRef(null); + let child = React.isValidElement(childArray[0]) ? childArray[0] : null; + if (child) { + lastChild.current = child; + } + + let context = { + type, + onClose: onDismiss, + isDismissable + }; + + return ( + + + {lastChild.current} + + + ); +} diff --git a/packages/@react-spectrum/dialog/src/DialogTrigger.tsx b/packages/@react-spectrum/dialog/src/DialogTrigger.tsx index 2e39169f649..5246d2f1782 100644 --- a/packages/@react-spectrum/dialog/src/DialogTrigger.tsx +++ b/packages/@react-spectrum/dialog/src/DialogTrigger.tsx @@ -15,7 +15,7 @@ import {DOMRefValue} from '@react-types/shared'; import {Modal, Popover, Tray} from '@react-spectrum/overlays'; import {OverlayTriggerState, useOverlayTriggerState} from '@react-stately/overlays'; import {PressResponder} from '@react-aria/interactions'; -import React, {Fragment, ReactElement, useRef} from 'react'; +import React, {Fragment, ReactElement, useEffect, useRef} from 'react'; import {SpectrumDialogClose, SpectrumDialogProps, SpectrumDialogTriggerProps} from '@react-types/dialog'; import {unwrapDOMRef, useMediaQuery} from '@react-spectrum/utils'; import {useOverlayPosition, useOverlayTrigger} from '@react-aria/overlays'; @@ -50,6 +50,20 @@ function DialogTrigger(props: SpectrumDialogTriggerProps) { } let state = useOverlayTriggerState(props); + let wasOpen = useRef(false); + wasOpen.current = state.isOpen; + let isExiting = useRef(false); + let onExiting = () => isExiting.current = true; + let onExited = () => isExiting.current = false; + + // eslint-disable-next-line arrow-body-style + useEffect(() => { + return () => { + if ((wasOpen.current || isExiting.current) && type !== 'popover' && type !== 'tray') { + console.warn('A DialogTrigger unmounted while open. This is likely due to being placed within a trigger that unmounts or inside a conditional. Consider using a DialogContainer instead.'); + } + }; + }, []); if (type === 'popover') { return ( @@ -68,20 +82,25 @@ function DialogTrigger(props: SpectrumDialogTriggerProps) { switch (type) { case 'fullscreen': case 'fullscreenTakeover': - return ( - - {typeof content === 'function' ? content(state.close) : content} - - ); case 'modal': return ( - + {typeof content === 'function' ? content(state.close) : content} ); case 'tray': return ( - + {typeof content === 'function' ? content(state.close) : content} ); @@ -171,7 +190,7 @@ function PopoverTrigger({state, targetRef, trigger, content, hideArrow, isKeyboa } interface SpectrumDialogTriggerBase { - type?: 'modal' | 'popover' | 'tray' | 'fullscreen' | 'fullscreenTakeover', + type: 'modal' | 'popover' | 'tray' | 'fullscreen' | 'fullscreenTakeover', state: OverlayTriggerState, isDismissable?: boolean, dialogProps?: SpectrumDialogProps | {}, diff --git a/packages/@react-spectrum/dialog/src/context.ts b/packages/@react-spectrum/dialog/src/context.ts index f1fab8acba3..a427deb596a 100644 --- a/packages/@react-spectrum/dialog/src/context.ts +++ b/packages/@react-spectrum/dialog/src/context.ts @@ -15,7 +15,7 @@ import React, {HTMLAttributes} from 'react'; export interface DialogContextValue extends HTMLAttributes { type: 'modal' | 'popover' | 'tray' | 'fullscreen' | 'fullscreenTakeover', isDismissable?: boolean, - onClose?: () => void + onClose: () => void } export const DialogContext = React.createContext(null); diff --git a/packages/@react-spectrum/dialog/src/index.ts b/packages/@react-spectrum/dialog/src/index.ts index dc942ce113a..a15530d0596 100644 --- a/packages/@react-spectrum/dialog/src/index.ts +++ b/packages/@react-spectrum/dialog/src/index.ts @@ -15,3 +15,5 @@ export * from './AlertDialog'; export * from './Dialog'; export * from './DialogTrigger'; +export * from './DialogContainer'; +export * from './useDialogContainer'; diff --git a/packages/@react-spectrum/dialog/src/useDialogContainer.ts b/packages/@react-spectrum/dialog/src/useDialogContainer.ts new file mode 100644 index 00000000000..cb8ef04f89d --- /dev/null +++ b/packages/@react-spectrum/dialog/src/useDialogContainer.ts @@ -0,0 +1,33 @@ +/* + * Copyright 2020 Adobe. All rights reserved. + * This file is licensed to you under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. You may obtain a copy + * of the License at http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under + * the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS + * OF ANY KIND, either express or implied. See the License for the specific language + * governing permissions and limitations under the License. + */ + +import {DialogContext} from './context'; +import {useContext} from 'react'; + +interface DialogContainer { + type: 'modal' | 'popover' | 'tray' | 'fullscreen' | 'fullscreenTakeover', + dismiss(): void +} + +export function useDialogContainer(): DialogContainer { + let context = useContext(DialogContext); + if (!context) { + throw new Error('Cannot call useDialogContext outside a or .'); + } + + return { + type: context.type, + dismiss() { + context.onClose(); + } + }; +} diff --git a/packages/@react-spectrum/dialog/stories/DialogContainer.stories.tsx b/packages/@react-spectrum/dialog/stories/DialogContainer.stories.tsx new file mode 100644 index 00000000000..5763837b9d3 --- /dev/null +++ b/packages/@react-spectrum/dialog/stories/DialogContainer.stories.tsx @@ -0,0 +1,42 @@ +/* + * Copyright 2020 Adobe. All rights reserved. + * This file is licensed to you under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. You may obtain a copy + * of the License at http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under + * the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS + * OF ANY KIND, either express or implied. See the License for the specific language + * governing permissions and limitations under the License. + */ + +import {DialogContainerExample, MenuExample} from './DialogContainerExamples'; +import React from 'react'; +import {storiesOf} from '@storybook/react'; + +storiesOf('DialogContainer', module) + .addParameters({providerSwitcher: {status: 'notice'}}) + .add( + 'default', + () => + ) + .add( + 'in a menu', + () => + ) + .add( + 'type: fullscreen', + () => + ) + .add( + 'type: fullscreenTakeover', + () => + ) + .add( + 'isDismissable', + () => + ) + .add( + 'isKeyboardDismissDisabled', + () => + ); diff --git a/packages/@react-spectrum/dialog/stories/DialogContainerExamples.tsx b/packages/@react-spectrum/dialog/stories/DialogContainerExamples.tsx new file mode 100644 index 00000000000..303325c8ab8 --- /dev/null +++ b/packages/@react-spectrum/dialog/stories/DialogContainerExamples.tsx @@ -0,0 +1,62 @@ +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 '../'; +import {Divider} from '@react-spectrum/divider'; +import {Heading, Text} from '@react-spectrum/text'; +import {Item, Menu, MenuTrigger} from '@react-spectrum/menu'; +import React from 'react'; + +export function DialogContainerExample(props) { + let [isOpen, setOpen] = React.useState(false); + + return ( + <> + setOpen(true)}>Open dialog + setOpen(false)} {...props}> + {isOpen && + + } + + + ); +} + +export function MenuExample(props) { + let [isOpen, setOpen] = React.useState(false); + + return ( + <> + + Open menu + setOpen(true)}> + Open dialog... + + + setOpen(false)}> + {isOpen && + + } + + + ); +} + +function ExampleDialog(props) { + let container = useDialogContainer(); + + return ( + + The Heading +
The Header
+ + 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. + {!props.isDismissable && + + + + + } +
+ ); +} diff --git a/packages/@react-spectrum/dialog/test/DialogContainer.test.js b/packages/@react-spectrum/dialog/test/DialogContainer.test.js new file mode 100644 index 00000000000..d77cec038de --- /dev/null +++ b/packages/@react-spectrum/dialog/test/DialogContainer.test.js @@ -0,0 +1,208 @@ +/* + * Copyright 2020 Adobe. All rights reserved. + * This file is licensed to you under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. You may obtain a copy + * of the License at http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under + * the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS + * OF ANY KIND, either express or implied. See the License for the specific language + * governing permissions and limitations under the License. + */ + +import {act, fireEvent, render, within} from '@testing-library/react'; +import {DialogContainerExample, MenuExample} from '../stories/DialogContainerExamples'; +import {Provider} from '@react-spectrum/provider'; +import React from 'react'; +import {theme} from '@react-spectrum/theme-default'; +import {triggerPress} from '@react-spectrum/test-utils'; + +describe('DialogContainer', function () { + beforeAll(() => { + jest.useFakeTimers(); + }); + + afterAll(() => { + jest.useRealTimers(); + }); + + beforeEach(() => { + jest.spyOn(window, 'requestAnimationFrame').mockImplementation(cb => cb()); + }); + + afterEach(() => { + jest.runAllTimers(); + window.requestAnimationFrame.mockRestore(); + }); + + it('should open and close a dialog based on controlled state', function () { + let {getByRole} = render( + + + + ); + + let button = getByRole('button'); + expect(() => getByRole('dialog')).toThrow(); + + act(() => { + triggerPress(button); + jest.runAllTimers(); + }); + + let dialog = getByRole('dialog'); + expect(document.activeElement).toBe(dialog); + + button = within(dialog).getByText('Confirm'); + + act(() => { + triggerPress(button); + jest.runAllTimers(); + }); + + expect(() => getByRole('dialog')).toThrow(); + }); + + it('should support closing a dialog via the Escape key', function () { + let {getByRole} = render( + + + + ); + + let button = getByRole('button'); + expect(() => getByRole('dialog')).toThrow(); + + act(() => { + triggerPress(button); + jest.runAllTimers(); + }); + + let dialog = getByRole('dialog'); + expect(document.activeElement).toBe(dialog); + + act(() => { + fireEvent.keyDown(dialog, {key: 'Escape'}); + fireEvent.keyUp(dialog, {key: 'Escape'}); + jest.runAllTimers(); + }); + + expect(() => getByRole('dialog')).toThrow(); + }); + + it('should not close a dialog via the Escape key if isKeyboardDismissDisabled', function () { + let {getByRole} = render( + + + + ); + + let button = getByRole('button'); + expect(() => getByRole('dialog')).toThrow(); + + act(() => { + triggerPress(button); + jest.runAllTimers(); + }); + + let dialog = getByRole('dialog'); + expect(document.activeElement).toBe(dialog); + + act(() => { + fireEvent.keyDown(dialog, {key: 'Escape'}); + fireEvent.keyUp(dialog, {key: 'Escape'}); + jest.runAllTimers(); + }); + + expect(getByRole('dialog')).toBeVisible(); + }); + + it('should not close when clicking outside the dialog by default', function () { + let {getByRole} = render( + + + + ); + + let button = getByRole('button'); + expect(() => getByRole('dialog')).toThrow(); + + act(() => { + triggerPress(button); + jest.runAllTimers(); + }); + + expect(getByRole('dialog')).toBeVisible(); + + act(() => { + triggerPress(document.body); + jest.runAllTimers(); + }); + + expect(getByRole('dialog')).toBeVisible(); + }); + + it('should close when clicking outside the dialog when isDismissible', function () { + let {getByRole} = render( + + + + ); + + let button = getByRole('button'); + expect(() => getByRole('dialog')).toThrow(); + + act(() => { + triggerPress(button); + jest.runAllTimers(); + }); + + expect(getByRole('dialog')).toBeVisible(); + + act(() => { + triggerPress(document.body); + jest.runAllTimers(); + }); + + expect(() => getByRole('dialog')).toThrow(); + }); + + it('should not close the dialog when a trigger unmounts', function () { + let {getByRole} = render( + + + + ); + + let button = getByRole('button'); + expect(() => getByRole('dialog')).toThrow(); + + act(() => { + triggerPress(button); + jest.runAllTimers(); + }); + + expect(() => getByRole('dialog')).toThrow(); + + let menu = getByRole('menu'); + let menuitem = within(menu).getByRole('menuitem'); + + act(() => { + triggerPress(menuitem); + jest.runAllTimers(); + }); + + expect(() => getByRole('menu')).toThrow(); + expect(() => getByRole('menuitem')).toThrow(); + + let dialog = getByRole('dialog'); + button = within(dialog).getByText('Confirm'); + + act(() => { + triggerPress(button); + jest.runAllTimers(); + }); + + expect(() => getByRole('dialog')).toThrow(); + }); +}); diff --git a/packages/@react-spectrum/dialog/test/DialogTrigger.test.js b/packages/@react-spectrum/dialog/test/DialogTrigger.test.js index a3a1aa40e4e..4d8fe6f2c46 100644 --- a/packages/@react-spectrum/dialog/test/DialogTrigger.test.js +++ b/packages/@react-spectrum/dialog/test/DialogTrigger.test.js @@ -14,6 +14,7 @@ import {act, fireEvent, render, waitFor, within} from '@testing-library/react'; import {ActionButton, Button} from '@react-spectrum/button'; import {ButtonGroup} from '@react-spectrum/buttongroup'; import {Dialog, DialogTrigger} from '../'; +import {Item, Menu, MenuTrigger} from '@react-spectrum/menu'; import MatchMediaMock from 'jest-matchmedia-mock'; import {Provider} from '@react-spectrum/provider'; import React from 'react'; @@ -35,7 +36,16 @@ describe('DialogTrigger', function () { }); afterEach(() => { - jest.runAllTimers(); + // Ensure we close any dialogs before unmounting to avoid warning. + let dialog = document.querySelector('[role="dialog"]'); + if (dialog) { + act(() => { + fireEvent.keyDown(dialog, {key: 'Escape'}); + fireEvent.keyUp(dialog, {key: 'Escape'}); + jest.runAllTimers(); + }); + } + matchMedia.clear(); window.requestAnimationFrame.mockRestore(); }); @@ -671,7 +681,7 @@ describe('DialogTrigger', function () { Trigger - Content body + {close => Close} ); @@ -703,5 +713,53 @@ describe('DialogTrigger', function () { }); // wait for animation expect(document.activeElement).toBe(dialog); + + // Close the dialog by clicking the button inside + button = within(dialog).getByRole('button'); + act(() => { + triggerPress(button); + jest.runAllTimers(); + }); + + expect(() => getByRole('dialog')).toThrow(); + }); + + it('should warn when unmounting a dialog trigger while a modal is open', function () { + let warn = jest.spyOn(console, 'warn').mockImplementation(() => {}); + let {getByRole} = render( + + + Trigger + + + Open menu + Content body + + + + + ); + + let button = getByRole('button'); + + act(() => { + triggerPress(button); + jest.runAllTimers(); + }); + + let menu = getByRole('menu'); + let menuitem = within(menu).getByRole('menuitem'); + + act(() => { + triggerPress(menuitem); + jest.runAllTimers(); + }); + + expect(() => getByRole('menu')).toThrow(); + expect(() => getByRole('menuitem')).toThrow(); + expect(() => getByRole('dialog')).toThrow(); + + expect(warn).toHaveBeenCalledTimes(1); + expect(warn).toHaveBeenCalledWith('A DialogTrigger unmounted while open. This is likely due to being placed within a trigger that unmounts or inside a conditional. Consider using a DialogContainer instead.'); }); }); diff --git a/packages/@react-spectrum/menu/test/Menu.test.js b/packages/@react-spectrum/menu/test/Menu.test.js index fb041c8d607..3b3dafd2b02 100644 --- a/packages/@react-spectrum/menu/test/Menu.test.js +++ b/packages/@react-spectrum/menu/test/Menu.test.js @@ -10,9 +10,9 @@ * governing permissions and limitations under the License. */ +import {act, fireEvent, render, within} from '@testing-library/react'; import Bell from '@spectrum-icons/workflow/Bell'; import {Dialog, DialogTrigger} from '@react-spectrum/dialog'; -import {fireEvent, render, within} from '@testing-library/react'; import {Item, Menu, Section} from '../'; import {Keyboard, Text} from '@react-spectrum/text'; import {MenuContext} from '../src/context'; @@ -639,10 +639,22 @@ describe('Menu', function () { let menu = tree.getByRole('menu'); let menuItem = within(menu).getByRole('menuitem'); - triggerPress(menuItem); + + act(() => { + triggerPress(menuItem); + jest.runAllTimers(); + }); let dialog = tree.getByRole('dialog'); expect(dialog).toBeVisible(); + + act(() => { + fireEvent.keyDown(dialog, {key: 'Escape'}); + fireEvent.keyUp(dialog, {key: 'Escape'}); + jest.runAllTimers(); + }); + + expect(() => tree.getByRole('dialog')).toThrow(); }); describe('supports onAction', function () { diff --git a/packages/@react-spectrum/overlays/src/Modal.tsx b/packages/@react-spectrum/overlays/src/Modal.tsx index 2d57256764b..3ad8e60c89f 100644 --- a/packages/@react-spectrum/overlays/src/Modal.tsx +++ b/packages/@react-spectrum/overlays/src/Modal.tsx @@ -25,7 +25,7 @@ interface ModalWrapperProps extends HTMLAttributes { children: ReactNode, isOpen?: boolean, onClose?: () => void, - type?: 'fullscreen' | 'fullscreenTakeover', + type?: 'modal' | 'fullscreen' | 'fullscreenTakeover', isDismissable?: boolean, isKeyboardDismissDisabled?: boolean } diff --git a/packages/@react-spectrum/table/stories/CRUDExample.tsx b/packages/@react-spectrum/table/stories/CRUDExample.tsx index 4cba79aa3aa..d07c4d7c085 100644 --- a/packages/@react-spectrum/table/stories/CRUDExample.tsx +++ b/packages/@react-spectrum/table/stories/CRUDExample.tsx @@ -13,7 +13,7 @@ import {ActionButton, Button} from '@react-spectrum/button'; import {ActionGroup} from '@react-spectrum/actiongroup'; import Add from '@spectrum-icons/workflow/Add'; -import {AlertDialog, Dialog, DialogTrigger} from '@react-spectrum/dialog'; +import {AlertDialog, Dialog, DialogContainer, useDialogContainer} from '@react-spectrum/dialog'; import {ButtonGroup} from '@react-spectrum/buttongroup'; import {Cell, Column, Row, Table, TableBody, TableHeader} from '../'; import {Content} from '@react-spectrum/view'; @@ -23,7 +23,6 @@ import {Flex} from '@react-spectrum/layout'; import {Form} from '@react-spectrum/form'; import {Heading} from '@react-spectrum/text'; import {Item, Menu, MenuTrigger} from '@react-spectrum/menu'; -import {Modal} from '@react-spectrum/overlays'; import More from '@spectrum-icons/workflow/More'; import React, {useState} from 'react'; import {TextField} from '@react-spectrum/textfield'; @@ -37,54 +36,19 @@ export function CRUDExample() { ] }); - let [editingItem, setEditingItem] = useState(null); - let [deletingItem, setDeletingItem] = useState(null); - + let [dialog, setDialog] = useState(null); let createItem = (item) => { list.prepend({...item, id: Date.now()}); }; - let editItem = (id, item) => { - list.update(id, item); - setEditingItem(null); - }; - - let deleteItem = (item) => { - list.remove(item.id); - setDeletingItem(null); - }; - - let deleteSelectedItems = () => { - list.removeSelectedItems(); - }; - - let onAction = (action, item) => { - switch (action) { - case 'edit': - setEditingItem(item); - break; - case 'delete': - setDeletingItem(item); - break; - } - }; - let selectedCount = list.selectedKeys === 'all' ? list.items.length : list.selectedKeys.size; return ( - - - - {onClose => } - + setDialog({action})}> + {selectedCount > 0 && - - - - Are you sure you want to delete {selectedCount === 1 ? '1 item' : `${selectedCount} items`}? - - + } @@ -102,7 +66,7 @@ export function CRUDExample() { {column === 'actions' ? - onAction(action, item)}> + setDialog({action, item})}> Edit... Delete... @@ -115,22 +79,42 @@ export function CRUDExample() { }
- setEditingItem(null)}> - setEditingItem(null)} - onConfirm={item => editItem(editingItem.id, item)} /> - - setDeletingItem(null)}> - deleteItem(deletingItem)}> - Are you sure you want to delete {deletingItem?.firstName} {deletingItem?.lastName}? - - + setDialog(null)}> + {dialog?.action === 'add' && + + } + {dialog?.action === 'edit' && + list.update(dialog.item.id, item)} /> + } + {dialog?.action === 'delete' && + list.remove(dialog.item.id)}> + Are you sure you want to delete {dialog.item.firstName} {dialog.item.lastName}? + + } + {dialog?.action === 'bulk-delete' && + list.removeSelectedItems()}> + Are you sure you want to delete {selectedCount === 1 ? '1 item' : `${selectedCount} items`}? + + } +
); } -function EditDialog({item, onClose, onConfirm}) { +function EditDialog({item, onConfirm}) { + let dialog = useDialogContainer(); let [state, setState] = useState({ firstName: '', lastName: '', @@ -150,8 +134,8 @@ function EditDialog({item, onClose, onConfirm}) { - - + + ); diff --git a/packages/@react-types/dialog/src/index.d.ts b/packages/@react-types/dialog/src/index.d.ts index d49b79e1d36..74804878686 100644 --- a/packages/@react-types/dialog/src/index.d.ts +++ b/packages/@react-types/dialog/src/index.d.ts @@ -38,6 +38,22 @@ export interface SpectrumDialogTriggerProps extends OverlayTriggerProps, Positio isKeyboardDismissDisabled?: boolean } +export interface SpectrumDialogContainerProps { + /** The Dialog to display, if any. */ + children: ReactNode, + /** Handler that is called when the 'x' button of a dismissable Dialog is clicked. */ + onDismiss: () => void, + /** + * The type of Dialog that should be rendered. See the visual options below for examples of each. + * @default 'modal' + */ + type?: 'modal' | 'fullscreen' | 'fullscreenTakeover', + /** Whether the Dialog is dismissable. See the [Dialog docs](Dialog.html#dismissable-dialogs) for more details. */ + isDismissable?: boolean, + /** Whether pressing the escape key to close the dialog should be disabled. */ + isKeyboardDismissDisabled?: boolean +} + export interface AriaDialogProps extends DOMProps, AriaLabelingProps { /** * The accessibility role for the dialog. diff --git a/packages/@react-types/overlays/src/index.d.ts b/packages/@react-types/overlays/src/index.d.ts index a8ab9e416b7..13c36097a0c 100644 --- a/packages/@react-types/overlays/src/index.d.ts +++ b/packages/@react-types/overlays/src/index.d.ts @@ -78,7 +78,7 @@ export interface ModalProps extends StyleProps, OverlayProps { children: ReactElement, isOpen?: boolean, onClose?: () => void, - type?: 'fullscreen' | 'fullscreenTakeover', + type?: 'modal' | 'fullscreen' | 'fullscreenTakeover', isDismissable?: boolean }