Change avatarsetting componment to use a menu (#12585)

* 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 edit icon on avatarsetting comnponent

* Change avatarsetting componment to use a menu

As per the designs, remove the 'remove' link and instead have a menu
pop up to either upload a new file or remove the avatar.

This also changes the room profile viw, since that uses the same view.

* 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

* Make clickable with no avatar again and fix tests

and renmove a test for the remove button which is no longer there

* Put back missing CSS to make the menu entry red

* Fix type error

* Fix tests

* Supply open / onOpenChange props

* Fix tests

* There is no hover anymore

* Use the computed name, not the name which may be null

* Fix room avatar remove behaviour

* Remove redundant else
This commit is contained in:
David Baker 2024-06-07 14:25:21 +01:00 committed by GitHub
parent c0cbd687c2
commit 1696c5cd0a
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
12 changed files with 107 additions and 134 deletions

View file

@ -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 }) => {

Binary file not shown.

Before

Width:  |  Height:  |  Size: 50 KiB

After

Width:  |  Height:  |  Size: 50 KiB

Binary file not shown.

Before

Width:  |  Height:  |  Size: 49 KiB

After

Width:  |  Height:  |  Size: 49 KiB

Binary file not shown.

Before

Width:  |  Height:  |  Size: 30 KiB

After

Width:  |  Height:  |  Size: 30 KiB

View file

@ -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;
}

View file

@ -218,6 +218,10 @@ export default class RoomProfileSettings extends React.Component<IProps, IState>
);
}
const canRemove = this.state.profileFieldsTouched.avatar
? Boolean(this.state.avatarFile)
: Boolean(this.state.originalAvatarUrl);
return (
<form onSubmit={this.saveProfile} autoComplete="off" noValidate={true} className="mx_RoomProfileSettings">
<div className="mx_RoomProfileSettings_profile">
@ -254,9 +258,9 @@ export default class RoomProfileSettings extends React.Component<IProps, IState>
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}
/>
</div>
{profileSettingsButtons}

View file

@ -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<MenuProps> = ({ trigger, onUploadSelect, onRemoveSelect }) => {
const [menuOpen, setMenuOpen] = useState(false);
const onOpenChange = useCallback((newOpen: boolean) => {
setMenuOpen(newOpen);
}, []);
return (
<Menu
trigger={trigger}
title={_t("action|set_avatar")}
showTitle={false}
open={menuOpen}
onOpenChange={onOpenChange}
>
<MenuItem
as="div"
Icon={<UploadIcon width="24px" height="24px" />}
label={_t("action|upload_file")}
onSelect={onUploadSelect}
/>
{onRemoveSelect && (
<MenuItem
as="div"
Icon={<DeleteIcon width="24px" height="24px" />}
className="mx_AvatarSetting_removeMenuItem"
label={_t("action|remove")}
onSelect={onRemoveSelect}
/>
)}
</Menu>
);
};
interface IProps {
/**
* The current value of the avatar URL, as an mxc URL or a File.
@ -136,15 +180,28 @@ const AvatarSetting: React.FC<IProps> = ({
}
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 = (
<div className="mx_AvatarSetting_uploadButton">
<EditIcon width="20px" height="20px" />
</div>
);
}
const content = (
<div className="mx_AvatarSetting_avatar" role="group" aria-label={avatarAltText}>
{avatarElement}
{uploadAvatarBtn}
</div>
);
if (disabled) {
return content;
}
return (
<>
<AccessibleButton
onClick={uploadAvatar}
className="mx_AvatarSetting_uploadButton"
aria-labelledby={a11yId}
/>
<AvatarSettingContextMenu trigger={content} onUploadSelect={uploadAvatar} onRemoveSelect={removeAvatar} />
<input
type="file"
style={{ display: "none" }}
@ -156,28 +213,6 @@ const AvatarSetting: React.FC<IProps> = ({
/>
</>
);
}
let removeAvatarBtn: JSX.Element | undefined;
if (avatarURL && removeAvatar && !disabled) {
removeAvatarBtn = (
<AccessibleButton onClick={removeAvatar} kind="link_sm">
{_t("action|remove")}
</AccessibleButton>
);
}
return (
<div className="mx_AvatarSetting_avatar" role="group" aria-label={avatarAltText}>
{avatarElement}
<div className="mx_AvatarSetting_hover" aria-hidden="true">
<div className="mx_AvatarSetting_hoverBg" />
{!disabled && <span id={a11yId}>{_t("action|upload")}</span>}
</div>
{uploadAvatarBtn}
{removeAvatarBtn}
</div>
);
};
export default AvatarSetting;

View file

@ -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() ?? ""}
/>

View file

@ -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",

View file

@ -85,7 +85,8 @@ describe("RoomProfileSetting", () => {
render(<RoomProfileSettings roomId="!floob:itty" />);
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(() =>

View file

@ -44,35 +44,6 @@ describe("<AvatarSetting />", () => {
expect(imgElement).toBeInTheDocument();
});
it("renders avatar with remove button", async () => {
const { queryByText } = render(
<AvatarSetting
avatarAltText="Avatar of Peter Fox"
avatar="mxc://example.org/my-avatar"
removeAvatar={jest.fn()}
placeholderId="blee"
placeholderName="boo"
/>,
);
const removeButton = queryByText("Remove");
expect(removeButton).toBeInTheDocument();
});
it("renders avatar without remove button", async () => {
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
@ -102,9 +73,6 @@ describe("<AvatarSetting />", () => {
/>,
);
// 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);

View file

@ -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();