Skip to content
This repository has been archived by the owner on Sep 11, 2024. It is now read-only.

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

Merged
merged 6 commits into from
Jan 5, 2023
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 19 additions & 5 deletions src/components/views/avatars/RoomAvatar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand All @@ -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}
/>
Expand Down
16 changes: 7 additions & 9 deletions test/components/structures/auth/ForgotPassword-test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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 () => {
Expand All @@ -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);

Expand All @@ -87,7 +86,6 @@ describe("<ForgotPassword>", () => {
afterEach(() => {
// clean up modals
Modal.closeCurrentModal("force");
restoreConsole?.();
});

beforeAll(() => {
Expand Down
78 changes: 78 additions & 0 deletions test/components/views/avatars/RoomAvatar-test.tsx
Original file line number Diff line number Diff line change
@@ -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);
});
});
Original file line number Diff line number Diff line change
@@ -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"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately this does not assert the rendered colour. But at least the text content is different per test: D, L and T.

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>
`;
12 changes: 5 additions & 7 deletions test/components/views/rooms/RoomTile-test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -105,7 +104,6 @@ describe("RoomTile", () => {
});

afterEach(() => {
restoreConsole();
jest.clearAllMocks();
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 () => {
Expand All @@ -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();
Expand All @@ -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);
Expand Down
8 changes: 4 additions & 4 deletions test/test-utils/console.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,11 @@ const originalFunctions: FilteredConsole = {

/**
* Allows to filter out specific messages in console.*.
* 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) => {
export const filterConsole = (...ignoreList: string[]): void => {
for (const [key, originalFunction] of Object.entries(originalFunctions)) {
weeman1337 marked this conversation as resolved.
Show resolved Hide resolved
window.console[key as keyof FilteredConsole] = (...data: any[]) => {
const message = data?.[0]?.message || data?.[0];
Expand All @@ -43,9 +43,9 @@ export const filterConsole = (...ignoreList: string[]): (() => void) => {
};
}

return () => {
afterAll(() => {
for (const [key, originalFunction] of Object.entries(originalFunctions)) {
window.console[key as keyof FilteredConsole] = originalFunction;
}
};
});
};
Original file line number Diff line number Diff line change
Expand Up @@ -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() || "");
Expand All @@ -85,6 +84,8 @@ describe("VoiceBroadcastRecordingPip", () => {
});
};

filterConsole("Starting load of AsyncWrapper for modal");

beforeAll(() => {
client = stubClient();
mocked(requestMediaPermissions).mockResolvedValue({
Expand All @@ -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", () => {
Expand Down