From 6cd16cf1173c7e105de2d155a68995f8697ab676 Mon Sep 17 00:00:00 2001
From: Fedor Indutny <79877362+indutny-signal@users.noreply.github.com>
Date: Tue, 13 Dec 2022 17:56:32 -0800
Subject: [PATCH] Make sure that closed Modals are always removed

---
 ts/components/Modal.tsx  | 44 +++++++++++++++++++++++++++++++++++-----
 ts/hooks/useAnimated.tsx | 24 +++++++++++++++++++---
 2 files changed, 60 insertions(+), 8 deletions(-)

diff --git a/ts/components/Modal.tsx b/ts/components/Modal.tsx
index 6a119b3a66..f097108c92 100644
--- a/ts/components/Modal.tsx
+++ b/ts/components/Modal.tsx
@@ -2,7 +2,7 @@
 // SPDX-License-Identifier: AGPL-3.0-only
 
 import type { ReactElement, ReactNode } from 'react';
-import React, { useRef, useState } from 'react';
+import React, { useEffect, useRef, useState } from 'react';
 import type { ContentRect, MeasuredComponentProps } from 'react-measure';
 import Measure from 'react-measure';
 import classNames from 'classnames';
@@ -12,10 +12,12 @@ import { animated } from '@react-spring/web';
 import type { LocalizerType } from '../types/Util';
 import { ModalHost } from './ModalHost';
 import type { Theme } from '../util/theme';
+import { assertDev } from '../util/assert';
 import { getClassNamesFor } from '../util/getClassNamesFor';
 import { useAnimated } from '../hooks/useAnimated';
 import { useHasWrapped } from '../hooks/useHasWrapped';
 import { useRefMerger } from '../hooks/useRefMerger';
+import * as log from '../logging/log';
 
 type PropsType = {
   children: ReactNode;
@@ -56,8 +58,8 @@ export function Modal({
   hasHeaderDivider = false,
   hasFooterDivider = false,
   padded = true,
-}: Readonly<ModalPropsType>): ReactElement {
-  const { close, modalStyles, overlayStyles } = useAnimated(onClose, {
+}: Readonly<ModalPropsType>): JSX.Element | null {
+  const { close, isClosed, modalStyles, overlayStyles } = useAnimated(onClose, {
     getFrom: () => ({ opacity: 0, transform: 'translateY(48px)' }),
     getTo: isOpen =>
       isOpen
@@ -65,6 +67,20 @@ export function Modal({
         : { opacity: 0, transform: 'translateY(48px)' },
   });
 
+  useEffect(() => {
+    if (!isClosed) {
+      return noop;
+    }
+
+    const timer = setTimeout(() => {
+      log.error(`Modal ${modalName} is closed, but still visible`);
+      assertDev(false, `Invisible modal ${modalName}`);
+    }, 0);
+    return () => {
+      clearTimeout(timer);
+    };
+  }, [modalName, isClosed]);
+
   return (
     <ModalHost
       modalName={modalName}
@@ -286,8 +302,8 @@ export function PagedModal({
   onClose = noop,
   theme,
   useFocusTrap,
-}: PagedModalProps): ReactElement {
-  const { close, modalStyles, overlayStyles } = useAnimated(onClose, {
+}: PagedModalProps): JSX.Element | null {
+  const { close, isClosed, modalStyles, overlayStyles } = useAnimated(onClose, {
     getFrom: () => ({ opacity: 0, transform: 'translateY(48px)' }),
     getTo: isOpen =>
       isOpen
@@ -295,6 +311,24 @@ export function PagedModal({
         : { opacity: 0, transform: 'translateY(48px)' },
   });
 
+  useEffect(() => {
+    if (!isClosed) {
+      return noop;
+    }
+
+    const timer = setTimeout(() => {
+      log.error(`PagedModal ${modalName} is closed, but still visible`);
+      assertDev(false, `Invisible paged modal ${modalName}`);
+    }, 0);
+    return () => {
+      clearTimeout(timer);
+    };
+  }, [modalName, isClosed]);
+
+  if (isClosed) {
+    return null;
+  }
+
   return (
     <ModalHost
       modalName={modalName}
diff --git a/ts/hooks/useAnimated.tsx b/ts/hooks/useAnimated.tsx
index 0901da1748..40ec1d0d63 100644
--- a/ts/hooks/useAnimated.tsx
+++ b/ts/hooks/useAnimated.tsx
@@ -10,6 +10,12 @@ export type ModalConfigType = {
   transform?: string;
 };
 
+enum ModalState {
+  Open = 'Open',
+  Closing = 'Closing',
+  Closed = 'Closed',
+}
+
 export function useAnimated(
   onClose: () => unknown,
   {
@@ -21,10 +27,13 @@ export function useAnimated(
   }
 ): {
   close: () => unknown;
+  isClosed: boolean;
   modalStyles: SpringValues<ModalConfigType>;
   overlayStyles: SpringValues<ModalConfigType>;
 } {
-  const [isOpen, setIsOpen] = useState(true);
+  const [state, setState] = useState(ModalState.Open);
+  const isOpen = state === ModalState.Open;
+  const isClosed = state === ModalState.Closed;
 
   const modalRef = useSpringRef();
 
@@ -32,7 +41,8 @@ export function useAnimated(
     from: getFrom(isOpen),
     to: getTo(isOpen),
     onRest: () => {
-      if (!isOpen) {
+      if (state === ModalState.Closing) {
+        setState(ModalState.Closed);
         onClose();
       }
     },
@@ -59,10 +69,18 @@ export function useAnimated(
   });
 
   useChain(isOpen ? [overlayRef, modalRef] : [modalRef, overlayRef]);
-  const close = useCallback(() => setIsOpen(false), []);
+  const close = useCallback(() => {
+    setState(currentState => {
+      if (currentState === ModalState.Open) {
+        return ModalState.Closing;
+      }
+      return currentState;
+    });
+  }, []);
 
   return {
     close,
+    isClosed,
     overlayStyles,
     modalStyles,
   };