From 0820994463ddb01ce520079be7e6fd3241d39a24 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Thu, 11 Jan 2024 11:49:24 +0000 Subject: [PATCH] Use Compound tooltips more widely (#12128) * 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> --- playwright/e2e/crypto/crypto.spec.ts | 18 ++-- playwright/e2e/editing/editing.spec.ts | 1 + res/css/views/messages/_MStickerBody.pcss | 5 - src/components/views/messages/MImageBody.tsx | 19 +++- .../views/messages/MStickerBody.tsx | 18 ++-- src/components/views/rooms/EventTile.tsx | 65 ++++--------- .../views/messages/MStickerBody-test.tsx | 94 +++++++++++++++++++ .../components/views/rooms/EventTile-test.tsx | 6 +- 8 files changed, 150 insertions(+), 76 deletions(-) create mode 100644 test/components/views/messages/MStickerBody-test.tsx diff --git a/playwright/e2e/crypto/crypto.spec.ts b/playwright/e2e/crypto/crypto.spec.ts index 7a18a1dd49..c9cad8af77 100644 --- a/playwright/e2e/crypto/crypto.spec.ts +++ b/playwright/e2e/crypto/crypto.spec.ts @@ -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( @@ -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 @@ -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 @@ -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) @@ -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 }) => { diff --git a/playwright/e2e/editing/editing.spec.ts b/playwright/e2e/editing/editing.spec.ts index 2a95b4884a..7ed5d136fe 100644 --- a/playwright/e2e/editing/editing.spec.ts +++ b/playwright/e2e/editing/editing.spec.ts @@ -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( diff --git a/res/css/views/messages/_MStickerBody.pcss b/res/css/views/messages/_MStickerBody.pcss index 4d1b49b5ff..5fea4adf86 100644 --- a/res/css/views/messages/_MStickerBody.pcss +++ b/res/css/views/messages/_MStickerBody.pcss @@ -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; diff --git a/src/components/views/messages/MImageBody.tsx b/src/components/views/messages/MImageBody.tsx index 4cb7d281cb..ff4d573e05 100644 --- a/src/components/views/messages/MImageBody.tsx +++ b/src/components/views/messages/MImageBody.tsx @@ -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"; @@ -520,10 +521,12 @@ export default class MImageBody extends React.Component { ); } - const thumbnail = ( + const tooltipProps = this.getTooltipProps(); + let thumbnail = (
{placeholder} @@ -537,11 +540,19 @@ export default class MImageBody extends React.Component { {!this.props.forExport && !this.state.imgLoaded && (
)} - - {this.state.hover && this.getTooltip()}
); + 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 = ( + + {thumbnail} + + ); + } + return this.wrapImage(contentUrl, thumbnail); } @@ -578,7 +589,7 @@ export default class MImageBody extends React.Component { } // Overridden by MStickerBody - protected getTooltip(): ReactNode { + protected getTooltipProps(): ComponentProps | null { return null; } diff --git a/src/components/views/messages/MStickerBody.tsx b/src/components/views/messages/MStickerBody.tsx index 092b948fc6..26c33e89fc 100644 --- a/src/components/views/messages/MStickerBody.tsx +++ b/src/components/views/messages/MStickerBody.tsx @@ -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 { @@ -63,16 +63,16 @@ export default class MStickerBody extends MImageBody { } // Tooltip to show on mouse over - protected getTooltip(): ReactNode { + protected getTooltipProps(): ComponentProps | 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 ( -
- -
- ); + return { + align: "center", + side: "right", + label: content.body, + }; } // Don't show "Download this_file.png ..." diff --git a/src/components/views/rooms/EventTile.tsx b/src/components/views/rooms/EventTile.tsx index 54d874d785..8ea3e35f67 100644 --- a/src/components/views/rooms/EventTile.tsx +++ b/src/components/views/rooms/EventTile.tsx @@ -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, @@ -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"; @@ -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"; @@ -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"; @@ -1493,11 +1492,7 @@ interface IE2ePadlockProps { title: string; } -interface IE2ePadlockState { - hover: boolean; -} - -class E2ePadlock extends React.Component { +class E2ePadlock extends React.Component { public constructor(props: IE2ePadlockProps) { super(props); @@ -1506,30 +1501,14 @@ class E2ePadlock extends React.Component { }; } - 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 = ; - } - + 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 ( -
- {tooltip} -
+ +
+ ); } } @@ -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({ @@ -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 (
-
- - {nonCssBadge} - -
- {tooltip} + +
+ + {nonCssBadge} + +
+
); diff --git a/test/components/views/messages/MStickerBody-test.tsx b/test/components/views/messages/MStickerBody-test.tsx new file mode 100644 index 0000000000..e541149a79 --- /dev/null +++ b/test/components/views/messages/MStickerBody-test.tsx @@ -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("", () => { + 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(); + + expect(screen.queryByRole("tooltip")).toBeNull(); + await userEvent.hover(screen.getByRole("img")); + await expect(screen.findByRole("tooltip")).resolves.toHaveTextContent("sticker description"); + }); +}); diff --git a/test/components/views/rooms/EventTile-test.tsx b/test/components/views/rooms/EventTile-test.tsx index 19b539d583..ddc0b8c2b0 100644 --- a/test/components/views/rooms/EventTile-test.tsx +++ b/test/components/views/rooms/EventTile-test.tsx @@ -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", () => {