Switch Pill & E2EIcon to using Compound Tooltips (#12080)

* Switch Pill to using Compound Tooltips

Signed-off-by: Michael Telatynski <7t3chguy@gmail.com>

* Switch E2EIcon to using Compound Tooltips

Signed-off-by: Michael Telatynski <7t3chguy@gmail.com>

* Update tests & snapshots

Signed-off-by: Michael Telatynski <7t3chguy@gmail.com>

---------

Signed-off-by: Michael Telatynski <7t3chguy@gmail.com>
This commit is contained in:
Michael Telatynski 2023-12-21 15:21:41 +00:00 committed by GitHub
parent 42ac2272b2
commit 07747e24d4
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
10 changed files with 105 additions and 129 deletions

View file

@ -14,13 +14,13 @@ See the License for the specific language governing permissions and
limitations under the License. limitations under the License.
*/ */
import React, { ReactElement, useRef, useState } from "react"; import React, { ReactElement } from "react";
import classNames from "classnames"; import classNames from "classnames";
import { Room, RoomMember } from "matrix-js-sdk/src/matrix"; import { Room, RoomMember } from "matrix-js-sdk/src/matrix";
import { Tooltip } from "@vector-im/compound-web";
import { MatrixClientPeg } from "../../../MatrixClientPeg"; import { MatrixClientPeg } from "../../../MatrixClientPeg";
import MatrixClientContext from "../../../contexts/MatrixClientContext"; import MatrixClientContext from "../../../contexts/MatrixClientContext";
import Tooltip, { Alignment } from "../elements/Tooltip";
import { usePermalink } from "../../../hooks/usePermalink"; import { usePermalink } from "../../../hooks/usePermalink";
import RoomAvatar from "../avatars/RoomAvatar"; import RoomAvatar from "../avatars/RoomAvatar";
import MemberAvatar from "../avatars/MemberAvatar"; import MemberAvatar from "../avatars/MemberAvatar";
@ -88,8 +88,6 @@ export interface PillProps {
} }
export const Pill: React.FC<PillProps> = ({ type: propType, url, inMessage, room, shouldShowPillAvatar = true }) => { export const Pill: React.FC<PillProps> = ({ type: propType, url, inMessage, room, shouldShowPillAvatar = true }) => {
const tooltipId = useRef(`mx_Pill_${Math.random()}`).current;
const [hover, setHover] = useState(false);
const { event, member, onClick, resourceId, targetRoom, text, type } = usePermalink({ const { event, member, onClick, resourceId, targetRoom, text, type } = usePermalink({
room, room,
type: propType, type: propType,
@ -109,15 +107,6 @@ export const Pill: React.FC<PillProps> = ({ type: propType, url, inMessage, room
mx_EventPill: type === PillType.EventInOtherRoom || type === PillType.EventInSameRoom, mx_EventPill: type === PillType.EventInOtherRoom || type === PillType.EventInSameRoom,
}); });
const onMouseOver = (): void => {
setHover(true);
};
const onMouseLeave = (): void => {
setHover(false);
};
const tip = hover && resourceId ? <Tooltip id={tooltipId} label={resourceId} alignment={Alignment.Right} /> : null;
let avatar: ReactElement | null = null; let avatar: ReactElement | null = null;
let pillText: string | null = text; let pillText: string | null = text;
@ -155,34 +144,28 @@ export const Pill: React.FC<PillProps> = ({ type: propType, url, inMessage, room
return null; return null;
} }
const isAnchor = !!inMessage && !!url;
return ( return (
<bdi> <bdi>
<MatrixClientContext.Provider value={MatrixClientPeg.safeGet()}> <MatrixClientContext.Provider value={MatrixClientPeg.safeGet()}>
{inMessage && url ? ( <Tooltip
<a label={resourceId ?? ""}
className={classes} open={resourceId ? undefined : false}
href={url} side="right"
onClick={onClick} isTriggerInteractive={isAnchor}
onMouseOver={onMouseOver} >
onMouseLeave={onMouseLeave} {isAnchor ? (
aria-describedby={tooltipId} <a className={classes} href={url} onClick={onClick}>
> {avatar}
{avatar} <span className="mx_Pill_text">{pillText}</span>
<span className="mx_Pill_text">{pillText}</span> </a>
{tip} ) : (
</a> <span className={classes}>
) : ( {avatar}
<span <span className="mx_Pill_text">{pillText}</span>
className={classes} </span>
onMouseOver={onMouseOver} )}
onMouseLeave={onMouseLeave} </Tooltip>
aria-describedby={tooltipId}
>
{avatar}
<span className="mx_Pill_text">{pillText}</span>
{tip}
</span>
)}
</MatrixClientContext.Provider> </MatrixClientContext.Provider>
</bdi> </bdi>
); );

View file

@ -15,12 +15,12 @@ See the License for the specific language governing permissions and
limitations under the License. limitations under the License.
*/ */
import React, { CSSProperties, useState } from "react"; import React, { ComponentProps, CSSProperties } from "react";
import classNames from "classnames"; import classNames from "classnames";
import { Tooltip } from "@vector-im/compound-web";
import { _t, _td, TranslationKey } from "../../../languageHandler"; import { _t, _td, TranslationKey } from "../../../languageHandler";
import AccessibleButton from "../elements/AccessibleButton"; import AccessibleButton from "../elements/AccessibleButton";
import Tooltip, { Alignment } from "../elements/Tooltip";
import { E2EStatus } from "../../../utils/ShieldUtils"; import { E2EStatus } from "../../../utils/ShieldUtils";
import { XOR } from "../../../@types/common"; import { XOR } from "../../../@types/common";
@ -48,7 +48,7 @@ interface Props {
size?: number; size?: number;
onClick?: () => void; onClick?: () => void;
hideTooltip?: boolean; hideTooltip?: boolean;
tooltipAlignment?: Alignment; tooltipSide?: ComponentProps<typeof Tooltip>["side"];
bordered?: boolean; bordered?: boolean;
} }
@ -69,11 +69,9 @@ const E2EIcon: React.FC<XOR<UserProps, RoomProps>> = ({
size, size,
onClick, onClick,
hideTooltip, hideTooltip,
tooltipAlignment, tooltipSide,
bordered, bordered,
}) => { }) => {
const [hover, setHover] = useState(false);
const classes = classNames( const classes = classNames(
{ {
mx_E2EIcon: true, mx_E2EIcon: true,
@ -97,35 +95,23 @@ const E2EIcon: React.FC<XOR<UserProps, RoomProps>> = ({
style = { width: `${size}px`, height: `${size}px` }; style = { width: `${size}px`, height: `${size}px` };
} }
const onMouseOver = (): void => setHover(true);
const onMouseLeave = (): void => setHover(false);
const label = e2eTitle ? _t(e2eTitle) : ""; const label = e2eTitle ? _t(e2eTitle) : "";
let tip: JSX.Element | undefined; let content: JSX.Element;
if (hover && !hideTooltip && label) { if (onClick) {
tip = <Tooltip label={label} alignment={tooltipAlignment} />; content = <AccessibleButton onClick={onClick} className={classes} style={style} />;
} else {
content = <div className={classes} style={style} />;
} }
if (onClick) { if (!e2eTitle || hideTooltip) {
return ( return content;
<AccessibleButton
onClick={onClick}
onMouseOver={onMouseOver}
onMouseLeave={onMouseLeave}
className={classes}
style={style}
aria-label={label}
>
{tip}
</AccessibleButton>
);
} }
return ( return (
<div onMouseOver={onMouseOver} onMouseLeave={onMouseLeave} className={classes} style={style} aria-label={label}> <Tooltip label={label} side={tooltipSide} isTriggerInteractive={!!onClick}>
{tip} {content}
</div> </Tooltip>
); );
}; };

View file

@ -780,11 +780,7 @@ export default class RoomHeader extends React.Component<IProps, IState> {
const icon = this.props.viewingCall ? ( const icon = this.props.viewingCall ? (
<div className="mx_LegacyRoomHeader_icon mx_LegacyRoomHeader_icon_video" /> <div className="mx_LegacyRoomHeader_icon mx_LegacyRoomHeader_icon_video" />
) : this.props.e2eStatus ? ( ) : this.props.e2eStatus ? (
<E2EIcon <E2EIcon className="mx_LegacyRoomHeader_icon" status={this.props.e2eStatus} tooltipSide="bottom" />
className="mx_LegacyRoomHeader_icon"
status={this.props.e2eStatus}
tooltipAlignment={Alignment.Bottom}
/>
) : // If we're expecting an E2EE status to come in, but it hasn't ) : // If we're expecting an E2EE status to come in, but it hasn't
// yet been loaded, insert a blank div to reserve space // yet been loaded, insert a blank div to reserve space
this.client.isRoomEncrypted(this.props.room.roomId) && this.client.isCryptoEnabled() ? ( this.client.isRoomEncrypted(this.props.room.roomId) && this.client.isCryptoEnabled() ? (

View file

@ -522,10 +522,14 @@ exports[`RoomView for a local room in state NEW that is encrypted should match t
</span> </span>
</div> </div>
</div> </div>
<div <span
aria-label="This room is end-to-end encrypted" data-state="closed"
class="mx_E2EIcon mx_E2EIcon_normal mx_LegacyRoomHeader_icon" tabindex="0"
/> >
<div
class="mx_E2EIcon mx_E2EIcon_normal mx_LegacyRoomHeader_icon"
/>
</span>
<div <div
class="mx_LegacyRoomHeader_name mx_LegacyRoomHeader_name--textonly" class="mx_LegacyRoomHeader_name mx_LegacyRoomHeader_name--textonly"
> >

View file

@ -142,8 +142,8 @@ describe("<Pill>", () => {
await userEvent.hover(screen.getByText("Room 1")); await userEvent.hover(screen.getByText("Room 1"));
}); });
it("should show a tooltip with the room Id", () => { it("should show a tooltip with the room Id", async () => {
expect(screen.getByRole("tooltip", { name: room1Id })).toBeInTheDocument(); expect(await screen.findByRole("tooltip", { name: room1Id })).toBeInTheDocument();
}); });
describe("when not hovering the pill any more", () => { describe("when not hovering the pill any more", () => {

View file

@ -11,13 +11,17 @@ exports[`<Pill> should not render an avatar or link when called with inMessage =
<div> <div>
<bdi> <bdi>
<span <span
aria-describedby="mx_Pill_0.123456" data-state="closed"
class="mx_Pill mx_RoomPill" tabindex="0"
> >
<span <span
class="mx_Pill_text" class="mx_Pill mx_RoomPill"
> >
Room 1 <span
class="mx_Pill_text"
>
Room 1
</span>
</span> </span>
</span> </span>
</bdi> </bdi>
@ -30,24 +34,28 @@ exports[`<Pill> should render the expected pill for @room 1`] = `
<div> <div>
<bdi> <bdi>
<span <span
aria-describedby="mx_Pill_0.123456" data-state="closed"
class="mx_Pill mx_AtRoomPill" tabindex="0"
> >
<span <span
aria-hidden="true" class="mx_Pill mx_AtRoomPill"
class="_avatar_1o69u_17 mx_BaseAvatar _avatar-imageless_1o69u_60"
data-color="3"
data-testid="avatar-img"
data-type="round"
role="presentation"
style="--cpd-avatar-size: 16px;"
> >
R <span
</span> aria-hidden="true"
<span class="_avatar_1o69u_17 mx_BaseAvatar _avatar-imageless_1o69u_60"
class="mx_Pill_text" data-color="3"
> data-testid="avatar-img"
@room data-type="round"
role="presentation"
style="--cpd-avatar-size: 16px;"
>
R
</span>
<span
class="mx_Pill_text"
>
@room
</span>
</span> </span>
</span> </span>
</bdi> </bdi>
@ -60,8 +68,8 @@ exports[`<Pill> should render the expected pill for a known user not in the room
<div> <div>
<bdi> <bdi>
<a <a
aria-describedby="mx_Pill_0.123456"
class="mx_Pill mx_UserPill" class="mx_Pill mx_UserPill"
data-state="closed"
href="https://matrix.to/#/@user2:example.com" href="https://matrix.to/#/@user2:example.com"
> >
<span <span
@ -91,8 +99,8 @@ exports[`<Pill> should render the expected pill for a message in another room 1`
<div> <div>
<bdi> <bdi>
<a <a
aria-describedby="mx_Pill_0.123456"
class="mx_Pill mx_EventPill" class="mx_Pill mx_EventPill"
data-state="closed"
href="https://matrix.to/#/!room1:example.com/$123-456" href="https://matrix.to/#/!room1:example.com/$123-456"
> >
<span <span
@ -122,8 +130,8 @@ exports[`<Pill> should render the expected pill for a message in the same room 1
<div> <div>
<bdi> <bdi>
<a <a
aria-describedby="mx_Pill_0.123456"
class="mx_Pill mx_EventPill" class="mx_Pill mx_EventPill"
data-state="closed"
href="https://matrix.to/#/!room1:example.com/$123-456" href="https://matrix.to/#/!room1:example.com/$123-456"
> >
<span <span
@ -153,8 +161,8 @@ exports[`<Pill> should render the expected pill for a room alias 1`] = `
<div> <div>
<bdi> <bdi>
<a <a
aria-describedby="mx_Pill_0.123456"
class="mx_Pill mx_RoomPill" class="mx_Pill mx_RoomPill"
data-state="closed"
href="https://matrix.to/#/#room1:example.com" href="https://matrix.to/#/#room1:example.com"
> >
<span <span
@ -184,8 +192,8 @@ exports[`<Pill> should render the expected pill for a space 1`] = `
<div> <div>
<bdi> <bdi>
<a <a
aria-describedby="mx_Pill_0.123456"
class="mx_Pill mx_RoomPill" class="mx_Pill mx_RoomPill"
data-state="closed"
href="https://matrix.to/#/!space1:example.com" href="https://matrix.to/#/!space1:example.com"
> >
<span <span
@ -215,8 +223,8 @@ exports[`<Pill> should render the expected pill for an uknown user not in the ro
<div> <div>
<bdi> <bdi>
<a <a
aria-describedby="mx_Pill_0.123456"
class="mx_Pill mx_UserPill" class="mx_Pill mx_UserPill"
data-state="closed"
href="https://matrix.to/#/@user3:example.com" href="https://matrix.to/#/@user3:example.com"
> >
<div <div
@ -238,8 +246,8 @@ exports[`<Pill> when rendering a pill for a room should render the expected pill
<div> <div>
<bdi> <bdi>
<a <a
aria-describedby="mx_Pill_0.123456"
class="mx_Pill mx_RoomPill" class="mx_Pill mx_RoomPill"
data-state="closed"
href="https://matrix.to/#/!room1:example.com" href="https://matrix.to/#/!room1:example.com"
> >
<span <span
@ -269,8 +277,8 @@ exports[`<Pill> when rendering a pill for a user in the room should render as ex
<div> <div>
<bdi> <bdi>
<a <a
aria-describedby="mx_Pill_0.123456"
class="mx_Pill mx_UserPill" class="mx_Pill mx_UserPill"
data-state="closed"
href="https://matrix.to/#/@user1:example.com" href="https://matrix.to/#/@user1:example.com"
> >
<span <span

View file

@ -199,7 +199,7 @@ describe("<TextualBody />", () => {
const { container } = getComponent({ mxEvent: ev }); const { container } = getComponent({ mxEvent: ev });
const content = container.querySelector(".mx_EventTile_body"); const content = container.querySelector(".mx_EventTile_body");
expect(content.innerHTML).toMatchInlineSnapshot( expect(content.innerHTML).toMatchInlineSnapshot(
`"Chat with <span><bdi><a class="mx_Pill mx_UserPill mx_UserPill_me" href="https://matrix.to/#/@user:example.com" aria-describedby="mx_Pill_0.123456"><span aria-label="Profile picture" aria-hidden="true" data-testid="avatar-img" data-type="round" data-color="8" class="_avatar_1o69u_17 mx_BaseAvatar" style="--cpd-avatar-size: 16px;"><img loading="lazy" alt="" src="mxc://avatar.url/image.png" crossorigin="anonymous" referrerpolicy="no-referrer" class="_image_1o69u_49" data-type="round" width="16px" height="16px"></span><span class="mx_Pill_text">Member</span></a></bdi></span>"`, `"Chat with <span><bdi><a class="mx_Pill mx_UserPill mx_UserPill_me" href="https://matrix.to/#/@user:example.com" data-state="closed"><span aria-label="Profile picture" aria-hidden="true" data-testid="avatar-img" data-type="round" data-color="8" class="_avatar_1o69u_17 mx_BaseAvatar" style="--cpd-avatar-size: 16px;"><img loading="lazy" alt="" src="mxc://avatar.url/image.png" crossorigin="anonymous" referrerpolicy="no-referrer" class="_image_1o69u_49" data-type="round" width="16px" height="16px"></span><span class="mx_Pill_text">Member</span></a></bdi></span>"`,
); );
}); });
@ -217,7 +217,7 @@ describe("<TextualBody />", () => {
const { container } = getComponent({ mxEvent: ev }); const { container } = getComponent({ mxEvent: ev });
const content = container.querySelector(".mx_EventTile_body"); const content = container.querySelector(".mx_EventTile_body");
expect(content.innerHTML).toMatchInlineSnapshot( expect(content.innerHTML).toMatchInlineSnapshot(
`"Visit <span><bdi><a class="mx_Pill mx_RoomPill" href="https://matrix.to/#/#room:example.com" aria-describedby="mx_Pill_0.123456"><div class="mx_Pill_LinkIcon mx_BaseAvatar"></div><span class="mx_Pill_text">#room:example.com</span></a></bdi></span>"`, `"Visit <span><bdi><a class="mx_Pill mx_RoomPill" href="https://matrix.to/#/#room:example.com" data-state="closed"><div class="mx_Pill_LinkIcon mx_BaseAvatar"></div><span class="mx_Pill_text">#room:example.com</span></a></bdi></span>"`,
); );
}); });

View file

@ -50,8 +50,8 @@ exports[`<TextualBody /> renders formatted m.text correctly pills appear for an
<span> <span>
<bdi> <bdi>
<a <a
aria-describedby="mx_Pill_0.123456"
class="mx_Pill mx_UserPill" class="mx_Pill mx_UserPill"
data-state="closed"
href="https://matrix.to/#/@user:example.com" href="https://matrix.to/#/@user:example.com"
> >
<span <span
@ -99,8 +99,8 @@ exports[`<TextualBody /> renders formatted m.text correctly pills appear for eve
<span> <span>
<bdi> <bdi>
<a <a
aria-describedby="mx_Pill_0.123456"
class="mx_Pill mx_EventPill" class="mx_Pill mx_EventPill"
data-state="closed"
href="https://matrix.to/#/!ZxbRYPQXDXKGmDnJNg:example.com/$16085560162aNpaH:example.com?via=example.com" href="https://matrix.to/#/!ZxbRYPQXDXKGmDnJNg:example.com/$16085560162aNpaH:example.com?via=example.com"
> >
<span <span
@ -150,8 +150,8 @@ exports[`<TextualBody /> renders formatted m.text correctly pills appear for roo
<span> <span>
<bdi> <bdi>
<a <a
aria-describedby="mx_Pill_0.123456"
class="mx_Pill mx_RoomPill" class="mx_Pill mx_RoomPill"
data-state="closed"
href="https://matrix.to/#/!ZxbRYPQXDXKGmDnJNg:example.com?via=example.com&via=bob.com" href="https://matrix.to/#/!ZxbRYPQXDXKGmDnJNg:example.com?via=example.com&via=bob.com"
> >
<span <span
@ -260,8 +260,8 @@ exports[`<TextualBody /> renders formatted m.text correctly pills get injected c
<span> <span>
<bdi> <bdi>
<a <a
aria-describedby="mx_Pill_0.123456"
class="mx_Pill mx_UserPill" class="mx_Pill mx_UserPill"
data-state="closed"
href="https://matrix.to/#/@user:server" href="https://matrix.to/#/@user:server"
> >
<span <span
@ -303,7 +303,7 @@ exports[`<TextualBody /> renders plain-text m.text correctly should pillify a pe
><a ><a
class="mx_Pill mx_EventPill" class="mx_Pill mx_EventPill"
href="https://matrix.to/#/!room1:example.com/%event_id%" href="https://matrix.to/#/!room1:example.com/%event_id%"
aria-describedby="mx_Pill_0.123456" data-state="closed"
><span ><span
aria-label="Profile picture" aria-label="Profile picture"
aria-hidden="true" aria-hidden="true"
@ -336,7 +336,7 @@ exports[`<TextualBody /> renders plain-text m.text correctly should pillify a pe
><a ><a
class="mx_Pill mx_EventPill" class="mx_Pill mx_EventPill"
href="https://matrix.to/#/!room2:example.com/%event_id%" href="https://matrix.to/#/!room2:example.com/%event_id%"
aria-describedby="mx_Pill_0.123456" data-state="closed"
><span ><span
aria-label="Avatar" aria-label="Avatar"
aria-hidden="true" aria-hidden="true"
@ -371,8 +371,8 @@ exports[`<TextualBody /> renders plain-text m.text correctly should pillify a pe
<span> <span>
<bdi> <bdi>
<a <a
aria-describedby="mx_Pill_0.123456"
class="mx_Pill mx_EventPill" class="mx_Pill mx_EventPill"
data-state="closed"
href="https://matrix.to/#/!room1:example.com/!abc123" href="https://matrix.to/#/!room1:example.com/!abc123"
> >
<div <div

View file

@ -20,6 +20,7 @@ import { render, screen, waitFor } from "@testing-library/react";
import { MatrixClient, RoomMember, Device } from "matrix-js-sdk/src/matrix"; import { MatrixClient, RoomMember, Device } from "matrix-js-sdk/src/matrix";
import { UserVerificationStatus, DeviceVerificationStatus } from "matrix-js-sdk/src/crypto-api"; import { UserVerificationStatus, DeviceVerificationStatus } from "matrix-js-sdk/src/crypto-api";
import { mocked } from "jest-mock"; import { mocked } from "jest-mock";
import userEvent from "@testing-library/user-event";
import * as TestUtils from "../../../test-utils"; import * as TestUtils from "../../../test-utils";
import MemberTile from "../../../../src/components/views/rooms/MemberTile"; import MemberTile from "../../../../src/components/views/rooms/MemberTile";
@ -48,10 +49,13 @@ describe("MemberTile", () => {
const { container } = render(<MemberTile member={member} />); const { container } = render(<MemberTile member={member} />);
await waitFor(() =>
expect(screen.getByLabelText("This user has not verified all of their sessions.")).toBeInTheDocument(),
);
expect(container).toMatchSnapshot(); expect(container).toMatchSnapshot();
await waitFor(async () => {
await userEvent.hover(container.querySelector(".mx_E2EIcon")!);
expect(
screen.getByRole("tooltip", { name: "This user has not verified all of their sessions." }),
).toBeInTheDocument();
});
}); });
it("should display an verified E2EIcon when the e2E status = Verified", async () => { it("should display an verified E2EIcon when the e2E status = Verified", async () => {
@ -69,11 +73,14 @@ describe("MemberTile", () => {
const { container } = render(<MemberTile member={member} />); const { container } = render(<MemberTile member={member} />);
await waitFor(() =>
expect(
screen.getByLabelText("You have verified this user. This user has verified all of their sessions."),
).toBeInTheDocument(),
);
expect(container).toMatchSnapshot(); expect(container).toMatchSnapshot();
await waitFor(async () => {
await userEvent.hover(container.querySelector(".mx_E2EIcon")!);
expect(
screen.getByRole("tooltip", {
name: "You have verified this user. This user has verified all of their sessions.",
}),
).toBeInTheDocument();
});
}); });
}); });

View file

@ -24,10 +24,6 @@ exports[`MemberTile should display an verified E2EIcon when the e2E status = Ver
> >
u u
</span> </span>
<div
aria-label="You have verified this user. This user has verified all of their sessions."
class="mx_E2EIcon mx_E2EIcon_bordered mx_E2EIcon_verified"
/>
</div> </div>
<div <div
class="mx_EntityTile_details" class="mx_EntityTile_details"
@ -81,10 +77,6 @@ exports[`MemberTile should display an warning E2EIcon when the e2E status = Warn
> >
u u
</span> </span>
<div
aria-label="This user has not verified all of their sessions."
class="mx_E2EIcon mx_E2EIcon_bordered mx_E2EIcon_warning"
/>
</div> </div>
<div <div
class="mx_EntityTile_details" class="mx_EntityTile_details"