Fix closing all modals

src/Modal.tsx

@@ -47,8 +47,8 @@ export interface IModal<C extends ComponentType> {
     elem: React.ReactNode;
     className?: string;
     beforeClosePromise?: Promise<boolean>;
-    closeReason?: string;
-    onBeforeClose?(reason?: string): Promise<boolean>;
+    closeReason?: ModalCloseReason;
+    onBeforeClose?(reason?: ModalCloseReason): Promise<boolean>;
     onFinished: ComponentProps<C>["onFinished"];
     close(...args: Parameters<ComponentProps<C>["onFinished"]>): void;
     hidden?: boolean;
@@ -73,6 +73,8 @@ type HandlerMap = {
     [ModalManagerEvent.Closed]: () => void;
 };
 
+type ModalCloseReason = "backgroundClick";
+
 export class ModalManager extends TypedEventEmitter<ModalManagerEvent, HandlerMap> {
     private counter = 0;
     // The modal to prioritise over all others. If this is set, only show
@@ -147,18 +149,12 @@ export class ModalManager extends TypedEventEmitter<ModalManagerEvent, HandlerMa
         return this.appendDialogAsync<C>(Promise.resolve(Element), props, className);
     }
 
-    /**
-     * @param reason either "backgroundClick" or undefined
-     * @return whether a modal was closed
-     */
-    public closeCurrentModal(reason?: string): boolean {
-        const modal = this.getCurrentModal();
-        if (!modal) {
-            return false;
+    public closeAllModals(reason?: ModalCloseReason): void {
+        let modal: IModal<any>;
+        while ((modal = this.getCurrentModal())) {
+            modal.closeReason = reason;
+            modal.close();
         }
-        modal.closeReason = reason;
-        modal.close();
-        return true;
     }
 
     private buildModal<C extends ComponentType>(

src/components/structures/LoggedInView.tsx

@@ -491,7 +491,7 @@ class LoggedInView extends React.Component<IProps, IState> {
                 dis.dispatch({
                     action: Action.ViewHomePage,
                 });
-                Modal.closeCurrentModal("homeKeyboardShortcut");
+                Modal.closeAllModals();
                 handled = true;
                 break;
             case KeyBindingAction.ToggleSpacePanel:

src/components/structures/MatrixChat.tsx

@@ -1544,7 +1544,7 @@ export default class MatrixChat extends React.PureComponent<IProps, IState> {
             if (Lifecycle.isLoggingOut()) return;
 
             // A modal might have been open when we were logged out by the server
-            Modal.closeCurrentModal("Session.logged_out");
+            Modal.closeAllModals();
 
             if (errObj.httpStatus === 401 && errObj.data && errObj.data["soft_logout"]) {
                 logger.warn("Soft logout issued by server - avoiding data deletion");

test/components/views/context_menus/RoomGeneralContextMenu-test.tsx

@@ -36,7 +36,7 @@ import { mkMessage, stubClient } from "../../../test-utils/test-utils";
 import { shouldShowComponent } from "../../../../src/customisations/helpers/UIComponents";
 import { UIComponent } from "../../../../src/settings/UIFeature";
 import SettingsStore from "../../../../src/settings/SettingsStore";
-import Modal from "../../../../src/Modal";
+import { clearAllModals } from "../../../test-utils";
 
 jest.mock("../../../../src/customisations/helpers/UIComponents", () => ({
     shouldShowComponent: jest.fn(),
@@ -90,8 +90,8 @@ describe("RoomGeneralContextMenu", () => {
         onFinished = jest.fn();
     });
 
-    afterEach(() => {
-        Modal.closeCurrentModal("force");
+    afterEach(async () => {
+        await clearAllModals();
     });
 
     it("renders an empty context menu for archived rooms", async () => {

test/components/views/dialogs/InviteDialog-test.tsx

@@ -25,6 +25,7 @@ import { mocked, Mocked } from "jest-mock";
 import InviteDialog from "../../../../src/components/views/dialogs/InviteDialog";
 import { InviteKind } from "../../../../src/components/views/dialogs/InviteDialogTypes";
 import {
+    clearAllModals,
     filterConsole,
     flushPromises,
     getMockClientWithEventEmitter,
@@ -40,7 +41,6 @@ import { SdkContextClass } from "../../../../src/contexts/SDKContext";
 import { IProfileInfo } from "../../../../src/hooks/useProfileInfo";
 import { DirectoryMember, startDmOnFirstMessage } from "../../../../src/utils/direct-messages";
 import SettingsStore from "../../../../src/settings/SettingsStore";
-import Modal from "../../../../src/Modal";
 
 const mockGetAccessToken = jest.fn().mockResolvedValue("getAccessToken");
 jest.mock("../../../../src/IdentityAuthClient", () =>
@@ -178,8 +178,8 @@ describe("InviteDialog", () => {
         SdkContextClass.instance.client = mockClient;
     });
 
-    afterEach(() => {
-        Modal.closeCurrentModal();
+    afterEach(async () => {
+        await clearAllModals();
         SdkContextClass.instance.onLoggedOut();
         SdkContextClass.instance.client = undefined;
     });

test/test-utils/utilities.ts

@@ -202,27 +202,24 @@ export const waitEnoughCyclesForModal = async ({
  */
 export const clearAllModals = async (): Promise<void> => {
     // Prevent modals from leaking and polluting other tests
-    let keepClosingModals = true;
-    while (keepClosingModals) {
-        keepClosingModals = Modal.closeCurrentModal("End of test (clean-up)");
+    Modal.closeAllModals();
 
-        // Then wait for the screen to update (probably React rerender and async/await).
-        // Important for tests using Jest fake timers to not get into an infinite loop
-        // of removing the same modal because the promises don't flush otherwise.
-        //
-        // XXX: Maybe in the future with Jest 29.5.0+, we could use `runAllTimersAsync` instead.
+    // Then wait for the screen to update (probably React rerender and async/await).
+    // Important for tests using Jest fake timers to not get into an infinite loop
+    // of removing the same modal because the promises don't flush otherwise.
+    //
+    // XXX: Maybe in the future with Jest 29.5.0+, we could use `runAllTimersAsync` instead.
 
-        // this is called in some places where timers are not faked
-        // which causes a lot of noise in the console
-        // to make a hack even hackier check if timers are faked using a weird trick from github
-        // then call the appropriate promise flusher
-        // https://github.com/facebook/jest/issues/10555#issuecomment-1136466942
-        const jestTimersFaked = setTimeout.name === "setTimeout";
-        if (jestTimersFaked) {
-            await flushPromisesWithFakeTimers();
-        } else {
-            await flushPromises();
-        }
+    // this is called in some places where timers are not faked
+    // which causes a lot of noise in the console
+    // to make a hack even hackier check if timers are faked using a weird trick from github
+    // then call the appropriate promise flusher
+    // https://github.com/facebook/jest/issues/10555#issuecomment-1136466942
+    const jestTimersFaked = setTimeout.name === "setTimeout";
+    if (jestTimersFaked) {
+        await flushPromisesWithFakeTimers();
+    } else {
+        await flushPromises();
     }
 };