diff --git a/src/Avatar.ts b/src/Avatar.ts index b23d06a55cf1..32bc4f6544ec 100644 --- a/src/Avatar.ts +++ b/src/Avatar.ts @@ -24,6 +24,8 @@ import DMRoomMap from "./utils/DMRoomMap"; import { mediaFromMxc } from "./customisations/Media"; import { isLocalRoom } from "./utils/localRoom/isLocalRoom"; +const DEFAULT_COLORS: Readonly = ["#0DBD8B", "#368bd6", "#ac3ba8"]; + // Not to be used for BaseAvatar urls as that has similar default avatar fallback already export function avatarUrlForMember( member: RoomMember | null | undefined, @@ -89,16 +91,8 @@ const colorToDataURLCache = new Map(); export function defaultAvatarUrlForString(s: string | undefined): string { if (!s) return ""; // XXX: should never happen but empirically does by evidence of a rageshake - const defaultColors = ["#0DBD8B", "#368bd6", "#ac3ba8"]; - let total = 0; - for (let i = 0; i < s.length; ++i) { - total += s.charCodeAt(i); - } - const colorIndex = total % defaultColors.length; - // overwritten color value in custom themes - const cssVariable = `--avatar-background-colors_${colorIndex}`; - const cssValue = document.body.style.getPropertyValue(cssVariable); - const color = cssValue || defaultColors[colorIndex]!; + + const color = getColorForString(s); let dataUrl = colorToDataURLCache.get(color); if (!dataUrl) { // validate color as this can come from account_data @@ -113,6 +107,16 @@ export function defaultAvatarUrlForString(s: string | undefined): string { return dataUrl; } +export function getColorForString(input: string): string { + const charSum = [...input].reduce((s, c) => s + c.charCodeAt(0), 0); + const index = charSum % DEFAULT_COLORS.length; + + // overwritten color value in custom themes + const cssVariable = `--avatar-background-colors_${index}`; + const cssValue = document.body.style.getPropertyValue(cssVariable); + return cssValue || DEFAULT_COLORS[index]!; +} + /** * returns the first (non-sigil) character of 'name', * converted to uppercase diff --git a/src/components/views/avatars/BaseAvatar.tsx b/src/components/views/avatars/BaseAvatar.tsx index 92c469a0bff7..7be4a0e659ac 100644 --- a/src/components/views/avatars/BaseAvatar.tsx +++ b/src/components/views/avatars/BaseAvatar.tsx @@ -161,6 +161,10 @@ const BaseAvatar: React.FC = (props) => { className={classNames("mx_BaseAvatar", className)} ref={inputRef} {...otherProps} + style={{ + width: toPx(width), + height: toPx(height), + }} role="presentation" > {avatar} @@ -219,38 +223,22 @@ const TextAvatar: React.FC<{ title?: string; }> = ({ name, idName, width, height, title }) => { const initialLetter = AvatarLogic.getInitialLetter(name); - const textNode = ( + + return ( ); - const imgNode = ( - - ); - - return ( - <> - {textNode} - {imgNode} - - ); }; diff --git a/test/Avatar-test.ts b/test/Avatar-test.ts index 0188acd9cda5..eb35f74cc4e5 100644 --- a/test/Avatar-test.ts +++ b/test/Avatar-test.ts @@ -22,6 +22,7 @@ import { avatarUrlForRoom, avatarUrlForUser, defaultAvatarUrlForString, + getColorForString, getInitialLetter, } from "../src/Avatar"; import { mediaFromMxc } from "../src/customisations/Media"; @@ -84,6 +85,16 @@ describe("defaultAvatarUrlForString", () => { }); }); +describe("getColorForString", () => { + it.each(["a", "abc", "abcde", "@".repeat(150)])("should return a value for %s", (s) => { + expect(getColorForString(s)).toMatch(/^#\w+$/); + }); + + it("should return different values for different strings", () => { + expect(getColorForString("a")).not.toBe(getColorForString("b")); + }); +}); + describe("getInitialLetter", () => { filterConsole("argument to `getInitialLetter` not supplied"); diff --git a/test/components/structures/__snapshots__/RoomView-test.tsx.snap b/test/components/structures/__snapshots__/RoomView-test.tsx.snap index 47318525d56d..af653efe9cc2 100644 --- a/test/components/structures/__snapshots__/RoomView-test.tsx.snap +++ b/test/components/structures/__snapshots__/RoomView-test.tsx.snap @@ -20,22 +20,16 @@ exports[`RoomView for a local room in state CREATING should match the snapshot 1 - @@ -119,22 +113,16 @@ exports[`RoomView for a local room in state ERROR should match the snapshot 1`] - @@ -219,19 +207,12 @@ exports[`RoomView for a local room in state ERROR should match the snapshot 1`] > -

@user:example.com @@ -314,22 +295,16 @@ exports[`RoomView for a local room in state NEW should match the snapshot 1`] = - @@ -414,19 +389,12 @@ exports[`RoomView for a local room in state NEW should match the snapshot 1`] = > -

@user:example.com @@ -581,22 +549,16 @@ exports[`RoomView for a local room in state NEW that is encrypted should match t - @@ -676,19 +638,12 @@ exports[`RoomView for a local room in state NEW that is encrypted should match t > -

@user:example.com diff --git a/test/components/structures/__snapshots__/UserMenu-test.tsx.snap b/test/components/structures/__snapshots__/UserMenu-test.tsx.snap index 769711434a87..0546900abb39 100644 --- a/test/components/structures/__snapshots__/UserMenu-test.tsx.snap +++ b/test/components/structures/__snapshots__/UserMenu-test.tsx.snap @@ -20,22 +20,16 @@ exports[` when rendered should render as expected 1`] = ` - diff --git a/test/components/views/avatars/MemberAvatar-test.tsx b/test/components/views/avatars/MemberAvatar-test.tsx index dc282c8adc91..3dc793bd9298 100644 --- a/test/components/views/avatars/MemberAvatar-test.tsx +++ b/test/components/views/avatars/MemberAvatar-test.tsx @@ -110,9 +110,8 @@ describe("MemberAvatar", () => { const { container } = render(getComponent({ member })); - const img = container.querySelector("img"); + const img = container.querySelector(".mx_BaseAvatar_image"); expect(img).not.toBeNull(); - expect(img!.src).toMatch(/^data:/); }); it("dispatches on click", () => { diff --git a/test/components/views/avatars/RoomAvatar-test.tsx b/test/components/views/avatars/RoomAvatar-test.tsx index e23cd96f02da..7be7dd65e905 100644 --- a/test/components/views/avatars/RoomAvatar-test.tsx +++ b/test/components/views/avatars/RoomAvatar-test.tsx @@ -39,7 +39,7 @@ describe("RoomAvatar", () => { const dmRoomMap = new DMRoomMap(client); jest.spyOn(dmRoomMap, "getUserIdForRoomId"); jest.spyOn(DMRoomMap, "shared").mockReturnValue(dmRoomMap); - jest.spyOn(AvatarModule, "defaultAvatarUrlForString"); + jest.spyOn(AvatarModule, "getColorForString"); }); afterAll(() => { @@ -48,14 +48,14 @@ describe("RoomAvatar", () => { afterEach(() => { mocked(DMRoomMap.shared().getUserIdForRoomId).mockReset(); - mocked(AvatarModule.defaultAvatarUrlForString).mockClear(); + mocked(AvatarModule.getColorForString).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); + expect(AvatarModule.getColorForString).toHaveBeenCalledWith(room.roomId); }); it("should render as expected for a DM room", () => { @@ -64,7 +64,7 @@ describe("RoomAvatar", () => { room.name = "DM room"; mocked(DMRoomMap.shared().getUserIdForRoomId).mockReturnValue(userId); expect(render().container).toMatchSnapshot(); - expect(AvatarModule.defaultAvatarUrlForString).toHaveBeenCalledWith(userId); + expect(AvatarModule.getColorForString).toHaveBeenCalledWith(userId); }); it("should render as expected for a LocalRoom", () => { @@ -73,6 +73,6 @@ describe("RoomAvatar", () => { localRoom.name = "local test room"; localRoom.targets.push(new DirectoryMember({ user_id: userId })); expect(render().container).toMatchSnapshot(); - expect(AvatarModule.defaultAvatarUrlForString).toHaveBeenCalledWith(userId); + expect(AvatarModule.getColorForString).toHaveBeenCalledWith(userId); }); }); diff --git a/test/components/views/avatars/__snapshots__/BaseAvatar-test.tsx.snap b/test/components/views/avatars/__snapshots__/BaseAvatar-test.tsx.snap index 38202a82eade..a5789297a651 100644 --- a/test/components/views/avatars/__snapshots__/BaseAvatar-test.tsx.snap +++ b/test/components/views/avatars/__snapshots__/BaseAvatar-test.tsx.snap @@ -39,20 +39,13 @@ exports[` matches snapshot (no avatar + click) 1`] = ` > - `; @@ -62,23 +55,17 @@ exports[` matches snapshot (no avatar) 1`] = ` - `; diff --git a/test/components/views/avatars/__snapshots__/RoomAvatar-test.tsx.snap b/test/components/views/avatars/__snapshots__/RoomAvatar-test.tsx.snap index 6bffa157b637..699113689e40 100644 --- a/test/components/views/avatars/__snapshots__/RoomAvatar-test.tsx.snap +++ b/test/components/views/avatars/__snapshots__/RoomAvatar-test.tsx.snap @@ -5,22 +5,16 @@ exports[`RoomAvatar should render as expected for a DM room 1`] = ` - `; @@ -30,22 +24,16 @@ exports[`RoomAvatar should render as expected for a LocalRoom 1`] = ` - `; @@ -55,22 +43,16 @@ exports[`RoomAvatar should render as expected for a Room 1`] = ` - `; diff --git a/test/components/views/beacon/__snapshots__/BeaconMarker-test.tsx.snap b/test/components/views/beacon/__snapshots__/BeaconMarker-test.tsx.snap index b42ccb83ee31..b965f50b2f98 100644 --- a/test/components/views/beacon/__snapshots__/BeaconMarker-test.tsx.snap +++ b/test/components/views/beacon/__snapshots__/BeaconMarker-test.tsx.snap @@ -13,23 +13,17 @@ exports[` renders marker when beacon has location 1`] = ` - diff --git a/test/components/views/rooms/RoomHeader-test.tsx b/test/components/views/rooms/RoomHeader-test.tsx index 5857e2829574..c27d4c0c20f6 100644 --- a/test/components/views/rooms/RoomHeader-test.tsx +++ b/test/components/views/rooms/RoomHeader-test.tsx @@ -72,7 +72,7 @@ describe("RoomHeader (Enzyme)", () => { // And there is no image avatar (because it's not set on this room) const image = findImg(rendered, ".mx_BaseAvatar_image"); - expect(image.prop("src")).toEqual("data:image/png;base64,00"); + expect(image).toBeTruthy(); }); it("shows the room avatar in a room with 2 people", () => { @@ -86,7 +86,7 @@ describe("RoomHeader (Enzyme)", () => { // And there is no image avatar (because it's not set on this room) const image = findImg(rendered, ".mx_BaseAvatar_image"); - expect(image.prop("src")).toEqual("data:image/png;base64,00"); + expect(image).toBeTruthy(); }); it("shows the room avatar in a room with >2 people", () => { @@ -100,7 +100,7 @@ describe("RoomHeader (Enzyme)", () => { // And there is no image avatar (because it's not set on this room) const image = findImg(rendered, ".mx_BaseAvatar_image"); - expect(image.prop("src")).toEqual("data:image/png;base64,00"); + expect(image).toBeTruthy(); }); it("shows the room avatar in a DM with only ourselves", () => { @@ -114,7 +114,7 @@ describe("RoomHeader (Enzyme)", () => { // And there is no image avatar (because it's not set on this room) const image = findImg(rendered, ".mx_BaseAvatar_image"); - expect(image.prop("src")).toEqual("data:image/png;base64,00"); + expect(image).toBeTruthy(); }); it("shows the user avatar in a DM with 2 people", () => { @@ -148,7 +148,7 @@ describe("RoomHeader (Enzyme)", () => { // And there is no image avatar (because it's not set on this room) const image = findImg(rendered, ".mx_BaseAvatar_image"); - expect(image.prop("src")).toEqual("data:image/png;base64,00"); + expect(image).toBeTruthy(); }); it("renders call buttons normally", () => { diff --git a/test/components/views/rooms/__snapshots__/RoomPreviewBar-test.tsx.snap b/test/components/views/rooms/__snapshots__/RoomPreviewBar-test.tsx.snap index f35467e1efd7..a78a452e8909 100644 --- a/test/components/views/rooms/__snapshots__/RoomPreviewBar-test.tsx.snap +++ b/test/components/views/rooms/__snapshots__/RoomPreviewBar-test.tsx.snap @@ -161,22 +161,16 @@ exports[` with an invite without an invited email for a dm roo -

@@ -236,22 +230,16 @@ exports[` with an invite without an invited email for a non-dm -

diff --git a/test/components/views/rooms/__snapshots__/RoomTile-test.tsx.snap b/test/components/views/rooms/__snapshots__/RoomTile-test.tsx.snap index bcbb7932c692..557d97c243e9 100644 --- a/test/components/views/rooms/__snapshots__/RoomTile-test.tsx.snap +++ b/test/components/views/rooms/__snapshots__/RoomTile-test.tsx.snap @@ -15,22 +15,16 @@ exports[`RoomTile should render the room 1`] = ` -