chore: GenericModal dismissing strategy (#35026)

RocketChat · Jan 25, 2025 · 3913bd2 · 3913bd2
1 parent 492eba9
commit 3913bd2
Show file tree

Hide file tree

Showing 4 changed files with 37 additions and 22 deletions.
diff --git 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 (
 		<Modal aria-labelledby={`${genericModalId}-title`} wrapperFunction={wrapperFunction} {...props}>

diff --git 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 (
-			<Suspense fallback={<div />}>
-				{createElement(descriptor.component, {
-					key: Math.random(),
-					...descriptor.props,
-				})}
-			</Suspense>
-		);
+		return createElement(descriptor.component, {
+			key: Math.random(),
+			...descriptor.props,
+		});
 	}
 };
 

diff --git a/...eor/client/views/admin/subscription/components/cards/AppsUsageCard/AppsUsageCard.spec.tsx b/...eor/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' }));
 });
diff --git 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 (
 		<ModalPortal>
-			<ModalBackdrop onDismiss={handleDismiss}>
-				<FocusScope contain restoreFocus autoFocus>
-					{currentModal}
-				</FocusScope>
-			</ModalBackdrop>
+			<Suspense fallback={null}>
+				<ModalBackdrop onDismiss={handleDismiss}>
+					<FocusScope contain restoreFocus autoFocus>
+						<Suspense fallback={<div />}>{currentModal}</Suspense>
+					</FocusScope>
+				</ModalBackdrop>
+			</Suspense>
 		</ModalPortal>
 	);
 };