From 8e3353aed62e8a0fc9846210222b9b6e3359636f Mon Sep 17 00:00:00 2001 From: Grzegorz Zdunek Date: Tue, 12 Nov 2024 13:56:38 +0100 Subject: [PATCH 01/14] Convert `DialogConfirmation` to TS --- ...story.jsx => DialogConfirmation.story.tsx} | 2 -- ...n.test.jsx => DialogConfirmation.test.tsx} | 4 +-- ...onfirmation.jsx => DialogConfirmation.tsx} | 26 ++++++++++++------- .../DialogConfirmation/{index.js => index.ts} | 2 +- 4 files changed, 19 insertions(+), 15 deletions(-) rename web/packages/design/src/DialogConfirmation/{DialogConfirmation.story.jsx => DialogConfirmation.story.tsx} (97%) rename web/packages/design/src/DialogConfirmation/{DialogConfirmation.test.jsx => DialogConfirmation.test.tsx} (92%) rename web/packages/design/src/DialogConfirmation/{DialogConfirmation.jsx => DialogConfirmation.tsx} (60%) rename web/packages/design/src/DialogConfirmation/{index.js => index.ts} (94%) diff --git a/web/packages/design/src/DialogConfirmation/DialogConfirmation.story.jsx b/web/packages/design/src/DialogConfirmation/DialogConfirmation.story.tsx similarity index 97% rename from web/packages/design/src/DialogConfirmation/DialogConfirmation.story.jsx rename to web/packages/design/src/DialogConfirmation/DialogConfirmation.story.tsx index 5c8ea7fc371df..51e5e25ed7b8a 100644 --- a/web/packages/design/src/DialogConfirmation/DialogConfirmation.story.jsx +++ b/web/packages/design/src/DialogConfirmation/DialogConfirmation.story.tsx @@ -16,8 +16,6 @@ * along with this program. If not, see . */ -import React from 'react'; - import { ButtonPrimary } from '../Button'; import DialogConfirmation, { diff --git a/web/packages/design/src/DialogConfirmation/DialogConfirmation.test.jsx b/web/packages/design/src/DialogConfirmation/DialogConfirmation.test.tsx similarity index 92% rename from web/packages/design/src/DialogConfirmation/DialogConfirmation.test.jsx rename to web/packages/design/src/DialogConfirmation/DialogConfirmation.test.tsx index a1974fe1fc247..4887037fd3974 100644 --- a/web/packages/design/src/DialogConfirmation/DialogConfirmation.test.jsx +++ b/web/packages/design/src/DialogConfirmation/DialogConfirmation.test.tsx @@ -16,11 +16,9 @@ * along with this program. If not, see . */ -import React from 'react'; - import { render, fireEvent } from 'design/utils/testing'; -import DialogConfirmation from './DialogConfirmation'; +import { DialogConfirmation } from './DialogConfirmation'; test('onClose is respected', () => { const onClose = jest.fn(); diff --git a/web/packages/design/src/DialogConfirmation/DialogConfirmation.jsx b/web/packages/design/src/DialogConfirmation/DialogConfirmation.tsx similarity index 60% rename from web/packages/design/src/DialogConfirmation/DialogConfirmation.jsx rename to web/packages/design/src/DialogConfirmation/DialogConfirmation.tsx index f15f1b5c8604c..11b1758dc480a 100644 --- a/web/packages/design/src/DialogConfirmation/DialogConfirmation.jsx +++ b/web/packages/design/src/DialogConfirmation/DialogConfirmation.tsx @@ -16,22 +16,30 @@ * along with this program. If not, see . */ -import React from 'react'; +import { ReactNode } from 'react'; +import { StyleFunction } from 'styled-components'; import Dialog from 'design/Dialog'; -function DialogConfirmation(props) { - const { children, open, onClose, dialogCss } = props; +export function DialogConfirmation(props: { + open: boolean; + /** @deprecated This props has no effect, it was never passed down to `Dialog`. */ + disableEscapeKeyDown?: boolean; + children?: ReactNode; + onClose?: ( + event: KeyboardEvent | React.MouseEvent, + reason: 'escapeKeyDown' | 'backdropClick' + ) => void; + dialogCss?: StyleFunction; +}) { return ( - {children} + {props.children} ); } - -export default DialogConfirmation; diff --git a/web/packages/design/src/DialogConfirmation/index.js b/web/packages/design/src/DialogConfirmation/index.ts similarity index 94% rename from web/packages/design/src/DialogConfirmation/index.js rename to web/packages/design/src/DialogConfirmation/index.ts index 4a2b0c46d6057..62a4d56409401 100644 --- a/web/packages/design/src/DialogConfirmation/index.js +++ b/web/packages/design/src/DialogConfirmation/index.ts @@ -16,7 +16,7 @@ * along with this program. If not, see . */ -import DialogConfirmation from './DialogConfirmation'; +import { DialogConfirmation } from './DialogConfirmation'; import { DialogTitle, DialogContent, From 198a73d13fd1cdbd92238f7512feae78e086cd66 Mon Sep 17 00:00:00 2001 From: Grzegorz Zdunek Date: Tue, 12 Nov 2024 13:58:23 +0100 Subject: [PATCH 02/14] Allow `DialogConfirmation` and `Modal` to be hidden using CSS while closed --- .../DialogConfirmation/DialogConfirmation.tsx | 6 + web/packages/design/src/Modal/Modal.test.tsx | 192 ++++++++++-------- web/packages/design/src/Modal/Modal.tsx | 23 ++- 3 files changed, 129 insertions(+), 92 deletions(-) diff --git a/web/packages/design/src/DialogConfirmation/DialogConfirmation.tsx b/web/packages/design/src/DialogConfirmation/DialogConfirmation.tsx index 11b1758dc480a..0bf3e4f046f94 100644 --- a/web/packages/design/src/DialogConfirmation/DialogConfirmation.tsx +++ b/web/packages/design/src/DialogConfirmation/DialogConfirmation.tsx @@ -23,6 +23,11 @@ import Dialog from 'design/Dialog'; export function DialogConfirmation(props: { open: boolean; + /** + * Prevent unmounting the component and its children from the DOM when closed. + * Instead, hides it with CSS. + */ + keepMounted?: boolean; /** @deprecated This props has no effect, it was never passed down to `Dialog`. */ disableEscapeKeyDown?: boolean; children?: ReactNode; @@ -38,6 +43,7 @@ export function DialogConfirmation(props: { disableEscapeKeyDown={false} onClose={props.onClose} open={props.open} + keepMounted={props.keepMounted} > {props.children} diff --git a/web/packages/design/src/Modal/Modal.test.tsx b/web/packages/design/src/Modal/Modal.test.tsx index 0c0e33aa965db..9840a0c4099e3 100644 --- a/web/packages/design/src/Modal/Modal.test.tsx +++ b/web/packages/design/src/Modal/Modal.test.tsx @@ -36,128 +36,144 @@ const escapeKey = { key: 'Escape', }; -describe('design/Modal', () => { - it('respects open prop set to false', () => { - render( - -
Hello
-
- ); +test('respects open prop set to false', () => { + render( + +
Hello
+
+ ); - expect(screen.queryByRole('dialog')).not.toBeInTheDocument(); + expect(screen.queryByRole('dialog')).not.toBeInTheDocument(); +}); + +test('respects onBackdropClick prop', () => { + const mockFn = jest.fn(); + + renderModal({ + onBackdropClick: mockFn, }); - it('respects onBackdropClick prop', () => { - const mockFn = jest.fn(); + // handlebackdropClick + fireEvent.click(screen.getByTestId('backdrop')); + expect(mockFn).toHaveBeenCalled(); +}); - renderModal({ - onBackdropClick: mockFn, - }); +test('respects onEscapeKeyDown props', () => { + const mockFn = jest.fn(); - // handlebackdropClick - fireEvent.click(screen.getByTestId('backdrop')); - expect(mockFn).toHaveBeenCalled(); + const { container } = renderModal({ + onEscapeKeyDown: mockFn, }); - it('respects onEscapeKeyDown props', () => { - const mockFn = jest.fn(); + // handleDocumentKeyDown + fireEvent.keyDown(container, escapeKey); + expect(mockFn).toHaveBeenCalled(); +}); - const { container } = renderModal({ - onEscapeKeyDown: mockFn, - }); +test('respects onClose prop', () => { + const mockFn = jest.fn(); - // handleDocumentKeyDown - fireEvent.keyDown(container, escapeKey); - expect(mockFn).toHaveBeenCalled(); + const { container } = renderModal({ + onClose: mockFn, }); - it('respects onClose prop', () => { - const mockFn = jest.fn(); - - const { container } = renderModal({ - onClose: mockFn, - }); + // handlebackdropClick + fireEvent.click(screen.getByTestId('backdrop')); + expect(mockFn).toHaveBeenCalled(); - // handlebackdropClick - fireEvent.click(screen.getByTestId('backdrop')); - expect(mockFn).toHaveBeenCalled(); + // handleDocumentKeyDown + fireEvent.keyDown(container, escapeKey); + expect(mockFn).toHaveBeenCalled(); +}); - // handleDocumentKeyDown - fireEvent.keyDown(container, escapeKey); - expect(mockFn).toHaveBeenCalled(); +test('respects hideBackDrop prop', () => { + renderModal({ + hideBackdrop: true, }); - it('respects hideBackDrop prop', () => { - renderModal({ - hideBackdrop: true, - }); + expect(screen.queryByTestId('backdrop')).not.toBeInTheDocument(); +}); - expect(screen.queryByTestId('backdrop')).not.toBeInTheDocument(); +test('respects disableBackdropClick prop', () => { + const mockFn = jest.fn(); + renderModal({ + disableBackdropClick: true, + onClose: mockFn, }); - it('respects disableBackdropClick prop', () => { - const mockFn = jest.fn(); - renderModal({ - disableBackdropClick: true, - onClose: mockFn, - }); + // handleBackdropClick + fireEvent.click(screen.getByTestId('backdrop')); + expect(mockFn).not.toHaveBeenCalled(); +}); - // handleBackdropClick - fireEvent.click(screen.getByTestId('backdrop')); - expect(mockFn).not.toHaveBeenCalled(); +test('respects disableEscapeKeyDown prop', () => { + const mockFn = jest.fn(); + const { container } = renderModal({ + disableEscapeKeyDown: true, + onClose: mockFn, }); - it('respects disableEscapeKeyDown prop', () => { - const mockFn = jest.fn(); - const { container } = renderModal({ - disableEscapeKeyDown: true, - onClose: mockFn, - }); + // handleDocumentKeyDown + fireEvent.keyDown(container, escapeKey); + expect(mockFn).not.toHaveBeenCalled(); +}); - // handleDocumentKeyDown - fireEvent.keyDown(container, escapeKey); - expect(mockFn).not.toHaveBeenCalled(); +test('unmount cleans up event listeners and closes modal', () => { + const mockFn = jest.fn(); + const { container, unmount } = renderModal({ + onEscapeKeyDown: mockFn, }); - test('unmount cleans up event listeners and closes modal', () => { - const mockFn = jest.fn(); - const { container, unmount } = renderModal({ - onEscapeKeyDown: mockFn, - }); + unmount(); - unmount(); + expect(screen.queryByTestId('Modal')).not.toBeInTheDocument(); - expect(screen.queryByTestId('Modal')).not.toBeInTheDocument(); + fireEvent.keyDown(container, escapeKey); + expect(mockFn).not.toHaveBeenCalled(); +}); - fireEvent.keyDown(container, escapeKey); - expect(mockFn).not.toHaveBeenCalled(); +test('respects backdropProps prop invisible', () => { + renderModal({ + BackdropProps: { invisible: true }, }); - test('respects backdropProps prop invisible', () => { - renderModal({ - BackdropProps: { invisible: true }, - }); - - expect(screen.getByTestId('backdrop')).toHaveStyle({ - 'background-color': 'transparent', - }); + expect(screen.getByTestId('backdrop')).toHaveStyle({ + 'background-color': 'transparent', }); +}); - it('restores focus on close', async () => { - const user = userEvent.setup(); - render(); - const toggleModalButton = screen.getByRole('button', { name: 'Toggle' }); +test('restores focus on close', async () => { + const user = userEvent.setup(); + render(); + const toggleModalButton = screen.getByRole('button', { name: 'Toggle' }); - await user.click(toggleModalButton); - // Type in the input inside the modal to shift focus into another element. - const input = screen.getByLabelText('Input in modal'); - await user.type(input, 'a'); + await user.click(toggleModalButton); + // Type in the input inside the modal to shift focus into another element. + const input = screen.getByLabelText('Input in modal'); + await user.type(input, 'a'); - const closeModal = screen.getByRole('button', { name: 'Close modal' }); - await user.click(closeModal); + const closeModal = screen.getByRole('button', { name: 'Close modal' }); + await user.click(closeModal); - expect(toggleModalButton).toHaveFocus(); - }); + expect(toggleModalButton).toHaveFocus(); +}); + +test('closing dialog when keepMounted is set only hides it with DOM', async () => { + const { rerender } = render( + +
Hello
+
+ ); + + expect(screen.getByText('Hello')).toBeVisible(); + + rerender( + +
Hello
+
+ ); + + expect(screen.getByText('Hello')).not.toBeVisible(); }); const ToggleableModal = () => { diff --git a/web/packages/design/src/Modal/Modal.tsx b/web/packages/design/src/Modal/Modal.tsx index e93085d426e7f..cd40d15021781 100644 --- a/web/packages/design/src/Modal/Modal.tsx +++ b/web/packages/design/src/Modal/Modal.tsx @@ -25,6 +25,11 @@ export type ModalProps = { * If `true`, the modal is open. */ open: boolean; + /** + * Prevent unmounting the component and its children from the DOM when closed. + * Instead, hides it with CSS. + */ + keepMounted?: boolean; className?: string; @@ -200,15 +205,23 @@ export default class Modal extends React.Component { } render() { - const { BackdropProps, children, modalCss, hideBackdrop, open, className } = - this.props; - - if (!open) { + const { + BackdropProps, + children, + modalCss, + hideBackdrop, + open, + className, + keepMounted, + } = this.props; + + if (!open && !keepMounted) { return null; } return createPortal( ` const StyledModal = styled.div<{ modalCss?: StyleFunction; ref: React.RefObject; + hiddenInDom?: boolean; }>` position: fixed; z-index: 1200; @@ -268,5 +282,6 @@ const StyledModal = styled.div<{ bottom: 0; top: 0; left: 0; + ${props => props.hiddenInDom && `display: none;`}; ${props => props.modalCss?.(props)} `; From 804716bc19185396db31829be1733387178c258e Mon Sep 17 00:00:00 2001 From: Grzegorz Zdunek Date: Tue, 12 Nov 2024 14:02:05 +0100 Subject: [PATCH 03/14] Allow hiding all dialogs that are displayed as important --- .../teleterm/src/ui/ClusterConnect/ClusterConnect.tsx | 8 ++++++-- .../src/ui/HeadlessAuthn/HeadlessAuthentication.tsx | 2 ++ .../ui/HeadlessAuthn/HeadlessPrompt/HeadlessPrompt.tsx | 6 ++++-- .../src/ui/ModalsHost/modals/HardwareKeys/AskPin.tsx | 4 +++- .../src/ui/ModalsHost/modals/HardwareKeys/ChangePin.tsx | 4 +++- .../ui/ModalsHost/modals/HardwareKeys/OverwriteSlot.tsx | 4 +++- .../src/ui/ModalsHost/modals/HardwareKeys/Touch.tsx | 4 +++- .../ModalsHost/modals/ReAuthenticate/ReAuthenticate.tsx | 4 +++- 8 files changed, 27 insertions(+), 9 deletions(-) diff --git a/web/packages/teleterm/src/ui/ClusterConnect/ClusterConnect.tsx b/web/packages/teleterm/src/ui/ClusterConnect/ClusterConnect.tsx index 5a88fd2f71a2c..95fe60a76f2be 100644 --- a/web/packages/teleterm/src/ui/ClusterConnect/ClusterConnect.tsx +++ b/web/packages/teleterm/src/ui/ClusterConnect/ClusterConnect.tsx @@ -28,7 +28,10 @@ import { ClusterAdd } from './ClusterAdd'; import { ClusterLogin } from './ClusterLogin'; import { dialogCss } from './spacing'; -export function ClusterConnect(props: { dialog: DialogClusterConnect }) { +export function ClusterConnect(props: { + dialog: DialogClusterConnect; + hidden?: boolean; +}) { const [createdClusterUri, setCreatedClusterUri] = useState< RootClusterUri | undefined >(); @@ -49,7 +52,8 @@ export function ClusterConnect(props: { dialog: DialogClusterConnect }) { dialogCss={dialogCss} disableEscapeKeyDown={false} onClose={props.dialog.onCancel} - open={true} + open={!props.hidden} + keepMounted={true} > {!clusterUri ? (