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

Fix display of no avatar in avatar setting controls #12558

Merged
merged 43 commits into from
Jun 6, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
43 commits
Select commit Hold shift + click to select a range
06896dc
New user profile UI in User Settings
dbkr May 21, 2024
f16cb80
Show avatar upload error
dbkr May 22, 2024
1b60ea8
Fix avatar upload error
dbkr May 23, 2024
d0624af
Wire up errors & feedback for display name setting
dbkr May 24, 2024
ac46104
Implement avatar upload / remove progress toast
dbkr May 24, 2024
a01d94a
Add 768px breakpoint
dbkr May 24, 2024
5da2969
Fix display of no avatar in avatar setting controls
dbkr May 28, 2024
236de62
Fix room profile display
dbkr May 28, 2024
6786697
Update to released compund-web with required components / fixes
dbkr May 30, 2024
237570e
Require compound-web 4.4.0
dbkr May 30, 2024
500fb3a
Update snapshots
dbkr May 30, 2024
f0c9d83
Fix duplicate import
dbkr May 30, 2024
513687c
Fix CSS comment
dbkr May 30, 2024
c93ab3d
Update snapshot
dbkr May 30, 2024
be18de1
Run all the tests so the ids stay the same
dbkr May 30, 2024
7330b34
Start of a test for ProfileSettings
dbkr May 30, 2024
b244866
More tests
dbkr May 30, 2024
137df66
Test that a toast appears
dbkr May 30, 2024
48a3d39
Test ToastRack
dbkr May 30, 2024
a99dced
Update snapshots
dbkr Jun 4, 2024
605555a
Add the usernamee control
dbkr Jun 4, 2024
0f56520
Fix playwright tests
dbkr Jun 5, 2024
96802a7
Put ^ back on compound-web version
dbkr Jun 5, 2024
304836d
Split CSS for room & user profile settings
dbkr Jun 5, 2024
d97d8d9
Fix playwright test
dbkr Jun 5, 2024
6a8a981
Update room settings screenshot
dbkr Jun 5, 2024
b2a7f8a
Use original screenshot instead
dbkr Jun 5, 2024
41bee22
Merge branch 'dbkr/new_profilesettings' into dbkr/fix_blank_avatar_di…
dbkr Jun 5, 2024
072681d
Merge branch 'develop' into dbkr/new_profilesettings
dbkr Jun 5, 2024
85b2266
Merge branch 'dbkr/new_profilesettings' into dbkr/fix_blank_avatar_di…
dbkr Jun 5, 2024
8d05f12
Add required props in test
dbkr Jun 5, 2024
54cb655
Fix test
dbkr Jun 5, 2024
3bd2aa1
Also here
dbkr Jun 5, 2024
6082f97
Update screenshots
dbkr Jun 5, 2024
52fbe95
Remove user icon
dbkr Jun 5, 2024
f0c6f94
Fix styling of unrelated buttons
dbkr Jun 5, 2024
ecb6730
Add copyright year
dbkr Jun 5, 2024
672d7cf
Fix copyright year
dbkr Jun 5, 2024
f72d7e7
Merge remote-tracking branch 'origin/develop' into dbkr/new_profilese…
dbkr Jun 6, 2024
145fc75
Merge branch 'dbkr/new_profilesettings' into dbkr/fix_blank_avatar_di…
dbkr Jun 6, 2024
2d7672d
Merge branch 'develop' into dbkr/fix_blank_avatar_display
dbkr Jun 6, 2024
a40340f
Switch to useMatrixClientContext
dbkr Jun 6, 2024
7f08efd
Fix other test
dbkr Jun 6, 2024
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
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
22 changes: 1 addition & 21 deletions res/css/views/settings/_AvatarSetting.pcss
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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;
}
11 changes: 0 additions & 11 deletions res/img/feather-customised/user.svg

This file was deleted.

26 changes: 16 additions & 10 deletions src/components/views/avatars/RoomAvatar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -117,17 +130,10 @@ 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 {
Expand Down
3 changes: 3 additions & 0 deletions src/components/views/room_settings/RoomProfileSettings.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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}
Expand Down
25 changes: 23 additions & 2 deletions src/components/views/settings/AvatarSetting.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
/**
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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 = (
Expand Down
25 changes: 14 additions & 11 deletions src/components/views/settings/UserProfileSettings.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand All @@ -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 }) => (
<>
Expand Down Expand Up @@ -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) => {
Expand All @@ -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);
Expand All @@ -112,7 +113,7 @@ const UserProfileSettings: React.FC = () => {
removeToast();
}
},
[toastRack],
[toastRack, client],
);

const onDisplayNameChanged = useCallback((e: ChangeEvent<HTMLInputElement>) => {
Expand All @@ -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 (
Expand All @@ -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"
Expand Down
6 changes: 6 additions & 0 deletions test/components/views/dialogs/RoomSettingsDialog-test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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) =>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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", {
Expand All @@ -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);
});
Expand Down
29 changes: 26 additions & 3 deletions test/components/views/settings/AvatarSetting-test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand All @@ -45,6 +50,8 @@ describe("<AvatarSetting />", () => {
avatarAltText="Avatar of Peter Fox"
avatar="mxc://example.org/my-avatar"
removeAvatar={jest.fn()}
placeholderId="blee"
placeholderName="boo"
/>,
);

Expand All @@ -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();
Expand All @@ -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}
Expand Down
Loading
Loading