From 0d1fce37b22c0565d25c517ad8e50ff42cab78ff Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Tue, 31 Jan 2023 13:56:48 +0000 Subject: [PATCH] Fix layout and visual regressions around default avatars (#10031) --- cypress/e2e/spaces/spaces.spec.ts | 6 ++-- res/css/structures/_SpacePanel.pcss | 9 ++---- res/css/structures/_UserMenu.pcss | 2 ++ src/components/views/avatars/BaseAvatar.tsx | 30 ++++++++----------- .../__snapshots__/MemberAvatar-test.tsx.snap | 2 +- 5 files changed, 21 insertions(+), 28 deletions(-) diff --git a/cypress/e2e/spaces/spaces.spec.ts b/cypress/e2e/spaces/spaces.spec.ts index f07746c0f5..2deb58574b 100644 --- a/cypress/e2e/spaces/spaces.spec.ts +++ b/cypress/e2e/spaces/spaces.spec.ts @@ -153,10 +153,7 @@ describe("Spaces", () => { openSpaceCreateMenu().within(() => { cy.get(".mx_SpaceCreateMenuType_private").click(); - cy.get('.mx_SpaceBasicSettings_avatarContainer input[type="file"]').selectFile( - "cypress/fixtures/riot.png", - { force: true }, - ); + // We don't set an avatar here to get a Percy snapshot of the default avatar style for spaces cy.get('input[label="Address"]').should("not.exist"); cy.get('textarea[label="Description"]').type("This is a personal space to mourn Riot.im..."); cy.get('input[label="Name"]').type("This is my Riot{enter}"); @@ -169,6 +166,7 @@ describe("Spaces", () => { cy.contains(".mx_RoomList .mx_RoomTile", "Sample Room").should("exist"); cy.contains(".mx_SpaceHierarchy_list .mx_SpaceHierarchy_roomTile", "Sample Room").should("exist"); + cy.get(".mx_LeftPanel_outerWrapper").percySnapshotElement("Left panel with default avatar space"); }); it("should allow user to invite another to a space", () => { diff --git a/res/css/structures/_SpacePanel.pcss b/res/css/structures/_SpacePanel.pcss index 70c65b3f74..859714bfb9 100644 --- a/res/css/structures/_SpacePanel.pcss +++ b/res/css/structures/_SpacePanel.pcss @@ -277,14 +277,11 @@ $activeBorderColor: $primary-content; .mx_BaseAvatar:not(.mx_UserMenu_userAvatar_BaseAvatar) .mx_BaseAvatar_initial { color: $secondary-content; border-radius: 8px; - background-color: $panel-actions; - font-size: $font-15px !important; /* override inline style */ font-weight: $font-semi-bold; line-height: $font-18px; - - & + .mx_BaseAvatar_image { - visibility: hidden; - } + /* override inline styles which are part of the default avatar style as these uses a monochrome style */ + background-color: $panel-actions !important; + font-size: $font-15px !important; } .mx_SpaceTreeLevel { diff --git a/res/css/structures/_UserMenu.pcss b/res/css/structures/_UserMenu.pcss index 08ef8947ee..f43d773e3d 100644 --- a/res/css/structures/_UserMenu.pcss +++ b/res/css/structures/_UserMenu.pcss @@ -25,6 +25,8 @@ limitations under the License. .mx_UserMenu_userAvatar { position: relative; + /* without this a default avatar will cause this to be 4px oversized and out of alignment */ + display: inherit; .mx_BaseAvatar { pointer-events: none; /* makes the avatar non-draggable */ diff --git a/src/components/views/avatars/BaseAvatar.tsx b/src/components/views/avatars/BaseAvatar.tsx index d1dbe7743d..7fb0ba458f 100644 --- a/src/components/views/avatars/BaseAvatar.tsx +++ b/src/components/views/avatars/BaseAvatar.tsx @@ -15,7 +15,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -import React, { useCallback, useContext, useEffect, useState } from "react"; +import React, { CSSProperties, useCallback, useContext, useEffect, useState } from "react"; import classNames from "classnames"; import { ResizeMethod } from "matrix-js-sdk/src/@types/partials"; import { ClientEvent } from "matrix-js-sdk/src/client"; @@ -51,6 +51,7 @@ interface IProps { inputRef?: React.RefObject; className?: string; tabIndex?: number; + style?: CSSProperties; } const calculateUrls = (url: string | undefined, urls: string[] | undefined, lowBandwidth: boolean): string[] => { @@ -132,10 +133,17 @@ const BaseAvatar: React.FC = (props) => { onClick, inputRef, className, + style: parentStyle, resizeMethod: _unused, // to keep it from being in `otherProps` ...otherProps } = props; + const style = { + ...parentStyle, + width: toPx(width), + height: toPx(height), + }; + const [imageUrl, onError] = useImageUrl({ url, urls }); if (!imageUrl && defaultToInitialLetter && name) { @@ -151,10 +159,7 @@ const BaseAvatar: React.FC = (props) => { className={classNames("mx_BaseAvatar", className)} onClick={onClick} inputRef={inputRef} - style={{ - width: toPx(width), - height: toPx(height), - }} + style={style} > {avatar} @@ -165,10 +170,7 @@ const BaseAvatar: React.FC = (props) => { className={classNames("mx_BaseAvatar", className)} ref={inputRef} {...otherProps} - style={{ - width: toPx(width), - height: toPx(height), - }} + style={style} role="presentation" > {avatar} @@ -185,10 +187,7 @@ const BaseAvatar: React.FC = (props) => { src={imageUrl} onClick={onClick} onError={onError} - style={{ - width: toPx(width), - height: toPx(height), - }} + style={style} title={title} alt={_t("Avatar")} inputRef={inputRef} @@ -202,10 +201,7 @@ const BaseAvatar: React.FC = (props) => { className={classNames("mx_BaseAvatar mx_BaseAvatar_image", className)} src={imageUrl} onError={onError} - style={{ - width: toPx(width), - height: toPx(height), - }} + style={style} title={title} alt="" ref={inputRef} diff --git a/test/components/views/avatars/__snapshots__/MemberAvatar-test.tsx.snap b/test/components/views/avatars/__snapshots__/MemberAvatar-test.tsx.snap index feda79035c..ef29b03bc8 100644 --- a/test/components/views/avatars/__snapshots__/MemberAvatar-test.tsx.snap +++ b/test/components/views/avatars/__snapshots__/MemberAvatar-test.tsx.snap @@ -7,7 +7,7 @@ exports[`MemberAvatar matches the snapshot 1`] = ` class="mx_BaseAvatar mx_BaseAvatar_image" data-testid="avatar-img" src="http://this.is.a.url//placekitten.com/400/400" - style="color: pink;" + style="color: pink; width: 35px; height: 35px;" title="Hover title" />