Use the same avatar colour when creating 1:1 DM rooms (#9850)

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<ComponentProps<typeof BaseAvatar>, "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<IProps, IState> {
         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 (
             <BaseAvatar
@@ -132,7 +146,7 @@ export default class RoomAvatar extends React.Component<IProps, IState> {
                     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}
             />

test/components/structures/auth/ForgotPassword-test.tsx

@@ -40,7 +40,6 @@ describe("<ForgotPassword>", () => {
     let onComplete: () => void;
     let onLoginClick: () => void;
     let renderResult: RenderResult;
-    let restoreConsole: () => void;
 
     const typeIntoField = async (label: string, value: string): Promise<void> => {
         await act(async () => {
@@ -63,14 +62,14 @@ describe("<ForgotPassword>", () => {
         });
     };
 
-    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("<ForgotPassword>", () => {
     afterEach(() => {
         // clean up modals
         Modal.closeCurrentModal("force");
-        restoreConsole?.();
     });
 
     beforeAll(() => {

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(<RoomAvatar room={room} />).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(<RoomAvatar room={room} />).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(<RoomAvatar room={localRoom} />).container).toMatchSnapshot();
+        expect(AvatarModule.defaultAvatarUrlForString).toHaveBeenCalledWith(userId);
+    });
+});

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`] = `
+<div>
+  <span
+    class="mx_BaseAvatar"
+    role="presentation"
+  >
+    <span
+      aria-hidden="true"
+      class="mx_BaseAvatar_initial"
+      style="font-size: 23.400000000000002px; width: 36px; line-height: 36px;"
+    >
+      D
+    </span>
+    <img
+      alt=""
+      aria-hidden="true"
+      class="mx_BaseAvatar_image"
+      data-testid="avatar-img"
+      src="data:image/png;base64,00"
+      style="width: 36px; height: 36px;"
+    />
+  </span>
+</div>
+`;
+
+exports[`RoomAvatar should render as expected for a LocalRoom 1`] = `
+<div>
+  <span
+    class="mx_BaseAvatar"
+    role="presentation"
+  >
+    <span
+      aria-hidden="true"
+      class="mx_BaseAvatar_initial"
+      style="font-size: 23.400000000000002px; width: 36px; line-height: 36px;"
+    >
+      L
+    </span>
+    <img
+      alt=""
+      aria-hidden="true"
+      class="mx_BaseAvatar_image"
+      data-testid="avatar-img"
+      src="data:image/png;base64,00"
+      style="width: 36px; height: 36px;"
+    />
+  </span>
+</div>
+`;
+
+exports[`RoomAvatar should render as expected for a Room 1`] = `
+<div>
+  <span
+    class="mx_BaseAvatar"
+    role="presentation"
+  >
+    <span
+      aria-hidden="true"
+      class="mx_BaseAvatar_initial"
+      style="font-size: 23.400000000000002px; width: 36px; line-height: 36px;"
+    >
+      T
+    </span>
+    <img
+      alt=""
+      aria-hidden="true"
+      class="mx_BaseAvatar_image"
+      data-testid="avatar-img"
+      src="data:image/png;base64,00"
+      style="width: 36px; height: 36px;"
+    />
+  </span>
+</div>
+`;

test/components/views/rooms/RoomTile-test.tsx

@@ -75,20 +75,19 @@ describe("RoomTile", () => {
     };
 
     let client: Mocked<MatrixClient>;
-    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();
     });
 

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<RenderResult> => {
         const renderResult = render(<UserOnboardingPage />);
         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);

test/test-utils/console.ts

@@ -16,36 +16,39 @@ limitations under the License.
 
 type FilteredConsole = Pick<Console, "log" | "error" | "info" | "debug" | "warn">;
 
-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;
         }
-    };
+    });
 };