PrivateCoffee/element-web
4
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions 3

Switch ModalManager to the React 18 createRoot API (#28336)

Browse source 
* Remove boilerplate around dispatcher and settings watchers

Signed-off-by: Michael Telatynski <7t3chguy@gmail.com>

* Iterate

Signed-off-by: Michael Telatynski <7t3chguy@gmail.com>

* Move state update listeners from constructor to componentDidMount

Signed-off-by: Michael Telatynski <7t3chguy@gmail.com>

* Iterate

Signed-off-by: Michael Telatynski <7t3chguy@gmail.com>

* Switch ModalManager to the React 18 createRoot API

Signed-off-by: Michael Telatynski <7t3chguy@gmail.com>

* Iterate

Signed-off-by: Michael Telatynski <7t3chguy@gmail.com>

* Iterate

Signed-off-by: Michael Telatynski <7t3chguy@gmail.com>

---------

Signed-off-by: Michael Telatynski <7t3chguy@gmail.com>
This commit is contained in:
Michael Telatynski 2024-11-04 11:34:00 +00:00 committed by GitHub
parent 38e5eeea00
commit 502cc91dfe
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
9 changed files with 66 additions and 60 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

61
src/Modal.tsx
View file

@ -8,9 +8,9 @@ Please see LICENSE files in the repository root for full details.
*/ */
import React, { StrictMode } from "react"; import React, { StrictMode } from "react";
import ReactDOM from "react-dom"; import { createRoot, Root } from "react-dom/client";
import classNames from "classnames"; import classNames from "classnames";
import { IDeferred, defer, sleep } from "matrix-js-sdk/src/utils"; import { IDeferred, defer } from "matrix-js-sdk/src/utils";
import { TypedEventEmitter } from "matrix-js-sdk/src/matrix"; import { TypedEventEmitter } from "matrix-js-sdk/src/matrix";
import { Glass, TooltipProvider } from "@vector-im/compound-web"; import { Glass, TooltipProvider } from "@vector-im/compound-web";
@ -69,6 +69,16 @@ type HandlerMap = {
type ModalCloseReason = "backgroundClick"; type ModalCloseReason = "backgroundClick";
function getOrCreateContainer(id: string): HTMLDivElement {
let container = document.getElementById(id) as HTMLDivElement | null;
if (!container) {
container = document.createElement("div");
container.id = id;
document.body.appendChild(container);
}
return container;
}
export class ModalManager extends TypedEventEmitter<ModalManagerEvent, HandlerMap> { export class ModalManager extends TypedEventEmitter<ModalManagerEvent, HandlerMap> {
private counter = 0; private counter = 0;
// The modal to prioritise over all others. If this is set, only show // The modal to prioritise over all others. If this is set, only show
@ -83,28 +93,22 @@ export class ModalManager extends TypedEventEmitter<ModalManagerEvent, HandlerMa
// Neither the static nor priority modal will be in this list. // Neither the static nor priority modal will be in this list.
private modals: IModal<any>[] = []; private modals: IModal<any>[] = [];
private static getOrCreateContainer(): HTMLElement { private static root?: Root;
let container = document.getElementById(DIALOG_CONTAINER_ID); private static getOrCreateRoot(): Root {
if (!ModalManager.root) {
if (!container) { const container = getOrCreateContainer(DIALOG_CONTAINER_ID);
container = document.createElement("div"); ModalManager.root = createRoot(container);
container.id = DIALOG_CONTAINER_ID;
document.body.appendChild(container);
} }
return ModalManager.root;
return container;
} }
private static getOrCreateStaticContainer(): HTMLElement { private static staticRoot?: Root;
let container = document.getElementById(STATIC_DIALOG_CONTAINER_ID); private static getOrCreateStaticRoot(): Root {
if (!ModalManager.staticRoot) {
if (!container) { const container = getOrCreateContainer(STATIC_DIALOG_CONTAINER_ID);
container = document.createElement("div"); ModalManager.staticRoot = createRoot(container);
container.id = STATIC_DIALOG_CONTAINER_ID;
document.body.appendChild(container);
} }
return ModalManager.staticRoot;
return container;
} }
public constructor() { public constructor() {
@ -389,19 +393,14 @@ export class ModalManager extends TypedEventEmitter<ModalManagerEvent, HandlerMa
} }
private async reRender(): Promise<void> { private async reRender(): Promise<void> {
// TODO: We should figure out how to remove this weird sleep. It also makes testing harder
//
// await next tick because sometimes ReactDOM can race with itself and cause the modal to wrongly stick around
await sleep(0);
if (this.modals.length === 0 && !this.priorityModal && !this.staticModal) { if (this.modals.length === 0 && !this.priorityModal && !this.staticModal) {
// If there is no modal to render, make all of Element available // If there is no modal to render, make all of Element available
// to screen reader users again // to screen reader users again
dis.dispatch({ dis.dispatch({
action: "aria_unhide_main_app", action: "aria_unhide_main_app",
}); });
ReactDOM.unmountComponentAtNode(ModalManager.getOrCreateContainer()); ModalManager.getOrCreateRoot().render(<></>);
ReactDOM.unmountComponentAtNode(ModalManager.getOrCreateStaticContainer()); ModalManager.getOrCreateStaticRoot().render(<></>);
return; return;
} }
@ -432,10 +431,10 @@ export class ModalManager extends TypedEventEmitter<ModalManagerEvent, HandlerMa
</StrictMode> </StrictMode>
); );
ReactDOM.render(staticDialog, ModalManager.getOrCreateStaticContainer()); ModalManager.getOrCreateStaticRoot().render(staticDialog);
} else { } else {
// This is safe to call repeatedly if we happen to do that // This is safe to call repeatedly if we happen to do that
ReactDOM.unmountComponentAtNode(ModalManager.getOrCreateStaticContainer()); ModalManager.getOrCreateStaticRoot().render(<></>);
} }
const modal = this.getCurrentModal(); const modal = this.getCurrentModal();
@ -461,10 +460,10 @@ export class ModalManager extends TypedEventEmitter<ModalManagerEvent, HandlerMa
</StrictMode> </StrictMode>
); );
setTimeout(() => ReactDOM.render(dialog, ModalManager.getOrCreateContainer()), 0); ModalManager.getOrCreateRoot().render(dialog);
} else { } else {
// This is safe to call repeatedly if we happen to do that // This is safe to call repeatedly if we happen to do that
ReactDOM.unmountComponentAtNode(ModalManager.getOrCreateContainer()); ModalManager.getOrCreateRoot().render(<></>);
} }
} }
} }

6
test/unit-tests/components/structures/MatrixChat-test.tsx
View file

@ -62,6 +62,7 @@ import { DRAFT_LAST_CLEANUP_KEY } from "../../../../src/DraftCleaner";
import { UIFeature } from "../../../../src/settings/UIFeature"; import { UIFeature } from "../../../../src/settings/UIFeature";
import AutoDiscoveryUtils from "../../../../src/utils/AutoDiscoveryUtils"; import AutoDiscoveryUtils from "../../../../src/utils/AutoDiscoveryUtils";
import { ValidatedServerConfig } from "../../../../src/utils/ValidatedServerConfig"; import { ValidatedServerConfig } from "../../../../src/utils/ValidatedServerConfig";
import Modal from "../../../../src/Modal.tsx";
jest.mock("matrix-js-sdk/src/oidc/authorize", () => ({ jest.mock("matrix-js-sdk/src/oidc/authorize", () => ({
completeAuthorizationCodeGrant: jest.fn(), completeAuthorizationCodeGrant: jest.fn(),
@ -1514,7 +1515,9 @@ describe("<MatrixChat />", () => {
describe("when key backup failed", () => { describe("when key backup failed", () => {
it("should show the new recovery method dialog", async () => { it("should show the new recovery method dialog", async () => {
const spy = jest.spyOn(Modal, "createDialogAsync");
jest.mock("../../../../src/async-components/views/dialogs/security/NewRecoveryMethodDialog", () => ({ jest.mock("../../../../src/async-components/views/dialogs/security/NewRecoveryMethodDialog", () => ({
__test: true,
__esModule: true, __esModule: true,
default: () => <span>mocked dialog</span>, default: () => <span>mocked dialog</span>,
})); }));
@ -1526,7 +1529,8 @@ describe("<MatrixChat />", () => {
}); });
await flushPromises(); await flushPromises();
mockClient.emit(CryptoEvent.KeyBackupFailed, "error code"); mockClient.emit(CryptoEvent.KeyBackupFailed, "error code");
await waitFor(() => expect(screen.getByText("mocked dialog")).toBeInTheDocument()); await waitFor(() => expect(spy).toHaveBeenCalledTimes(1));
expect(await spy.mock.lastCall![0]).toEqual(expect.objectContaining({ __test: true }));
}); });
}); });
}); });

36
test/unit-tests/components/structures/auth/ForgotPassword-test.tsx
View file

@ -8,7 +8,7 @@ Please see LICENSE files in the repository root for full details.
import React from "react"; import React from "react";
import { mocked } from "jest-mock"; import { mocked } from "jest-mock";
import { act, render, RenderResult, screen } from "jest-matrix-react"; import { act, render, RenderResult, screen, waitFor } from "jest-matrix-react";
import userEvent from "@testing-library/user-event"; import userEvent from "@testing-library/user-event";
import { MatrixClient, createClient } from "matrix-js-sdk/src/matrix"; import { MatrixClient, createClient } from "matrix-js-sdk/src/matrix";
@ -47,14 +47,12 @@ describe("<ForgotPassword>", () => {
}; };
const click = async (element: Element): Promise<void> => { const click = async (element: Element): Promise<void> => {
await act(async () => { await userEvent.click(element, { delay: null });
await userEvent.click(element, { delay: null });
});
}; };
const itShouldCloseTheDialogAndShowThePasswordInput = (): void => { const itShouldCloseTheDialogAndShowThePasswordInput = (): void => {
it("should close the dialog and show the password input", () => { it("should close the dialog and show the password input", async () => {
expect(screen.queryByText("Verify your email to continue")).not.toBeInTheDocument(); await waitFor(() => expect(screen.queryByText("Verify your email to continue")).not.toBeInTheDocument());
expect(screen.getByText("Reset your password")).toBeInTheDocument(); expect(screen.getByText("Reset your password")).toBeInTheDocument();
}); });
}; };
@ -314,7 +312,7 @@ describe("<ForgotPassword>", () => {
}); });
}); });
it("should send the new password and show the click validation link dialog", () => { it("should send the new password and show the click validation link dialog", async () => {
expect(client.setPassword).toHaveBeenCalledWith( expect(client.setPassword).toHaveBeenCalledWith(
{ {
type: "m.login.email.identity", type: "m.login.email.identity",
@ -326,15 +324,15 @@ describe("<ForgotPassword>", () => {
testPassword, testPassword,
false, false,
); );
expect(screen.getByText("Verify your email to continue")).toBeInTheDocument(); await expect(
screen.findByText("Verify your email to continue"),
).resolves.toBeInTheDocument();
expect(screen.getByText(testEmail)).toBeInTheDocument(); expect(screen.getByText(testEmail)).toBeInTheDocument();
}); });
describe("and dismissing the dialog by clicking the background", () => { describe("and dismissing the dialog by clicking the background", () => {
beforeEach(async () => { beforeEach(async () => {
await act(async () => { await userEvent.click(await screen.findByTestId("dialog-background"), { delay: null });
await userEvent.click(screen.getByTestId("dialog-background"), { delay: null });
});
await waitEnoughCyclesForModal({ await waitEnoughCyclesForModal({
useFakeTimers: true, useFakeTimers: true,
}); });
@ -345,7 +343,7 @@ describe("<ForgotPassword>", () => {
describe("and dismissing the dialog", () => { describe("and dismissing the dialog", () => {
beforeEach(async () => { beforeEach(async () => {
await click(screen.getByLabelText("Close dialog")); await click(await screen.findByLabelText("Close dialog"));
await waitEnoughCyclesForModal({ await waitEnoughCyclesForModal({
useFakeTimers: true, useFakeTimers: true,
}); });
@ -356,14 +354,16 @@ describe("<ForgotPassword>", () => {
describe("and clicking »Re-enter email address«", () => { describe("and clicking »Re-enter email address«", () => {
beforeEach(async () => { beforeEach(async () => {
await click(screen.getByText("Re-enter email address")); await click(await screen.findByText("Re-enter email address"));
await waitEnoughCyclesForModal({ await waitEnoughCyclesForModal({
useFakeTimers: true, useFakeTimers: true,
}); });
}); });
it("should close the dialog and go back to the email input", () => { it("should close the dialog and go back to the email input", async () => {
expect(screen.queryByText("Verify your email to continue")).not.toBeInTheDocument(); await waitFor(() =>
expect(screen.queryByText("Verify your email to continue")).not.toBeInTheDocument(),
);
expect(screen.queryByText("Enter your email to reset password")).toBeInTheDocument(); expect(screen.queryByText("Enter your email to reset password")).toBeInTheDocument();
}); });
}); });
@ -397,11 +397,11 @@ describe("<ForgotPassword>", () => {
}); });
it("should show the sign out warning dialog", async () => { it("should show the sign out warning dialog", async () => {
expect( await expect(
screen.getByText( screen.findByText(
"Signing out your devices will delete the message encryption keys stored on them, making encrypted chat history unreadable.", "Signing out your devices will delete the message encryption keys stored on them, making encrypted chat history unreadable.",
), ),
).toBeInTheDocument(); ).resolves.toBeInTheDocument();
// confirm dialog // confirm dialog
await click(screen.getByText("Continue")); await click(screen.getByText("Continue"));

6
test/unit-tests/components/views/dialogs/ConfirmRedactDialog-test.tsx
View file

@ -8,7 +8,7 @@ Please see LICENSE files in the repository root for full details.
import { Feature, ServerSupport } from "matrix-js-sdk/src/feature"; import { Feature, ServerSupport } from "matrix-js-sdk/src/feature";
import { MatrixClient, MatrixEvent, RelationType } from "matrix-js-sdk/src/matrix"; import { MatrixClient, MatrixEvent, RelationType } from "matrix-js-sdk/src/matrix";
import { screen } from "jest-matrix-react"; import { screen, act } from "jest-matrix-react";
import userEvent from "@testing-library/user-event"; import userEvent from "@testing-library/user-event";
import { flushPromises, mkEvent, stubClient } from "../../../../test-utils"; import { flushPromises, mkEvent, stubClient } from "../../../../test-utils";
@ -31,12 +31,12 @@ describe("ConfirmRedactDialog", () => {
}; };
const confirmDeleteVoiceBroadcastStartedEvent = async () => { const confirmDeleteVoiceBroadcastStartedEvent = async () => {
createRedactEventDialog({ mxEvent }); act(() => createRedactEventDialog({ mxEvent }));
// double-flush promises required for the dialog to show up // double-flush promises required for the dialog to show up
await flushPromises(); await flushPromises();
await flushPromises(); await flushPromises();
await userEvent.click(screen.getByTestId("dialog-primary-button")); await userEvent.click(await screen.findByTestId("dialog-primary-button"));
}; };
beforeEach(() => { beforeEach(() => {

6
test/unit-tests/components/views/messages/DateSeparator-test.tsx
View file

@ -8,7 +8,7 @@ Please see LICENSE files in the repository root for full details.
import React from "react"; import React from "react";
import { mocked } from "jest-mock"; import { mocked } from "jest-mock";
import { fireEvent, render, screen } from "jest-matrix-react"; import { fireEvent, render, screen, waitFor } from "jest-matrix-react";
import { TimestampToEventResponse, ConnectionError, HTTPError, MatrixError } from "matrix-js-sdk/src/matrix"; import { TimestampToEventResponse, ConnectionError, HTTPError, MatrixError } from "matrix-js-sdk/src/matrix";
import dispatcher from "../../../../../src/dispatcher/dispatcher"; import dispatcher from "../../../../../src/dispatcher/dispatcher";
@ -291,7 +291,9 @@ describe("DateSeparator", () => {
// The submit debug logs option should *NOT* be shown for network errors. // The submit debug logs option should *NOT* be shown for network errors.
// //
// We have to use `queryBy` so that it can return `null` for something that does not exist. // We have to use `queryBy` so that it can return `null` for something that does not exist.
expect(screen.queryByTestId("jump-to-date-error-submit-debug-logs-button")).not.toBeInTheDocument(); await waitFor(() =>
expect(screen.queryByTestId("jump-to-date-error-submit-debug-logs-button")).not.toBeInTheDocument(),
);
}); });
}); });
}); });

2
test/unit-tests/components/views/rooms/wysiwyg_composer/utils/message-test.ts
View file

@ -417,7 +417,7 @@ describe("message", () => {
expect(mockClient.sendMessage).toHaveBeenCalledTimes(0); expect(mockClient.sendMessage).toHaveBeenCalledTimes(0);
expect(mockClient.cancelPendingEvent).toHaveBeenCalledTimes(1); expect(mockClient.cancelPendingEvent).toHaveBeenCalledTimes(1);
expect(mockCreateRedactEventDialog).toHaveBeenCalledTimes(1); expect(mockCreateRedactEventDialog).toHaveBeenCalledTimes(1);
expect(spyDispatcher).toHaveBeenCalledTimes(0); expect(spyDispatcher).toHaveBeenCalledTimes(1);
}); });
it("Should do nothing if the content is unmodified", async () => { it("Should do nothing if the content is unmodified", async () => {

4
test/unit-tests/components/views/settings/JoinRuleSettings-test.tsx
View file

@ -202,7 +202,7 @@ describe("<JoinRuleSettings />", () => {
await flushPromises(); await flushPromises();
expect(within(dialog).getByText("Loading new room")).toBeInTheDocument(); await expect(within(dialog).findByText("Loading new room")).resolves.toBeInTheDocument();
// "create" our new room, have it come thru sync // "create" our new room, have it come thru sync
client.getRoom.mockImplementation((id) => { client.getRoom.mockImplementation((id) => {
@ -250,7 +250,7 @@ describe("<JoinRuleSettings />", () => {
await flushPromises(); await flushPromises();
expect(within(dialog).getByText("Loading new room")).toBeInTheDocument(); await expect(within(dialog).findByText("Loading new room")).resolves.toBeInTheDocument();
// "create" our new room, have it come thru sync // "create" our new room, have it come thru sync
client.getRoom.mockImplementation((id) => { client.getRoom.mockImplementation((id) => {

1
test/unit-tests/components/views/settings/tabs/user/SessionManagerTab-test.tsx
View file

@ -119,6 +119,7 @@ describe("<SessionManagerTab />", () => {
const mockVerificationRequest = { const mockVerificationRequest = {
cancel: jest.fn(), cancel: jest.fn(),
on: jest.fn(), on: jest.fn(),
off: jest.fn(),
} as unknown as VerificationRequest; } as unknown as VerificationRequest;
const mockCrypto = mocked({ const mockCrypto = mocked({

4
test/unit-tests/utils/media/requestMediaPermissions-test.tsx
View file

@ -19,9 +19,9 @@ describe("requestMediaPermissions", () => {
const audioStream = {} as MediaStream; const audioStream = {} as MediaStream;
const itShouldLogTheErrorAndShowTheNoMediaPermissionsModal = () => { const itShouldLogTheErrorAndShowTheNoMediaPermissionsModal = () => {
it("should log the error and show the »No media permissions« modal", () => { it("should log the error and show the »No media permissions« modal", async () => {
expect(logger.log).toHaveBeenCalledWith("Failed to list userMedia devices", error); expect(logger.log).toHaveBeenCalledWith("Failed to list userMedia devices", error);
screen.getByText("No media permissions"); await screen.findByText("No media permissions");
}); });
}; };