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

Commit 21c3967

Browse files
authored
Revert "Member avatars without canvas" (#10057
* Revert "Apply more general fix for base avatar regressions (#10045)" This reverts commit 371a3c0. * Revert "Fix layout and visual regressions around default avatars (#10031)" This reverts commit 0d1fce3. * Revert "Member avatars without canvas (#9990)" This reverts commit a8aa4de. * Update snapshots
1 parent 43e7870 commit 21c3967

File tree

27 files changed

+353
-824
lines changed

27 files changed

+353
-824
lines changed

cypress/e2e/spaces/spaces.spec.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -153,7 +153,10 @@ describe("Spaces", () => {
153153

154154
openSpaceCreateMenu().within(() => {
155155
cy.get(".mx_SpaceCreateMenuType_private").click();
156-
// We don't set an avatar here to get a Percy snapshot of the default avatar style for spaces
156+
cy.get('.mx_SpaceBasicSettings_avatarContainer input[type="file"]').selectFile(
157+
"cypress/fixtures/riot.png",
158+
{ force: true },
159+
);
157160
cy.get('input[label="Address"]').should("not.exist");
158161
cy.get('textarea[label="Description"]').type("This is a personal space to mourn Riot.im...");
159162
cy.get('input[label="Name"]').type("This is my Riot{enter}");
@@ -166,7 +169,6 @@ describe("Spaces", () => {
166169

167170
cy.contains(".mx_RoomList .mx_RoomTile", "Sample Room").should("exist");
168171
cy.contains(".mx_SpaceHierarchy_list .mx_SpaceHierarchy_roomTile", "Sample Room").should("exist");
169-
cy.get(".mx_LeftPanel_outerWrapper").percySnapshotElement("Left panel with default avatar space");
170172
});
171173

172174
it("should allow user to invite another to a space", () => {

res/css/structures/_SpacePanel.pcss

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -277,11 +277,14 @@ $activeBorderColor: $primary-content;
277277
.mx_BaseAvatar:not(.mx_UserMenu_userAvatar_BaseAvatar) .mx_BaseAvatar_initial {
278278
color: $secondary-content;
279279
border-radius: 8px;
280+
background-color: $panel-actions;
281+
font-size: $font-15px !important; /* override inline style */
280282
font-weight: $font-semi-bold;
281283
line-height: $font-18px;
282-
/* override inline styles which are part of the default avatar style as these uses a monochrome style */
283-
background-color: $panel-actions !important;
284-
font-size: $font-15px !important;
284+
285+
& + .mx_BaseAvatar_image {
286+
visibility: hidden;
287+
}
285288
}
286289

287290
.mx_SpaceTreeLevel {

res/css/views/avatars/_BaseAvatar.pcss

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,16 @@ limitations under the License.
1616

1717
.mx_BaseAvatar {
1818
position: relative;
19-
display: block;
19+
/* In at least Firefox, the case of relative positioned inline elements */
20+
/* (such as mx_BaseAvatar) with absolute positioned children (such as */
21+
/* mx_BaseAvatar_initial) is a dark corner full of spider webs. It will give */
22+
/* different results during full reflow of the page vs. incremental reflow */
23+
/* of small portions. While that's surely a browser bug, we can avoid it by */
24+
/* using `inline-block` instead of the default `inline`. */
25+
/* https://github.com/vector-im/element-web/issues/5594 */
26+
/* https://bugzilla.mozilla.org/show_bug.cgi?id=1535053 */
27+
/* https://bugzilla.mozilla.org/show_bug.cgi?id=255139 */
28+
display: inline-block;
2029
user-select: none;
2130

2231
&.mx_RoomAvatar_isSpaceRoom {

res/css/views/rooms/_BasicMessageComposer.pcss

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ limitations under the License.
7878
min-width: $font-16px; /* ensure the avatar is not compressed */
7979
height: $font-16px;
8080
margin-inline-end: 0.24rem;
81-
background: var(--avatar-background);
81+
background: var(--avatar-background), $background;
8282
color: $avatar-initial-color;
8383
background-repeat: no-repeat;
8484
background-size: $font-16px;

src/Avatar.ts

Lines changed: 23 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
Copyright 2015, 2016, 2023 The Matrix.org Foundation C.I.C.
2+
Copyright 2015, 2016 OpenMarket Ltd
33
44
Licensed under the Apache License, Version 2.0 (the "License");
55
you may not use this file except in compliance with the License.
@@ -24,19 +24,16 @@ import DMRoomMap from "./utils/DMRoomMap";
2424
import { mediaFromMxc } from "./customisations/Media";
2525
import { isLocalRoom } from "./utils/localRoom/isLocalRoom";
2626

27-
const DEFAULT_COLORS: Readonly<string[]> = ["#0DBD8B", "#368bd6", "#ac3ba8"];
28-
2927
// Not to be used for BaseAvatar urls as that has similar default avatar fallback already
3028
export function avatarUrlForMember(
31-
member: RoomMember | null | undefined,
29+
member: RoomMember,
3230
width: number,
3331
height: number,
3432
resizeMethod: ResizeMethod,
3533
): string {
36-
let url: string | undefined;
37-
const mxcUrl = member?.getMxcAvatarUrl();
38-
if (mxcUrl) {
39-
url = mediaFromMxc(mxcUrl).getThumbnailOfSourceHttp(width, height, resizeMethod);
34+
let url: string;
35+
if (member?.getMxcAvatarUrl()) {
36+
url = mediaFromMxc(member.getMxcAvatarUrl()).getThumbnailOfSourceHttp(width, height, resizeMethod);
4037
}
4138
if (!url) {
4239
// member can be null here currently since on invites, the JS SDK
@@ -47,17 +44,6 @@ export function avatarUrlForMember(
4744
return url;
4845
}
4946

50-
export function getMemberAvatar(
51-
member: RoomMember | null | undefined,
52-
width: number,
53-
height: number,
54-
resizeMethod: ResizeMethod,
55-
): string | undefined {
56-
const mxcUrl = member?.getMxcAvatarUrl();
57-
if (!mxcUrl) return undefined;
58-
return mediaFromMxc(mxcUrl).getThumbnailOfSourceHttp(width, height, resizeMethod);
59-
}
60-
6147
export function avatarUrlForUser(
6248
user: Pick<User, "avatarUrl">,
6349
width: number,
@@ -100,10 +86,18 @@ function urlForColor(color: string): string {
10086
// hard to install a listener here, even if there were a clear event to listen to
10187
const colorToDataURLCache = new Map<string, string>();
10288

103-
export function defaultAvatarUrlForString(s: string | undefined): string {
89+
export function defaultAvatarUrlForString(s: string): string {
10490
if (!s) return ""; // XXX: should never happen but empirically does by evidence of a rageshake
105-
106-
const color = getColorForString(s);
91+
const defaultColors = ["#0DBD8B", "#368bd6", "#ac3ba8"];
92+
let total = 0;
93+
for (let i = 0; i < s.length; ++i) {
94+
total += s.charCodeAt(i);
95+
}
96+
const colorIndex = total % defaultColors.length;
97+
// overwritten color value in custom themes
98+
const cssVariable = `--avatar-background-colors_${colorIndex}`;
99+
const cssValue = document.body.style.getPropertyValue(cssVariable);
100+
const color = cssValue || defaultColors[colorIndex];
107101
let dataUrl = colorToDataURLCache.get(color);
108102
if (!dataUrl) {
109103
// validate color as this can come from account_data
@@ -118,23 +112,13 @@ export function defaultAvatarUrlForString(s: string | undefined): string {
118112
return dataUrl;
119113
}
120114

121-
export function getColorForString(input: string): string {
122-
const charSum = [...input].reduce((s, c) => s + c.charCodeAt(0), 0);
123-
const index = charSum % DEFAULT_COLORS.length;
124-
125-
// overwritten color value in custom themes
126-
const cssVariable = `--avatar-background-colors_${index}`;
127-
const cssValue = document.body.style.getPropertyValue(cssVariable);
128-
return cssValue || DEFAULT_COLORS[index]!;
129-
}
130-
131115
/**
132116
* returns the first (non-sigil) character of 'name',
133117
* converted to uppercase
134118
* @param {string} name
135119
* @return {string} the first letter
136120
*/
137-
export function getInitialLetter(name: string): string | undefined {
121+
export function getInitialLetter(name: string): string {
138122
if (!name) {
139123
// XXX: We should find out what causes the name to sometimes be falsy.
140124
console.trace("`name` argument to `getInitialLetter` not supplied");
@@ -150,20 +134,19 @@ export function getInitialLetter(name: string): string | undefined {
150134
}
151135

152136
// rely on the grapheme cluster splitter in lodash so that we don't break apart compound emojis
153-
return split(name, "", 1)[0]!.toUpperCase();
137+
return split(name, "", 1)[0].toUpperCase();
154138
}
155139

156140
export function avatarUrlForRoom(
157-
room: Room | undefined,
141+
room: Room,
158142
width: number,
159143
height: number,
160144
resizeMethod?: ResizeMethod,
161145
): string | null {
162146
if (!room) return null; // null-guard
163147

164-
const mxcUrl = room.getMxcAvatarUrl();
165-
if (mxcUrl) {
166-
return mediaFromMxc(mxcUrl).getThumbnailOfSourceHttp(width, height, resizeMethod);
148+
if (room.getMxcAvatarUrl()) {
149+
return mediaFromMxc(room.getMxcAvatarUrl()).getThumbnailOfSourceHttp(width, height, resizeMethod);
167150
}
168151

169152
// space rooms cannot be DMs so skip the rest
@@ -176,9 +159,8 @@ export function avatarUrlForRoom(
176159

177160
// If there are only two members in the DM use the avatar of the other member
178161
const otherMember = room.getAvatarFallbackMember();
179-
const otherMemberMxc = otherMember?.getMxcAvatarUrl();
180-
if (otherMemberMxc) {
181-
return mediaFromMxc(otherMemberMxc).getThumbnailOfSourceHttp(width, height, resizeMethod);
162+
if (otherMember?.getMxcAvatarUrl()) {
163+
return mediaFromMxc(otherMember.getMxcAvatarUrl()).getThumbnailOfSourceHttp(width, height, resizeMethod);
182164
}
183165
return null;
184166
}

0 commit comments

Comments
 (0)