diff --git a/playwright/snapshots/settings/general-room-settings-tab.spec.ts/General-room-settings-tab-should-be-rendered-properly-1-linux.png b/playwright/snapshots/settings/general-room-settings-tab.spec.ts/General-room-settings-tab-should-be-rendered-properly-1-linux.png index 253b230419..47a9733ccf 100644 Binary files a/playwright/snapshots/settings/general-room-settings-tab.spec.ts/General-room-settings-tab-should-be-rendered-properly-1-linux.png and b/playwright/snapshots/settings/general-room-settings-tab.spec.ts/General-room-settings-tab-should-be-rendered-properly-1-linux.png differ diff --git a/playwright/snapshots/settings/general-user-settings-tab.spec.ts/general-linux.png b/playwright/snapshots/settings/general-user-settings-tab.spec.ts/general-linux.png index f6463ffd22..0cea98d8c2 100644 Binary files a/playwright/snapshots/settings/general-user-settings-tab.spec.ts/general-linux.png and b/playwright/snapshots/settings/general-user-settings-tab.spec.ts/general-linux.png differ diff --git a/playwright/snapshots/settings/general-user-settings-tab.spec.ts/general-smallscreen-linux.png b/playwright/snapshots/settings/general-user-settings-tab.spec.ts/general-smallscreen-linux.png index 7f22193662..1035a54f6f 100644 Binary files a/playwright/snapshots/settings/general-user-settings-tab.spec.ts/general-smallscreen-linux.png and b/playwright/snapshots/settings/general-user-settings-tab.spec.ts/general-smallscreen-linux.png differ diff --git a/res/css/views/settings/_AvatarSetting.pcss b/res/css/views/settings/_AvatarSetting.pcss index a6d70a697a..4627921de5 100644 --- a/res/css/views/settings/_AvatarSetting.pcss +++ b/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; -} diff --git a/res/img/feather-customised/user.svg b/res/img/feather-customised/user.svg deleted file mode 100644 index 210ef99e6a..0000000000 --- a/res/img/feather-customised/user.svg +++ /dev/null @@ -1,11 +0,0 @@ - - - - - - - - - - - diff --git a/src/components/views/avatars/RoomAvatar.tsx b/src/components/views/avatars/RoomAvatar.tsx index 22dbb4dcf6..8380fb477a 100644 --- a/src/components/views/avatars/RoomAvatar.tsx +++ b/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 { public static defaultProps = { size: "36px", @@ -117,17 +130,10 @@ export default class RoomAvatar extends React.Component { 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 { diff --git a/src/components/views/room_settings/RoomProfileSettings.tsx b/src/components/views/room_settings/RoomProfileSettings.tsx index 7923dbfa22..bb229cc2a0 100644 --- a/src/components/views/room_settings/RoomProfileSettings.tsx +++ b/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 disabled={!this.state.canSetAvatar} onChange={this.onAvatarChanged} removeAvatar={this.removeAvatar} + placeholderId={idNameForRoom(MatrixClientPeg.safeGet().getRoom(this.props.roomId)!)} + placeholderName={this.state.displayName} /> {profileSettingsButtons} diff --git a/src/components/views/settings/AvatarSetting.tsx b/src/components/views/settings/AvatarSetting.tsx index 99b4d6020f..cdec5fdfed 100644 --- a/src/components/views/settings/AvatarSetting.tsx +++ b/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 = ({ avatar, avatarAltText, onChange, removeAvatar, disabled }) => { +const AvatarSetting: React.FC = ({ + avatar, + avatarAltText, + onChange, + removeAvatar, + disabled, + placeholderId, + placeholderName, +}) => { const fileInputRef = createRef(); // 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 = ({ avatar, avatarAltText, onChange, remo aria-labelledby={disabled ? undefined : a11yId} // Inhibit tab stop as we have explicit upload/remove buttons tabIndex={-1} - /> + > + + ); if (avatarURL) { avatarElement = ( diff --git a/src/components/views/settings/UserProfileSettings.tsx b/src/components/views/settings/UserProfileSettings.tsx index 9e54e1485a..34da232a01 100644 --- a/src/components/views/settings/UserProfileSettings.tsx +++ b/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( {_t("settings|general|avatar_remove_progress")}, ); 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) => { @@ -126,19 +127,19 @@ const UserProfileSettings: React.FC = () => { const onDisplayNameSave = useCallback(async (): Promise => { 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() ?? ""} /> ", () => { const userId = "@alice:server.org"; @@ -62,6 +63,11 @@ describe("", () => { }); 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) => diff --git a/test/components/views/room_settings/RoomProfileSettings-test.tsx b/test/components/views/room_settings/RoomProfileSettings-test.tsx index f52c2eff94..a7406ede7c 100644 --- a/test/components/views/room_settings/RoomProfileSettings-test.tsx +++ b/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); }); diff --git a/test/components/views/settings/AvatarSetting-test.tsx b/test/components/views/settings/AvatarSetting-test.tsx index d2ed79d889..59218eb28d 100644 --- a/test/components/views/settings/AvatarSetting-test.tsx +++ b/test/components/views/settings/AvatarSetting-test.tsx @@ -32,7 +32,12 @@ describe("", () => { it("renders avatar with specified alt text", async () => { const { queryByAltText } = render( - , + , ); const imgElement = queryByAltText("Avatar of Peter Fox"); @@ -45,6 +50,8 @@ describe("", () => { avatarAltText="Avatar of Peter Fox" avatar="mxc://example.org/my-avatar" removeAvatar={jest.fn()} + placeholderId="blee" + placeholderName="boo" />, ); @@ -53,14 +60,28 @@ describe("", () => { }); it("renders avatar without remove button", async () => { - const { queryByText } = render(); + const { queryByText } = render( + , + ); const removeButton = queryByText("Remove"); expect(removeButton).toBeNull(); }); it("renders a file as the avatar when supplied", async () => { - render(); + render( + , + ); const imgElement = await screen.findByRole("button", { name: "Avatar of Peter Fox" }); expect(imgElement).toBeInTheDocument(); @@ -73,6 +94,8 @@ describe("", () => { render( void; @@ -62,6 +63,16 @@ jest.mock("@vector-im/compound-web", () => ({ }) as React.FC, })); +const renderProfileSettings = (toastRack: Partial, client: MatrixClient) => { + return render( + + + + + , + ); +}; + describe("ProfileSettings", () => { let client: MatrixClient; let toastRack: Partial; @@ -74,11 +85,7 @@ describe("ProfileSettings", () => { }); it("removes avatar", async () => { - render( - - - , - ); + renderProfileSettings(toastRack, client); expect(await screen.findByText("Mocked AvatarSetting")).toBeInTheDocument(); expect(removeAvatarFn).toBeDefined(); @@ -91,11 +98,7 @@ describe("ProfileSettings", () => { }); it("changes avatar", async () => { - render( - - - , - ); + 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( - - - , - ); + 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( - - - , - ); + 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( - - - , - ); + renderProfileSettings(toastRack, client); expect(await screen.findByText("Mocked EditInPlace: Alice")).toBeInTheDocument(); expect(editInPlaceOnChange).toBeDefined(); diff --git a/test/components/views/settings/tabs/user/GeneralUserSettingsTab-test.tsx b/test/components/views/settings/tabs/user/GeneralUserSettingsTab-test.tsx index 9ed18fa225..2532e98c60 100644 --- a/test/components/views/settings/tabs/user/GeneralUserSettingsTab-test.tsx +++ b/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("", () => { const defaultProps = { @@ -48,9 +49,11 @@ describe("", () => { let stores: SdkContextClass; const getComponent = () => ( - - - + + + + + ); beforeEach(() => {