From 5c43054bfe242b0fb6d19a04ce16981438e457a1 Mon Sep 17 00:00:00 2001 From: Dominik Henneke Date: Mon, 21 Aug 2023 14:09:17 +0200 Subject: [PATCH] Implement updated open dialog method of the Module API (#11395) * Implement updated open dialog method Signed-off-by: Dominik Henneke * Apply the review comments Signed-off-by: Dominik Henneke * Add unit tests for the module system dialog Signed-off-by: Dominik Henneke * Bump @matrix-org/react-sdk-module-api from 1.0.0 to 2.0.0 Signed-off-by: Dominik Henneke * Run prettier Signed-off-by: Dominik Henneke * Apply review comments Signed-off-by: Dominik Henneke --------- Signed-off-by: Dominik Henneke --- package.json | 2 +- .../views/dialogs/ModuleUiDialog.tsx | 38 ++- .../views/dialogs/ScrollableBaseModal.tsx | 3 +- src/modules/ProxiedModuleApi.ts | 15 +- test/modules/ProxiedModuleApi-test.ts | 105 ------- test/modules/ProxiedModuleApi-test.tsx | 257 ++++++++++++++++++ yarn.lock | 8 +- 7 files changed, 299 insertions(+), 129 deletions(-) delete mode 100644 test/modules/ProxiedModuleApi-test.ts create mode 100644 test/modules/ProxiedModuleApi-test.tsx diff --git a/package.json b/package.json index 1d5f9a7fb3..0bbd4dffdb 100644 --- a/package.json +++ b/package.json @@ -62,7 +62,7 @@ "@babel/runtime": "^7.12.5", "@matrix-org/analytics-events": "^0.6.0", "@matrix-org/matrix-wysiwyg": "^2.4.1", - "@matrix-org/react-sdk-module-api": "^1.0.0", + "@matrix-org/react-sdk-module-api": "^2.0.0", "@matrix-org/spec": "^1.7.0", "@sentry/browser": "^7.0.0", "@sentry/tracing": "^7.0.0", diff --git a/src/components/views/dialogs/ModuleUiDialog.tsx b/src/components/views/dialogs/ModuleUiDialog.tsx index acf9d1eda4..b43356666a 100644 --- a/src/components/views/dialogs/ModuleUiDialog.tsx +++ b/src/components/views/dialogs/ModuleUiDialog.tsx @@ -17,15 +17,18 @@ limitations under the License. import React, { createRef } from "react"; import { DialogContent, DialogProps } from "@matrix-org/react-sdk-module-api/lib/components/DialogContent"; import { logger } from "matrix-js-sdk/src/logger"; +import { ModuleApi } from "@matrix-org/react-sdk-module-api/lib/ModuleApi"; +import { ModuleUiDialogOptions } from "@matrix-org/react-sdk-module-api/lib/types/ModuleUiDialogOptions"; import ScrollableBaseModal, { IScrollableBaseState } from "./ScrollableBaseModal"; import { _t } from "../../../languageHandler"; interface IProps

> { contentFactory: (props: P, ref: React.RefObject) => React.ReactNode; - contentProps: P; - title: string; - onFinished(ok?: boolean, model?: Awaited["trySubmit"]>>): void; + additionalContentProps: Omit | undefined; + initialOptions: ModuleUiDialogOptions; + moduleApi: ModuleApi; + onFinished(ok?: boolean, model?: Awaited["trySubmit"]>>): void; } interface IState extends IScrollableBaseState { @@ -42,9 +45,10 @@ export class ModuleUiDialog

> e super(props); this.state = { - title: this.props.title, - canSubmit: true, - actionLabel: _t("OK"), + title: this.props.initialOptions.title, + actionLabel: this.props.initialOptions.actionLabel ?? _t("OK"), + cancelLabel: this.props.initialOptions.cancelLabel, + canSubmit: this.props.initialOptions.canSubmit ?? true, }; } @@ -61,11 +65,23 @@ export class ModuleUiDialog

> e this.props.onFinished(false); } + private setOptions(options: ModuleUiDialogOptions): void { + this.setState((state) => ({ ...state, ...options })); + } + protected renderContent(): React.ReactNode { - return ( -

- {this.props.contentFactory(this.props.contentProps, this.contentRef)} -
- ); + const dialogProps: DialogProps = { + moduleApi: this.props.moduleApi, + setOptions: this.setOptions.bind(this), + cancel: this.cancel.bind(this), + }; + + // Typescript isn't very happy understanding that `contentProps` satisfies `P` + const contentProps: P = { + ...this.props.additionalContentProps, + ...dialogProps, + } as unknown as P; + + return
{this.props.contentFactory(contentProps, this.contentRef)}
; } } diff --git a/src/components/views/dialogs/ScrollableBaseModal.tsx b/src/components/views/dialogs/ScrollableBaseModal.tsx index be06807408..5ffdfafb71 100644 --- a/src/components/views/dialogs/ScrollableBaseModal.tsx +++ b/src/components/views/dialogs/ScrollableBaseModal.tsx @@ -29,6 +29,7 @@ export interface IScrollableBaseState { canSubmit: boolean; title: string; actionLabel: string; + cancelLabel?: string; } /** @@ -103,7 +104,7 @@ export default abstract class ScrollableBaseModal<
{this.renderContent()}
- {_t("Cancel")} + {this.state.cancelLabel ?? _t("Cancel")} >( - title: string, + initialTitleOrOptions: string | ModuleUiDialogOptions, body: (props: P, ref: React.RefObject) => React.ReactNode, props?: Omit, ): Promise<{ didOkOrSubmit: boolean; model: M }> { + const initialOptions: ModuleUiDialogOptions = + typeof initialTitleOrOptions === "string" ? { title: initialTitleOrOptions } : initialTitleOrOptions; + return new Promise<{ didOkOrSubmit: boolean; model: M }>((resolve) => { Modal.createDialog( ModuleUiDialog, { - title: title, + initialOptions, contentFactory: body, - // Typescript isn't very happy understanding that `props` satisfies `Omit` - contentProps: { - ...props, - moduleApi: this, - } as unknown as P, + moduleApi: this, + additionalContentProps: props, }, "mx_CompoundDialog", ).finished.then(([didOkOrSubmit, model]) => { diff --git a/test/modules/ProxiedModuleApi-test.ts b/test/modules/ProxiedModuleApi-test.ts deleted file mode 100644 index 881e98c9df..0000000000 --- a/test/modules/ProxiedModuleApi-test.ts +++ /dev/null @@ -1,105 +0,0 @@ -/* -Copyright 2022 The Matrix.org Foundation C.I.C. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -import { TranslationStringsObject } from "@matrix-org/react-sdk-module-api/lib/types/translations"; -import { AccountAuthInfo } from "@matrix-org/react-sdk-module-api/lib/types/AccountAuthInfo"; - -import { ProxiedModuleApi } from "../../src/modules/ProxiedModuleApi"; -import { stubClient } from "../test-utils"; -import { setLanguage } from "../../src/languageHandler"; -import { ModuleRunner } from "../../src/modules/ModuleRunner"; -import { registerMockModule } from "./MockModule"; -import defaultDispatcher from "../../src/dispatcher/dispatcher"; -import { Action } from "../../src/dispatcher/actions"; - -describe("ProxiedApiModule", () => { - afterEach(() => { - ModuleRunner.instance.reset(); - }); - - // Note: Remainder is implicitly tested from end-to-end tests of modules. - - describe("translations", () => { - it("should cache translations", () => { - const api = new ProxiedModuleApi(); - expect(api.translations).toBeFalsy(); - - const translations: TranslationStringsObject = { - ["custom string"]: { - en: "custom string", - fr: "custom french string", - }, - }; - api.registerTranslations(translations); - expect(api.translations).toBe(translations); - }); - - it("should overwriteAccountAuth", async () => { - const dispatchSpy = jest.spyOn(defaultDispatcher, "dispatch"); - - const api = new ProxiedModuleApi(); - const accountInfo = {} as unknown as AccountAuthInfo; - const promise = api.overwriteAccountAuth(accountInfo); - - expect(dispatchSpy).toHaveBeenCalledWith( - expect.objectContaining({ - action: Action.OverwriteLogin, - credentials: { - ...accountInfo, - guest: false, - }, - }), - expect.anything(), - ); - - defaultDispatcher.fire(Action.OnLoggedIn); - - await expect(promise).resolves.toBeUndefined(); - }); - - describe("integration", () => { - it("should translate strings using translation system", async () => { - // Test setup - stubClient(); - - // Set up a module to pull translations through - const module = registerMockModule(); - const en = "custom string"; - const de = "custom german string"; - const enVars = "custom variable %(var)s"; - const varVal = "string"; - const deVars = "custom german variable %(var)s"; - const deFull = `custom german variable ${varVal}`; - expect(module.apiInstance).toBeInstanceOf(ProxiedModuleApi); - module.apiInstance.registerTranslations({ - [en]: { - en: en, - de: de, - }, - [enVars]: { - en: enVars, - de: deVars, - }, - }); - await setLanguage("de"); // calls `registerCustomTranslations()` for us - - // See if we can pull the German string out - expect(module.apiInstance.translateString(en)).toEqual(de); - expect(module.apiInstance.translateString(enVars, { var: varVal })).toEqual(deFull); - }); - }); - }); -}); diff --git a/test/modules/ProxiedModuleApi-test.tsx b/test/modules/ProxiedModuleApi-test.tsx new file mode 100644 index 0000000000..de796048a1 --- /dev/null +++ b/test/modules/ProxiedModuleApi-test.tsx @@ -0,0 +1,257 @@ +/* +Copyright 2022 The Matrix.org Foundation C.I.C. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +import React from "react"; +import { TranslationStringsObject } from "@matrix-org/react-sdk-module-api/lib/types/translations"; +import { AccountAuthInfo } from "@matrix-org/react-sdk-module-api/lib/types/AccountAuthInfo"; +import { DialogContent, DialogProps } from "@matrix-org/react-sdk-module-api/lib/components/DialogContent"; +import { screen, within } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; + +import { ProxiedModuleApi } from "../../src/modules/ProxiedModuleApi"; +import { stubClient } from "../test-utils"; +import { setLanguage } from "../../src/languageHandler"; +import { ModuleRunner } from "../../src/modules/ModuleRunner"; +import { registerMockModule } from "./MockModule"; +import defaultDispatcher from "../../src/dispatcher/dispatcher"; +import { Action } from "../../src/dispatcher/actions"; + +describe("ProxiedApiModule", () => { + afterEach(() => { + ModuleRunner.instance.reset(); + }); + + // Note: Remainder is implicitly tested from end-to-end tests of modules. + + describe("translations", () => { + it("should cache translations", () => { + const api = new ProxiedModuleApi(); + expect(api.translations).toBeFalsy(); + + const translations: TranslationStringsObject = { + ["custom string"]: { + en: "custom string", + fr: "custom french string", + }, + }; + api.registerTranslations(translations); + expect(api.translations).toBe(translations); + }); + + it("should overwriteAccountAuth", async () => { + const dispatchSpy = jest.spyOn(defaultDispatcher, "dispatch"); + + const api = new ProxiedModuleApi(); + const accountInfo = {} as unknown as AccountAuthInfo; + const promise = api.overwriteAccountAuth(accountInfo); + + expect(dispatchSpy).toHaveBeenCalledWith( + expect.objectContaining({ + action: Action.OverwriteLogin, + credentials: { + ...accountInfo, + guest: false, + }, + }), + expect.anything(), + ); + + defaultDispatcher.fire(Action.OnLoggedIn); + + await expect(promise).resolves.toBeUndefined(); + }); + + describe("integration", () => { + it("should translate strings using translation system", async () => { + // Test setup + stubClient(); + + // Set up a module to pull translations through + const module = registerMockModule(); + const en = "custom string"; + const de = "custom german string"; + const enVars = "custom variable %(var)s"; + const varVal = "string"; + const deVars = "custom german variable %(var)s"; + const deFull = `custom german variable ${varVal}`; + expect(module.apiInstance).toBeInstanceOf(ProxiedModuleApi); + module.apiInstance.registerTranslations({ + [en]: { + en: en, + de: de, + }, + [enVars]: { + en: enVars, + de: deVars, + }, + }); + await setLanguage("de"); // calls `registerCustomTranslations()` for us + + // See if we can pull the German string out + expect(module.apiInstance.translateString(en)).toEqual(de); + expect(module.apiInstance.translateString(enVars, { var: varVal })).toEqual(deFull); + }); + + afterEach(async () => { + await setLanguage("en"); // reset the language + }); + }); + }); + + describe("openDialog", () => { + it("should open dialog with a custom title and default options", async () => { + class MyDialogContent extends DialogContent { + trySubmit = async () => ({ result: true }); + render = () =>

This is my example content.

; + } + + const api = new ProxiedModuleApi(); + + const resultPromise = api.openDialog<{ result: boolean }, DialogProps, MyDialogContent>( + "My Dialog Title", + (props, ref) => , + ); + + const dialog = await screen.findByRole("dialog"); + + expect(within(dialog).getByRole("heading", { name: "My Dialog Title" })).toBeInTheDocument(); + expect(within(dialog).getByText("This is my example content.")).toBeInTheDocument(); + expect(within(dialog).getByRole("button", { name: "Cancel" })).toBeInTheDocument(); + + await userEvent.click(within(dialog).getByRole("button", { name: "OK" })); + + expect(await resultPromise).toEqual({ + didOkOrSubmit: true, + model: { result: true }, + }); + + expect(dialog).not.toBeInTheDocument(); + }); + + it("should open dialog with custom options", async () => { + class MyDialogContent extends DialogContent { + trySubmit = async () => ({ result: true }); + render = () =>

This is my example content.

; + } + + const api = new ProxiedModuleApi(); + + const resultPromise = api.openDialog<{ result: boolean }, DialogProps, MyDialogContent>( + { + title: "My Custom Dialog Title", + actionLabel: "Submit it", + cancelLabel: "Cancel it", + canSubmit: false, + }, + (props, ref) => , + ); + + const dialog = await screen.findByRole("dialog"); + + expect(within(dialog).getByRole("heading", { name: "My Custom Dialog Title" })).toBeInTheDocument(); + expect(within(dialog).getByText("This is my example content.")).toBeInTheDocument(); + expect(within(dialog).getByRole("button", { name: "Submit it" })).toBeDisabled(); + + await userEvent.click(within(dialog).getByRole("button", { name: "Cancel it" })); + + expect(await resultPromise).toEqual({ didOkOrSubmit: false }); + + expect(dialog).not.toBeInTheDocument(); + }); + + it("should update the options from the opened dialog", async () => { + class MyDialogContent extends DialogContent { + trySubmit = async () => ({ result: true }); + render = () => { + const onClick = () => { + this.props.setOptions({ + title: "My New Title", + actionLabel: "New Action", + cancelLabel: "New Cancel", + }); + + // check if delta updates work + this.props.setOptions({ + canSubmit: false, + }); + }; + + return ( + + ); + }; + } + + const api = new ProxiedModuleApi(); + + const resultPromise = api.openDialog<{ result: boolean }, DialogProps, MyDialogContent>( + "My Dialog Title", + (props, ref) => , + ); + + const dialog = await screen.findByRole("dialog"); + + expect(within(dialog).getByRole("heading", { name: "My Dialog Title" })).toBeInTheDocument(); + expect(within(dialog).getByRole("button", { name: "Cancel" })).toBeInTheDocument(); + expect(within(dialog).getByRole("button", { name: "OK" })).toBeEnabled(); + + await userEvent.click(within(dialog).getByRole("button", { name: "Change the settings!" })); + + expect(within(dialog).getByRole("heading", { name: "My New Title" })).toBeInTheDocument(); + expect(within(dialog).getByRole("button", { name: "New Action" })).toBeDisabled(); + + await userEvent.click(within(dialog).getByRole("button", { name: "New Cancel" })); + + expect(await resultPromise).toEqual({ + didOkOrSubmit: false, + model: undefined, + }); + + expect(dialog).not.toBeInTheDocument(); + }); + + it("should cancel the dialog from within the dialog", async () => { + class MyDialogContent extends DialogContent { + trySubmit = async () => ({ result: true }); + render = () => ( + + ); + } + + const api = new ProxiedModuleApi(); + + const resultPromise = api.openDialog<{ result: boolean }, DialogProps, MyDialogContent>( + "My Dialog Title", + (props, ref) => , + ); + + const dialog = await screen.findByRole("dialog"); + + await userEvent.click(within(dialog).getByRole("button", { name: "No need for action" })); + + expect(await resultPromise).toEqual({ + didOkOrSubmit: false, + model: undefined, + }); + + expect(dialog).not.toBeInTheDocument(); + }); + }); +}); diff --git a/yarn.lock b/yarn.lock index c5d5ff2215..690291c7e2 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1867,10 +1867,10 @@ version "3.2.14" resolved "https://gitlab.matrix.org/api/v4/projects/27/packages/npm/@matrix-org/olm/-/@matrix-org/olm-3.2.14.tgz#acd96c00a881d0f462e1f97a56c73742c8dbc984" -"@matrix-org/react-sdk-module-api@^1.0.0": - version "1.0.0" - resolved "https://registry.yarnpkg.com/@matrix-org/react-sdk-module-api/-/react-sdk-module-api-1.0.0.tgz#de73e163a439fe330f6971a6a0cef2ccb090d616" - integrity sha512-drhPkoPWitAv9bXS2q8cyaqPta/KGF+Ph3aZSmaYiOPyY5S84e4Ju3JI6/HExqF8+HyBsajlCKtyvTZsMsTIFA== +"@matrix-org/react-sdk-module-api@^2.0.0": + version "2.0.0" + resolved "https://registry.yarnpkg.com/@matrix-org/react-sdk-module-api/-/react-sdk-module-api-2.0.0.tgz#f894af429ad352d5151dc7240cc2f987d9dab780" + integrity sha512-o/M+IfB3bu4S3yTO10zMRiEtTQagV9AJ9cNmq8a/ksniCx3QLShtzWeL5FkTa8co0ab/VdxdqTlEux0aStT/dg== dependencies: "@babel/runtime" "^7.17.9"