From 6386d8a27d351c7e3d4141c1e86dd342f983ccd2 Mon Sep 17 00:00:00 2001 From: Tasso Date: Fri, 24 Jan 2025 17:22:45 -0300 Subject: [PATCH 1/3] Change dismissal handling of `GenericModal` --- .../components/GenericModal/GenericModal.tsx | 20 +++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/apps/meteor/client/components/GenericModal/GenericModal.tsx b/apps/meteor/client/components/GenericModal/GenericModal.tsx index 9a73298ffc343..c3705ffafa69d 100644 --- a/apps/meteor/client/components/GenericModal/GenericModal.tsx +++ b/apps/meteor/client/components/GenericModal/GenericModal.tsx @@ -7,6 +7,7 @@ import { useTranslation } from 'react-i18next'; import type { RequiredModalProps } from './withDoNotAskAgain'; import { withDoNotAskAgain } from './withDoNotAskAgain'; +import { modalStore } from '../../providers/ModalProvider/ModalStore'; type VariantType = 'danger' | 'warning' | 'info' | 'success'; @@ -97,13 +98,20 @@ const GenericModal = ({ onClose?.(); }); - useEffect( - () => () => { + const handleDismiss = useEffectEvent(() => { + dismissedRef.current = true; + onDismiss?.(); + }); + + useEffect(() => { + const thisModal = modalStore.current; + + return () => { + if (thisModal === modalStore.current) return; if (!dismissedRef.current) return; - onDismiss?.(); - }, - [onDismiss], - ); + handleDismiss(); + }; + }, [handleDismiss]); return ( From 9e349574993c999e03ca2e001fa08780f449be7f Mon Sep 17 00:00:00 2001 From: Tasso Date: Fri, 24 Jan 2025 17:24:19 -0300 Subject: [PATCH 2/3] Rearrange `Suspense` elements --- apps/meteor/client/lib/imperativeModal.tsx | 14 +++++--------- apps/meteor/client/views/modal/ModalRegion.tsx | 16 +++++++++------- 2 files changed, 14 insertions(+), 16 deletions(-) diff --git a/apps/meteor/client/lib/imperativeModal.tsx b/apps/meteor/client/lib/imperativeModal.tsx index 27551edbe8d1b..c8f108ed2bb33 100644 --- a/apps/meteor/client/lib/imperativeModal.tsx +++ b/apps/meteor/client/lib/imperativeModal.tsx @@ -1,5 +1,5 @@ import { Emitter } from '@rocket.chat/emitter'; -import { Suspense, createElement } from 'react'; +import { createElement } from 'react'; import type { ComponentProps, ComponentType, ReactNode } from 'react'; import { modalStore } from '../providers/ModalProvider/ModalStore'; @@ -22,14 +22,10 @@ const mapCurrentModal = (descriptor: ModalDescriptor): ReactNode => { } if ('component' in descriptor) { - return ( - }> - {createElement(descriptor.component, { - key: Math.random(), - ...descriptor.props, - })} - - ); + return createElement(descriptor.component, { + key: Math.random(), + ...descriptor.props, + }); } }; diff --git a/apps/meteor/client/views/modal/ModalRegion.tsx b/apps/meteor/client/views/modal/ModalRegion.tsx index a672e61c107f3..66523d89184b2 100644 --- a/apps/meteor/client/views/modal/ModalRegion.tsx +++ b/apps/meteor/client/views/modal/ModalRegion.tsx @@ -1,12 +1,12 @@ import { useEffectEvent } from '@rocket.chat/fuselage-hooks'; import { useCurrentModal, useModal } from '@rocket.chat/ui-contexts'; import type { ReactElement } from 'react'; -import { lazy } from 'react'; +import { lazy, Suspense } from 'react'; import ModalBackdrop from '../../components/ModalBackdrop'; import ModalPortal from '../../portals/ModalPortal'; -const FocusScope = lazy(() => import('react-aria').then((module) => ({ default: module.FocusScope }))); +const FocusScope = lazy(() => import('react-aria').then(({ FocusScope }) => ({ default: FocusScope }))); const ModalRegion = (): ReactElement | null => { const currentModal = useCurrentModal(); @@ -21,11 +21,13 @@ const ModalRegion = (): ReactElement | null => { return ( - - - {currentModal} - - + + + + }>{currentModal} + + + ); }; From fef266f1549de58d10ac3f9abce0da7385a4f72f Mon Sep 17 00:00:00 2001 From: Tasso Date: Tue, 21 Jan 2025 01:37:08 -0300 Subject: [PATCH 3/3] Fix unit test --- .../cards/AppsUsageCard/AppsUsageCard.spec.tsx | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/apps/meteor/client/views/admin/subscription/components/cards/AppsUsageCard/AppsUsageCard.spec.tsx b/apps/meteor/client/views/admin/subscription/components/cards/AppsUsageCard/AppsUsageCard.spec.tsx index 135b78677b8b1..65295bbd02b3e 100644 --- a/apps/meteor/client/views/admin/subscription/components/cards/AppsUsageCard/AppsUsageCard.spec.tsx +++ b/apps/meteor/client/views/admin/subscription/components/cards/AppsUsageCard/AppsUsageCard.spec.tsx @@ -37,6 +37,9 @@ it('should render data as progress bars', async () => { await userEvent.click(screen.getByRole('button', { name: 'Click_here_for_more_info' })); expect(screen.getByRole('link', { name: 'premium plans' })).toHaveAttribute('href', PRICING_LINK); + + // TODO: discover how to automatically unmount all modals after each test + await userEvent.click(screen.getByRole('button', { name: 'Close' })); }); it('should render an upgrade button if marketplace apps reached 80% of the limit', async () => { @@ -53,6 +56,9 @@ it('should render an upgrade button if marketplace apps reached 80% of the limit await userEvent.click(screen.getByRole('button', { name: 'Click_here_for_more_info' })); expect(screen.getByRole('link', { name: 'premium plans' })).toHaveAttribute('href', PRICING_LINK); + + // TODO: discover how to automatically unmount all modals after each test + await userEvent.click(screen.getByRole('button', { name: 'Close' })); }); it('should render a full progress bar with private apps disabled', async () => { @@ -75,4 +81,7 @@ it('should render a full progress bar with private apps disabled', async () => { await userEvent.click(screen.getByRole('button', { name: 'Click_here_for_more_info' })); expect(screen.getByRole('link', { name: 'premium plans' })).toHaveAttribute('href', PRICING_LINK); + + // TODO: discover how to automatically unmount all modals after each test + await userEvent.click(screen.getByRole('button', { name: 'Close' })); });