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

Commit ab91520

Browse files
authored
Use the same avatar colour when creating 1:1 DM rooms (#9850)
1 parent ecfd173 commit ab91520

File tree

8 files changed

+215
-60
lines changed

8 files changed

+215
-60
lines changed

src/components/views/avatars/RoomAvatar.tsx

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ import * as Avatar from "../../../Avatar";
2929
import DMRoomMap from "../../../utils/DMRoomMap";
3030
import { mediaFromMxc } from "../../../customisations/Media";
3131
import { IOOBData } from "../../../stores/ThreepidInviteStore";
32+
import { LocalRoom } from "../../../models/LocalRoom";
3233

3334
interface IProps extends Omit<ComponentProps<typeof BaseAvatar>, "name" | "idName" | "url" | "onClick"> {
3435
// Room may be left unset here, but if it is,
@@ -117,13 +118,26 @@ export default class RoomAvatar extends React.Component<IProps, IState> {
117118
Modal.createDialog(ImageView, params, "mx_Dialog_lightbox", null, true);
118119
};
119120

121+
private get roomIdName(): string | undefined {
122+
const room = this.props.room;
123+
124+
if (room) {
125+
const dmMapUserId = DMRoomMap.shared().getUserIdForRoomId(room.roomId);
126+
// If the room is a DM, we use the other user's ID for the color hash
127+
// in order to match the room avatar with their avatar
128+
if (dmMapUserId) return dmMapUserId;
129+
130+
if (room instanceof LocalRoom && room.targets.length === 1) {
131+
return room.targets[0].userId;
132+
}
133+
}
134+
135+
return this.props.room?.roomId || this.props.oobData?.roomId;
136+
}
137+
120138
public render() {
121139
const { room, oobData, viewAvatarOnClick, onClick, className, ...otherProps } = this.props;
122-
123140
const roomName = room?.name ?? oobData.name;
124-
// If the room is a DM, we use the other user's ID for the color hash
125-
// in order to match the room avatar with their avatar
126-
const idName = room ? DMRoomMap.shared().getUserIdForRoomId(room.roomId) ?? room.roomId : oobData.roomId;
127141

128142
return (
129143
<BaseAvatar
@@ -132,7 +146,7 @@ export default class RoomAvatar extends React.Component<IProps, IState> {
132146
mx_RoomAvatar_isSpaceRoom: (room?.getType() ?? this.props.oobData?.roomType) === RoomType.Space,
133147
})}
134148
name={roomName}
135-
idName={idName}
149+
idName={this.roomIdName}
136150
urls={this.state.urls}
137151
onClick={viewAvatarOnClick && this.state.urls[0] ? this.onRoomAvatarClick : onClick}
138152
/>

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

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,6 @@ describe("<ForgotPassword>", () => {
4040
let onComplete: () => void;
4141
let onLoginClick: () => void;
4242
let renderResult: RenderResult;
43-
let restoreConsole: () => void;
4443

4544
const typeIntoField = async (label: string, value: string): Promise<void> => {
4645
await act(async () => {
@@ -63,14 +62,14 @@ describe("<ForgotPassword>", () => {
6362
});
6463
};
6564

66-
beforeEach(() => {
67-
restoreConsole = filterConsole(
68-
// not implemented by js-dom https://github.com/jsdom/jsdom/issues/1937
69-
"Not implemented: HTMLFormElement.prototype.requestSubmit",
70-
// not of interested for this test
71-
"Starting load of AsyncWrapper for modal",
72-
);
65+
filterConsole(
66+
// not implemented by js-dom https://github.com/jsdom/jsdom/issues/1937
67+
"Not implemented: HTMLFormElement.prototype.requestSubmit",
68+
// not of interested for this test
69+
"Starting load of AsyncWrapper for modal",
70+
);
7371

72+
beforeEach(() => {
7473
client = stubClient();
7574
mocked(createClient).mockReturnValue(client);
7675

@@ -87,7 +86,6 @@ describe("<ForgotPassword>", () => {
8786
afterEach(() => {
8887
// clean up modals
8988
Modal.closeCurrentModal("force");
90-
restoreConsole?.();
9189
});
9290

9391
beforeAll(() => {
Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,78 @@
1+
/*
2+
Copyright 2022 The Matrix.org Foundation C.I.C.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
import React from "react";
18+
import { render } from "@testing-library/react";
19+
import { MatrixClient, Room } from "matrix-js-sdk/src/matrix";
20+
import { mocked } from "jest-mock";
21+
22+
import RoomAvatar from "../../../../src/components/views/avatars/RoomAvatar";
23+
import { filterConsole, stubClient } from "../../../test-utils";
24+
import DMRoomMap from "../../../../src/utils/DMRoomMap";
25+
import { LocalRoom } from "../../../../src/models/LocalRoom";
26+
import * as AvatarModule from "../../../../src/Avatar";
27+
import { DirectoryMember } from "../../../../src/utils/direct-messages";
28+
29+
describe("RoomAvatar", () => {
30+
let client: MatrixClient;
31+
32+
filterConsole(
33+
// unrelated for this test
34+
"Room !room:example.com does not have an m.room.create event",
35+
);
36+
37+
beforeAll(() => {
38+
client = stubClient();
39+
const dmRoomMap = new DMRoomMap(client);
40+
jest.spyOn(dmRoomMap, "getUserIdForRoomId");
41+
jest.spyOn(DMRoomMap, "shared").mockReturnValue(dmRoomMap);
42+
jest.spyOn(AvatarModule, "defaultAvatarUrlForString");
43+
});
44+
45+
afterAll(() => {
46+
jest.restoreAllMocks();
47+
});
48+
49+
afterEach(() => {
50+
mocked(DMRoomMap.shared().getUserIdForRoomId).mockReset();
51+
mocked(AvatarModule.defaultAvatarUrlForString).mockClear();
52+
});
53+
54+
it("should render as expected for a Room", () => {
55+
const room = new Room("!room:example.com", client, client.getSafeUserId());
56+
room.name = "test room";
57+
expect(render(<RoomAvatar room={room} />).container).toMatchSnapshot();
58+
expect(AvatarModule.defaultAvatarUrlForString).toHaveBeenCalledWith(room.roomId);
59+
});
60+
61+
it("should render as expected for a DM room", () => {
62+
const userId = "@dm_user@example.com";
63+
const room = new Room("!room:example.com", client, client.getSafeUserId());
64+
room.name = "DM room";
65+
mocked(DMRoomMap.shared().getUserIdForRoomId).mockReturnValue(userId);
66+
expect(render(<RoomAvatar room={room} />).container).toMatchSnapshot();
67+
expect(AvatarModule.defaultAvatarUrlForString).toHaveBeenCalledWith(userId);
68+
});
69+
70+
it("should render as expected for a LocalRoom", () => {
71+
const userId = "@local_room_user@example.com";
72+
const localRoom = new LocalRoom("!room:example.com", client, client.getSafeUserId());
73+
localRoom.name = "local test room";
74+
localRoom.targets.push(new DirectoryMember({ user_id: userId }));
75+
expect(render(<RoomAvatar room={localRoom} />).container).toMatchSnapshot();
76+
expect(AvatarModule.defaultAvatarUrlForString).toHaveBeenCalledWith(userId);
77+
});
78+
});
Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
// Jest Snapshot v1, https://goo.gl/fbAQLP
2+
3+
exports[`RoomAvatar should render as expected for a DM room 1`] = `
4+
<div>
5+
<span
6+
class="mx_BaseAvatar"
7+
role="presentation"
8+
>
9+
<span
10+
aria-hidden="true"
11+
class="mx_BaseAvatar_initial"
12+
style="font-size: 23.400000000000002px; width: 36px; line-height: 36px;"
13+
>
14+
D
15+
</span>
16+
<img
17+
alt=""
18+
aria-hidden="true"
19+
class="mx_BaseAvatar_image"
20+
data-testid="avatar-img"
21+
src="data:image/png;base64,00"
22+
style="width: 36px; height: 36px;"
23+
/>
24+
</span>
25+
</div>
26+
`;
27+
28+
exports[`RoomAvatar should render as expected for a LocalRoom 1`] = `
29+
<div>
30+
<span
31+
class="mx_BaseAvatar"
32+
role="presentation"
33+
>
34+
<span
35+
aria-hidden="true"
36+
class="mx_BaseAvatar_initial"
37+
style="font-size: 23.400000000000002px; width: 36px; line-height: 36px;"
38+
>
39+
L
40+
</span>
41+
<img
42+
alt=""
43+
aria-hidden="true"
44+
class="mx_BaseAvatar_image"
45+
data-testid="avatar-img"
46+
src="data:image/png;base64,00"
47+
style="width: 36px; height: 36px;"
48+
/>
49+
</span>
50+
</div>
51+
`;
52+
53+
exports[`RoomAvatar should render as expected for a Room 1`] = `
54+
<div>
55+
<span
56+
class="mx_BaseAvatar"
57+
role="presentation"
58+
>
59+
<span
60+
aria-hidden="true"
61+
class="mx_BaseAvatar_initial"
62+
style="font-size: 23.400000000000002px; width: 36px; line-height: 36px;"
63+
>
64+
T
65+
</span>
66+
<img
67+
alt=""
68+
aria-hidden="true"
69+
class="mx_BaseAvatar_image"
70+
data-testid="avatar-img"
71+
src="data:image/png;base64,00"
72+
style="width: 36px; height: 36px;"
73+
/>
74+
</span>
75+
</div>
76+
`;

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

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -75,20 +75,19 @@ describe("RoomTile", () => {
7575
};
7676

7777
let client: Mocked<MatrixClient>;
78-
let restoreConsole: () => void;
7978
let voiceBroadcastInfoEvent: MatrixEvent;
8079
let room: Room;
8180
let renderResult: RenderResult;
8281
let sdkContext: TestSdkContext;
8382

83+
filterConsole(
84+
// irrelevant for this test
85+
"Room !1:example.org does not have an m.room.create event",
86+
);
87+
8488
beforeEach(() => {
8589
sdkContext = new TestSdkContext();
8690

87-
restoreConsole = filterConsole(
88-
// irrelevant for this test
89-
"Room !1:example.org does not have an m.room.create event",
90-
);
91-
9291
client = mocked(stubClient());
9392
sdkContext.client = client;
9493
DMRoomMap.makeShared();
@@ -105,7 +104,6 @@ describe("RoomTile", () => {
105104
});
106105

107106
afterEach(() => {
108-
restoreConsole();
109107
jest.clearAllMocks();
110108
});
111109

test/components/views/user-onboarding/UserOnboardingPage-test.tsx

Lines changed: 4 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -33,8 +33,6 @@ jest.mock("../../../../src/components/structures/HomePage", () => ({
3333
}));
3434

3535
describe("UserOnboardingPage", () => {
36-
let restoreConsole: () => void;
37-
3836
const renderComponent = async (): Promise<RenderResult> => {
3937
const renderResult = render(<UserOnboardingPage />);
4038
await act(async () => {
@@ -43,12 +41,10 @@ describe("UserOnboardingPage", () => {
4341
return renderResult;
4442
};
4543

46-
beforeAll(() => {
47-
restoreConsole = filterConsole(
48-
// unrelated for this test
49-
"could not update user onboarding context",
50-
);
51-
});
44+
filterConsole(
45+
// unrelated for this test
46+
"could not update user onboarding context",
47+
);
5248

5349
beforeEach(() => {
5450
stubClient();
@@ -60,10 +56,6 @@ describe("UserOnboardingPage", () => {
6056
jest.restoreAllMocks();
6157
});
6258

63-
afterAll(() => {
64-
restoreConsole();
65-
});
66-
6759
describe("when the user registered before the cutoff date", () => {
6860
beforeEach(() => {
6961
jest.spyOn(MatrixClientPeg, "userRegisteredAfter").mockReturnValue(false);

test/test-utils/console.ts

Lines changed: 24 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -16,36 +16,39 @@ limitations under the License.
1616

1717
type FilteredConsole = Pick<Console, "log" | "error" | "info" | "debug" | "warn">;
1818

19-
const originalFunctions: FilteredConsole = {
20-
log: console.log,
21-
error: console.error,
22-
info: console.info,
23-
debug: console.debug,
24-
warn: console.warn,
25-
};
26-
2719
/**
2820
* Allows to filter out specific messages in console.*.
21+
* Call this from any describe block.
22+
* Automagically restores the original function by implementing an afterAll hook.
2923
*
3024
* @param ignoreList Messages to be filtered
31-
* @returns function to restore the console
3225
*/
33-
export const filterConsole = (...ignoreList: string[]): (() => void) => {
34-
for (const [key, originalFunction] of Object.entries(originalFunctions)) {
35-
window.console[key as keyof FilteredConsole] = (...data: any[]) => {
36-
const message = data?.[0]?.message || data?.[0];
26+
export const filterConsole = (...ignoreList: string[]): void => {
27+
const originalFunctions: FilteredConsole = {
28+
log: console.log,
29+
error: console.error,
30+
info: console.info,
31+
debug: console.debug,
32+
warn: console.warn,
33+
};
3734

38-
if (typeof message === "string" && ignoreList.some((i) => message.includes(i))) {
39-
return;
40-
}
35+
beforeAll(() => {
36+
for (const [key, originalFunction] of Object.entries(originalFunctions)) {
37+
window.console[key as keyof FilteredConsole] = (...data: any[]) => {
38+
const message = data?.[0]?.message || data?.[0];
4139

42-
originalFunction(...data);
43-
};
44-
}
40+
if (typeof message === "string" && ignoreList.some((i) => message.includes(i))) {
41+
return;
42+
}
4543

46-
return () => {
44+
originalFunction(...data);
45+
};
46+
}
47+
});
48+
49+
afterAll(() => {
4750
for (const [key, originalFunction] of Object.entries(originalFunctions)) {
4851
window.console[key as keyof FilteredConsole] = originalFunction;
4952
}
50-
};
53+
});
5154
};

0 commit comments

Comments
 (0)