From e718677aad9d65fafb2928023e51985ccc767e4b Mon Sep 17 00:00:00 2001 From: Mayank <9084735+mayank99@users.noreply.github.com> Date: Wed, 3 Apr 2024 13:03:51 -0400 Subject: [PATCH 01/30] add / methods to Dialog ref --- .../itwinui-react/src/core/Dialog/Dialog.tsx | 30 +++++++++++++++---- .../src/core/Dialog/DialogContext.tsx | 2 +- 2 files changed, 25 insertions(+), 7 deletions(-) diff --git a/packages/itwinui-react/src/core/Dialog/Dialog.tsx b/packages/itwinui-react/src/core/Dialog/Dialog.tsx index a1f36b9c050..7729c9b51b8 100644 --- a/packages/itwinui-react/src/core/Dialog/Dialog.tsx +++ b/packages/itwinui-react/src/core/Dialog/Dialog.tsx @@ -11,7 +11,12 @@ import { DialogContext } from './DialogContext.js'; import type { DialogContextProps } from './DialogContext.js'; import { DialogButtonBar } from './DialogButtonBar.js'; import { DialogMain } from './DialogMain.js'; -import { useMergedRefs, Box, Portal } from '../utils/index.js'; +import { + useMergedRefs, + Box, + Portal, + useControlledState, +} from '../utils/index.js'; import type { PolymorphicForwardRefComponent } from '../utils/index.js'; import { Transition } from 'react-transition-group'; @@ -22,12 +27,12 @@ type DialogProps = { children: React.ReactNode; } & Omit; -const DialogComponent = React.forwardRef((props, ref) => { +const DialogComponent = React.forwardRef((props, forwardedRef) => { const { trapFocus = false, setFocus = false, preventDocumentScroll = false, - isOpen = false, + isOpen: isOpenProp = false, isDismissible = true, closeOnEsc = true, closeOnExternalClick = false, @@ -41,7 +46,19 @@ const DialogComponent = React.forwardRef((props, ref) => { ...rest } = props; - const dialogRootRef = React.useRef(null); + const dialogRootRef = React.useRef(null); + + const [isOpen, setIsOpen] = useControlledState(false, isOpenProp); + + React.useImperativeHandle( + forwardedRef, + () => ({ + ...(dialogRootRef.current as HTMLDialogElement), + show: () => setIsOpen(true), + close: () => setIsOpen(false), + }), + [dialogRootRef, setIsOpen], + ); return ( @@ -64,16 +81,17 @@ const DialogComponent = React.forwardRef((props, ref) => { > ); -}) as PolymorphicForwardRefComponent<'div', DialogProps>; +}) as PolymorphicForwardRefComponent<'dialog', DialogProps>; /** * Dialog component. diff --git a/packages/itwinui-react/src/core/Dialog/DialogContext.tsx b/packages/itwinui-react/src/core/Dialog/DialogContext.tsx index dfd8e9dacd1..2042070d2fe 100644 --- a/packages/itwinui-react/src/core/Dialog/DialogContext.tsx +++ b/packages/itwinui-react/src/core/Dialog/DialogContext.tsx @@ -84,7 +84,7 @@ export type DialogContextProps = { /** * Dialog root ref. For internal use. */ - dialogRootRef?: React.RefObject; + dialogRootRef?: React.RefObject; /** * Determines the positioning of Dialog on page. */ From 56100a87bc5d9cf7717a7666217937aeef9dc23a Mon Sep 17 00:00:00 2001 From: Mayank <9084735+mayank99@users.noreply.github.com> Date: Wed, 3 Apr 2024 14:19:30 -0400 Subject: [PATCH 02/30] add `show` and `close` methods to `Dialog` --- apps/react-workshop/src/Dialog.stories.tsx | 129 +++++------------- .../itwinui-react/src/core/Dialog/Dialog.tsx | 30 ++-- 2 files changed, 50 insertions(+), 109 deletions(-) diff --git a/apps/react-workshop/src/Dialog.stories.tsx b/apps/react-workshop/src/Dialog.stories.tsx index 20c36771095..550d90de68f 100644 --- a/apps/react-workshop/src/Dialog.stories.tsx +++ b/apps/react-workshop/src/Dialog.stories.tsx @@ -9,50 +9,33 @@ export default { title: 'Dialog', }; -export const Basic = () => { - const [isOpen, setIsOpen] = React.useState(false); - - const closeDialog = () => { - setIsOpen(false); - }; - - const onClose = () => { - console.log('onClose'); - closeDialog(); - }; +const lorem = `Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum.`; - const primaryButtonHandle = () => { - console.log('Primary button'); - closeDialog(); - }; - - const secondaryButtonHandle = () => { - console.log('Secondary button'); - closeDialog(); - }; +export const Basic = () => { + const dialogRef = React.useRef(null); return ( <> - - + dialogRef.current?.close()}> - - Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do - eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim - ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut - aliquip ex ea commodo consequat. Duis aute irure dolor in - reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla - pariatur. Excepteur sint occaecat cupidatat non proident, sunt in - culpa qui officia deserunt mollit anim id est laborum. - + {lorem} - - + @@ -61,35 +44,18 @@ export const Basic = () => { }; export const Modal = () => { - const [isOpen, setIsOpen] = React.useState(false); - - const closeDialog = () => { - setIsOpen(false); - }; - - const onClose = () => { - console.log('onClose'); - closeDialog(); - }; - - const primaryButtonHandle = () => { - console.log('Primary button'); - closeDialog(); - }; - - const secondaryButtonHandle = () => { - console.log('Secondary button'); - closeDialog(); - }; + const dialogRef = React.useRef(null); return ( <> - dialogRef.current?.close()} closeOnEsc closeOnExternalClick preventDocumentScroll @@ -97,23 +63,20 @@ export const Modal = () => { setFocus isDismissible > - console.log('onKeyDown')} /> + - - Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do - eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim - ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut - aliquip ex ea commodo consequat. Duis aute irure dolor in - reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla - pariatur. Excepteur sint occaecat cupidatat non proident, sunt in - culpa qui officia deserunt mollit anim id est laborum. - + {lorem} - - + @@ -158,15 +121,7 @@ export const DraggableAndResizable = () => { > - - Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do - eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim - ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut - aliquip ex ea commodo consequat. Duis aute irure dolor in - reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla - pariatur. Excepteur sint occaecat cupidatat non proident, sunt in - culpa qui officia deserunt mollit anim id est laborum. - + {lorem} */ -export const Dialog = Object.assign(DialogComponent, { +export const Dialog = Object.assign(DialogComponent as DialogComponentType, { Backdrop: DialogBackdrop, Main: DialogMain, TitleBar: DialogTitleBar, From 2bf437123d9adb94c568e0a13ee13ba05745d653 Mon Sep 17 00:00:00 2001 From: Mayank <9084735+mayank99@users.noreply.github.com> Date: Wed, 3 Apr 2024 14:26:08 -0400 Subject: [PATCH 03/30] handle `onClose` implicitly --- apps/react-workshop/src/Dialog.stories.tsx | 12 ++---------- packages/itwinui-react/src/core/Dialog/Dialog.tsx | 3 ++- .../itwinui-react/src/core/Dialog/DialogBackdrop.tsx | 5 +++-- .../itwinui-react/src/core/Dialog/DialogContext.tsx | 2 ++ .../itwinui-react/src/core/Dialog/DialogMain.tsx | 10 +++++++--- .../itwinui-react/src/core/Dialog/DialogTitleBar.tsx | 5 ++++- 6 files changed, 20 insertions(+), 17 deletions(-) diff --git a/apps/react-workshop/src/Dialog.stories.tsx b/apps/react-workshop/src/Dialog.stories.tsx index 550d90de68f..15465af48ca 100644 --- a/apps/react-workshop/src/Dialog.stories.tsx +++ b/apps/react-workshop/src/Dialog.stories.tsx @@ -22,7 +22,7 @@ export const Basic = () => { > Open Dialog - dialogRef.current?.close()}> + {lorem} @@ -54,15 +54,7 @@ export const Modal = () => { > Open Dialog - dialogRef.current?.close()} - closeOnEsc - closeOnExternalClick - preventDocumentScroll - trapFocus - setFocus - isDismissible - > + diff --git a/packages/itwinui-react/src/core/Dialog/Dialog.tsx b/packages/itwinui-react/src/core/Dialog/Dialog.tsx index a845ad6d95c..c54d0d580fa 100644 --- a/packages/itwinui-react/src/core/Dialog/Dialog.tsx +++ b/packages/itwinui-react/src/core/Dialog/Dialog.tsx @@ -20,7 +20,7 @@ type DialogProps = { * Dialog content. */ children: React.ReactNode; -} & Omit; +} & Omit; const DialogComponent = React.forwardRef((props, forwardedRef) => { const { @@ -60,6 +60,7 @@ const DialogComponent = React.forwardRef((props, forwardedRef) => { { if (event.target !== backdropRef.current) { return; } - if (isDismissible && closeOnExternalClick && onClose) { - onClose(event); + if (isDismissible && closeOnExternalClick) { + dialogContext.setIsOpen?.(false); + onClose?.(event); } onMouseDown?.(event); }; diff --git a/packages/itwinui-react/src/core/Dialog/DialogContext.tsx b/packages/itwinui-react/src/core/Dialog/DialogContext.tsx index 2042070d2fe..2a45cfe26ac 100644 --- a/packages/itwinui-react/src/core/Dialog/DialogContext.tsx +++ b/packages/itwinui-react/src/core/Dialog/DialogContext.tsx @@ -15,6 +15,8 @@ export type DialogContextProps = { * @default false */ isOpen?: boolean; + /** @private */ + setIsOpen?: (isOpen: boolean) => void; /** * Handler that is called when dialog is closed. */ diff --git a/packages/itwinui-react/src/core/Dialog/DialogMain.tsx b/packages/itwinui-react/src/core/Dialog/DialogMain.tsx index cf6917db481..3e23c1dc7cb 100644 --- a/packages/itwinui-react/src/core/Dialog/DialogMain.tsx +++ b/packages/itwinui-react/src/core/Dialog/DialogMain.tsx @@ -30,7 +30,10 @@ export type DialogMainProps = { * Content of the dialog. */ children: React.ReactNode; -} & Omit; +} & Omit< + DialogContextProps, + 'closeOnExternalClick' | 'dialogRootRef' | 'setIsOpen' +>; /** * Dialog component which can wrap any content. @@ -115,8 +118,9 @@ export const DialogMain = React.forwardRef((props, ref) => { } // Prevents React from resetting its properties event.persist(); - if (isDismissible && closeOnEsc && event.key === 'Escape' && onClose) { - onClose(event); + if (isDismissible && closeOnEsc && event.key === 'Escape') { + dialogContext.setIsOpen?.(false); + onClose?.(event); } onKeyDown?.(event); }; diff --git a/packages/itwinui-react/src/core/Dialog/DialogTitleBar.tsx b/packages/itwinui-react/src/core/Dialog/DialogTitleBar.tsx index fe91e9412e2..bf2c3b67a6f 100644 --- a/packages/itwinui-react/src/core/Dialog/DialogTitleBar.tsx +++ b/packages/itwinui-react/src/core/Dialog/DialogTitleBar.tsx @@ -74,7 +74,10 @@ export const DialogTitleBar = Object.assign( { + dialogContext.setIsOpen?.(false); + onClose?.(event); + }} aria-label='Close' data-iui-shift='right' > From e07d1531d65f5a9269e21d52eb1c93289e4d4848 Mon Sep 17 00:00:00 2001 From: Mayank <9084735+mayank99@users.noreply.github.com> Date: Wed, 3 Apr 2024 16:32:54 -0400 Subject: [PATCH 04/30] also add it to `Modal` --- apps/react-workshop/src/Modal.stories.tsx | 40 +++++-------------- .../itwinui-react/src/core/Dialog/Dialog.tsx | 7 +++- .../itwinui-react/src/core/Modal/Modal.tsx | 4 +- 3 files changed, 20 insertions(+), 31 deletions(-) diff --git a/apps/react-workshop/src/Modal.stories.tsx b/apps/react-workshop/src/Modal.stories.tsx index bbea7adf1b0..8c40ee324e7 100644 --- a/apps/react-workshop/src/Modal.stories.tsx +++ b/apps/react-workshop/src/Modal.stories.tsx @@ -15,38 +15,17 @@ export default { }; export const Basic = () => { - const [isModalOpen, setIsModalOpen] = React.useState(false); - - const closeModal = () => { - setIsModalOpen(false); - }; - - const onClose = () => { - console.log('onClose'); - closeModal(); - }; - - const primaryButtonHandle = () => { - console.log('Primary button'); - closeModal(); - }; - - const secondaryButtonHandle = () => { - console.log('Secondary button'); - closeModal(); - }; + const modalRef = React.useRef(null); return ( <> - - console.log('onKeyDown')} - > + Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad @@ -57,10 +36,13 @@ export const Basic = () => { culpa qui officia deserunt mollit anim id est laborum. - - + diff --git a/packages/itwinui-react/src/core/Dialog/Dialog.tsx b/packages/itwinui-react/src/core/Dialog/Dialog.tsx index c54d0d580fa..84a9eea856d 100644 --- a/packages/itwinui-react/src/core/Dialog/Dialog.tsx +++ b/packages/itwinui-react/src/core/Dialog/Dialog.tsx @@ -91,7 +91,12 @@ const DialogComponent = React.forwardRef((props, forwardedRef) => { // ---------------------------------------------------------------------------- type DialogComponentType = typeof DialogComponent & { - Ref: HTMLDivElement & { show: () => void; close: () => void }; + Ref: HTMLDivElement & { + /** Call this function to show (open) the dialog. */ + show: () => void; + /** Call this function to close the dialog. */ + close: () => void; + }; }; // ---------------------------------------------------------------------------- diff --git a/packages/itwinui-react/src/core/Modal/Modal.tsx b/packages/itwinui-react/src/core/Modal/Modal.tsx index de8a150d979..8484feea6c0 100644 --- a/packages/itwinui-react/src/core/Modal/Modal.tsx +++ b/packages/itwinui-react/src/core/Modal/Modal.tsx @@ -123,4 +123,6 @@ export const Modal = React.forwardRef((props, forwardedRef) => { ); -}) as PolymorphicForwardRefComponent<'div', ModalProps>; +}) as PolymorphicForwardRefComponent<'div', ModalProps> & { + Ref: typeof Dialog.Ref; +}; From 80a5a236d8390f16bc404fc3f7db135e01f2b2e2 Mon Sep 17 00:00:00 2001 From: Mayank <9084735+mayank99@users.noreply.github.com> Date: Wed, 3 Apr 2024 16:51:39 -0400 Subject: [PATCH 05/30] update examples --- examples/Dialog.draggable.jsx | 25 ++++++++++--------------- examples/Dialog.fullpage.jsx | 21 +++++++++++---------- examples/Dialog.main.jsx | 19 ++++++++++--------- examples/Dialog.modal.jsx | 16 +++++++++++----- examples/Dialog.nondismissible.jsx | 20 ++++++++------------ examples/Dialog.placement.jsx | 19 ++++++++++--------- 6 files changed, 60 insertions(+), 60 deletions(-) diff --git a/examples/Dialog.draggable.jsx b/examples/Dialog.draggable.jsx index 9d651263a81..4ac834e1260 100644 --- a/examples/Dialog.draggable.jsx +++ b/examples/Dialog.draggable.jsx @@ -3,7 +3,6 @@ * See LICENSE.md in the project root for license terms and full copyright notice. *--------------------------------------------------------------------------------------------*/ import * as React from 'react'; -// import * as ReactDOM from 'react-dom'; import { Dialog, Button, @@ -12,24 +11,18 @@ import { } from '@itwin/itwinui-react'; export default () => { - const [isOpen, setIsOpen] = React.useState(false); + const dialogRef = React.useRef(null); return ( <> - - setIsOpen(false)} - setFocus={false} - closeOnEsc - isDismissible - isDraggable - isResizable - portal - > + @@ -39,11 +32,13 @@ export default () => { - + diff --git a/examples/Dialog.fullpage.jsx b/examples/Dialog.fullpage.jsx index 5ca253bc6e6..ed24c0730f8 100644 --- a/examples/Dialog.fullpage.jsx +++ b/examples/Dialog.fullpage.jsx @@ -13,28 +13,29 @@ import { } from '@itwin/itwinui-react'; export default () => { - const [isOpen, setIsOpen] = React.useState(false); + const modalRef = React.useRef(null); return ( <> - - setIsOpen(false)} - > + - - + diff --git a/examples/Dialog.main.jsx b/examples/Dialog.main.jsx index 35f8bab9006..9e272119e56 100644 --- a/examples/Dialog.main.jsx +++ b/examples/Dialog.main.jsx @@ -3,26 +3,25 @@ * See LICENSE.md in the project root for license terms and full copyright notice. *--------------------------------------------------------------------------------------------*/ import * as React from 'react'; -// import * as ReactDOM from 'react-dom'; import { Dialog, Button } from '@itwin/itwinui-react'; export default () => { - const [isOpen, setIsOpen] = React.useState(false); + const dialogRef = React.useRef(null); return ( <> - setIsOpen(false)} - closeOnEsc + ref={dialogRef} closeOnExternalClick preventDocumentScroll trapFocus setFocus - isDismissible portal > @@ -37,11 +36,13 @@ export default () => { - + diff --git a/examples/Dialog.modal.jsx b/examples/Dialog.modal.jsx index 9acd3e97dc0..64e82eed6be 100644 --- a/examples/Dialog.modal.jsx +++ b/examples/Dialog.modal.jsx @@ -11,17 +11,20 @@ import { } from '@itwin/itwinui-react'; export default () => { - const [isOpen, setIsOpen] = React.useState(false); + const modalRef = React.useRef(null); return ( <>
-
- setIsOpen(false)}> + Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad @@ -29,10 +32,13 @@ export default () => { aliquip ex ea commodo consequat. - - + diff --git a/examples/Dialog.nondismissible.jsx b/examples/Dialog.nondismissible.jsx index 51e9389c653..6914f221fdf 100644 --- a/examples/Dialog.nondismissible.jsx +++ b/examples/Dialog.nondismissible.jsx @@ -3,24 +3,20 @@ * See LICENSE.md in the project root for license terms and full copyright notice. *--------------------------------------------------------------------------------------------*/ import * as React from 'react'; -// import * as ReactDOM from 'react-dom'; import { Dialog, Button } from '@itwin/itwinui-react'; export default () => { - const [isOpen, setIsOpen] = React.useState(false); + const dialogRef = React.useRef(null); return ( <> - - setIsOpen(false)} - setFocus={false} - isDismissible={false} - portal - > + @@ -31,11 +27,11 @@ export default () => { - + diff --git a/examples/Dialog.placement.jsx b/examples/Dialog.placement.jsx index 9d4289ffb98..9c842fabd59 100644 --- a/examples/Dialog.placement.jsx +++ b/examples/Dialog.placement.jsx @@ -3,26 +3,25 @@ * See LICENSE.md in the project root for license terms and full copyright notice. *--------------------------------------------------------------------------------------------*/ import * as React from 'react'; -// import * as ReactDOM from 'react-dom'; import { Dialog, Button } from '@itwin/itwinui-react'; export default () => { - const [isOpen, setIsOpen] = React.useState(false); + const dialogRef = React.useRef(null); return ( <> - setIsOpen(false)} - closeOnEsc + ref={dialogRef} closeOnExternalClick preventDocumentScroll trapFocus setFocus - isDismissible portal placement='top-left' > @@ -33,11 +32,13 @@ export default () => { - + From 18ce887fa6eae09e6ccd43e75653b8a048235b38 Mon Sep 17 00:00:00 2001 From: Mayank <9084735+mayank99@users.noreply.github.com> Date: Wed, 3 Apr 2024 16:51:59 -0400 Subject: [PATCH 06/30] docs --- apps/website/src/content/docs/dialog.mdx | 28 ++++++++++++++++++++++-- 1 file changed, 26 insertions(+), 2 deletions(-) diff --git a/apps/website/src/content/docs/dialog.mdx b/apps/website/src/content/docs/dialog.mdx index d3735009307..6fadb355c09 100644 --- a/apps/website/src/content/docs/dialog.mdx +++ b/apps/website/src/content/docs/dialog.mdx @@ -15,7 +15,7 @@ A dialog informs users about a task and can contain critical information, requir Dialogs are purposefully interruptive, so they should be used sparingly. -## Dialog VS Modal +## Dialog vs Modal The terms dialog and modal are often used interchangeably. Within iTwinUI we differentiate the two terms like so: @@ -26,7 +26,31 @@ The terms dialog and modal are often used interchangeably. Within iTwinUI we dif -## Variants +## Usage + +The dialog can be opened and closed in two ways: + +1. By using the `show()` and `close()` methods from its ref. + + ```tsx + const dialogRef = React.useRef(null); + + + + + + ``` + +2. By using the `isOpen` prop, often paired with `onClose` and a state variable. + + ```tsx + const [isDialogOpen, setIsDialogOpen] = React.useState(false); + + + setIsDialogOpen(false)}> + + + ``` ### Dismissible From 35ec876907a5eb52743bbfa7c73befc2e368e228 Mon Sep 17 00:00:00 2001 From: Mayank <9084735+mayank99@users.noreply.github.com> Date: Wed, 3 Apr 2024 17:25:24 -0400 Subject: [PATCH 07/30] fix refs --- .../itwinui-react/src/core/Dialog/Dialog.tsx | 28 +++++++++++++------ 1 file changed, 19 insertions(+), 9 deletions(-) diff --git a/packages/itwinui-react/src/core/Dialog/Dialog.tsx b/packages/itwinui-react/src/core/Dialog/Dialog.tsx index 84a9eea856d..3971692a05e 100644 --- a/packages/itwinui-react/src/core/Dialog/Dialog.tsx +++ b/packages/itwinui-react/src/core/Dialog/Dialog.tsx @@ -11,7 +11,12 @@ import { DialogContext } from './DialogContext.js'; import type { DialogContextProps } from './DialogContext.js'; import { DialogButtonBar } from './DialogButtonBar.js'; import { DialogMain } from './DialogMain.js'; -import { Box, Portal, useControlledState } from '../utils/index.js'; +import { + Box, + Portal, + useControlledState, + useMergedRefs, +} from '../utils/index.js'; import type { PolymorphicForwardRefComponent } from '../utils/index.js'; import { Transition } from 'react-transition-group'; @@ -42,17 +47,22 @@ const DialogComponent = React.forwardRef((props, forwardedRef) => { } = props; const [isOpen, setIsOpen] = useControlledState(false, isOpenProp); - const dialogRootRef = React.useRef(null); + const [dialog, setDialog] = React.useState(null); React.useImperativeHandle( forwardedRef, - () => ({ - ...(dialogRootRef.current as HTMLDivElement), - show: () => setIsOpen(true), - close: () => setIsOpen(false), - }), - [dialogRootRef, setIsOpen], + () => { + const _dialog = (dialog as any) || {}; + + Object.assign(_dialog, { + show: () => setIsOpen(true), + close: () => setIsOpen(false), + }); + + return _dialog; + }, + [dialog, setIsOpen], ); return ( @@ -79,7 +89,7 @@ const DialogComponent = React.forwardRef((props, forwardedRef) => { From 04e98c9c3129bd41b09c7fdccd50735f3ac879cf Mon Sep 17 00:00:00 2001 From: Mayank <9084735+mayank99@users.noreply.github.com> Date: Wed, 3 Apr 2024 17:25:44 -0400 Subject: [PATCH 08/30] test --- .../src/core/Dialog/Dialog.test.tsx | 30 ++++++++++++++++++- 1 file changed, 29 insertions(+), 1 deletion(-) diff --git a/packages/itwinui-react/src/core/Dialog/Dialog.test.tsx b/packages/itwinui-react/src/core/Dialog/Dialog.test.tsx index 44022e16c1d..fbf010cdfea 100644 --- a/packages/itwinui-react/src/core/Dialog/Dialog.test.tsx +++ b/packages/itwinui-react/src/core/Dialog/Dialog.test.tsx @@ -3,7 +3,7 @@ * See LICENSE.md in the project root for license terms and full copyright notice. *--------------------------------------------------------------------------------------------*/ import * as React from 'react'; -import { render, act } from '@testing-library/react'; +import { render, act, screen, fireEvent } from '@testing-library/react'; import { Dialog } from './Dialog.js'; import { Button } from '../Buttons/Button.js'; import { userEvent } from '@testing-library/user-event'; @@ -188,3 +188,31 @@ it('should not stay in the DOM when isOpen=false', () => { dialogWrapper = container.querySelector('.iui-dialog-wrapper') as HTMLElement; expect(dialogWrapper).toBeFalsy(); }); + +it.only('should expose show() and close() methods', () => { + vi.useFakeTimers(); + const dialogRef = React.createRef(); + + render( + + Hello + , + ); + + act(() => dialogRef.current?.show()); + const dialogElement = screen.getByRole('dialog'); + expect(dialogElement).toBeVisible(); + + act(() => dialogRef.current?.close()); + act(() => vi.runAllTimers()); + expect(dialogElement).not.toBeVisible(); + + act(() => dialogRef.current?.show()); + + // Built-in close should still work + act(() => { + fireEvent.keyDown(dialogElement, { key: 'Escape' }); + }); + + expect(dialogElement).not.toBeVisible(); +}); From 97dab3d8d0500eaaf431e7ab2e0c08fbf8c58fd3 Mon Sep 17 00:00:00 2001 From: Mayank <9084735+mayank99@users.noreply.github.com> Date: Wed, 3 Apr 2024 17:28:14 -0400 Subject: [PATCH 09/30] typo --- apps/react-workshop/src/Dialog.stories.tsx | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/apps/react-workshop/src/Dialog.stories.tsx b/apps/react-workshop/src/Dialog.stories.tsx index 15465af48ca..f10f94f846c 100644 --- a/apps/react-workshop/src/Dialog.stories.tsx +++ b/apps/react-workshop/src/Dialog.stories.tsx @@ -54,7 +54,13 @@ export const Modal = () => { > Open Dialog - + From 2f267c118e955196a947aff81ba03cbe85098d55 Mon Sep 17 00:00:00 2001 From: Mayank <9084735+mayank99@users.noreply.github.com> Date: Wed, 3 Apr 2024 17:53:40 -0400 Subject: [PATCH 10/30] cleanup --- packages/itwinui-react/src/core/Dialog/Dialog.tsx | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/packages/itwinui-react/src/core/Dialog/Dialog.tsx b/packages/itwinui-react/src/core/Dialog/Dialog.tsx index 3971692a05e..762305765ed 100644 --- a/packages/itwinui-react/src/core/Dialog/Dialog.tsx +++ b/packages/itwinui-react/src/core/Dialog/Dialog.tsx @@ -53,13 +53,11 @@ const DialogComponent = React.forwardRef((props, forwardedRef) => { React.useImperativeHandle( forwardedRef, () => { + const show = () => setIsOpen(true); + const close = () => setIsOpen(false); const _dialog = (dialog as any) || {}; - Object.assign(_dialog, { - show: () => setIsOpen(true), - close: () => setIsOpen(false), - }); - + Object.assign(_dialog, { show, close }); return _dialog; }, [dialog, setIsOpen], From b45ae79fc34bae833df33370ae61894bfaef29ce Mon Sep 17 00:00:00 2001 From: Mayank <9084735+mayank99@users.noreply.github.com> Date: Wed, 3 Apr 2024 18:01:47 -0400 Subject: [PATCH 11/30] whoops --- packages/itwinui-react/src/core/Dialog/Dialog.test.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/itwinui-react/src/core/Dialog/Dialog.test.tsx b/packages/itwinui-react/src/core/Dialog/Dialog.test.tsx index fbf010cdfea..dc3366f139a 100644 --- a/packages/itwinui-react/src/core/Dialog/Dialog.test.tsx +++ b/packages/itwinui-react/src/core/Dialog/Dialog.test.tsx @@ -189,7 +189,7 @@ it('should not stay in the DOM when isOpen=false', () => { expect(dialogWrapper).toBeFalsy(); }); -it.only('should expose show() and close() methods', () => { +it('should expose show() and close() methods', () => { vi.useFakeTimers(); const dialogRef = React.createRef(); From 6785ea5058669107f2c1130b06f8fb83d9efaf2f Mon Sep 17 00:00:00 2001 From: Mayank <9084735+mayank99@users.noreply.github.com> Date: Wed, 3 Apr 2024 18:14:49 -0400 Subject: [PATCH 12/30] revert `Modal` --- apps/react-workshop/src/Modal.stories.tsx | 40 ++++++++++++++----- examples/Dialog.fullpage.jsx | 21 +++++----- examples/Dialog.modal.jsx | 16 +++----- .../itwinui-react/src/core/Modal/Modal.tsx | 4 +- 4 files changed, 45 insertions(+), 36 deletions(-) diff --git a/apps/react-workshop/src/Modal.stories.tsx b/apps/react-workshop/src/Modal.stories.tsx index 8c40ee324e7..bbea7adf1b0 100644 --- a/apps/react-workshop/src/Modal.stories.tsx +++ b/apps/react-workshop/src/Modal.stories.tsx @@ -15,17 +15,38 @@ export default { }; export const Basic = () => { - const modalRef = React.useRef(null); + const [isModalOpen, setIsModalOpen] = React.useState(false); + + const closeModal = () => { + setIsModalOpen(false); + }; + + const onClose = () => { + console.log('onClose'); + closeModal(); + }; + + const primaryButtonHandle = () => { + console.log('Primary button'); + closeModal(); + }; + + const secondaryButtonHandle = () => { + console.log('Secondary button'); + closeModal(); + }; return ( <> - - + console.log('onKeyDown')} + > Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad @@ -36,13 +57,10 @@ export const Basic = () => { culpa qui officia deserunt mollit anim id est laborum. - - + diff --git a/examples/Dialog.fullpage.jsx b/examples/Dialog.fullpage.jsx index ed24c0730f8..5ca253bc6e6 100644 --- a/examples/Dialog.fullpage.jsx +++ b/examples/Dialog.fullpage.jsx @@ -13,29 +13,28 @@ import { } from '@itwin/itwinui-react'; export default () => { - const modalRef = React.useRef(null); + const [isOpen, setIsOpen] = React.useState(false); return ( <> - - + setIsOpen(false)} + > - - + diff --git a/examples/Dialog.modal.jsx b/examples/Dialog.modal.jsx index 64e82eed6be..9acd3e97dc0 100644 --- a/examples/Dialog.modal.jsx +++ b/examples/Dialog.modal.jsx @@ -11,20 +11,17 @@ import { } from '@itwin/itwinui-react'; export default () => { - const modalRef = React.useRef(null); + const [isOpen, setIsOpen] = React.useState(false); return ( <>
-
- + setIsOpen(false)}> Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad @@ -32,13 +29,10 @@ export default () => { aliquip ex ea commodo consequat. - - + diff --git a/packages/itwinui-react/src/core/Modal/Modal.tsx b/packages/itwinui-react/src/core/Modal/Modal.tsx index 8484feea6c0..de8a150d979 100644 --- a/packages/itwinui-react/src/core/Modal/Modal.tsx +++ b/packages/itwinui-react/src/core/Modal/Modal.tsx @@ -123,6 +123,4 @@ export const Modal = React.forwardRef((props, forwardedRef) => { ); -}) as PolymorphicForwardRefComponent<'div', ModalProps> & { - Ref: typeof Dialog.Ref; -}; +}) as PolymorphicForwardRefComponent<'div', ModalProps>; From 84b9a4d290c148d2d443cb3188121d4d2e5bb498 Mon Sep 17 00:00:00 2001 From: Mayank <9084735+mayank99@users.noreply.github.com> Date: Thu, 4 Apr 2024 19:01:10 -0400 Subject: [PATCH 13/30] wip --- apps/react-workshop/src/Dialog.stories.tsx | 36 +++------- apps/website/src/content/docs/dialog.mdx | 10 +-- examples/Dialog.draggable.jsx | 18 ++--- examples/Dialog.main.jsx | 18 ++--- examples/Dialog.nondismissible.jsx | 16 ++--- examples/Dialog.placement.jsx | 18 ++--- .../src/core/Dialog/Dialog.test.tsx | 10 +-- .../itwinui-react/src/core/Dialog/Dialog.tsx | 72 ++++++++++++------- 8 files changed, 85 insertions(+), 113 deletions(-) diff --git a/apps/react-workshop/src/Dialog.stories.tsx b/apps/react-workshop/src/Dialog.stories.tsx index f10f94f846c..b2f01f1e273 100644 --- a/apps/react-workshop/src/Dialog.stories.tsx +++ b/apps/react-workshop/src/Dialog.stories.tsx @@ -12,30 +12,22 @@ export default { const lorem = `Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum.`; export const Basic = () => { - const dialogRef = React.useRef(null); + const dialog = Dialog.useInstance(); return ( <> - - + {lorem} - - + @@ -44,18 +36,15 @@ export const Basic = () => { }; export const Modal = () => { - const dialogRef = React.useRef(null); + const dialog = Dialog.useInstance(); return ( <> - { {lorem} - - + diff --git a/apps/website/src/content/docs/dialog.mdx b/apps/website/src/content/docs/dialog.mdx index 6fadb355c09..791b5e1ad11 100644 --- a/apps/website/src/content/docs/dialog.mdx +++ b/apps/website/src/content/docs/dialog.mdx @@ -30,14 +30,14 @@ The terms dialog and modal are often used interchangeably. Within iTwinUI we dif The dialog can be opened and closed in two ways: -1. By using the `show()` and `close()` methods from its ref. +1. By using the `show()` and `close()` methods from its instance. ```tsx - const dialogRef = React.useRef(null); + const dialog = Dialog.useInstance(); - - - + + + ``` diff --git a/examples/Dialog.draggable.jsx b/examples/Dialog.draggable.jsx index 4ac834e1260..b2cc35ad0e7 100644 --- a/examples/Dialog.draggable.jsx +++ b/examples/Dialog.draggable.jsx @@ -11,18 +11,15 @@ import { } from '@itwin/itwinui-react'; export default () => { - const dialogRef = React.useRef(null); + const dialog = Dialog.useInstance(); return ( <> - - + @@ -30,15 +27,10 @@ export default () => { - - + diff --git a/examples/Dialog.main.jsx b/examples/Dialog.main.jsx index 9e272119e56..97c22900f2a 100644 --- a/examples/Dialog.main.jsx +++ b/examples/Dialog.main.jsx @@ -6,18 +6,15 @@ import * as React from 'react'; import { Dialog, Button } from '@itwin/itwinui-react'; export default () => { - const dialogRef = React.useRef(null); + const dialog = Dialog.useInstance(); return ( <> - { ask for a decision. - - + diff --git a/examples/Dialog.nondismissible.jsx b/examples/Dialog.nondismissible.jsx index 6914f221fdf..55e46ff854d 100644 --- a/examples/Dialog.nondismissible.jsx +++ b/examples/Dialog.nondismissible.jsx @@ -6,17 +6,14 @@ import * as React from 'react'; import { Dialog, Button } from '@itwin/itwinui-react'; export default () => { - const dialogRef = React.useRef(null); + const dialog = Dialog.useInstance(); return ( <> - - + @@ -25,13 +22,10 @@ export default () => { You can't undo this action. - - + diff --git a/examples/Dialog.placement.jsx b/examples/Dialog.placement.jsx index 9c842fabd59..53698c8b0d0 100644 --- a/examples/Dialog.placement.jsx +++ b/examples/Dialog.placement.jsx @@ -6,18 +6,15 @@ import * as React from 'react'; import { Dialog, Button } from '@itwin/itwinui-react'; export default () => { - const dialogRef = React.useRef(null); + const dialog = Dialog.useInstance(); return ( <> - { - - + diff --git a/packages/itwinui-react/src/core/Dialog/Dialog.test.tsx b/packages/itwinui-react/src/core/Dialog/Dialog.test.tsx index dc3366f139a..01579ae1780 100644 --- a/packages/itwinui-react/src/core/Dialog/Dialog.test.tsx +++ b/packages/itwinui-react/src/core/Dialog/Dialog.test.tsx @@ -191,23 +191,23 @@ it('should not stay in the DOM when isOpen=false', () => { it('should expose show() and close() methods', () => { vi.useFakeTimers(); - const dialogRef = React.createRef(); + const dialog = Dialog.useInstance(); render( - + Hello , ); - act(() => dialogRef.current?.show()); + act(() => dialog.show()); const dialogElement = screen.getByRole('dialog'); expect(dialogElement).toBeVisible(); - act(() => dialogRef.current?.close()); + act(() => dialog.close()); act(() => vi.runAllTimers()); expect(dialogElement).not.toBeVisible(); - act(() => dialogRef.current?.show()); + act(() => dialog.show()); // Built-in close should still work act(() => { diff --git a/packages/itwinui-react/src/core/Dialog/Dialog.tsx b/packages/itwinui-react/src/core/Dialog/Dialog.tsx index 762305765ed..1814e6cec8b 100644 --- a/packages/itwinui-react/src/core/Dialog/Dialog.tsx +++ b/packages/itwinui-react/src/core/Dialog/Dialog.tsx @@ -20,11 +20,50 @@ import { import type { PolymorphicForwardRefComponent } from '../utils/index.js'; import { Transition } from 'react-transition-group'; +// ---------------------------------------------------------------------------- + +const internals = Symbol('_'); + +class Instance { + [internals] = new Map(); + show() { + this[internals].get('setIsOpen')(true); + } + close() { + this[internals].get('setIsOpen')(false); + } +} + +const useInstance = () => { + return React.useMemo(() => new Instance(), []); +}; + +const setupInstance = (instance: any, properties: Record) => { + if (instance?.[internals]?.created) { + return instance; + } + + instance ||= new Instance(); + + // Copy all properties into instance internals. + // This needs to be done here rather than inside the constructor or an instance method, + // in order to maintain referential stability. + for (const [key, value] of Object.entries(properties)) { + instance[internals].set(key, value); + } + instance[internals].set('created', true); + + return instance; +}; + +// ---------------------------------------------------------------------------- + type DialogProps = { /** * Dialog content. */ children: React.ReactNode; + instance?: Instance; } & Omit; const DialogComponent = React.forwardRef((props, forwardedRef) => { @@ -43,25 +82,14 @@ const DialogComponent = React.forwardRef((props, forwardedRef) => { placement, className, portal = false, + instance: instanceProp, ...rest } = props; const [isOpen, setIsOpen] = useControlledState(false, isOpenProp); + setupInstance(instanceProp, { setIsOpen }); + const dialogRootRef = React.useRef(null); - const [dialog, setDialog] = React.useState(null); - - React.useImperativeHandle( - forwardedRef, - () => { - const show = () => setIsOpen(true); - const close = () => setIsOpen(false); - const _dialog = (dialog as any) || {}; - - Object.assign(_dialog, { show, close }); - return _dialog; - }, - [dialog, setIsOpen], - ); return ( @@ -87,7 +115,7 @@ const DialogComponent = React.forwardRef((props, forwardedRef) => { @@ -98,17 +126,6 @@ const DialogComponent = React.forwardRef((props, forwardedRef) => { // ---------------------------------------------------------------------------- -type DialogComponentType = typeof DialogComponent & { - Ref: HTMLDivElement & { - /** Call this function to show (open) the dialog. */ - show: () => void; - /** Call this function to close the dialog. */ - close: () => void; - }; -}; - -// ---------------------------------------------------------------------------- - /** * Dialog component. * @example @@ -131,10 +148,11 @@ type DialogComponentType = typeof DialogComponent & { * * */ -export const Dialog = Object.assign(DialogComponent as DialogComponentType, { +export const Dialog = Object.assign(DialogComponent, { Backdrop: DialogBackdrop, Main: DialogMain, TitleBar: DialogTitleBar, Content: DialogContent, ButtonBar: DialogButtonBar, + useInstance, }); From e3defd85ec43fa9e80112fe4b78e4435298651f2 Mon Sep 17 00:00:00 2001 From: Mayank <9084735+mayank99@users.noreply.github.com> Date: Thu, 4 Apr 2024 19:06:38 -0400 Subject: [PATCH 14/30] cleanup --- .../itwinui-react/src/core/Dialog/Dialog.tsx | 37 ++++++++----------- 1 file changed, 15 insertions(+), 22 deletions(-) diff --git a/packages/itwinui-react/src/core/Dialog/Dialog.tsx b/packages/itwinui-react/src/core/Dialog/Dialog.tsx index 1814e6cec8b..b0b14fde183 100644 --- a/packages/itwinui-react/src/core/Dialog/Dialog.tsx +++ b/packages/itwinui-react/src/core/Dialog/Dialog.tsx @@ -22,15 +22,14 @@ import { Transition } from 'react-transition-group'; // ---------------------------------------------------------------------------- -const internals = Symbol('_'); +const internals = Symbol('internals'); class Instance { - [internals] = new Map(); - show() { - this[internals].get('setIsOpen')(true); - } - close() { - this[internals].get('setIsOpen')(false); + [internals] = {} as any; + constructor() { + this[internals].initialize = (properties = {}) => { + Object.assign(this, properties); + }; } } @@ -38,21 +37,12 @@ const useInstance = () => { return React.useMemo(() => new Instance(), []); }; -const setupInstance = (instance: any, properties: Record) => { - if (instance?.[internals]?.created) { - return instance; - } - +const synchronizeInstance = ( + instance: any, + properties: Record, +) => { instance ||= new Instance(); - - // Copy all properties into instance internals. - // This needs to be done here rather than inside the constructor or an instance method, - // in order to maintain referential stability. - for (const [key, value] of Object.entries(properties)) { - instance[internals].set(key, value); - } - instance[internals].set('created', true); - + instance[internals].initialize(properties); return instance; }; @@ -87,7 +77,10 @@ const DialogComponent = React.forwardRef((props, forwardedRef) => { } = props; const [isOpen, setIsOpen] = useControlledState(false, isOpenProp); - setupInstance(instanceProp, { setIsOpen }); + synchronizeInstance(instanceProp, { + show: React.useCallback(() => setIsOpen(true), [setIsOpen]), + close: React.useCallback(() => setIsOpen(false), [setIsOpen]), + }); const dialogRootRef = React.useRef(null); From d8a858c944a821ce38c396176c777c01f751679a Mon Sep 17 00:00:00 2001 From: Mayank <9084735+mayank99@users.noreply.github.com> Date: Thu, 4 Apr 2024 19:14:46 -0400 Subject: [PATCH 15/30] fix types --- packages/itwinui-react/src/core/Dialog/Dialog.tsx | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/packages/itwinui-react/src/core/Dialog/Dialog.tsx b/packages/itwinui-react/src/core/Dialog/Dialog.tsx index b0b14fde183..a3727d39460 100644 --- a/packages/itwinui-react/src/core/Dialog/Dialog.tsx +++ b/packages/itwinui-react/src/core/Dialog/Dialog.tsx @@ -48,12 +48,20 @@ const synchronizeInstance = ( // ---------------------------------------------------------------------------- +type DialogInstance = { + show: () => void; + close: () => void; +}; + type DialogProps = { /** * Dialog content. */ children: React.ReactNode; - instance?: Instance; + /** + * Pass an instance created by `useInstance` to control the dialog programmatically. + */ + instance?: DialogInstance; } & Omit; const DialogComponent = React.forwardRef((props, forwardedRef) => { @@ -147,5 +155,5 @@ export const Dialog = Object.assign(DialogComponent, { TitleBar: DialogTitleBar, Content: DialogContent, ButtonBar: DialogButtonBar, - useInstance, + useInstance: useInstance as unknown as () => DialogInstance, }); From caf402dd6580fc123d632b848e43367a4a8d81db Mon Sep 17 00:00:00 2001 From: Mayank <9084735+mayank99@users.noreply.github.com> Date: Thu, 4 Apr 2024 19:29:25 -0400 Subject: [PATCH 16/30] fix test --- .../src/core/Dialog/Dialog.test.tsx | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/packages/itwinui-react/src/core/Dialog/Dialog.test.tsx b/packages/itwinui-react/src/core/Dialog/Dialog.test.tsx index 01579ae1780..1fddee07e22 100644 --- a/packages/itwinui-react/src/core/Dialog/Dialog.test.tsx +++ b/packages/itwinui-react/src/core/Dialog/Dialog.test.tsx @@ -191,13 +191,19 @@ it('should not stay in the DOM when isOpen=false', () => { it('should expose show() and close() methods', () => { vi.useFakeTimers(); - const dialog = Dialog.useInstance(); - render( - - Hello - , - ); + let dialog: ReturnType; + + const DialogTest = () => { + dialog = Dialog.useInstance(); + return ( + + Hello + + ); + }; + + render(); act(() => dialog.show()); const dialogElement = screen.getByRole('dialog'); From 72c3f45c985cd9cbed5fe3678d9f7ea4946eb1c3 Mon Sep 17 00:00:00 2001 From: Mayank <9084735+mayank99@users.noreply.github.com> Date: Thu, 4 Apr 2024 19:32:42 -0400 Subject: [PATCH 17/30] simplify --- packages/itwinui-react/src/core/Dialog/Dialog.tsx | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/packages/itwinui-react/src/core/Dialog/Dialog.tsx b/packages/itwinui-react/src/core/Dialog/Dialog.tsx index a3727d39460..3ef3ad01baf 100644 --- a/packages/itwinui-react/src/core/Dialog/Dialog.tsx +++ b/packages/itwinui-react/src/core/Dialog/Dialog.tsx @@ -25,12 +25,11 @@ import { Transition } from 'react-transition-group'; const internals = Symbol('internals'); class Instance { - [internals] = {} as any; - constructor() { - this[internals].initialize = (properties = {}) => { + [internals] = { + initialize: (properties = {}) => { Object.assign(this, properties); - }; - } + }, + }; } const useInstance = () => { From 4c8861685eac9b5821a70b028055039ecea44dad Mon Sep 17 00:00:00 2001 From: Mayank <9084735+mayank99@users.noreply.github.com> Date: Fri, 5 Apr 2024 10:16:46 -0400 Subject: [PATCH 18/30] move helpers to separate module --- .../itwinui-react/src/core/Dialog/Dialog.tsx | 45 +++++++---------- .../src/core/utils/hooks/index.ts | 1 + .../src/core/utils/hooks/useInstance.ts | 48 +++++++++++++++++++ 3 files changed, 65 insertions(+), 29 deletions(-) create mode 100644 packages/itwinui-react/src/core/utils/hooks/useInstance.ts diff --git a/packages/itwinui-react/src/core/Dialog/Dialog.tsx b/packages/itwinui-react/src/core/Dialog/Dialog.tsx index 3ef3ad01baf..e776842110f 100644 --- a/packages/itwinui-react/src/core/Dialog/Dialog.tsx +++ b/packages/itwinui-react/src/core/Dialog/Dialog.tsx @@ -15,40 +15,19 @@ import { Box, Portal, useControlledState, + useInstance, useMergedRefs, + useSynchronizeInstance, } from '../utils/index.js'; import type { PolymorphicForwardRefComponent } from '../utils/index.js'; import { Transition } from 'react-transition-group'; // ---------------------------------------------------------------------------- -const internals = Symbol('internals'); - -class Instance { - [internals] = { - initialize: (properties = {}) => { - Object.assign(this, properties); - }, - }; -} - -const useInstance = () => { - return React.useMemo(() => new Instance(), []); -}; - -const synchronizeInstance = ( - instance: any, - properties: Record, -) => { - instance ||= new Instance(); - instance[internals].initialize(properties); - return instance; -}; - -// ---------------------------------------------------------------------------- - type DialogInstance = { + /** Call this function to show (open) the dialog. */ show: () => void; + /** Call this function to hide (close) the dialog. */ close: () => void; }; @@ -63,6 +42,8 @@ type DialogProps = { instance?: DialogInstance; } & Omit; +// ---------------------------------------------------------------------------- + const DialogComponent = React.forwardRef((props, forwardedRef) => { const { trapFocus = false, @@ -84,10 +65,16 @@ const DialogComponent = React.forwardRef((props, forwardedRef) => { } = props; const [isOpen, setIsOpen] = useControlledState(false, isOpenProp); - synchronizeInstance(instanceProp, { - show: React.useCallback(() => setIsOpen(true), [setIsOpen]), - close: React.useCallback(() => setIsOpen(false), [setIsOpen]), - }); + useSynchronizeInstance( + instanceProp, + React.useMemo( + () => ({ + show: () => setIsOpen(true), + close: () => setIsOpen(false), + }), + [setIsOpen], + ), + ); const dialogRootRef = React.useRef(null); diff --git a/packages/itwinui-react/src/core/utils/hooks/index.ts b/packages/itwinui-react/src/core/utils/hooks/index.ts index c60c0d923d6..d10532a7f62 100644 --- a/packages/itwinui-react/src/core/utils/hooks/index.ts +++ b/packages/itwinui-react/src/core/utils/hooks/index.ts @@ -17,3 +17,4 @@ export * from './useIsClient.js'; export * from './useId.js'; export * from './useControlledState.js'; export * from './useSyncExternalStore.js'; +export * from './useInstance.js'; diff --git a/packages/itwinui-react/src/core/utils/hooks/useInstance.ts b/packages/itwinui-react/src/core/utils/hooks/useInstance.ts new file mode 100644 index 00000000000..1d4fe7c64b8 --- /dev/null +++ b/packages/itwinui-react/src/core/utils/hooks/useInstance.ts @@ -0,0 +1,48 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Bentley Systems, Incorporated. All rights reserved. + * See LICENSE.md in the project root for license terms and full copyright notice. + *--------------------------------------------------------------------------------------------*/ +import * as React from 'react'; +import { useSyncExternalStore } from './useSyncExternalStore.js'; + +const internals = Symbol(); + +class Instance { + [internals] = { + initialize: (properties = {}) => { + Object.assign(this, properties); + }, + }; +} + +export const useInstance = () => React.useMemo(() => new Instance(), []); + +/** + * Synchronizes the instance with the provided properties. + * + * @param instance Instance created by `useInstance`. + * @param properties Memoized object containing properties to be synchronized. + * + * @usage + * ```tsx + * const instance = useInstance(); + * + * const properties = React.useMemo(() => ({ + * show: () => console.log('show'), + * }), []); + * + * useSynchronizeInstance(instance, properties); + * ``` + */ +export const useSynchronizeInstance = (instance: T, properties: T) => { + const synchronize = React.useCallback(() => { + (instance as any)?.[internals]?.initialize?.(properties); + return () => {}; + }, [instance, properties]); + + return useSyncExternalStore( + synchronize, + () => instance, + () => instance, + ); +}; From 4f4d8fb1fd9e2927cdcee2cb006a2b8beb68d9ed Mon Sep 17 00:00:00 2001 From: Mayank <9084735+mayank99@users.noreply.github.com> Date: Fri, 5 Apr 2024 10:25:00 -0400 Subject: [PATCH 19/30] simplify even more --- packages/itwinui-react/src/core/Dialog/Dialog.tsx | 2 +- .../src/core/utils/hooks/useInstance.ts | 14 ++++---------- 2 files changed, 5 insertions(+), 11 deletions(-) diff --git a/packages/itwinui-react/src/core/Dialog/Dialog.tsx b/packages/itwinui-react/src/core/Dialog/Dialog.tsx index e776842110f..380993cbe13 100644 --- a/packages/itwinui-react/src/core/Dialog/Dialog.tsx +++ b/packages/itwinui-react/src/core/Dialog/Dialog.tsx @@ -141,5 +141,5 @@ export const Dialog = Object.assign(DialogComponent, { TitleBar: DialogTitleBar, Content: DialogContent, ButtonBar: DialogButtonBar, - useInstance: useInstance as unknown as () => DialogInstance, + useInstance: useInstance as () => DialogInstance, }); diff --git a/packages/itwinui-react/src/core/utils/hooks/useInstance.ts b/packages/itwinui-react/src/core/utils/hooks/useInstance.ts index 1d4fe7c64b8..c9292f065e1 100644 --- a/packages/itwinui-react/src/core/utils/hooks/useInstance.ts +++ b/packages/itwinui-react/src/core/utils/hooks/useInstance.ts @@ -5,15 +5,7 @@ import * as React from 'react'; import { useSyncExternalStore } from './useSyncExternalStore.js'; -const internals = Symbol(); - -class Instance { - [internals] = { - initialize: (properties = {}) => { - Object.assign(this, properties); - }, - }; -} +export class Instance {} export const useInstance = () => React.useMemo(() => new Instance(), []); @@ -36,7 +28,9 @@ export const useInstance = () => React.useMemo(() => new Instance(), []); */ export const useSynchronizeInstance = (instance: T, properties: T) => { const synchronize = React.useCallback(() => { - (instance as any)?.[internals]?.initialize?.(properties); + if (instance instanceof Instance) { + Object.assign(instance, properties); + } return () => {}; }, [instance, properties]); From 9551838dfe1cdaa9517703e16d5f360da2a9ce1d Mon Sep 17 00:00:00 2001 From: Mayank <9084735+mayank99@users.noreply.github.com> Date: Fri, 5 Apr 2024 10:29:03 -0400 Subject: [PATCH 20/30] changeset --- .changeset/eight-rocks-dream.md | 14 ++++++++++++++ 1 file changed, 14 insertions(+) create mode 100644 .changeset/eight-rocks-dream.md diff --git a/.changeset/eight-rocks-dream.md b/.changeset/eight-rocks-dream.md new file mode 100644 index 00000000000..47ddb60c630 --- /dev/null +++ b/.changeset/eight-rocks-dream.md @@ -0,0 +1,14 @@ +--- +"@itwin/itwinui-react": minor +--- + +Added the ability to control `Dialog` imperatively using `show()` and `close()` methods from its instance. + +```tsx +const dialog = Dialog.useInstance(); + + + + + +``` From 1a899637ad15fd1ed47587c1117bc0578eddac31 Mon Sep 17 00:00:00 2001 From: Mayank <9084735+mayank99@users.noreply.github.com> Date: Fri, 5 Apr 2024 12:51:08 -0400 Subject: [PATCH 21/30] consistency --- .changeset/eight-rocks-dream.md | 4 ++-- apps/website/src/content/docs/dialog.mdx | 2 +- packages/itwinui-react/src/core/Dialog/Dialog.tsx | 6 +++++- 3 files changed, 8 insertions(+), 4 deletions(-) diff --git a/.changeset/eight-rocks-dream.md b/.changeset/eight-rocks-dream.md index 47ddb60c630..780ccb6cb87 100644 --- a/.changeset/eight-rocks-dream.md +++ b/.changeset/eight-rocks-dream.md @@ -7,8 +7,8 @@ Added the ability to control `Dialog` imperatively using `show()` and `close()` ```tsx const dialog = Dialog.useInstance(); - + - + ``` diff --git a/apps/website/src/content/docs/dialog.mdx b/apps/website/src/content/docs/dialog.mdx index 791b5e1ad11..c0a9c8813f7 100644 --- a/apps/website/src/content/docs/dialog.mdx +++ b/apps/website/src/content/docs/dialog.mdx @@ -35,7 +35,7 @@ The dialog can be opened and closed in two ways: ```tsx const dialog = Dialog.useInstance(); - + diff --git a/packages/itwinui-react/src/core/Dialog/Dialog.tsx b/packages/itwinui-react/src/core/Dialog/Dialog.tsx index 380993cbe13..b508ad6d3a4 100644 --- a/packages/itwinui-react/src/core/Dialog/Dialog.tsx +++ b/packages/itwinui-react/src/core/Dialog/Dialog.tsx @@ -37,7 +37,11 @@ type DialogProps = { */ children: React.ReactNode; /** - * Pass an instance created by `useInstance` to control the dialog programmatically. + * Pass an instance created by `useInstance` to control the dialog imperatively. + * + * @example + * const dialog = Dialog.useInstance(); + * */ instance?: DialogInstance; } & Omit; From e75d73bcfb59c25cea3115729f92a27e45ff1473 Mon Sep 17 00:00:00 2001 From: Mayank <9084735+mayank99@users.noreply.github.com> Date: Fri, 5 Apr 2024 13:03:59 -0400 Subject: [PATCH 22/30] remove unnecessary export --- packages/itwinui-react/src/core/utils/hooks/useInstance.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/itwinui-react/src/core/utils/hooks/useInstance.ts b/packages/itwinui-react/src/core/utils/hooks/useInstance.ts index c9292f065e1..cb714f60519 100644 --- a/packages/itwinui-react/src/core/utils/hooks/useInstance.ts +++ b/packages/itwinui-react/src/core/utils/hooks/useInstance.ts @@ -5,7 +5,7 @@ import * as React from 'react'; import { useSyncExternalStore } from './useSyncExternalStore.js'; -export class Instance {} +class Instance {} export const useInstance = () => React.useMemo(() => new Instance(), []); From 465a8d5f152886c865fb74a8adb4f075ae7d3e4e Mon Sep 17 00:00:00 2001 From: Mayank <9084735+mayank99@users.noreply.github.com> Date: Fri, 5 Apr 2024 13:05:09 -0400 Subject: [PATCH 23/30] =?UTF-8?q?`@usage`=20=E2=86=92=20`@example`?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- packages/itwinui-react/src/core/utils/hooks/useInstance.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/packages/itwinui-react/src/core/utils/hooks/useInstance.ts b/packages/itwinui-react/src/core/utils/hooks/useInstance.ts index cb714f60519..8a371be73ab 100644 --- a/packages/itwinui-react/src/core/utils/hooks/useInstance.ts +++ b/packages/itwinui-react/src/core/utils/hooks/useInstance.ts @@ -15,8 +15,7 @@ export const useInstance = () => React.useMemo(() => new Instance(), []); * @param instance Instance created by `useInstance`. * @param properties Memoized object containing properties to be synchronized. * - * @usage - * ```tsx + * @example * const instance = useInstance(); * * const properties = React.useMemo(() => ({ @@ -24,7 +23,6 @@ export const useInstance = () => React.useMemo(() => new Instance(), []); * }), []); * * useSynchronizeInstance(instance, properties); - * ``` */ export const useSynchronizeInstance = (instance: T, properties: T) => { const synchronize = React.useCallback(() => { From 50775997cc6e6f57ae7e7a3236d191fe281b6320 Mon Sep 17 00:00:00 2001 From: Mayank <9084735+mayank99@users.noreply.github.com> Date: Fri, 5 Apr 2024 13:05:42 -0400 Subject: [PATCH 24/30] better example --- packages/itwinui-react/src/core/utils/hooks/useInstance.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/itwinui-react/src/core/utils/hooks/useInstance.ts b/packages/itwinui-react/src/core/utils/hooks/useInstance.ts index 8a371be73ab..0073b304398 100644 --- a/packages/itwinui-react/src/core/utils/hooks/useInstance.ts +++ b/packages/itwinui-react/src/core/utils/hooks/useInstance.ts @@ -23,6 +23,8 @@ export const useInstance = () => React.useMemo(() => new Instance(), []); * }), []); * * useSynchronizeInstance(instance, properties); + * + * instance.show(); // logs 'show' */ export const useSynchronizeInstance = (instance: T, properties: T) => { const synchronize = React.useCallback(() => { From 3542b0043a1bbe9e93a6198fa57dc3bdb2e8bddf Mon Sep 17 00:00:00 2001 From: Mayank <9084735+mayank99@users.noreply.github.com> Date: Fri, 5 Apr 2024 13:25:06 -0400 Subject: [PATCH 25/30] cleanup --- .../itwinui-react/src/core/utils/hooks/useInstance.ts | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/packages/itwinui-react/src/core/utils/hooks/useInstance.ts b/packages/itwinui-react/src/core/utils/hooks/useInstance.ts index 0073b304398..127148ae546 100644 --- a/packages/itwinui-react/src/core/utils/hooks/useInstance.ts +++ b/packages/itwinui-react/src/core/utils/hooks/useInstance.ts @@ -28,10 +28,12 @@ export const useInstance = () => React.useMemo(() => new Instance(), []); */ export const useSynchronizeInstance = (instance: T, properties: T) => { const synchronize = React.useCallback(() => { - if (instance instanceof Instance) { - Object.assign(instance, properties); + if (!(instance instanceof Instance)) { + return () => {}; } - return () => {}; + + Object.assign(instance, properties); + return () => Object.assign(instance, {}); }, [instance, properties]); return useSyncExternalStore( From b447d0735d7856b2bd09893b1f487290b64ff2f2 Mon Sep 17 00:00:00 2001 From: Mayank <9084735+mayank99@users.noreply.github.com> Date: Fri, 5 Apr 2024 13:27:29 -0400 Subject: [PATCH 26/30] fix cleanup --- packages/itwinui-react/src/core/utils/hooks/useInstance.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/packages/itwinui-react/src/core/utils/hooks/useInstance.ts b/packages/itwinui-react/src/core/utils/hooks/useInstance.ts index 127148ae546..b371d7d9c32 100644 --- a/packages/itwinui-react/src/core/utils/hooks/useInstance.ts +++ b/packages/itwinui-react/src/core/utils/hooks/useInstance.ts @@ -33,7 +33,11 @@ export const useSynchronizeInstance = (instance: T, properties: T) => { } Object.assign(instance, properties); - return () => Object.assign(instance, {}); + return () => { + for (const key in properties) { + delete instance[key]; + } + }; }, [instance, properties]); return useSyncExternalStore( From 6a13b90c1f3fe1f5698ebe936f852dc076223797 Mon Sep 17 00:00:00 2001 From: Mayank <9084735+mayank99@users.noreply.github.com> Date: Mon, 8 Apr 2024 16:03:28 -0400 Subject: [PATCH 27/30] Update apps/website/src/content/docs/dialog.mdx Co-authored-by: Rohan Kadkol <45748283+rohan-kadkol@users.noreply.github.com> --- apps/website/src/content/docs/dialog.mdx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/website/src/content/docs/dialog.mdx b/apps/website/src/content/docs/dialog.mdx index c0a9c8813f7..7ae13221ba8 100644 --- a/apps/website/src/content/docs/dialog.mdx +++ b/apps/website/src/content/docs/dialog.mdx @@ -47,7 +47,7 @@ The dialog can be opened and closed in two ways: const [isDialogOpen, setIsDialogOpen] = React.useState(false); - setIsDialogOpen(false)}> + setIsDialogOpen(false)}> ``` From 57fed2d0f4c373f462e377d32a9b5106b6d7eac3 Mon Sep 17 00:00:00 2001 From: Mayank <9084735+mayank99@users.noreply.github.com> Date: Mon, 8 Apr 2024 16:04:00 -0400 Subject: [PATCH 28/30] Update examples/Dialog.draggable.jsx Co-authored-by: Rohan Kadkol <45748283+rohan-kadkol@users.noreply.github.com> --- examples/Dialog.draggable.jsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/examples/Dialog.draggable.jsx b/examples/Dialog.draggable.jsx index b2cc35ad0e7..8f8ef3cbb71 100644 --- a/examples/Dialog.draggable.jsx +++ b/examples/Dialog.draggable.jsx @@ -15,7 +15,7 @@ export default () => { return ( <> - From 76c9556b7fb515383a044c00f8d736f2bd5ae99c Mon Sep 17 00:00:00 2001 From: Mayank <9084735+mayank99@users.noreply.github.com> Date: Tue, 9 Apr 2024 10:11:50 -0400 Subject: [PATCH 29/30] update remaining instances of `dialog.show` --- examples/Dialog.main.jsx | 2 +- examples/Dialog.nondismissible.jsx | 2 +- examples/Dialog.placement.jsx | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/examples/Dialog.main.jsx b/examples/Dialog.main.jsx index 97c22900f2a..0420259aff4 100644 --- a/examples/Dialog.main.jsx +++ b/examples/Dialog.main.jsx @@ -10,7 +10,7 @@ export default () => { return ( <> - { return ( <> - diff --git a/examples/Dialog.placement.jsx b/examples/Dialog.placement.jsx index 53698c8b0d0..ee83ef860ee 100644 --- a/examples/Dialog.placement.jsx +++ b/examples/Dialog.placement.jsx @@ -10,7 +10,7 @@ export default () => { return ( <> - Date: Wed, 10 Apr 2024 14:18:06 -0400 Subject: [PATCH 30/30] fix tests --- .../src/core/Dialog/Dialog.test.tsx | 11 +---------- testing/e2e/app/routes/Dialog/route.tsx | 16 ++++++++++++++++ testing/e2e/app/routes/Dialog/spec.ts | 13 +++++++++++++ 3 files changed, 30 insertions(+), 10 deletions(-) create mode 100644 testing/e2e/app/routes/Dialog/route.tsx create mode 100644 testing/e2e/app/routes/Dialog/spec.ts diff --git a/packages/itwinui-react/src/core/Dialog/Dialog.test.tsx b/packages/itwinui-react/src/core/Dialog/Dialog.test.tsx index 1fddee07e22..40b935dfd49 100644 --- a/packages/itwinui-react/src/core/Dialog/Dialog.test.tsx +++ b/packages/itwinui-react/src/core/Dialog/Dialog.test.tsx @@ -3,7 +3,7 @@ * See LICENSE.md in the project root for license terms and full copyright notice. *--------------------------------------------------------------------------------------------*/ import * as React from 'react'; -import { render, act, screen, fireEvent } from '@testing-library/react'; +import { render, act, screen } from '@testing-library/react'; import { Dialog } from './Dialog.js'; import { Button } from '../Buttons/Button.js'; import { userEvent } from '@testing-library/user-event'; @@ -212,13 +212,4 @@ it('should expose show() and close() methods', () => { act(() => dialog.close()); act(() => vi.runAllTimers()); expect(dialogElement).not.toBeVisible(); - - act(() => dialog.show()); - - // Built-in close should still work - act(() => { - fireEvent.keyDown(dialogElement, { key: 'Escape' }); - }); - - expect(dialogElement).not.toBeVisible(); }); diff --git a/testing/e2e/app/routes/Dialog/route.tsx b/testing/e2e/app/routes/Dialog/route.tsx new file mode 100644 index 00000000000..0cecec62705 --- /dev/null +++ b/testing/e2e/app/routes/Dialog/route.tsx @@ -0,0 +1,16 @@ +import * as React from 'react'; +import { Dialog } from '@itwin/itwinui-react'; + +export default function DialogTest() { + const dialog = Dialog.useInstance(); + + React.useEffect(() => { + dialog.show(); + }, [dialog]); + + return ( + + Hello + + ); +} diff --git a/testing/e2e/app/routes/Dialog/spec.ts b/testing/e2e/app/routes/Dialog/spec.ts new file mode 100644 index 00000000000..63ec0f9dac8 --- /dev/null +++ b/testing/e2e/app/routes/Dialog/spec.ts @@ -0,0 +1,13 @@ +import { test, expect } from '@playwright/test'; + +test.describe('Dialog triggers', () => { + test('should close dialog when pressing Esc', async ({ page }) => { + await page.goto('/Dialog'); + + const dialog = page.getByRole('dialog'); + await expect(dialog).toBeVisible(); + + await page.keyboard.press('Escape'); + await expect(dialog).not.toBeVisible(); + }); +});