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

Commit

Permalink
Use Compound tooltips more widely (#12128)
Browse files Browse the repository at this point in the history
* Switch MIMageBody & MStickerBody to Compound Tooltip

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

* Switch E2ePadlock & SentReceipt to Compound Tooltips

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

* Workaround compound bug

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

* Fix tests

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

* Improve coverage

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

* Iterate tests

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

* Iterate tests

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

* Fix tests

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

---------

Signed-off-by: Michael Telatynski <7t3chguy@gmail.com>
  • Loading branch information
t3chguy authored Jan 11, 2024
1 parent 88a57fb commit 0820994
Show file tree
Hide file tree
Showing 8 changed files with 150 additions and 76 deletions.
18 changes: 10 additions & 8 deletions playwright/e2e/crypto/crypto.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -336,7 +336,8 @@ test.describe("Cryptography", function () {
await expect(last).toContainText("Unable to decrypt message");
const lastE2eIcon = last.locator(".mx_EventTile_e2eIcon");
await expect(lastE2eIcon).toHaveClass(/mx_EventTile_e2eIcon_decryption_failure/);
await expect(lastE2eIcon).toHaveAttribute("aria-label", "This message could not be decrypted");
await lastE2eIcon.hover();
await expect(page.getByRole("tooltip")).toContainText("This message could not be decrypted");

/* Should show a red padlock for an unencrypted message in an e2e room */
await bob.evaluate(
Expand All @@ -355,7 +356,8 @@ test.describe("Cryptography", function () {

await expect(last).toContainText("test unencrypted");
await expect(lastE2eIcon).toHaveClass(/mx_EventTile_e2eIcon_warning/);
await expect(lastE2eIcon).toHaveAttribute("aria-label", "Not encrypted");
await lastE2eIcon.hover();
await expect(page.getByRole("tooltip")).toContainText("Not encrypted");

/* Should show no padlock for an unverified user */
// bob sends a valid event
Expand Down Expand Up @@ -387,10 +389,8 @@ test.describe("Cryptography", function () {
await bobSecondDevice.sendMessage(testRoomId, "test encrypted from unverified");
await expect(lastTile).toContainText("test encrypted from unverified");
await expect(lastTileE2eIcon).toHaveClass(/mx_EventTile_e2eIcon_warning/);
await expect(lastTileE2eIcon).toHaveAttribute(
"aria-label",
"Encrypted by a device not verified by its owner.",
);
await lastTileE2eIcon.hover();
await expect(page.getByRole("tooltip")).toContainText("Encrypted by a device not verified by its owner.");

/* Should show a grey padlock for a message from an unknown device */
// bob deletes his second device
Expand Down Expand Up @@ -428,7 +428,8 @@ test.describe("Cryptography", function () {
} else {
await expect(lastE2eIcon).toHaveClass(/mx_EventTile_e2eIcon_normal/);
}
await expect(lastE2eIcon).toHaveAttribute("aria-label", "Encrypted by an unknown or deleted device.");
await lastE2eIcon.hover();
await expect(page.getByRole("tooltip")).toContainText("Encrypted by an unknown or deleted device.");
});

// XXX: Failed since migration to Playwright (https://github.com/element-hq/element-web/issues/26811)
Expand Down Expand Up @@ -462,7 +463,8 @@ test.describe("Cryptography", function () {
await app.viewRoomById(testRoomId);
await expect(lastTile).toContainText("test encrypted 1");
await expect(lastTileE2eIcon).toHaveClass(/mx_EventTile_e2eIcon_warning/);
await expect(lastTileE2eIcon).toHaveAttribute("aria-label", "Encrypted by an unknown or deleted device.");
await lastTileE2eIcon.hover();
await expect(page.getByRole("tooltip")).toContainText("Encrypted by an unknown or deleted device.");
});

test("should show the correct shield on edited e2e events", async ({ page, app, bot: bob, homeserver }) => {
Expand Down
1 change: 1 addition & 0 deletions playwright/e2e/editing/editing.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -291,6 +291,7 @@ test.describe("Editing", () => {
await editComposer.press("Backspace");
await editComposer.press("Backspace");
await editComposer.press("Enter");
await app.getComposerField().hover(); // XXX: move the hover to get rid of the "Edit" tooltip
await checkA11y();
}
await expect(
Expand Down
5 changes: 0 additions & 5 deletions res/css/views/messages/_MStickerBody.pcss
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,6 @@ limitations under the License.
padding: 12px 0px;
}

.mx_MStickerBody_tooltip {
position: absolute;
top: 50%;
}

.mx_MStickerBody_hidden {
max-width: 220px;
text-decoration: none;
Expand Down
19 changes: 15 additions & 4 deletions src/components/views/messages/MImageBody.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import classNames from "classnames";
import { CSSTransition, SwitchTransition } from "react-transition-group";
import { logger } from "matrix-js-sdk/src/logger";
import { ClientEvent, ClientEventHandlerMap } from "matrix-js-sdk/src/matrix";
import { Tooltip } from "@vector-im/compound-web";

import MFileBody from "./MFileBody";
import Modal from "../../../Modal";
Expand Down Expand Up @@ -520,10 +521,12 @@ export default class MImageBody extends React.Component<IBodyProps, IState> {
);
}

const thumbnail = (
const tooltipProps = this.getTooltipProps();
let thumbnail = (
<div
className="mx_MImageBody_thumbnail_container"
style={{ maxHeight, maxWidth, aspectRatio: `${infoWidth}/${infoHeight}` }}
tabIndex={tooltipProps ? 0 : undefined}
>
{placeholder}

Expand All @@ -537,11 +540,19 @@ export default class MImageBody extends React.Component<IBodyProps, IState> {
{!this.props.forExport && !this.state.imgLoaded && (
<div style={{ height: maxHeight, width: maxWidth }} />
)}

{this.state.hover && this.getTooltip()}
</div>
);

if (tooltipProps) {
// We specify isTriggerInteractive=true and make the div interactive manually as a workaround for
// https://github.com/element-hq/compound/issues/294
thumbnail = (
<Tooltip {...tooltipProps} isTriggerInteractive={true}>
{thumbnail}
</Tooltip>
);
}

return this.wrapImage(contentUrl, thumbnail);
}

Expand Down Expand Up @@ -578,7 +589,7 @@ export default class MImageBody extends React.Component<IBodyProps, IState> {
}

// Overridden by MStickerBody
protected getTooltip(): ReactNode {
protected getTooltipProps(): ComponentProps<typeof Tooltip> | null {
return null;
}

Expand Down
18 changes: 9 additions & 9 deletions src/components/views/messages/MStickerBody.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,11 @@ See the License for the specific language governing permissions and
limitations under the License.
*/

import React, { ReactNode } from "react";
import React, { ComponentProps, ReactNode } from "react";
import { Tooltip } from "@vector-im/compound-web";

import MImageBody from "./MImageBody";
import { BLURHASH_FIELD } from "../../../utils/image-media";
import Tooltip from "../elements/Tooltip";
import { IMediaEventContent } from "../../../customisations/models/IMediaEventContent";

export default class MStickerBody extends MImageBody {
Expand Down Expand Up @@ -63,16 +63,16 @@ export default class MStickerBody extends MImageBody {
}

// Tooltip to show on mouse over
protected getTooltip(): ReactNode {
protected getTooltipProps(): ComponentProps<typeof Tooltip> | null {
const content = this.props.mxEvent && this.props.mxEvent.getContent();

if (!content || !content.body || !content.info || !content.info.w) return null;
if (!content?.body || !content.info?.w) return null;

return (
<div style={{ left: content.info.w + "px" }} className="mx_MStickerBody_tooltip">
<Tooltip label={content.body} />
</div>
);
return {
align: "center",
side: "right",
label: content.body,
};
}

// Don't show "Download this_file.png ..."
Expand Down
65 changes: 16 additions & 49 deletions src/components/views/rooms/EventTile.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ See the License for the specific language governing permissions and
limitations under the License.
*/

import React, { createRef, forwardRef, MouseEvent, ReactNode, useRef } from "react";
import React, { createRef, forwardRef, MouseEvent, ReactNode } from "react";
import classNames from "classnames";
import {
EventStatus,
Expand All @@ -37,6 +37,7 @@ import { CallErrorCode } from "matrix-js-sdk/src/webrtc/call";
import { CryptoEvent } from "matrix-js-sdk/src/crypto";
import { UserTrustLevel } from "matrix-js-sdk/src/crypto/CrossSigning";
import { EventShieldColour, EventShieldReason } from "matrix-js-sdk/src/crypto-api";
import { Tooltip } from "@vector-im/compound-web";

import ReplyChain from "../elements/ReplyChain";
import { _t } from "../../../languageHandler";
Expand All @@ -49,7 +50,6 @@ import RoomAvatar from "../avatars/RoomAvatar";
import MessageContextMenu from "../context_menus/MessageContextMenu";
import { aboveRightOf } from "../../structures/ContextMenu";
import { objectHasDiff } from "../../../utils/objects";
import Tooltip, { Alignment } from "../elements/Tooltip";
import EditorStateTransfer from "../../../utils/EditorStateTransfer";
import { RoomPermalinkCreator } from "../../../utils/permalinks/Permalinks";
import { StaticNotificationState } from "../../../stores/notifications/StaticNotificationState";
Expand Down Expand Up @@ -79,7 +79,6 @@ import TileErrorBoundary from "../messages/TileErrorBoundary";
import { haveRendererForEvent, isMessageEvent, renderTile } from "../../../events/EventTileFactory";
import ThreadSummary, { ThreadMessagePreview } from "./ThreadSummary";
import { ReadReceiptGroup } from "./ReadReceiptGroup";
import { useTooltip } from "../../../utils/useTooltip";
import { ShowThreadPayload } from "../../../dispatcher/payloads/ShowThreadPayload";
import { isLocalRoom } from "../../../utils/localRoom/isLocalRoom";
import { ElementCall } from "../../../models/Call";
Expand Down Expand Up @@ -1493,11 +1492,7 @@ interface IE2ePadlockProps {
title: string;
}

interface IE2ePadlockState {
hover: boolean;
}

class E2ePadlock extends React.Component<IE2ePadlockProps, IE2ePadlockState> {
class E2ePadlock extends React.Component<IE2ePadlockProps> {
public constructor(props: IE2ePadlockProps) {
super(props);

Expand All @@ -1506,30 +1501,14 @@ class E2ePadlock extends React.Component<IE2ePadlockProps, IE2ePadlockState> {
};
}

private onHoverStart = (): void => {
this.setState({ hover: true });
};

private onHoverEnd = (): void => {
this.setState({ hover: false });
};

public render(): React.ReactNode {
let tooltip: JSX.Element | undefined;
if (this.state.hover) {
tooltip = <Tooltip className="mx_EventTile_e2eIcon_tooltip" label={this.props.title} />;
}

public render(): ReactNode {
const classes = `mx_EventTile_e2eIcon mx_EventTile_e2eIcon_${this.props.icon}`;
// We specify isTriggerInteractive=true and make the div interactive manually as a workaround for
// https://github.com/element-hq/compound/issues/294
return (
<div
className={classes}
onMouseEnter={this.onHoverStart}
onMouseLeave={this.onHoverEnd}
aria-label={this.props.title}
>
{tooltip}
</div>
<Tooltip label={this.props.title} isTriggerInteractive={true}>
<div className={classes} tabIndex={0} />
</Tooltip>
);
}
}
Expand All @@ -1539,7 +1518,6 @@ interface ISentReceiptProps {
}

function SentReceipt({ messageState }: ISentReceiptProps): JSX.Element {
const tooltipId = useRef(`mx_SentReceipt_${Math.random()}`).current;
const isSent = !messageState || messageState === "sent";
const isFailed = messageState === "not_sent";
const receiptClasses = classNames({
Expand All @@ -1560,28 +1538,17 @@ function SentReceipt({ messageState }: ISentReceiptProps): JSX.Element {
} else if (isFailed) {
label = _t("timeline|send_state_failed");
}
const [{ showTooltip, hideTooltip }, tooltip] = useTooltip({
id: tooltipId,
label: label,
alignment: Alignment.TopRight,
});

return (
<div className="mx_EventTile_msgOption">
<div className="mx_ReadReceiptGroup">
<div
className="mx_ReadReceiptGroup_button"
onMouseOver={showTooltip}
onMouseLeave={hideTooltip}
onFocus={showTooltip}
onBlur={hideTooltip}
aria-describedby={tooltipId}
>
<span className="mx_ReadReceiptGroup_container">
<span className={receiptClasses}>{nonCssBadge}</span>
</span>
</div>
{tooltip}
<Tooltip label={label} side="top" align="end">
<div className="mx_ReadReceiptGroup_button">
<span className="mx_ReadReceiptGroup_container">
<span className={receiptClasses}>{nonCssBadge}</span>
</span>
</div>
</Tooltip>
</div>
</div>
);
Expand Down
94 changes: 94 additions & 0 deletions test/components/views/messages/MStickerBody-test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
/*
Copyright 2023 The Matrix.org Foundation C.I.C.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

import React from "react";
import { render, screen } from "@testing-library/react";
import { EventType, getHttpUriForMxc, MatrixEvent, Room } from "matrix-js-sdk/src/matrix";
import fetchMock from "fetch-mock-jest";
import userEvent from "@testing-library/user-event";

import { RoomPermalinkCreator } from "../../../../src/utils/permalinks/Permalinks";
import {
getMockClientWithEventEmitter,
mockClientMethodsCrypto,
mockClientMethodsDevice,
mockClientMethodsServer,
mockClientMethodsUser,
} from "../../../test-utils";
import SettingsStore from "../../../../src/settings/SettingsStore";
import MStickerBody from "../../../../src/components/views/messages/MStickerBody";

describe("<MStickerBody/>", () => {
const userId = "@user:server";
const deviceId = "DEADB33F";
const cli = getMockClientWithEventEmitter({
...mockClientMethodsUser(userId),
...mockClientMethodsServer(),
...mockClientMethodsDevice(deviceId),
...mockClientMethodsCrypto(),
getRooms: jest.fn().mockReturnValue([]),
getIgnoredUsers: jest.fn(),
getVersions: jest.fn().mockResolvedValue({
unstable_features: {
"org.matrix.msc3882": true,
"org.matrix.msc3886": true,
},
}),
});
const url = "https://server/_matrix/media/v3/download/server/sticker";
// eslint-disable-next-line no-restricted-properties
cli.mxcUrlToHttp.mockImplementation(
(mxcUrl: string, width?: number, height?: number, resizeMethod?: string, allowDirectLinks?: boolean) => {
return getHttpUriForMxc("https://server", mxcUrl, width, height, resizeMethod, allowDirectLinks);
},
);
const mediaEvent = new MatrixEvent({
room_id: "!room:server",
sender: userId,
type: EventType.RoomMessage,
content: {
body: "sticker description",
info: {
w: 40,
h: 50,
},
file: {
url: "mxc://server/sticker",
},
},
});

const props = {
onHeightChanged: jest.fn(),
onMessageAllowed: jest.fn(),
permalinkCreator: new RoomPermalinkCreator(new Room(mediaEvent.getRoomId()!, cli, cli.getUserId()!)),
};

beforeEach(() => {
jest.spyOn(SettingsStore, "getValue").mockRestore();
fetchMock.mockReset();
});

it("should show a tooltip on hover", async () => {
fetchMock.getOnce(url, { status: 200 });

render(<MStickerBody {...props} mxEvent={mediaEvent} />);

expect(screen.queryByRole("tooltip")).toBeNull();
await userEvent.hover(screen.getByRole("img"));
await expect(screen.findByRole("tooltip")).resolves.toHaveTextContent("sticker description");
});
});
6 changes: 5 additions & 1 deletion test/components/views/rooms/EventTile-test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -294,7 +294,11 @@ describe("EventTile", () => {
const e2eIcons = container.getElementsByClassName("mx_EventTile_e2eIcon");
expect(e2eIcons).toHaveLength(1);
expect(e2eIcons[0].classList).toContain("mx_EventTile_e2eIcon_normal");
expect(e2eIcons[0].getAttribute("aria-label")).toContain(expectedText);
fireEvent.focus(e2eIcons[0]);
expect(e2eIcons[0].getAttribute("aria-describedby")).toBeTruthy();
expect(document.getElementById(e2eIcons[0].getAttribute("aria-describedby")!)).toHaveTextContent(
expectedText,
);
});

describe("undecryptable event", () => {
Expand Down

0 comments on commit 0820994

Please sign in to comment.