From 3913bd2b15771615e63c6e777adf4aeaa34df377 Mon Sep 17 00:00:00 2001 From: Tasso Evangelista Date: Fri, 24 Jan 2025 22:03:59 -0300 Subject: [PATCH 1/2] chore: `GenericModal` dismissing strategy (#35026) --- .../components/GenericModal/GenericModal.tsx | 20 +++++++++++++------ apps/meteor/client/lib/imperativeModal.tsx | 14 +++++-------- .../AppsUsageCard/AppsUsageCard.spec.tsx | 9 +++++++++ .../meteor/client/views/modal/ModalRegion.tsx | 16 ++++++++------- 4 files changed, 37 insertions(+), 22 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 ( 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/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' })); }); 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 003c92dee8afc43e17cc014d352f798ed8b53d04 Mon Sep 17 00:00:00 2001 From: Tasso Evangelista Date: Fri, 24 Jan 2025 22:04:43 -0300 Subject: [PATCH 2/2] chore: Discard stale effect on `RoomProvider` (#35034) --- .../client/views/room/providers/RoomProvider.tsx | 9 --------- .../views/room/providers/hooks/useRoomQuery.ts | 15 ++++++++++++--- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/apps/meteor/client/views/room/providers/RoomProvider.tsx b/apps/meteor/client/views/room/providers/RoomProvider.tsx index 94f37dfafe2ef..e4c380475c894 100644 --- a/apps/meteor/client/views/room/providers/RoomProvider.tsx +++ b/apps/meteor/client/views/room/providers/RoomProvider.tsx @@ -1,5 +1,4 @@ import type { IRoom } from '@rocket.chat/core-typings'; -import { useRouter } from '@rocket.chat/ui-contexts'; import type { ReactNode, ContextType, ReactElement } from 'react'; import { useMemo, memo, useEffect, useCallback } from 'react'; @@ -38,14 +37,6 @@ const RoomProvider = ({ rid, children }: RoomProviderProps): ReactElement => { const resultFromLocal = useRoomQuery(rid); - // TODO: the following effect is a workaround while we don't have a general and definitive solution for it - const router = useRouter(); - useEffect(() => { - if (resultFromLocal.isSuccess && !resultFromLocal.data) { - router.navigate('/home'); - } - }, [resultFromLocal.data, resultFromLocal.isSuccess, resultFromServer, router]); - const subscriptionQuery = useReactiveQuery(subscriptionsQueryKeys.subscription(rid), () => Subscriptions.findOne({ rid }) ?? null); useRedirectOnSettingsChanged(subscriptionQuery.data); diff --git a/apps/meteor/client/views/room/providers/hooks/useRoomQuery.ts b/apps/meteor/client/views/room/providers/hooks/useRoomQuery.ts index 63e60e43e8ac4..65f07f8b85885 100644 --- a/apps/meteor/client/views/room/providers/hooks/useRoomQuery.ts +++ b/apps/meteor/client/views/room/providers/hooks/useRoomQuery.ts @@ -4,17 +4,26 @@ import { useQuery } from '@tanstack/react-query'; import { useEffect } from 'react'; import { Rooms } from '../../../../../app/models/client'; +import { RoomNotFoundError } from '../../../../lib/errors/RoomNotFoundError'; import { queueMicrotask } from '../../../../lib/utils/queueMicrotask'; export function useRoomQuery( rid: IRoom['_id'], - options?: UseQueryOptions, -): UseQueryResult { + options?: UseQueryOptions, +): UseQueryResult { const queryKey = ['rooms', rid] as const; const queryResult = useQuery({ queryKey, - queryFn: async (): Promise => Rooms.findOne({ _id: rid }, { reactive: false }) ?? null, + queryFn: async () => { + const room = Rooms.findOne({ _id: rid }, { reactive: false }); + + if (!room) { + throw new RoomNotFoundError(undefined, { rid }); + } + + return room; + }, staleTime: Infinity, ...options, });