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

Fix colour of avatar and colour matching with username #11470

Merged
merged 8 commits into from
Aug 24, 2023
Merged

Fix colour of avatar and colour matching with username #11470

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
e3df662
Use consistent colouring of username with avatar
Aug 24, 2023
ec16765
Upgrade Compound to fix Firefox issue
Aug 24, 2023
e580440
Use the approapriate color shade for usernames
Aug 24, 2023
45e9671
Use the approapriate color shade for usernames
Aug 24, 2023
d8129e5
Merge branch 'develop' into germain-gg/tooltip-fix
Aug 24, 2023
57e055b
Merge branch 'develop' into germain-gg/tooltip-fix
Aug 24, 2023
f1ab043
Upgrade Compound
Aug 24, 2023
c2ad167
Fix tests
Aug 24, 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
16 changes: 8 additions & 8 deletions res/themes/legacy-light/css/_legacy-light.pcss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -186,14 +186,14 @@ $call-background: #15191e;
$call-primary-content: #ffffff;
$call-light-quaternary-content: #c1c6cd;

$username-variant1-color: var(--cpd-color-blue-900);
$username-variant2-color: var(--cpd-color-fuchsia-900);
$username-variant3-color: var(--cpd-color-green-900);
$username-variant4-color: var(--cpd-color-pink-900);
$username-variant5-color: var(--cpd-color-orange-900);
$username-variant6-color: var(--cpd-color-cyan-900);
$username-variant7-color: var(--cpd-color-purple-900);
$username-variant8-color: var(--cpd-color-lime-900);
$username-variant1-color: var(--cpd-color-blue-1200);
$username-variant2-color: var(--cpd-color-fuchsia-1200);
$username-variant3-color: var(--cpd-color-green-1200);
$username-variant4-color: var(--cpd-color-pink-1200);
$username-variant5-color: var(--cpd-color-orange-1200);
$username-variant6-color: var(--cpd-color-cyan-1200);
$username-variant7-color: var(--cpd-color-purple-1200);
$username-variant8-color: var(--cpd-color-lime-1200);

/**
* Creating a `semantic` color scale. This will not be needed with the new
Expand Down
16 changes: 8 additions & 8 deletions res/themes/light-high-contrast/css/_light-high-contrast.pcss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,14 @@ $tertiary-content: var(--cpd-color-gray-800);
$quaternary-content: var(--cpd-color-gray-600);
$quinary-content: var(--cpd-color-gray-400);

$username-variant1-color: var(--cpd-color-blue-900);
$username-variant2-color: var(--cpd-color-fuchsia-900);
$username-variant3-color: var(--cpd-color-green-900);
$username-variant4-color: var(--cpd-color-pink-900);
$username-variant5-color: var(--cpd-color-orange-900);
$username-variant6-color: var(--cpd-color-cyan-900);
$username-variant7-color: var(--cpd-color-purple-900);
$username-variant8-color: var(--cpd-color-lime-900);
$username-variant1-color: var(--cpd-color-blue-1200);
$username-variant2-color: var(--cpd-color-fuchsia-1200);
$username-variant3-color: var(--cpd-color-green-1200);
$username-variant4-color: var(--cpd-color-pink-1200);
$username-variant5-color: var(--cpd-color-orange-1200);
$username-variant6-color: var(--cpd-color-cyan-1200);
$username-variant7-color: var(--cpd-color-purple-1200);
$username-variant8-color: var(--cpd-color-lime-1200);

$accent-alt: $links;
$input-border-color: $secondary-content;
Expand Down
16 changes: 8 additions & 8 deletions res/themes/light/css/_light.pcss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,14 +36,14 @@ $alert: var(--cpd-color-text-critical-primary);
$links: var(--cpd-color-text-link-external);
$link-external: var(--cpd-color-text-link-external);

$username-variant1-color: var(--cpd-color-blue-900);
$username-variant2-color: var(--cpd-color-fuchsia-900);
$username-variant3-color: var(--cpd-color-green-900);
$username-variant4-color: var(--cpd-color-pink-900);
$username-variant5-color: var(--cpd-color-orange-900);
$username-variant6-color: var(--cpd-color-cyan-900);
$username-variant7-color: var(--cpd-color-purple-900);
$username-variant8-color: var(--cpd-color-lime-900);
$username-variant1-color: var(--cpd-color-blue-1200);
$username-variant2-color: var(--cpd-color-fuchsia-1200);
$username-variant3-color: var(--cpd-color-green-1200);
$username-variant4-color: var(--cpd-color-pink-1200);
$username-variant5-color: var(--cpd-color-orange-1200);
$username-variant6-color: var(--cpd-color-cyan-1200);
$username-variant7-color: var(--cpd-color-purple-1200);
$username-variant8-color: var(--cpd-color-lime-1200);
/* ******************** */

/**
Expand Down
2 changes: 1 addition & 1 deletion src/components/views/messages/DisambiguatedProfile.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ export default class DisambiguatedProfile extends React.Component<IProps> {

let colorClass: string | undefined;
if (colored) {
colorClass = getUserNameColorClass(fallbackName);
colorClass = getUserNameColorClass(mxid ?? "");
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 test for this? I'd love to see one if not. Happy for you to merge and add it after if needed though.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will do that! thank you 🙏

}

let mxidElement;
Expand Down
28 changes: 5 additions & 23 deletions src/utils/FormattingUtils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ limitations under the License.
*/

import { ReactElement, ReactNode } from "react";
import { useIdColorHash } from "@vector-im/compound-web";

import { _t, getCurrentLanguage } from "../languageHandler";
import { jsxJoin } from "./ReactUtils";
Expand Down Expand Up @@ -73,30 +74,11 @@ export function formatBytes(bytes: number, decimals = 2): string {
export function formatCryptoKey(key: string): string {
return key.match(/.{1,4}/g)!.join(" ");
}
/**
* calculates a numeric hash for a given string
*
* @param {string} str string to hash
*
* @return {number}
*/
export function hashCode(str?: string): number {
let hash = 0;
let chr: number;
if (!str?.length) {
return hash;
}
for (let i = 0; i < str.length; i++) {
chr = str.charCodeAt(i);
hash = (hash << 5) - hash + chr;
hash |= 0;
}
return Math.abs(hash);
}

export function getUserNameColorClass(userId?: string): string {
const colorNumber = (hashCode(userId) % 8) + 1;
return `mx_Username_color${colorNumber}`;
export function getUserNameColorClass(userId: string): string {
// eslint-disable-next-line react-hooks/rules-of-hooks
Copy link
Contributor

Choose a reason for hiding this comment

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

Can I have a comment explaining why this is OK to disable?

Copy link
Contributor

Choose a reason for hiding this comment

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

Same comment about a test as above.

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 function starts with use but isn't actually a hook. The underlying class is tested within Compound but not here. I might land this to fix the dev build and get back to it

Copy link
Contributor

Choose a reason for hiding this comment

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

Great, can we have that comment in the code please?

const number = useIdColorHash(userId);
return `mx_Username_color${number}`;
}

/**
Expand Down
2 changes: 1 addition & 1 deletion test/components/structures/__snapshots__/MessagePanel-test.tsx.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ exports[`MessagePanel should handle lots of membership events quickly 1`] = `
class="mx_GenericEventListSummary_avatars"
>
<span
class="_avatar_i91o9_17 mx_BaseAvatar"
class="_avatar_2lhia_17 mx_BaseAvatar _avatar-imageless_2lhia_56"
data-color="7"
data-testid="avatar-img"
data-type="round"
Expand Down
14 changes: 7 additions & 7 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 @@ -18,7 +18,7 @@ exports[`RoomView for a local room in state CREATING should match the snapshot 1
class="mx_DecoratedRoomAvatar"
>
<span
class="_avatar_i91o9_17 mx_BaseAvatar"
class="_avatar_2lhia_17 mx_BaseAvatar _avatar-imageless_2lhia_56"
data-color="3"
data-testid="avatar-img"
data-type="round"
Expand Down Expand Up @@ -100,7 +100,7 @@ exports[`RoomView for a local room in state ERROR should match the snapshot 1`]
class="mx_DecoratedRoomAvatar"
>
<span
class="_avatar_i91o9_17 mx_BaseAvatar"
class="_avatar_2lhia_17 mx_BaseAvatar _avatar-imageless_2lhia_56"
data-color="3"
data-testid="avatar-img"
data-type="round"
Expand Down Expand Up @@ -179,7 +179,7 @@ exports[`RoomView for a local room in state ERROR should match the snapshot 1`]
<button
aria-label="Avatar"
aria-live="off"
class="_avatar_i91o9_17 mx_BaseAvatar"
class="_avatar_2lhia_17 mx_BaseAvatar _avatar-imageless_2lhia_56"
data-color="3"
data-testid="avatar-img"
data-type="round"
Expand Down Expand Up @@ -267,7 +267,7 @@ exports[`RoomView for a local room in state NEW should match the snapshot 1`] =
class="mx_DecoratedRoomAvatar"
>
<span
class="_avatar_i91o9_17 mx_BaseAvatar"
class="_avatar_2lhia_17 mx_BaseAvatar _avatar-imageless_2lhia_56"
data-color="3"
data-testid="avatar-img"
data-type="round"
Expand Down Expand Up @@ -346,7 +346,7 @@ exports[`RoomView for a local room in state NEW should match the snapshot 1`] =
<button
aria-label="Avatar"
aria-live="off"
class="_avatar_i91o9_17 mx_BaseAvatar"
class="_avatar_2lhia_17 mx_BaseAvatar _avatar-imageless_2lhia_56"
data-color="3"
data-testid="avatar-img"
data-type="round"
Expand Down Expand Up @@ -509,7 +509,7 @@ exports[`RoomView for a local room in state NEW that is encrypted should match t
class="mx_DecoratedRoomAvatar"
>
<span
class="_avatar_i91o9_17 mx_BaseAvatar"
class="_avatar_2lhia_17 mx_BaseAvatar _avatar-imageless_2lhia_56"
data-color="3"
data-testid="avatar-img"
data-type="round"
Expand Down Expand Up @@ -587,7 +587,7 @@ exports[`RoomView for a local room in state NEW that is encrypted should match t
<button
aria-label="Avatar"
aria-live="off"
class="_avatar_i91o9_17 mx_BaseAvatar"
class="_avatar_2lhia_17 mx_BaseAvatar _avatar-imageless_2lhia_56"
data-color="3"
data-testid="avatar-img"
data-type="round"
Expand Down
8 changes: 4 additions & 4 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 @@ -71,7 +71,7 @@ exports[`SpaceHierarchy <SpaceHierarchy /> renders 1`] = `
class="mx_SpaceHierarchy_roomTile_avatar"
>
<span
class="_avatar_i91o9_17 mx_BaseAvatar"
class="_avatar_2lhia_17 mx_BaseAvatar _avatar-imageless_2lhia_56"
data-color="7"
data-testid="avatar-img"
data-type="round"
Expand Down Expand Up @@ -141,7 +141,7 @@ exports[`SpaceHierarchy <SpaceHierarchy /> renders 1`] = `
class="mx_SpaceHierarchy_roomTile_avatar"
>
<span
class="_avatar_i91o9_17 mx_BaseAvatar"
class="_avatar_2lhia_17 mx_BaseAvatar _avatar-imageless_2lhia_56"
data-color="8"
data-testid="avatar-img"
data-type="round"
Expand Down Expand Up @@ -212,7 +212,7 @@ exports[`SpaceHierarchy <SpaceHierarchy /> renders 1`] = `
class="mx_SpaceHierarchy_roomTile_avatar"
>
<span
class="_avatar_i91o9_17 mx_BaseAvatar"
class="_avatar_2lhia_17 mx_BaseAvatar _avatar-imageless_2lhia_56"
data-color="8"
data-testid="avatar-img"
data-type="round"
Expand Down Expand Up @@ -289,7 +289,7 @@ exports[`SpaceHierarchy <SpaceHierarchy /> renders 1`] = `
class="mx_SpaceHierarchy_roomTile_avatar"
>
<span
class="_avatar_i91o9_17 mx_BaseAvatar"
class="_avatar_2lhia_17 mx_BaseAvatar _avatar-imageless_2lhia_56"
data-color="2"
data-testid="avatar-img"
data-type="round"
Expand Down
2 changes: 1 addition & 1 deletion 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 @@ -18,7 +18,7 @@ exports[`<UserMenu> when rendered should render as expected 1`] = `
class="mx_UserMenu_userAvatar"
>
<span
class="_avatar_i91o9_17 mx_BaseAvatar mx_UserMenu_userAvatar_BaseAvatar"
class="_avatar_2lhia_17 mx_BaseAvatar mx_UserMenu_userAvatar_BaseAvatar _avatar-imageless_2lhia_56"
data-color="2"
data-testid="avatar-img"
data-type="round"
Expand Down
6 changes: 3 additions & 3 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 @@ -3,7 +3,7 @@
exports[`RoomAvatar should render as expected for a DM room 1`] = `
<div>
<span
class="_avatar_i91o9_17 mx_BaseAvatar"
class="_avatar_2lhia_17 mx_BaseAvatar _avatar-imageless_2lhia_56"
data-color="1"
data-testid="avatar-img"
data-type="round"
Expand All @@ -18,7 +18,7 @@ exports[`RoomAvatar should render as expected for a DM room 1`] = `
exports[`RoomAvatar should render as expected for a LocalRoom 1`] = `
<div>
<span
class="_avatar_i91o9_17 mx_BaseAvatar"
class="_avatar_2lhia_17 mx_BaseAvatar _avatar-imageless_2lhia_56"
data-color="7"
data-testid="avatar-img"
data-type="round"
Expand All @@ -33,7 +33,7 @@ exports[`RoomAvatar should render as expected for a LocalRoom 1`] = `
exports[`RoomAvatar should render as expected for a Room 1`] = `
<div>
<span
class="_avatar_i91o9_17 mx_BaseAvatar"
class="_avatar_2lhia_17 mx_BaseAvatar _avatar-imageless_2lhia_56"
data-color="2"
data-testid="avatar-img"
data-type="round"
Expand Down
4 changes: 2 additions & 2 deletions test/components/views/beacon/__snapshots__/BeaconMarker-test.tsx.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,14 @@ exports[`<BeaconMarker /> renders marker when beacon has location 1`] = `
<DocumentFragment>
<span>
<div
class="mx_Marker mx_Username_color4"
class="mx_Marker mx_Username_color8"
id="!room:server_@alice:server"
>
<div
class="mx_Marker_border"
>
<span
class="_avatar_i91o9_17 mx_BaseAvatar"
class="_avatar_2lhia_17 mx_BaseAvatar _avatar-imageless_2lhia_56"
data-color="8"
data-testid="avatar-img"
data-type="round"
Expand Down
2 changes: 1 addition & 1 deletion test/components/views/beacon/__snapshots__/BeaconViewDialog-test.tsx.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ exports[`<BeaconViewDialog /> renders own beacon status when user is live sharin
class="mx_DialogOwnBeaconStatus"
>
<span
class="_avatar_i91o9_17 mx_BaseAvatar mx_DialogOwnBeaconStatus_avatar"
class="_avatar_2lhia_17 mx_BaseAvatar mx_DialogOwnBeaconStatus_avatar _avatar-imageless_2lhia_56"
data-color="8"
data-testid="avatar-img"
data-type="round"
Expand Down
2 changes: 1 addition & 1 deletion 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 @@ -32,7 +32,7 @@ exports[`<DialogSidebar /> renders sidebar correctly with beacons 1`] = `
class="mx_BeaconListItem"
>
<span
class="_avatar_i91o9_17 mx_BaseAvatar mx_BeaconListItem_avatar"
class="_avatar_2lhia_17 mx_BaseAvatar mx_BeaconListItem_avatar _avatar-imageless_2lhia_56"
data-color="1"
data-testid="avatar-img"
data-type="round"
Expand Down
2 changes: 1 addition & 1 deletion test/components/views/dialogs/__snapshots__/ConfirmUserActionDialog-test.tsx.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ exports[`ConfirmUserActionDialog renders 1`] = `
class="mx_ConfirmUserActionDialog_avatar"
>
<span
class="_avatar_i91o9_17 mx_BaseAvatar"
class="_avatar_2lhia_17 mx_BaseAvatar _avatar-imageless_2lhia_56"
data-color="7"
data-testid="avatar-img"
data-type="round"
Expand Down
2 changes: 1 addition & 1 deletion 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 @@ -71,7 +71,7 @@ exports[`<ManageRestrictedJoinRuleDialog /> should list spaces which are not par
<div>
<div>
<span
class="_avatar_i91o9_17 mx_BaseAvatar"
class="_avatar_2lhia_17 mx_BaseAvatar _avatar-imageless_2lhia_56"
data-color="7"
data-testid="avatar-img"
data-type="round"
Expand Down
14 changes: 7 additions & 7 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 @@ -104,15 +104,15 @@ exports[`AppTile for a pinned widget should render 1`] = `
<span>
<span
aria-label="Avatar"
class="_avatar_i91o9_17 mx_BaseAvatar mx_WidgetAvatar"
class="_avatar_2lhia_17 mx_BaseAvatar mx_WidgetAvatar"
data-color="1"
data-testid="avatar-img"
data-type="round"
style="--cpd-avatar-size: 20px;"
>
<img
alt=""
class="_image_i91o9_45"
class="_image_2lhia_45"
crossorigin="anonymous"
data-type="round"
height="20px"
Expand Down Expand Up @@ -192,15 +192,15 @@ exports[`AppTile for a pinned widget should render permission request 1`] = `
<span>
<span
aria-label="Avatar"
class="_avatar_i91o9_17 mx_BaseAvatar mx_WidgetAvatar"
class="_avatar_2lhia_17 mx_BaseAvatar mx_WidgetAvatar"
data-color="1"
data-testid="avatar-img"
data-type="round"
style="--cpd-avatar-size: 20px;"
>
<img
alt=""
class="_image_i91o9_45"
class="_image_2lhia_45"
crossorigin="anonymous"
data-type="round"
height="20px"
Expand Down Expand Up @@ -271,7 +271,7 @@ exports[`AppTile for a pinned widget should render permission request 1`] = `
</div>
<div>
<span
class="_avatar_i91o9_17 mx_BaseAvatar"
class="_avatar_2lhia_17 mx_BaseAvatar _avatar-imageless_2lhia_56"
data-color="1"
data-testid="avatar-img"
data-type="round"
Expand Down Expand Up @@ -347,15 +347,15 @@ exports[`AppTile preserves non-persisted widget on container move 1`] = `
<span>
<span
aria-label="Avatar"
class="_avatar_i91o9_17 mx_BaseAvatar mx_WidgetAvatar"
class="_avatar_2lhia_17 mx_BaseAvatar mx_WidgetAvatar"
data-color="1"
data-testid="avatar-img"
data-type="round"
style="--cpd-avatar-size: 20px;"
>
<img
alt=""
class="_image_i91o9_45"
class="_image_2lhia_45"
crossorigin="anonymous"
data-type="round"
height="20px"
Expand Down
2 changes: 1 addition & 1 deletion 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 @@ -11,7 +11,7 @@ exports[`<FacePile /> renders with a tooltip 1`] = `
tabindex="0"
>
<span
class="_avatar_i91o9_17 mx_BaseAvatar"
class="_avatar_2lhia_17 mx_BaseAvatar _avatar-imageless_2lhia_56"
data-color="8"
data-testid="avatar-img"
data-type="round"
Expand Down
Loading
Loading