diff --git a/playwright/e2e/settings/general-user-settings-tab.spec.ts b/playwright/e2e/settings/general-user-settings-tab.spec.ts index ff2d7c2207..41210292a3 100644 --- a/playwright/e2e/settings/general-user-settings-tab.spec.ts +++ b/playwright/e2e/settings/general-user-settings-tab.spec.ts @@ -45,14 +45,6 @@ test.describe("General user settings tab", () => { // Assert that a userId is rendered expect(uut.getByLabel("Username")).toHaveText(user.userId); - // Check avatar setting - const avatar = profile.locator(".mx_AvatarSetting_avatar"); - await avatar.hover(); - - // Hover effect - await expect(avatar.locator(".mx_AvatarSetting_hoverBg")).toBeVisible(); - await expect(avatar.locator(".mx_AvatarSetting_hover span").getByText("Upload")).toBeVisible(); - // Wait until spinners disappear await expect(uut.getByTestId("accountSection").locator(".mx_Spinner")).not.toBeVisible(); await expect(uut.getByTestId("discoverySection").locator(".mx_Spinner")).not.toBeVisible(); @@ -128,21 +120,20 @@ test.describe("General user settings tab", () => { await expect(uut).toMatchScreenshot("general-smallscreen.png"); }); - test("should support adding and removing a profile picture", async ({ uut }) => { + test("should support adding and removing a profile picture", async ({ uut, page }) => { const profileSettings = uut.locator(".mx_UserProfileSettings"); // Upload a picture await profileSettings.getByAltText("Upload").setInputFiles("playwright/sample-files/riot.png"); - // Find and click "Remove" link button - await profileSettings - .locator(".mx_UserProfileSettings_profile") - .getByRole("button", { name: "Remove" }) - .click(); + // Image should be visible + await expect(profileSettings.locator(".mx_AvatarSetting_avatar img")).toBeVisible(); - // Assert that the link button disappeared - await expect( - profileSettings.locator(".mx_AvatarSetting_avatar .mx_AccessibleButton_kind_link_sm"), - ).not.toBeVisible(); + // Open the menu & click remove + await profileSettings.getByRole("button", { name: "Profile Picture" }).click(); + await page.getByRole("menuitem", { name: "Remove" }).click(); + + // Assert that the image disappeared + await expect(profileSettings.locator(".mx_AvatarSetting_avatar img")).not.toBeVisible(); }); test("should set a country calling code based on default_country_code", async ({ uut }) => { 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 47a9733ccf..57e2a4026c 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 0cea98d8c2..af35cc8bb4 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 1035a54f6f..5d5cf27480 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 4627921de5..93fce06b13 100644 --- a/res/css/views/settings/_AvatarSetting.pcss +++ b/res/css/views/settings/_AvatarSetting.pcss @@ -21,36 +21,6 @@ limitations under the License. margin-top: 8px; position: relative; - .mx_AvatarSetting_hover { - transition: opacity var(--hover-transition); - opacity: 0; - - /* position to place the hover bg over the entire thing */ - position: absolute; - inset: 0; - - pointer-events: none; /* let the pointer fall through the underlying thing */ - - line-height: 90px; - text-align: center; - - > span { - color: $primary-content; - position: relative; /* tricks the layout engine into putting this on top of the bg */ - font-weight: 500; - } - - .mx_AvatarSetting_hoverBg { - /* absolute position to lazily fill the entire container */ - position: absolute; - inset: 0; - - opacity: 0.5; - background-color: $quinary-content; - border-radius: 90px; - } - } - &.mx_AvatarSetting_avatarDisplay:hover .mx_AvatarSetting_hover { opacity: 1; } @@ -77,25 +47,26 @@ limitations under the License. } .mx_AvatarSetting_uploadButton { - width: 32px; - height: 32px; + width: 28px; + height: 28px; border-radius: 32px; - background-color: $secondary-content; + border: 1px solid var(--cpd-color-bg-canvas-default); + background-color: var(--cpd-color-bg-subtle-primary); position: absolute; bottom: 0; right: 0; - } + text-align: center; + cursor: pointer; - .mx_AvatarSetting_uploadButton::before { - content: ""; - display: block; - width: 100%; - height: 100%; - mask-repeat: no-repeat; - mask-position: center; - mask-size: 55%; - background-color: $quinary-content; - mask-image: url("$(res)/img/feather-customised/edit.svg"); + svg { + position: relative; + top: 3px; + } } } + +.mx_AvatarSetting_removeMenuItem svg, +.mx_AvatarSetting_removeMenuItem span { + color: var(--cpd-color-text-critical-primary) !important; +} diff --git a/src/components/views/room_settings/RoomProfileSettings.tsx b/src/components/views/room_settings/RoomProfileSettings.tsx index bb229cc2a0..c829a26bc1 100644 --- a/src/components/views/room_settings/RoomProfileSettings.tsx +++ b/src/components/views/room_settings/RoomProfileSettings.tsx @@ -218,6 +218,10 @@ export default class RoomProfileSettings extends React.Component ); } + const canRemove = this.state.profileFieldsTouched.avatar + ? Boolean(this.state.avatarFile) + : Boolean(this.state.originalAvatarUrl); + return (
@@ -254,9 +258,9 @@ export default class RoomProfileSettings extends React.Component avatarAltText={_t("room_settings|general|avatar_field_label")} disabled={!this.state.canSetAvatar} onChange={this.onAvatarChanged} - removeAvatar={this.removeAvatar} + removeAvatar={canRemove ? this.removeAvatar : undefined} placeholderId={idNameForRoom(MatrixClientPeg.safeGet().getRoom(this.props.roomId)!)} - placeholderName={this.state.displayName} + placeholderName={MatrixClientPeg.safeGet().getRoom(this.props.roomId)!.name} />
{profileSettingsButtons} diff --git a/src/components/views/settings/AvatarSetting.tsx b/src/components/views/settings/AvatarSetting.tsx index cdec5fdfed..a3c0cf850b 100644 --- a/src/components/views/settings/AvatarSetting.tsx +++ b/src/components/views/settings/AvatarSetting.tsx @@ -14,15 +14,59 @@ See the License for the specific language governing permissions and limitations under the License. */ -import React, { createRef, useCallback, useEffect, useState } from "react"; +import React, { ReactNode, createRef, useCallback, useEffect, useState } from "react"; +import { Icon as EditIcon } from "@vector-im/compound-design-tokens/icons/edit.svg"; +import { Icon as UploadIcon } from "@vector-im/compound-design-tokens/icons/share.svg"; +import { Icon as DeleteIcon } from "@vector-im/compound-design-tokens/icons/delete.svg"; +import { Menu, MenuItem } from "@vector-im/compound-web"; import { _t } from "../../../languageHandler"; -import AccessibleButton from "../elements/AccessibleButton"; import { mediaFromMxc } from "../../../customisations/Media"; import { chromeFileInputFix } from "../../../utils/BrowserWorkarounds"; import { useId } from "../../../utils/useId"; +import AccessibleButton from "../elements/AccessibleButton"; import BaseAvatar from "../avatars/BaseAvatar"; +interface MenuProps { + trigger: ReactNode; + onUploadSelect: () => void; + onRemoveSelect?: () => void; +} + +const AvatarSettingContextMenu: React.FC = ({ trigger, onUploadSelect, onRemoveSelect }) => { + const [menuOpen, setMenuOpen] = useState(false); + + const onOpenChange = useCallback((newOpen: boolean) => { + setMenuOpen(newOpen); + }, []); + + return ( + + } + label={_t("action|upload_file")} + onSelect={onUploadSelect} + /> + {onRemoveSelect && ( + } + className="mx_AvatarSetting_removeMenuItem" + label={_t("action|remove")} + onSelect={onRemoveSelect} + /> + )} + + ); +}; + interface IProps { /** * The current value of the avatar URL, as an mxc URL or a File. @@ -136,47 +180,38 @@ const AvatarSetting: React.FC = ({ } let uploadAvatarBtn: JSX.Element | undefined; - if (uploadAvatar) { - // insert an empty div to be the host for a css mask containing the upload.svg + if (!disabled) { uploadAvatarBtn = ( - <> - - - +
+ +
); } - let removeAvatarBtn: JSX.Element | undefined; - if (avatarURL && removeAvatar && !disabled) { - removeAvatarBtn = ( - - {_t("action|remove")} - - ); + const content = ( +
+ {avatarElement} + {uploadAvatarBtn} +
+ ); + + if (disabled) { + return content; } return ( -
- {avatarElement} - + <> + + + ); }; diff --git a/src/components/views/settings/UserProfileSettings.tsx b/src/components/views/settings/UserProfileSettings.tsx index 34da232a01..f912fa44f5 100644 --- a/src/components/views/settings/UserProfileSettings.tsx +++ b/src/components/views/settings/UserProfileSettings.tsx @@ -151,7 +151,7 @@ const UserProfileSettings: React.FC = () => { avatar={avatarURL ?? undefined} avatarAltText={_t("common|user_avatar")} onChange={onAvatarChange} - removeAvatar={onAvatarRemove} + removeAvatar={avatarURL ? onAvatarRemove : undefined} placeholderName={displayName} placeholderId={client.getUserId() ?? ""} /> diff --git a/src/i18n/strings/en_EN.json b/src/i18n/strings/en_EN.json index 1dab84d5bf..d2a54b93f1 100644 --- a/src/i18n/strings/en_EN.json +++ b/src/i18n/strings/en_EN.json @@ -109,6 +109,7 @@ "save": "Save", "search": "Search", "send_report": "Send report", + "set_avatar": "Set profile picture", "share": "Share", "show": "Show", "show_advanced": "Show advanced", @@ -132,6 +133,7 @@ "update": "Update", "upgrade": "Upgrade", "upload": "Upload", + "upload_file": "Upload file", "verify": "Verify", "view": "View", "view_all": "View all", diff --git a/test/components/views/room_settings/RoomProfileSettings-test.tsx b/test/components/views/room_settings/RoomProfileSettings-test.tsx index a7406ede7c..f455055e22 100644 --- a/test/components/views/room_settings/RoomProfileSettings-test.tsx +++ b/test/components/views/room_settings/RoomProfileSettings-test.tsx @@ -85,7 +85,8 @@ describe("RoomProfileSetting", () => { render(); - await user.click(screen.getByRole("button", { name: "Remove" })); + await user.click(screen.getByRole("button", { name: "Room avatar" })); + await user.click(screen.getByRole("menuitem", { name: "Remove" })); await user.click(screen.getByRole("button", { name: "Save" })); await waitFor(() => diff --git a/test/components/views/settings/AvatarSetting-test.tsx b/test/components/views/settings/AvatarSetting-test.tsx index 59218eb28d..c3b1928232 100644 --- a/test/components/views/settings/AvatarSetting-test.tsx +++ b/test/components/views/settings/AvatarSetting-test.tsx @@ -44,35 +44,6 @@ describe("", () => { expect(imgElement).toBeInTheDocument(); }); - it("renders avatar with remove button", async () => { - const { queryByText } = render( - , - ); - - const removeButton = queryByText("Remove"); - expect(removeButton).toBeInTheDocument(); - }); - - it("renders avatar without remove button", async () => { - const { queryByText } = render( - , - ); - - const removeButton = queryByText("Remove"); - expect(removeButton).toBeNull(); - }); - it("renders a file as the avatar when supplied", async () => { render( ", () => { />, ); - // not really necessary, but to follow the expected user flow as much as possible - await user.click(screen.getByRole("button", { name: "Avatar of Peter Fox" })); - const fileInput = screen.getByAltText("Upload"); await user.upload(fileInput, AVATAR_FILE); diff --git a/test/components/views/settings/UserProfileSettings-test.tsx b/test/components/views/settings/UserProfileSettings-test.tsx index ff666b9f31..75c714036e 100644 --- a/test/components/views/settings/UserProfileSettings-test.tsx +++ b/test/components/views/settings/UserProfileSettings-test.tsx @@ -85,6 +85,7 @@ describe("ProfileSettings", () => { }); it("removes avatar", async () => { + jest.spyOn(OwnProfileStore.instance, "avatarMxc", "get").mockReturnValue("mxc://example.org/my-avatar"); renderProfileSettings(toastRack, client); expect(await screen.findByText("Mocked AvatarSetting")).toBeInTheDocument();