Modal: remove support for onFinished callback (#29852)

* Fix up type for `finished` result of Modal

The `finished` promise can be called with an empty array, for example if the
dialog is closed by a background click. This was not correctly represented in
the typing. Fix that, and add some documentation while we're at it.

* Type fixes to onFinished callbacks from Modal

These can all be called with zero arguments, despite what the type annotations
may say, so mark them accordingly.

* Remove uses of Modal `onFinished` property

... because it is confusing.

Instead, use the `finished` promise returned by `createDialog`.

* Modal: remove support for now-unused `onFinished` prop

* StopGapWidgetDriver: use `await` instead of promise chaining

* Fix up unit tests
This commit is contained in:
Richard van der Hoff
2025-04-30 16:56:21 +01:00
committed by GitHub
parent ce1055f5fe
commit f25fbdebc7
41 changed files with 345 additions and 315 deletions

View File

@@ -9,6 +9,7 @@ Please see LICENSE files in the repository root for full details.
import Modal from "../../src/Modal";
import QuestionDialog from "../../src/components/views/dialogs/QuestionDialog";
import defaultDispatcher from "../../src/dispatcher/dispatcher";
import { flushPromises } from "../test-utils";
describe("Modal", () => {
test("forceCloseAllModals should close all open modals", () => {
@@ -23,7 +24,7 @@ describe("Modal", () => {
expect(Modal.hasDialogs()).toBe(false);
});
test("open modals should be closed on logout", () => {
test("open modals should be closed on logout", async () => {
const modal1OnFinished = jest.fn();
const modal2OnFinished = jest.fn();
@@ -31,18 +32,18 @@ describe("Modal", () => {
title: "Test dialog 1",
description: "This is a test dialog",
button: "Word",
onFinished: modal1OnFinished,
});
}).finished.then(modal1OnFinished);
Modal.createDialog(QuestionDialog, {
title: "Test dialog 2",
description: "This is a test dialog",
button: "Word",
onFinished: modal2OnFinished,
});
}).finished.then(modal2OnFinished);
defaultDispatcher.dispatch({ action: "logout" }, true);
await flushPromises();
expect(modal1OnFinished).toHaveBeenCalled();
expect(modal2OnFinished).toHaveBeenCalled();
});

View File

@@ -35,7 +35,7 @@ describe("deleteDevices()", () => {
await deleteDevicesWithInteractiveAuth(mockClient, deviceIds, onFinished);
expect(mockClient.deleteMultipleDevices).toHaveBeenCalledWith(deviceIds, undefined);
expect(onFinished).toHaveBeenCalledWith(true, undefined);
expect(onFinished).toHaveBeenCalledWith(true);
// didnt open modal
expect(modalSpy).not.toHaveBeenCalled();

View File

@@ -9,6 +9,7 @@ Please see LICENSE files in the repository root for full details.
import { fireEvent, render, screen, within } from "jest-matrix-react";
import React from "react";
import { type MatrixClient, ThreepidMedium } from "matrix-js-sdk/src/matrix";
import { defer } from "matrix-js-sdk/src/utils";
import { logger } from "matrix-js-sdk/src/logger";
import userEvent from "@testing-library/user-event";
import { type MockedObject } from "jest-mock";
@@ -153,7 +154,8 @@ describe("<AccountUserSettingsTab />", () => {
(settingName) => settingName === UIFeature.Deactivate,
);
const createDialogFn = jest.fn();
const finishedDeferred = defer<[boolean]>();
const createDialogFn = jest.fn().mockReturnValue({ finished: finishedDeferred.promise });
jest.spyOn(Modal, "createDialog").mockImplementation(createDialogFn);
render(getComponent());
@@ -167,14 +169,16 @@ describe("<AccountUserSettingsTab />", () => {
(settingName) => settingName === UIFeature.Deactivate,
);
const createDialogFn = jest.fn();
const finishedDeferred = defer<[boolean]>();
const createDialogFn = jest.fn().mockReturnValue({ finished: finishedDeferred.promise });
jest.spyOn(Modal, "createDialog").mockImplementation(createDialogFn);
render(getComponent());
await userEvent.click(screen.getByRole("button", { name: "Deactivate Account" }));
createDialogFn.mock.calls[0][1].onFinished(true);
finishedDeferred.resolve([true]);
await flushPromises();
expect(defaultProps.closeSettingsFn).toHaveBeenCalled();
});
@@ -183,14 +187,16 @@ describe("<AccountUserSettingsTab />", () => {
(settingName) => settingName === UIFeature.Deactivate,
);
const createDialogFn = jest.fn();
const finishedDeferred = defer<[boolean]>();
const createDialogFn = jest.fn().mockReturnValue({ finished: finishedDeferred.promise });
jest.spyOn(Modal, "createDialog").mockImplementation(createDialogFn);
render(getComponent());
await userEvent.click(screen.getByRole("button", { name: "Deactivate Account" }));
createDialogFn.mock.calls[0][1].onFinished(false);
finishedDeferred.resolve([false]);
await flushPromises();
expect(defaultProps.closeSettingsFn).not.toHaveBeenCalled();
});

View File

@@ -62,7 +62,7 @@ describe("<EncryptionUserSettingsTab />", () => {
);
expect(asFragment()).toMatchSnapshot();
const spy = jest.spyOn(Modal, "createDialog").mockReturnValue({} as any);
const spy = jest.spyOn(Modal, "createDialog").mockReturnValue({ finished: new Promise(() => {}) } as any);
await user.click(screen.getByText("Verify this device"));
expect(spy).toHaveBeenCalled();
});

View File

@@ -631,9 +631,10 @@ describe("<SessionManagerTab />", () => {
// click verify button from current session section
fireEvent.click(getByTestId(`verification-status-button-${alicesMobileDevice.device_id}`));
const { onFinished: modalOnFinished } = modalSpy.mock.calls[0][1] as any;
// simulate modal completing process
await modalOnFinished();
// close the modal
const { close: closeModal } = modalSpy.mock.results[0].value;
closeModal();
await flushPromises();
// cancelled in case it was a failure exit from modal
expect(mockVerificationRequest.cancel).toHaveBeenCalled();