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

Fixes read receipt avatar offset #11483

Merged
merged 8 commits into from
Aug 30, 2023
Merged

Fixes read receipt avatar offset #11483

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
13ae4b3
Fixes read receipt avatar offset
Aug 29, 2023
2df6d87
Fix avatar collapsing in thread list
Aug 29, 2023
0b7d123
Make composer pills use new avatar design
Aug 29, 2023
c8fcc5a
Update snapshots
Aug 29, 2023
01ecabd
Merge branch 'develop' into germain-gg/avatar-fixes3
Aug 29, 2023
ac244ee
Update UserInfo snapshot
Aug 29, 2023
fa1d21d
Update HTMLExport snapshot
Aug 29, 2023
03cce45
Fixes avatar placeholder font
Aug 30, 2023
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
7 changes: 7 additions & 0 deletions res/css/views/avatars/_BaseAvatar.pcss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,3 +22,10 @@ limitations under the License.
color: white !important;
}
}

button.mx_BaseAvatar {
/* The user agent stylesheet overrides the font-size in this scenario
And that breaks the alignment, emojis, and all sorts of things
*/
font-size: inherit;
}
2 changes: 1 addition & 1 deletion res/css/views/rooms/_BasicMessageComposer.pcss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ limitations under the License.
height: $font-16px;
margin-inline-end: 0.24rem;
background: var(--avatar-background), $background;
color: $avatar-initial-color;
color: var(--avatar-color, $avatar-initial-color);
background-repeat: no-repeat;
background-size: $font-16px;
border-radius: $font-16px;
Expand Down
8 changes: 3 additions & 5 deletions res/css/views/rooms/_ReadReceiptGroup.pcss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,13 +49,11 @@ limitations under the License.
height: 100%;

.mx_BaseAvatar {
box-sizing: content-box;
position: absolute;
display: inline-block;
height: 14px;
width: 14px;
border: 1px solid $background;
border-radius: 100%;

width: 14px;
height: 14px;
will-change: left, top;
transition: left var(--transition-short) ease-out, top var(--transition-standard) ease-out;
}
Expand Down
1 change: 1 addition & 0 deletions res/css/views/rooms/_ThreadSummary.pcss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,7 @@ limitations under the License.

.mx_ThreadSummary_avatar {
margin-inline-end: $spacing-8;
flex-shrink: 0;
}

.mx_ThreadSummary_icon {
Expand Down
4 changes: 2 additions & 2 deletions res/css/views/rooms/wysiwyg_composer/components/_Editor.pcss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,8 +148,8 @@ limitations under the License.
background-size: $font-16px;
border-radius: $font-16px;

color: $avatar-initial-color;
font-weight: normal;
color: var(--avatar-color, $avatar-initial-color);
font-weight: bold;
font-size: $font-10-4px;
}
}
Expand Down
31 changes: 24 additions & 7 deletions src/Avatar.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,21 @@ limitations under the License.
*/

import { RoomMember, User, Room, ResizeMethod } from "matrix-js-sdk/src/matrix";
import { useIdColorHash } from "@vector-im/compound-web";

import DMRoomMap from "./utils/DMRoomMap";
import { mediaFromMxc } from "./customisations/Media";
import { isLocalRoom } from "./utils/localRoom/isLocalRoom";
import { getFirstGrapheme } from "./utils/strings";

/**
* Hardcoded from the Compound colors.
* Shade for background as defined in the compound web implementation
* https://github.com/vector-im/compound-web/blob/main/src/components/Avatar
*/
const AVATAR_BG_COLORS = ["#e9f2ff", "#faeefb", "#e3f7ed", "#ffecf0", "#ffefe4", "#e3f5f8", "#f1efff", "#e0f8d9"];
const AVATAR_TEXT_COLORS = ["#043894", "#671481", "#004933", "#7e0642", "#850000", "#004077", "#4c05b5", "#004b00"];
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we have a future plan to use a Compound component for this, or is it not possible here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, we can. But the design tokens does not export types yet, and TypeScript is shouting at me

Screenshot 2023-08-30 at 11 20 45

Copy link
Contributor

Choose a reason for hiding this comment

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

So you're planning to fix this with a new Compound release?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, pretty much have to unfortunately. I need to export types on that package


// Not to be used for BaseAvatar urls as that has similar default avatar fallback already
export function avatarUrlForMember(
member: RoomMember | undefined,
Expand All @@ -41,6 +50,18 @@ export function avatarUrlForMember(
return url;
}

/**
* Determines the HEX color to use in the avatar pills
* @param id the user or room ID
* @returns the text color to use on the avatar
*/
export function getAvatarTextColor(id: string): string {
// eslint-disable-next-line react-hooks/rules-of-hooks
const index = useIdColorHash(id);

return AVATAR_TEXT_COLORS[index - 1];
}

export function avatarUrlForUser(
user: Pick<User, "avatarUrl">,
width: number,
Expand Down Expand Up @@ -85,16 +106,12 @@ const colorToDataURLCache = new Map<string, string>();

export function defaultAvatarUrlForString(s: string): 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;
// eslint-disable-next-line react-hooks/rules-of-hooks
const colorIndex = useIdColorHash(s);
// overwritten color value in custom themes
const cssVariable = `--avatar-background-colors_${colorIndex}`;
const cssValue = getComputedStyle(document.body).getPropertyValue(cssVariable);
const color = cssValue || defaultColors[colorIndex];
const color = cssValue || AVATAR_BG_COLORS[colorIndex - 1];
let dataUrl = colorToDataURLCache.get(color);
if (!dataUrl) {
// validate color as this can come from account_data
Expand Down
2 changes: 1 addition & 1 deletion src/components/views/avatars/BaseAvatar.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ const BaseAvatar: React.FC<IProps> = (props) => {
const {
name,
idName,
title,
title = "",
Copy link
Contributor

Choose a reason for hiding this comment

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

Why are we setting an empty title on this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

To counter element-hq/element-web#26055, but we'll also deal with it on Compound's side.

Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a correct title we could add (e.g. the user's name?)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The previous implementation did not have that title set actually. So it was actually not required for me to add.
This is a patch till i fix the downstream Compound bug

url,
urls,
size = "40px",
Expand Down
46 changes: 43 additions & 3 deletions src/components/views/rooms/wysiwyg_composer/utils/autocomplete.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,25 @@ export function getMentionDisplayText(completion: ICompletion, client: MatrixCli
return "";
}

function getCSSProperties({
url,
initialLetter,
id = "",
}: {
url: string;
initialLetter?: string;
id: string;
}): string {
const cssProperties = [`--avatar-background: url(${url})`, `--avatar-letter: '${initialLetter}'`];

const textColor = Avatar.getAvatarTextColor(id);
if (textColor) {
cssProperties.push(textColor);
}

return cssProperties.join("; ");
}

/**
* For a given completion, the attributes will change depending on the completion type
*
Expand Down Expand Up @@ -118,7 +137,14 @@ export function getMentionAttributes(
}

attributes.set("data-mention-type", completion.type);
attributes.set("style", `--avatar-background: url(${avatarUrl}); --avatar-letter: '${initialLetter}'`);
attributes.set(
"style",
getCSSProperties({
url: avatarUrl,
initialLetter,
id: mentionedMember.userId,
}),
);
} else if (completion.type === "room") {
// logic as used in RoomPillPart.setAvatar in parts.ts
const mentionedRoom = getRoomFromCompletion(completion, client);
Expand All @@ -132,7 +158,14 @@ export function getMentionAttributes(
}

attributes.set("data-mention-type", completion.type);
attributes.set("style", `--avatar-background: url(${avatarUrl}); --avatar-letter: '${initialLetter}'`);
attributes.set(
"style",
getCSSProperties({
url: avatarUrl,
initialLetter,
id: mentionedRoom?.roomId ?? aliasFromCompletion,
}),
);
} else if (completion.type === "at-room") {
// logic as used in RoomPillPart.setAvatar in parts.ts, but now we know the current room
// from the arguments passed
Expand All @@ -145,7 +178,14 @@ export function getMentionAttributes(
}

attributes.set("data-mention-type", completion.type);
attributes.set("style", `--avatar-background: url(${avatarUrl}); --avatar-letter: '${initialLetter}'`);
attributes.set(
"style",
getCSSProperties({
url: avatarUrl,
initialLetter,
id: room.roomId,
}),
);
}

return attributes;
Expand Down
7 changes: 7 additions & 0 deletions test/components/structures/__snapshots__/RoomView-test.tsx.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ exports[`RoomView for a local room in state CREATING should match the snapshot 1
data-type="round"
role="presentation"
style="--cpd-avatar-size: 24px;"
title=""
>
u
</span>
Expand Down Expand Up @@ -106,6 +107,7 @@ exports[`RoomView for a local room in state ERROR should match the snapshot 1`]
data-type="round"
role="presentation"
style="--cpd-avatar-size: 24px;"
title=""
>
u
</span>
Expand Down Expand Up @@ -185,6 +187,7 @@ exports[`RoomView for a local room in state ERROR should match the snapshot 1`]
data-type="round"
role="button"
style="--cpd-avatar-size: 52px;"
title=""
>
u
</button>
Expand Down Expand Up @@ -273,6 +276,7 @@ exports[`RoomView for a local room in state NEW should match the snapshot 1`] =
data-type="round"
role="presentation"
style="--cpd-avatar-size: 24px;"
title=""
>
u
</span>
Expand Down Expand Up @@ -352,6 +356,7 @@ exports[`RoomView for a local room in state NEW should match the snapshot 1`] =
data-type="round"
role="button"
style="--cpd-avatar-size: 52px;"
title=""
>
u
</button>
Expand Down Expand Up @@ -515,6 +520,7 @@ exports[`RoomView for a local room in state NEW that is encrypted should match t
data-type="round"
role="presentation"
style="--cpd-avatar-size: 24px;"
title=""
>
u
</span>
Expand Down Expand Up @@ -593,6 +599,7 @@ exports[`RoomView for a local room in state NEW that is encrypted should match t
data-type="round"
role="button"
style="--cpd-avatar-size: 52px;"
title=""
>
u
</button>
Expand Down
4 changes: 4 additions & 0 deletions test/components/structures/__snapshots__/SpaceHierarchy-test.tsx.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ exports[`SpaceHierarchy <SpaceHierarchy /> renders 1`] = `
data-type="round"
role="presentation"
style="--cpd-avatar-size: 20px;"
title=""
>
U
</span>
Expand Down Expand Up @@ -147,6 +148,7 @@ exports[`SpaceHierarchy <SpaceHierarchy /> renders 1`] = `
data-type="round"
role="presentation"
style="--cpd-avatar-size: 20px;"
title=""
>
U
</span>
Expand Down Expand Up @@ -218,6 +220,7 @@ exports[`SpaceHierarchy <SpaceHierarchy /> renders 1`] = `
data-type="round"
role="presentation"
style="--cpd-avatar-size: 20px;"
title=""
>
N
</span>
Expand Down Expand Up @@ -295,6 +298,7 @@ exports[`SpaceHierarchy <SpaceHierarchy /> renders 1`] = `
data-type="round"
role="presentation"
style="--cpd-avatar-size: 20px;"
title=""
>
N
</span>
Expand Down
1 change: 1 addition & 0 deletions test/components/structures/__snapshots__/UserMenu-test.tsx.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ exports[`<UserMenu> when rendered should render as expected 1`] = `
data-type="round"
role="presentation"
style="--cpd-avatar-size: 32px;"
title=""
>
u
</span>
Expand Down
3 changes: 3 additions & 0 deletions test/components/views/avatars/__snapshots__/RoomAvatar-test.tsx.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ exports[`RoomAvatar should render as expected for a DM room 1`] = `
data-type="round"
role="presentation"
style="--cpd-avatar-size: 36px;"
title=""
>
D
</span>
Expand All @@ -24,6 +25,7 @@ exports[`RoomAvatar should render as expected for a LocalRoom 1`] = `
data-type="round"
role="presentation"
style="--cpd-avatar-size: 36px;"
title=""
>
l
</span>
Expand All @@ -39,6 +41,7 @@ exports[`RoomAvatar should render as expected for a Room 1`] = `
data-type="round"
role="presentation"
style="--cpd-avatar-size: 36px;"
title=""
>
t
</span>
Expand Down
1 change: 1 addition & 0 deletions test/components/views/beacon/__snapshots__/DialogSidebar-test.tsx.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ exports[`<DialogSidebar /> renders sidebar correctly with beacons 1`] = `
data-type="round"
role="presentation"
style="--cpd-avatar-size: 32px;"
title=""
>

</span>
Expand Down
1 change: 1 addition & 0 deletions test/components/views/dialogs/__snapshots__/ManageRestrictedJoinRuleDialog-test.tsx.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ exports[`<ManageRestrictedJoinRuleDialog /> should list spaces which are not par
data-type="round"
role="presentation"
style="--cpd-avatar-size: 20px;"
title=""
>
O
</span>
Expand Down
4 changes: 4 additions & 0 deletions test/components/views/elements/__snapshots__/AppTile-test.tsx.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@ exports[`AppTile for a pinned widget should render 1`] = `
data-testid="avatar-img"
data-type="round"
style="--cpd-avatar-size: 20px;"
title=""
>
<img
alt=""
Expand Down Expand Up @@ -197,6 +198,7 @@ exports[`AppTile for a pinned widget should render permission request 1`] = `
data-testid="avatar-img"
data-type="round"
style="--cpd-avatar-size: 20px;"
title=""
>
<img
alt=""
Expand Down Expand Up @@ -277,6 +279,7 @@ exports[`AppTile for a pinned widget should render permission request 1`] = `
data-type="round"
role="presentation"
style="--cpd-avatar-size: 38px;"
title=""
>
u
</span>
Expand Down Expand Up @@ -352,6 +355,7 @@ exports[`AppTile preserves non-persisted widget on container move 1`] = `
data-testid="avatar-img"
data-type="round"
style="--cpd-avatar-size: 20px;"
title=""
>
<img
alt=""
Expand Down
1 change: 1 addition & 0 deletions test/components/views/elements/__snapshots__/FacePile-test.tsx.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ exports[`<FacePile /> renders with a tooltip 1`] = `
data-type="round"
role="presentation"
style="--cpd-avatar-size: 36px;"
title=""
>
4
</span>
Expand Down
Loading
Loading