From ab9152044c03469cc583680d7b63a26e64dd4fc6 Mon Sep 17 00:00:00 2001 From: Michael Weimann Date: Thu, 5 Jan 2023 17:05:21 +0100 Subject: [PATCH] Use the same avatar colour when creating 1:1 DM rooms (#9850) --- src/components/views/avatars/RoomAvatar.tsx | 24 ++++-- .../structures/auth/ForgotPassword-test.tsx | 16 ++-- .../views/avatars/RoomAvatar-test.tsx | 78 +++++++++++++++++++ .../__snapshots__/RoomAvatar-test.tsx.snap | 76 ++++++++++++++++++ test/components/views/rooms/RoomTile-test.tsx | 12 ++- .../UserOnboardingPage-test.tsx | 16 +--- test/test-utils/console.ts | 45 ++++++----- .../VoiceBroadcastRecordingPip-test.tsx | 8 +- 8 files changed, 215 insertions(+), 60 deletions(-) create mode 100644 test/components/views/avatars/RoomAvatar-test.tsx create mode 100644 test/components/views/avatars/__snapshots__/RoomAvatar-test.tsx.snap diff --git a/src/components/views/avatars/RoomAvatar.tsx b/src/components/views/avatars/RoomAvatar.tsx index 383edd0792a..770ea01fce6 100644 --- a/src/components/views/avatars/RoomAvatar.tsx +++ b/src/components/views/avatars/RoomAvatar.tsx @@ -29,6 +29,7 @@ import * as Avatar from "../../../Avatar"; import DMRoomMap from "../../../utils/DMRoomMap"; import { mediaFromMxc } from "../../../customisations/Media"; import { IOOBData } from "../../../stores/ThreepidInviteStore"; +import { LocalRoom } from "../../../models/LocalRoom"; interface IProps extends Omit, "name" | "idName" | "url" | "onClick"> { // Room may be left unset here, but if it is, @@ -117,13 +118,26 @@ export default class RoomAvatar extends React.Component { Modal.createDialog(ImageView, params, "mx_Dialog_lightbox", null, true); }; + private get roomIdName(): string | undefined { + const room = this.props.room; + + if (room) { + const dmMapUserId = DMRoomMap.shared().getUserIdForRoomId(room.roomId); + // If the room is a DM, we use the other user's ID for the color hash + // in order to match the room avatar with their avatar + if (dmMapUserId) return dmMapUserId; + + if (room instanceof LocalRoom && room.targets.length === 1) { + return room.targets[0].userId; + } + } + + return this.props.room?.roomId || this.props.oobData?.roomId; + } + public render() { const { room, oobData, viewAvatarOnClick, onClick, className, ...otherProps } = this.props; - const roomName = room?.name ?? oobData.name; - // If the room is a DM, we use the other user's ID for the color hash - // in order to match the room avatar with their avatar - const idName = room ? DMRoomMap.shared().getUserIdForRoomId(room.roomId) ?? room.roomId : oobData.roomId; return ( { mx_RoomAvatar_isSpaceRoom: (room?.getType() ?? this.props.oobData?.roomType) === RoomType.Space, })} name={roomName} - idName={idName} + idName={this.roomIdName} urls={this.state.urls} onClick={viewAvatarOnClick && this.state.urls[0] ? this.onRoomAvatarClick : onClick} /> diff --git a/test/components/structures/auth/ForgotPassword-test.tsx b/test/components/structures/auth/ForgotPassword-test.tsx index 2c348f7efd6..57eccec0148 100644 --- a/test/components/structures/auth/ForgotPassword-test.tsx +++ b/test/components/structures/auth/ForgotPassword-test.tsx @@ -40,7 +40,6 @@ describe("", () => { let onComplete: () => void; let onLoginClick: () => void; let renderResult: RenderResult; - let restoreConsole: () => void; const typeIntoField = async (label: string, value: string): Promise => { await act(async () => { @@ -63,14 +62,14 @@ describe("", () => { }); }; - beforeEach(() => { - restoreConsole = filterConsole( - // not implemented by js-dom https://github.com/jsdom/jsdom/issues/1937 - "Not implemented: HTMLFormElement.prototype.requestSubmit", - // not of interested for this test - "Starting load of AsyncWrapper for modal", - ); + filterConsole( + // not implemented by js-dom https://github.com/jsdom/jsdom/issues/1937 + "Not implemented: HTMLFormElement.prototype.requestSubmit", + // not of interested for this test + "Starting load of AsyncWrapper for modal", + ); + beforeEach(() => { client = stubClient(); mocked(createClient).mockReturnValue(client); @@ -87,7 +86,6 @@ describe("", () => { afterEach(() => { // clean up modals Modal.closeCurrentModal("force"); - restoreConsole?.(); }); beforeAll(() => { diff --git a/test/components/views/avatars/RoomAvatar-test.tsx b/test/components/views/avatars/RoomAvatar-test.tsx new file mode 100644 index 00000000000..e23cd96f02d --- /dev/null +++ b/test/components/views/avatars/RoomAvatar-test.tsx @@ -0,0 +1,78 @@ +/* +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 { render } from "@testing-library/react"; +import { MatrixClient, Room } from "matrix-js-sdk/src/matrix"; +import { mocked } from "jest-mock"; + +import RoomAvatar from "../../../../src/components/views/avatars/RoomAvatar"; +import { filterConsole, stubClient } from "../../../test-utils"; +import DMRoomMap from "../../../../src/utils/DMRoomMap"; +import { LocalRoom } from "../../../../src/models/LocalRoom"; +import * as AvatarModule from "../../../../src/Avatar"; +import { DirectoryMember } from "../../../../src/utils/direct-messages"; + +describe("RoomAvatar", () => { + let client: MatrixClient; + + filterConsole( + // unrelated for this test + "Room !room:example.com does not have an m.room.create event", + ); + + beforeAll(() => { + client = stubClient(); + const dmRoomMap = new DMRoomMap(client); + jest.spyOn(dmRoomMap, "getUserIdForRoomId"); + jest.spyOn(DMRoomMap, "shared").mockReturnValue(dmRoomMap); + jest.spyOn(AvatarModule, "defaultAvatarUrlForString"); + }); + + afterAll(() => { + jest.restoreAllMocks(); + }); + + afterEach(() => { + mocked(DMRoomMap.shared().getUserIdForRoomId).mockReset(); + mocked(AvatarModule.defaultAvatarUrlForString).mockClear(); + }); + + it("should render as expected for a Room", () => { + const room = new Room("!room:example.com", client, client.getSafeUserId()); + room.name = "test room"; + expect(render().container).toMatchSnapshot(); + expect(AvatarModule.defaultAvatarUrlForString).toHaveBeenCalledWith(room.roomId); + }); + + it("should render as expected for a DM room", () => { + const userId = "@dm_user@example.com"; + const room = new Room("!room:example.com", client, client.getSafeUserId()); + room.name = "DM room"; + mocked(DMRoomMap.shared().getUserIdForRoomId).mockReturnValue(userId); + expect(render().container).toMatchSnapshot(); + expect(AvatarModule.defaultAvatarUrlForString).toHaveBeenCalledWith(userId); + }); + + it("should render as expected for a LocalRoom", () => { + const userId = "@local_room_user@example.com"; + const localRoom = new LocalRoom("!room:example.com", client, client.getSafeUserId()); + localRoom.name = "local test room"; + localRoom.targets.push(new DirectoryMember({ user_id: userId })); + expect(render().container).toMatchSnapshot(); + expect(AvatarModule.defaultAvatarUrlForString).toHaveBeenCalledWith(userId); + }); +}); diff --git a/test/components/views/avatars/__snapshots__/RoomAvatar-test.tsx.snap b/test/components/views/avatars/__snapshots__/RoomAvatar-test.tsx.snap new file mode 100644 index 00000000000..6bffa157b63 --- /dev/null +++ b/test/components/views/avatars/__snapshots__/RoomAvatar-test.tsx.snap @@ -0,0 +1,76 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`RoomAvatar should render as expected for a DM room 1`] = ` +
+ + + + +
+`; + +exports[`RoomAvatar should render as expected for a LocalRoom 1`] = ` +
+ + + + +
+`; + +exports[`RoomAvatar should render as expected for a Room 1`] = ` +
+ + + + +
+`; diff --git a/test/components/views/rooms/RoomTile-test.tsx b/test/components/views/rooms/RoomTile-test.tsx index e02a72b7165..b4bfde243c1 100644 --- a/test/components/views/rooms/RoomTile-test.tsx +++ b/test/components/views/rooms/RoomTile-test.tsx @@ -75,20 +75,19 @@ describe("RoomTile", () => { }; let client: Mocked; - let restoreConsole: () => void; let voiceBroadcastInfoEvent: MatrixEvent; let room: Room; let renderResult: RenderResult; let sdkContext: TestSdkContext; + filterConsole( + // irrelevant for this test + "Room !1:example.org does not have an m.room.create event", + ); + beforeEach(() => { sdkContext = new TestSdkContext(); - restoreConsole = filterConsole( - // irrelevant for this test - "Room !1:example.org does not have an m.room.create event", - ); - client = mocked(stubClient()); sdkContext.client = client; DMRoomMap.makeShared(); @@ -105,7 +104,6 @@ describe("RoomTile", () => { }); afterEach(() => { - restoreConsole(); jest.clearAllMocks(); }); diff --git a/test/components/views/user-onboarding/UserOnboardingPage-test.tsx b/test/components/views/user-onboarding/UserOnboardingPage-test.tsx index 0ff637e2c91..af94db64610 100644 --- a/test/components/views/user-onboarding/UserOnboardingPage-test.tsx +++ b/test/components/views/user-onboarding/UserOnboardingPage-test.tsx @@ -33,8 +33,6 @@ jest.mock("../../../../src/components/structures/HomePage", () => ({ })); describe("UserOnboardingPage", () => { - let restoreConsole: () => void; - const renderComponent = async (): Promise => { const renderResult = render(); await act(async () => { @@ -43,12 +41,10 @@ describe("UserOnboardingPage", () => { return renderResult; }; - beforeAll(() => { - restoreConsole = filterConsole( - // unrelated for this test - "could not update user onboarding context", - ); - }); + filterConsole( + // unrelated for this test + "could not update user onboarding context", + ); beforeEach(() => { stubClient(); @@ -60,10 +56,6 @@ describe("UserOnboardingPage", () => { jest.restoreAllMocks(); }); - afterAll(() => { - restoreConsole(); - }); - describe("when the user registered before the cutoff date", () => { beforeEach(() => { jest.spyOn(MatrixClientPeg, "userRegisteredAfter").mockReturnValue(false); diff --git a/test/test-utils/console.ts b/test/test-utils/console.ts index b4c4b98f910..468d2e3b8fb 100644 --- a/test/test-utils/console.ts +++ b/test/test-utils/console.ts @@ -16,36 +16,39 @@ limitations under the License. type FilteredConsole = Pick; -const originalFunctions: FilteredConsole = { - log: console.log, - error: console.error, - info: console.info, - debug: console.debug, - warn: console.warn, -}; - /** * Allows to filter out specific messages in console.*. + * Call this from any describe block. + * Automagically restores the original function by implementing an afterAll hook. * * @param ignoreList Messages to be filtered - * @returns function to restore the console */ -export const filterConsole = (...ignoreList: string[]): (() => void) => { - for (const [key, originalFunction] of Object.entries(originalFunctions)) { - window.console[key as keyof FilteredConsole] = (...data: any[]) => { - const message = data?.[0]?.message || data?.[0]; +export const filterConsole = (...ignoreList: string[]): void => { + const originalFunctions: FilteredConsole = { + log: console.log, + error: console.error, + info: console.info, + debug: console.debug, + warn: console.warn, + }; - if (typeof message === "string" && ignoreList.some((i) => message.includes(i))) { - return; - } + beforeAll(() => { + for (const [key, originalFunction] of Object.entries(originalFunctions)) { + window.console[key as keyof FilteredConsole] = (...data: any[]) => { + const message = data?.[0]?.message || data?.[0]; - originalFunction(...data); - }; - } + if (typeof message === "string" && ignoreList.some((i) => message.includes(i))) { + return; + } - return () => { + originalFunction(...data); + }; + } + }); + + afterAll(() => { for (const [key, originalFunction] of Object.entries(originalFunctions)) { window.console[key as keyof FilteredConsole] = originalFunction; } - }; + }); }; diff --git a/test/voice-broadcast/components/molecules/VoiceBroadcastRecordingPip-test.tsx b/test/voice-broadcast/components/molecules/VoiceBroadcastRecordingPip-test.tsx index 15b3348465e..662d7b26198 100644 --- a/test/voice-broadcast/components/molecules/VoiceBroadcastRecordingPip-test.tsx +++ b/test/voice-broadcast/components/molecules/VoiceBroadcastRecordingPip-test.tsx @@ -62,7 +62,6 @@ describe("VoiceBroadcastRecordingPip", () => { let infoEvent: MatrixEvent; let recording: VoiceBroadcastRecording; let renderResult: RenderResult; - let restoreConsole: () => void; const renderPip = async (state: VoiceBroadcastInfoState) => { infoEvent = mkVoiceBroadcastInfoStateEvent(roomId, state, client.getUserId() || "", client.getDeviceId() || ""); @@ -85,6 +84,8 @@ describe("VoiceBroadcastRecordingPip", () => { }); }; + filterConsole("Starting load of AsyncWrapper for modal"); + beforeAll(() => { client = stubClient(); mocked(requestMediaPermissions).mockResolvedValue({ @@ -105,11 +106,6 @@ describe("VoiceBroadcastRecordingPip", () => { [MediaDeviceKindEnum.VideoInput]: [], }); jest.spyOn(MediaDeviceHandler.instance, "setDevice").mockImplementation(); - restoreConsole = filterConsole("Starting load of AsyncWrapper for modal"); - }); - - afterAll(() => { - restoreConsole(); }); describe("when rendering a started recording", () => {