Fix display of no avatar in avatar setting controls (#12558)

* New user profile UI in User Settings

Using new Edit In Place component.

* Show avatar upload error

* Fix avatar upload error

* Wire up errors & feedback for display name setting

* Implement avatar upload / remove progress toast

* Add 768px breakpoint

* Fix display of no avatar in avatar setting controls

There was supposed to be a person icon but it was invisible, and also
would have been inappropriate for room avatars anyway.

This makes it match the designs by being the same as whatever the
default avatar is.

* Fix room profile display

* Update to released compund-web with required components / fixes

* Require compound-web 4.4.0

because we do need it

* Update snapshots

Because of course all the auto-generated IDs of unrelated things
have changed.

* Fix duplicate import

* Fix CSS comment

* Update snapshot

* Run all the tests so the ids stay the same

* Start of a test for ProfileSettings

* More tests

* Test that a toast appears

* Test ToastRack

* Update snapshots

* Add the usernamee control

* Fix playwright tests

 * New compound version for editinplace fixes
 * Fix useId to not just generate a constant ID
 * Use the label in the username component
 * Fix widths of test boxes
 * Update screenshots

* Put ^ back on compound-web version

* Split CSS for room & user profile settings

and name the components correspondingly

* Fix playwright test

* Update room settings screenshot

* Use original screenshot instead

* Add required props in test

* Fix test

* Also here

* Update screenshots

* Remove user icon

...which is unused now, as far as I can see.

* Fix styling of unrelated buttons

Needed to be added in other places otherwise the specificity changes.

Also put the old screenshots back.

* Add copyright year

* Fix copyright year

* Switch to useMatrixClientContext

* Fix other test

res/css/views/settings/_AvatarSetting.pcss

@@ -68,28 +68,12 @@ limitations under the License.
     }
 
     & > img {
-        cursor: pointer;
-        object-fit: cover;
-    }
-
-    & > img,
-    .mx_AvatarSetting_avatarPlaceholder {
         display: block;
         height: 90px;
         width: inherit;
         border-radius: 90px;
         cursor: pointer;
-    }
-
-    .mx_AvatarSetting_avatarPlaceholder::before {
-        background-color: $quinary-content;
-        mask: url("$(res)/img/feather-customised/user.svg");
-        mask-repeat: no-repeat;
-        mask-size: 36px;
-        mask-position: center;
-        content: "";
-        position: absolute;
-        inset: 0;
+        object-fit: cover;
     }
 
     .mx_AvatarSetting_uploadButton {
@@ -115,7 +99,3 @@ limitations under the License.
         mask-image: url("$(res)/img/feather-customised/edit.svg");
     }
 }
-
-.mx_AvatarSetting_avatar .mx_AvatarSetting_avatarPlaceholder {
-    background-color: $system;
-}

res/img/feather-customised/user.svg

@@ -1,11 +0,0 @@
-
-<svg width="15px" height="16px" viewBox="0 0 15 16" version="1.1" xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink">
-    <g id="Page-1" stroke="none" stroke-width="1" fill="none" fill-rule="evenodd" stroke-linecap="round" stroke-linejoin="round">
-        <g id="LifeBuoy" transform="translate(-1406.000000, -91.000000)" stroke="#B8BEC9" stroke-width="1">
-            <g id="user-copy" transform="translate(1407.000000, 92.000000)">
-                <path d="M13,14 L13,12.4444444 C13,10.7262252 11.5449254,9.33333333 9.75,9.33333333 L3.25,9.33333333 C1.45507456,9.33333333 0,10.7262252 0,12.4444444 L0,14" id="Path"></path>
-                <ellipse id="Oval" cx="6.5" cy="3.11111111" rx="3.25" ry="3.11111111"></ellipse>
-            </g>
-        </g>
-    </g>
-</svg>

src/components/views/avatars/RoomAvatar.tsx

@@ -44,6 +44,19 @@ interface IState {
     urls: string[];
 }
 
+export function idNameForRoom(room: Room): string {
+    const dmMapUserId = DMRoomMap.shared().getUserIdForRoomId(room.roomId);
+    // If the room is a DM, we use the other user's ID for the color hash
+    // in order to match the room avatar with their avatar
+    if (dmMapUserId) return dmMapUserId;
+
+    if (room instanceof LocalRoom && room.targets.length === 1) {
+        return room.targets[0].userId;
+    }
+
+    return room.roomId;
+}
+
 export default class RoomAvatar extends React.Component<IProps, IState> {
     public static defaultProps = {
         size: "36px",
@@ -117,19 +130,12 @@ export default class RoomAvatar extends React.Component<IProps, IState> {
         const room = this.props.room;
 
         if (room) {
-            const dmMapUserId = DMRoomMap.shared().getUserIdForRoomId(room.roomId);
-            // If the room is a DM, we use the other user's ID for the color hash
-            // in order to match the room avatar with their avatar
-            if (dmMapUserId) return dmMapUserId;
-
-            if (room instanceof LocalRoom && room.targets.length === 1) {
-                return room.targets[0].userId;
+            return idNameForRoom(room);
+        } else {
+            return this.props.oobData?.roomId;
         }
     }
 
-        return this.props.room?.roomId || this.props.oobData?.roomId;
-    }
-
     public render(): React.ReactNode {
         const { room, oobData, viewAvatarOnClick, onClick, className, ...otherProps } = this.props;
         const roomName = room?.name ?? oobData.name ?? "?";

src/components/views/room_settings/RoomProfileSettings.tsx

@@ -24,6 +24,7 @@ import Field from "../elements/Field";
 import AccessibleButton, { ButtonEvent } from "../elements/AccessibleButton";
 import AvatarSetting from "../settings/AvatarSetting";
 import { htmlSerializeFromMdIfNeeded } from "../../../editor/serialize";
+import { idNameForRoom } from "../avatars/RoomAvatar";
 
 interface IProps {
     roomId: string;
@@ -254,6 +255,8 @@ export default class RoomProfileSettings extends React.Component<IProps, IState>
                         disabled={!this.state.canSetAvatar}
                         onChange={this.onAvatarChanged}
                         removeAvatar={this.removeAvatar}
+                        placeholderId={idNameForRoom(MatrixClientPeg.safeGet().getRoom(this.props.roomId)!)}
+                        placeholderName={this.state.displayName}
                     />
                 </div>
                 {profileSettingsButtons}

src/components/views/settings/AvatarSetting.tsx

@@ -21,6 +21,7 @@ import AccessibleButton from "../elements/AccessibleButton";
 import { mediaFromMxc } from "../../../customisations/Media";
 import { chromeFileInputFix } from "../../../utils/BrowserWorkarounds";
 import { useId } from "../../../utils/useId";
+import BaseAvatar from "../avatars/BaseAvatar";
 
 interface IProps {
     /**
@@ -51,12 +52,30 @@ interface IProps {
      * The alt text for the avatar
      */
     avatarAltText: string;
+
+    /**
+     * String to use for computing the colour of the placeholder avatar if no avatar is set
+     */
+    placeholderId: string;
+
+    /**
+     * String to use for the placeholder display if no avatar is set
+     */
+    placeholderName: string;
 }
 
 /**
  * Component for setting or removing an avatar on something (eg. a user or a room)
  */
-const AvatarSetting: React.FC<IProps> = ({ avatar, avatarAltText, onChange, removeAvatar, disabled }) => {
+const AvatarSetting: React.FC<IProps> = ({
+    avatar,
+    avatarAltText,
+    onChange,
+    removeAvatar,
+    disabled,
+    placeholderId,
+    placeholderName,
+}) => {
     const fileInputRef = createRef<HTMLInputElement>();
 
     // Real URL that we can supply to the img element, either a data URL or whatever mediaFromMxc gives
@@ -98,7 +117,9 @@ const AvatarSetting: React.FC<IProps> = ({ avatar, avatarAltText, onChange, remo
             aria-labelledby={disabled ? undefined : a11yId}
             // Inhibit tab stop as we have explicit upload/remove buttons
             tabIndex={-1}
-        />
+        >
+            <BaseAvatar idName={placeholderId} name={placeholderName} size="90px" />
+        </AccessibleButton>
     );
     if (avatarURL) {
         avatarElement = (

src/components/views/settings/UserProfileSettings.tsx

@@ -19,7 +19,6 @@ import { logger } from "matrix-js-sdk/src/logger";
 import { EditInPlace, Alert } from "@vector-im/compound-web";
 
 import { _t } from "../../../languageHandler";
-import { MatrixClientPeg } from "../../../MatrixClientPeg";
 import { OwnProfileStore } from "../../../stores/OwnProfileStore";
 import AvatarSetting from "./AvatarSetting";
 import PosthogTrackers from "../../../PosthogTrackers";
@@ -29,6 +28,7 @@ import InlineSpinner from "../elements/InlineSpinner";
 import UserIdentifierCustomisations from "../../../customisations/UserIdentifier";
 import { useId } from "../../../utils/useId";
 import CopyableText from "../elements/CopyableText";
+import { useMatrixClientContext } from "../../../contexts/MatrixClientContext";
 
 const SpinnerToast: React.FC = ({ children }) => (
     <>
@@ -68,28 +68,30 @@ const UserProfileSettings: React.FC = () => {
 
     const toastRack = useToastContext();
 
+    const client = useMatrixClientContext();
+
     useEffect(() => {
         (async () => {
             try {
-                const mediaConfig = await MatrixClientPeg.safeGet().getMediaConfig();
+                const mediaConfig = await client.getMediaConfig();
                 setMaxUploadSize(mediaConfig["m.upload.size"]);
             } catch (e) {
                 logger.warn("Failed to get media config", e);
             }
         })();
-    }, []);
+    }, [client]);
 
     const onAvatarRemove = useCallback(async () => {
         const removeToast = toastRack.displayToast(
             <SpinnerToast>{_t("settings|general|avatar_remove_progress")}</SpinnerToast>,
         );
         try {
-            await MatrixClientPeg.safeGet().setAvatarUrl(""); // use empty string as Synapse 500s on undefined
+            await client.setAvatarUrl(""); // use empty string as Synapse 500s on undefined
             setAvatarURL("");
         } finally {
             removeToast();
         }
-    }, [toastRack]);
+    }, [toastRack, client]);
 
     const onAvatarChange = useCallback(
         async (avatarFile: File) => {
@@ -102,7 +104,6 @@ const UserProfileSettings: React.FC = () => {
             );
             try {
                 setAvatarError(false);
-                const client = MatrixClientPeg.safeGet();
                 const { content_uri: uri } = await client.uploadContent(avatarFile);
                 await client.setAvatarUrl(uri);
                 setAvatarURL(uri);
@@ -112,7 +113,7 @@ const UserProfileSettings: React.FC = () => {
                 removeToast();
             }
         },
-        [toastRack],
+        [toastRack, client],
     );
 
     const onDisplayNameChanged = useCallback((e: ChangeEvent<HTMLInputElement>) => {
@@ -126,19 +127,19 @@ const UserProfileSettings: React.FC = () => {
     const onDisplayNameSave = useCallback(async (): Promise<void> => {
         try {
             setDisplayNameError(false);
-            await MatrixClientPeg.safeGet().setDisplayName(displayName);
+            await client.setDisplayName(displayName);
             setInitialDisplayName(displayName);
         } catch (e) {
             setDisplayNameError(true);
         }
-    }, [displayName]);
+    }, [displayName, client]);
 
     const userIdentifier = useMemo(
         () =>
-            UserIdentifierCustomisations.getDisplayUserIdentifier(MatrixClientPeg.safeGet().getSafeUserId(), {
+            UserIdentifierCustomisations.getDisplayUserIdentifier(client.getSafeUserId(), {
                 withDisplayName: true,
             }),
-        [],
+        [client],
     );
 
     return (
@@ -151,6 +152,8 @@ const UserProfileSettings: React.FC = () => {
                     avatarAltText={_t("common|user_avatar")}
                     onChange={onAvatarChange}
                     removeAvatar={onAvatarRemove}
+                    placeholderName={displayName}
+                    placeholderId={client.getUserId() ?? ""}
                 />
                 <EditInPlace
                     className="mx_UserProfileSettings_profile_displayName"

test/components/views/dialogs/RoomSettingsDialog-test.tsx

@@ -31,6 +31,7 @@ import RoomSettingsDialog from "../../../../src/components/views/dialogs/RoomSet
 import MatrixClientContext from "../../../../src/contexts/MatrixClientContext";
 import SettingsStore from "../../../../src/settings/SettingsStore";
 import { UIFeature } from "../../../../src/settings/UIFeature";
+import DMRoomMap from "../../../../src/utils/DMRoomMap";
 
 describe("<RoomSettingsDialog />", () => {
     const userId = "@alice:server.org";
@@ -62,6 +63,11 @@ describe("<RoomSettingsDialog />", () => {
         });
 
         jest.spyOn(SettingsStore, "getValue").mockReset().mockReturnValue(false);
+
+        const dmRoomMap = {
+            getUserIdForRoomId: jest.fn(),
+        } as unknown as DMRoomMap;
+        jest.spyOn(DMRoomMap, "shared").mockReturnValue(dmRoomMap);
     });
 
     const getComponent = (onFinished = jest.fn(), propRoomId = roomId) =>

test/components/views/room_settings/RoomProfileSettings-test.tsx

@@ -22,6 +22,7 @@ import { EventType, MatrixClient, MatrixEvent, Room } from "matrix-js-sdk/src/ma
 
 import { mkStubRoom, stubClient } from "../../../test-utils";
 import RoomProfileSettings from "../../../../src/components/views/room_settings/RoomProfileSettings";
+import DMRoomMap from "../../../../src/utils/DMRoomMap";
 
 const BASE64_GIF = "R0lGODlhAQABAAAAACw=";
 const AVATAR_FILE = new File([Uint8Array.from(atob(BASE64_GIF), (c) => c.charCodeAt(0))], "avatar.gif", {
@@ -35,6 +36,11 @@ describe("RoomProfileSetting", () => {
     let room: Room;
 
     beforeEach(() => {
+        const dmRoomMap = {
+            getUserIdForRoomId: jest.fn(),
+        } as unknown as DMRoomMap;
+        jest.spyOn(DMRoomMap, "shared").mockReturnValue(dmRoomMap);
+
         client = stubClient();
         room = mkStubRoom(ROOM_ID, "Test room", client);
     });

test/components/views/settings/AvatarSetting-test.tsx

@@ -32,7 +32,12 @@ describe("<AvatarSetting />", () => {
 
     it("renders avatar with specified alt text", async () => {
         const { queryByAltText } = render(
-            <AvatarSetting avatarAltText="Avatar of Peter Fox" avatar="mxc://example.org/my-avatar" />,
+            <AvatarSetting
+                placeholderId="blee"
+                placeholderName="boo"
+                avatarAltText="Avatar of Peter Fox"
+                avatar="mxc://example.org/my-avatar"
+            />,
         );
 
         const imgElement = queryByAltText("Avatar of Peter Fox");
@@ -45,6 +50,8 @@ describe("<AvatarSetting />", () => {
                 avatarAltText="Avatar of Peter Fox"
                 avatar="mxc://example.org/my-avatar"
                 removeAvatar={jest.fn()}
+                placeholderId="blee"
+                placeholderName="boo"
             />,
         );
 
@@ -53,14 +60,28 @@ describe("<AvatarSetting />", () => {
     });
 
     it("renders avatar without remove button", async () => {
-        const { queryByText } = render(<AvatarSetting disabled={true} avatarAltText="Avatar of Peter Fox" />);
+        const { queryByText } = render(
+            <AvatarSetting
+                placeholderId="blee"
+                placeholderName="boo"
+                disabled={true}
+                avatarAltText="Avatar of Peter Fox"
+            />,
+        );
 
         const removeButton = queryByText("Remove");
         expect(removeButton).toBeNull();
     });
 
     it("renders a file as the avatar when supplied", async () => {
-        render(<AvatarSetting avatarAltText="Avatar of Peter Fox" avatar={AVATAR_FILE} />);
+        render(
+            <AvatarSetting
+                placeholderId="blee"
+                placeholderName="boo"
+                avatarAltText="Avatar of Peter Fox"
+                avatar={AVATAR_FILE}
+            />,
+        );
 
         const imgElement = await screen.findByRole("button", { name: "Avatar of Peter Fox" });
         expect(imgElement).toBeInTheDocument();
@@ -73,6 +94,8 @@ describe("<AvatarSetting />", () => {
 
         render(
             <AvatarSetting
+                placeholderId="blee"
+                placeholderName="boo"
                 avatar="mxc://example.org/my-avatar"
                 avatarAltText="Avatar of Peter Fox"
                 onChange={onChange}

test/components/views/settings/UserProfileSettings-test.tsx

@@ -23,6 +23,7 @@ import UserProfileSettings from "../../../../src/components/views/settings/UserP
 import { stubClient } from "../../../test-utils";
 import { ToastContext, ToastRack } from "../../../../src/contexts/ToastContext";
 import { OwnProfileStore } from "../../../../src/stores/OwnProfileStore";
+import MatrixClientContext from "../../../../src/contexts/MatrixClientContext";
 
 interface MockedAvatarSettingProps {
     removeAvatar: () => void;
@@ -62,6 +63,16 @@ jest.mock("@vector-im/compound-web", () => ({
     }) as React.FC<MockedEditInPlaceProps>,
 }));
 
+const renderProfileSettings = (toastRack: Partial<ToastRack>, client: MatrixClient) => {
+    return render(
+        <MatrixClientContext.Provider value={client}>
+            <ToastContext.Provider value={toastRack}>
+                <UserProfileSettings />
+            </ToastContext.Provider>
+        </MatrixClientContext.Provider>,
+    );
+};
+
 describe("ProfileSettings", () => {
     let client: MatrixClient;
     let toastRack: Partial<ToastRack>;
@@ -74,11 +85,7 @@ describe("ProfileSettings", () => {
     });
 
     it("removes avatar", async () => {
-        render(
-            <ToastContext.Provider value={toastRack}>
-                <UserProfileSettings />
-            </ToastContext.Provider>,
-        );
+        renderProfileSettings(toastRack, client);
 
         expect(await screen.findByText("Mocked AvatarSetting")).toBeInTheDocument();
         expect(removeAvatarFn).toBeDefined();
@@ -91,11 +98,7 @@ describe("ProfileSettings", () => {
     });
 
     it("changes avatar", async () => {
-        render(
-            <ToastContext.Provider value={toastRack}>
-                <UserProfileSettings />
-            </ToastContext.Provider>,
-        );
+        renderProfileSettings(toastRack, client);
 
         expect(await screen.findByText("Mocked AvatarSetting")).toBeInTheDocument();
         expect(changeAvatarFn).toBeDefined();
@@ -113,11 +116,7 @@ describe("ProfileSettings", () => {
     });
 
     it("displays toast while uploading avatar", async () => {
-        render(
-            <ToastContext.Provider value={toastRack}>
-                <UserProfileSettings />
-            </ToastContext.Provider>,
-        );
+        renderProfileSettings(toastRack, client);
 
         const clearToastFn = jest.fn();
         mocked(toastRack.displayToast!).mockReturnValue(clearToastFn);
@@ -149,11 +148,7 @@ describe("ProfileSettings", () => {
     it("changes display name", async () => {
         jest.spyOn(OwnProfileStore.instance, "displayName", "get").mockReturnValue("Alice");
 
-        render(
-            <ToastContext.Provider value={toastRack}>
-                <UserProfileSettings />
-            </ToastContext.Provider>,
-        );
+        renderProfileSettings(toastRack, client);
 
         expect(await screen.findByText("Mocked EditInPlace: Alice")).toBeInTheDocument();
         expect(editInPlaceOnSave).toBeDefined();
@@ -174,11 +169,7 @@ describe("ProfileSettings", () => {
     it("resets on cancel", async () => {
         jest.spyOn(OwnProfileStore.instance, "displayName", "get").mockReturnValue("Alice");
 
-        render(
-            <ToastContext.Provider value={toastRack}>
-                <UserProfileSettings />
-            </ToastContext.Provider>,
-        );
+        renderProfileSettings(toastRack, client);
 
         expect(await screen.findByText("Mocked EditInPlace: Alice")).toBeInTheDocument();
         expect(editInPlaceOnChange).toBeDefined();

test/components/views/settings/tabs/user/GeneralUserSettingsTab-test.tsx

@@ -29,6 +29,7 @@ import {
 import { UIFeature } from "../../../../../../src/settings/UIFeature";
 import { SettingLevel } from "../../../../../../src/settings/SettingLevel";
 import { OidcClientStore } from "../../../../../../src/stores/oidc/OidcClientStore";
+import MatrixClientContext from "../../../../../../src/contexts/MatrixClientContext";
 
 describe("<GeneralUserSettingsTab />", () => {
     const defaultProps = {
@@ -48,9 +49,11 @@ describe("<GeneralUserSettingsTab />", () => {
     let stores: SdkContextClass;
 
     const getComponent = () => (
+        <MatrixClientContext.Provider value={mockClient}>
             <SDKContext.Provider value={stores}>
                 <GeneralUserSettingsTab {...defaultProps} />
             </SDKContext.Provider>
+        </MatrixClientContext.Provider>
     );
 
     beforeEach(() => {