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

Commit

Permalink
Upgrade to latest compound-web package (#84)
Browse files Browse the repository at this point in the history
* Upgrade to latest compound-web package

* Use a custom render function for jest tests

This way we don't need to manually wrap our components with
<TooltipProvider>

* Pin wrap-ansi to fix broken yarn install

* Add playwright helper to find tooltip from element

and use it in the failing test

* Exclude floating-ui divs/spans from axe testing

This is rendered outside .MatrixChat by compound and contains all the
tooltips.

* Wrap outermost components with TooltipProvider

* Remove onChange and use onSelect for toggle

* Fix jest tests and update snapshots

* Use vector-im/matrix-wysiwig

---------

Co-authored-by: Michael Telatynski <[email protected]>
  • Loading branch information
MidhunSureshR and t3chguy authored Oct 14, 2024
1 parent 3bc0439 commit 91e84f7
Show file tree
Hide file tree
Showing 389 changed files with 1,262 additions and 1,085 deletions.
4 changes: 4 additions & 0 deletions .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,10 @@ module.exports = {
"error",
{
paths: [
{
name: "@testing-library/react",
message: "Please use jest-matrix-react instead",
},
{
name: "matrix-js-sdk",
message: "Please use matrix-js-sdk/src/matrix instead",
Expand Down
1 change: 1 addition & 0 deletions jest.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ const config: Config = {
coverageReporters: ["text-summary", "lcov"],
testResultsProcessor: "@casualbot/jest-sonar-reporter",
prettierPath: null,
moduleDirectories: ["node_modules", "test/test-utils"],
};

// if we're running under GHA, enable the GHA reporter
Expand Down
6 changes: 4 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,9 @@
"jwt-decode": "4.0.0",
"@floating-ui/react": "0.26.11",
"@radix-ui/react-id": "1.1.0",
"caniuse-lite": "1.0.30001655"
"caniuse-lite": "1.0.30001655",
"wrap-ansi-cjs": "npm:wrap-ansi@^7.0.0",
"wrap-ansi": "npm:wrap-ansi@^7.0.0"
},
"dependencies": {
"@babel/runtime": "^7.12.5",
Expand All @@ -80,7 +82,7 @@
"@sentry/browser": "^8.0.0",
"@testing-library/react-hooks": "^8.0.1",
"@vector-im/compound-design-tokens": "^1.8.0",
"@vector-im/compound-web": "^5.5.0",
"@vector-im/compound-web": "^6.3.1",
"@zxcvbn-ts/core": "^3.0.4",
"@zxcvbn-ts/language-common": "^3.0.4",
"@zxcvbn-ts/language-en": "^3.0.2",
Expand Down
18 changes: 12 additions & 6 deletions playwright/e2e/crypto/event-shields.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,9 @@ test.describe("Cryptography", function () {
const lastE2eIcon = last.locator(".mx_EventTile_e2eIcon");
await expect(lastE2eIcon).toHaveClass(/mx_EventTile_e2eIcon_decryption_failure/);
await lastE2eIcon.focus();
await expect(page.getByRole("tooltip")).toContainText("This message could not be decrypted");
await expect(await app.getTooltipForElement(lastE2eIcon)).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 @@ -90,7 +92,7 @@ test.describe("Cryptography", function () {
await expect(last).toContainText("test unencrypted");
await expect(lastE2eIcon).toHaveClass(/mx_EventTile_e2eIcon_warning/);
await lastE2eIcon.focus();
await expect(page.getByRole("tooltip")).toContainText("Not encrypted");
await expect(await app.getTooltipForElement(lastE2eIcon)).toContainText("Not encrypted");

/* Should show no padlock for an unverified user */
// bob sends a valid event
Expand Down Expand Up @@ -123,7 +125,9 @@ test.describe("Cryptography", function () {
await expect(lastTile).toContainText("test encrypted from unverified");
await expect(lastTileE2eIcon).toHaveClass(/mx_EventTile_e2eIcon_warning/);
await lastTileE2eIcon.focus();
await expect(page.getByRole("tooltip")).toContainText("Encrypted by a device not verified by its owner.");
await expect(await app.getTooltipForElement(lastTileE2eIcon)).toContainText(
"Encrypted by a device not verified by its owner.",
);

/* In legacy crypto: should show a grey padlock for a message from a deleted device.
* In rust crypto: should show a red padlock for a message from an unverified device.
Expand Down Expand Up @@ -159,7 +163,7 @@ test.describe("Cryptography", function () {
await expect(last).toContainText("test encrypted from unverified");
await expect(lastE2eIcon).toHaveClass(/mx_EventTile_e2eIcon_warning/);
await lastE2eIcon.focus();
await expect(page.getByRole("tooltip")).toContainText(
await expect(await app.getTooltipForElement(lastE2eIcon)).toContainText(
workerInfo.project.name === "Legacy Crypto"
? "Encrypted by an unknown or deleted device."
: "Encrypted by a device not verified by its owner.",
Expand Down Expand Up @@ -212,7 +216,7 @@ test.describe("Cryptography", function () {
// The key is coming from backup, so it is not anymore possible to establish if the claimed device
// creator of this key is authentic. The tooltip should be "The authenticity of this encrypted message can't be guaranteed on this device."
// It is not "Encrypted by an unknown or deleted device." even if the claimed device is actually deleted.
await expect(page.getByRole("tooltip")).toContainText(
await expect(await app.getTooltipForElement(lastTileE2eIcon)).toContainText(
"The authenticity of this encrypted message can't be guaranteed on this device.",
);
});
Expand Down Expand Up @@ -296,7 +300,9 @@ test.describe("Cryptography", function () {
const lastE2eIcon = last.locator(".mx_EventTile_e2eIcon");
await expect(lastE2eIcon).toHaveClass(/mx_EventTile_e2eIcon_warning/);
await lastE2eIcon.focus();
await expect(page.getByRole("tooltip")).toContainText("Encrypted by a device not verified by its owner.");
await expect(await app.getTooltipForElement(lastE2eIcon)).toContainText(
"Encrypted by a device not verified by its owner.",
);

const penultimate = page.locator(".mx_EventTile").filter({ hasText: "test encrypted from verified" });
await expect(penultimate.locator(".mx_EventTile_e2eIcon")).not.toBeVisible();
Expand Down
2 changes: 1 addition & 1 deletion playwright/element-web-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,7 @@ export const test = base.extend<{
},

axe: async ({ page }, use) => {
await use(new AxeBuilder({ page }));
await use(new AxeBuilder({ page }).exclude("[id^='floating-ui-']"));
},
checkA11y: async ({ axe }, use, testInfo) =>
use(async () => {
Expand Down
18 changes: 18 additions & 0 deletions playwright/pages/ElementAppPage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -172,4 +172,22 @@ export class ElementAppPage {
await this.page.getByRole("button", { name: "Room info" }).first().click();
return this.page.locator(".mx_RightPanel");
}

/**
* Get a locator for the tooltip associated with an element
* @param e The element with the tooltip
* @returns Locator to the tooltip
*/
public async getTooltipForElement(e: Locator): Promise<Locator> {
const [labelledById, describedById] = await Promise.all([
e.getAttribute("aria-labelledby"),
e.getAttribute("aria-describedby"),
]);
if (!labelledById && !describedById) {
throw new Error(
"Element has no aria-labelledby or aria-describedy attributes! The tooltip should have added either one of these.",
);
}
return this.page.locator(`#${labelledById ?? describedById}`);
}
}
46 changes: 25 additions & 21 deletions src/Modal.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import ReactDOM from "react-dom";
import classNames from "classnames";
import { IDeferred, defer, sleep } from "matrix-js-sdk/src/utils";
import { TypedEventEmitter } from "matrix-js-sdk/src/matrix";
import { Glass } from "@vector-im/compound-web";
import { Glass, TooltipProvider } from "@vector-im/compound-web";

import dis, { defaultDispatcher } from "./dispatcher/dispatcher";
import AsyncWrapper from "./AsyncWrapper";
Expand Down Expand Up @@ -416,16 +416,18 @@ export class ModalManager extends TypedEventEmitter<ModalManagerEvent, HandlerMa
const classes = classNames("mx_Dialog_wrapper mx_Dialog_staticWrapper", this.staticModal.className);

const staticDialog = (
<div className={classes}>
<Glass className="mx_Dialog_border">
<div className="mx_Dialog">{this.staticModal.elem}</div>
</Glass>
<div
data-testid="dialog-background"
className="mx_Dialog_background mx_Dialog_staticBackground"
onClick={this.onBackgroundClick}
/>
</div>
<TooltipProvider>
<div className={classes}>
<Glass className="mx_Dialog_border">
<div className="mx_Dialog">{this.staticModal.elem}</div>
</Glass>
<div
data-testid="dialog-background"
className="mx_Dialog_background mx_Dialog_staticBackground"
onClick={this.onBackgroundClick}
/>
</div>
</TooltipProvider>
);

ReactDOM.render(staticDialog, ModalManager.getOrCreateStaticContainer());
Expand All @@ -441,16 +443,18 @@ export class ModalManager extends TypedEventEmitter<ModalManagerEvent, HandlerMa
});

const dialog = (
<div className={classes}>
<Glass className="mx_Dialog_border">
<div className="mx_Dialog">{modal.elem}</div>
</Glass>
<div
data-testid="dialog-background"
className="mx_Dialog_background"
onClick={this.onBackgroundClick}
/>
</div>
<TooltipProvider>
<div className={classes}>
<Glass className="mx_Dialog_border">
<div className="mx_Dialog">{modal.elem}</div>
</Glass>
<div
data-testid="dialog-background"
className="mx_Dialog_background"
onClick={this.onBackgroundClick}
/>
</div>
</TooltipProvider>
);

setTimeout(() => ReactDOM.render(dialog, ModalManager.getOrCreateContainer()), 0);
Expand Down
21 changes: 12 additions & 9 deletions src/components/structures/ContextMenu.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import React, { CSSProperties, RefObject, SyntheticEvent, useRef, useState } fro
import ReactDOM from "react-dom";
import classNames from "classnames";
import FocusLock from "react-focus-lock";
import { TooltipProvider } from "@vector-im/compound-web";

import { Writeable } from "../../@types/common";
import UIStore from "../../stores/UIStore";
Expand Down Expand Up @@ -621,15 +622,17 @@ export function createMenu(
};

const menu = (
<ContextMenu
{...props}
mountAsChild={true}
hasBackground={false}
onFinished={onFinished} // eslint-disable-line react/jsx-no-bind
windowResize={onFinished} // eslint-disable-line react/jsx-no-bind
>
<ElementClass {...props} onFinished={onFinished} />
</ContextMenu>
<TooltipProvider>
<ContextMenu
{...props}
mountAsChild={true}
hasBackground={false}
onFinished={onFinished} // eslint-disable-line react/jsx-no-bind
windowResize={onFinished} // eslint-disable-line react/jsx-no-bind
>
<ElementClass {...props} onFinished={onFinished} />
</ContextMenu>
</TooltipProvider>
);

ReactDOM.render(menu, getOrCreateContainer());
Expand Down
5 changes: 4 additions & 1 deletion src/components/structures/MatrixChat.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import { logger } from "matrix-js-sdk/src/logger";
import { throttle } from "lodash";
import { CryptoEvent } from "matrix-js-sdk/src/crypto";
import { KeyBackupInfo } from "matrix-js-sdk/src/crypto-api";
import { TooltipProvider } from "@vector-im/compound-web";

// what-input helps improve keyboard accessibility
import "what-input";
Expand Down Expand Up @@ -2181,7 +2182,9 @@ export default class MatrixChat extends React.PureComponent<IProps, IState> {

return (
<ErrorBoundary>
<SDKContext.Provider value={this.stores}>{view}</SDKContext.Provider>
<SDKContext.Provider value={this.stores}>
<TooltipProvider>{view}</TooltipProvider>
</SDKContext.Provider>
</ErrorBoundary>
);
}
Expand Down
2 changes: 1 addition & 1 deletion src/components/structures/ThreadPanel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ export const ThreadPanelHeader: React.FC<{
return (
<div className="mx_BaseCard_header_title">
<Tooltip label={_t("threads|mark_all_read")}>
<IconButton onClick={onMarkAllThreadsReadClick} aria-label={_t("threads|mark_all_read")} size="24px">
<IconButton onClick={onMarkAllThreadsReadClick} size="24px">
<MarkAllThreadsReadIcon />
</IconButton>
</Tooltip>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ export const CheckEmail: React.FC<CheckEmailProps> = ({
<input onClick={onSubmitForm} type="button" className="mx_Login_submit" value={_t("action|next")} />
<div className="mx_AuthBody_did-not-receive">
<span className="mx_VerifyEMailDialog_text-light">{_t("auth|check_email_resend_prompt")}</span>
<Tooltip label={_t("auth|check_email_resend_tooltip")} placement="top" open={tooltipVisible}>
<Tooltip description={_t("auth|check_email_resend_tooltip")} placement="top" open={tooltipVisible}>
<AccessibleButton className="mx_AuthBody_resend-button" kind="link" onClick={onResendClickFn}>
<RetryIcon className="mx_Icon mx_Icon_16" />
{_t("action|resend")}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ export const VerifyEmailModal: React.FC<Props> = ({

<div className="mx_AuthBody_did-not-receive">
<span className="mx_VerifyEMailDialog_text-light">{_t("auth|check_email_resend_prompt")}</span>
<Tooltip label={_t("auth|check_email_resend_tooltip")} placement="top" open={tooltipVisible}>
<Tooltip description={_t("auth|check_email_resend_tooltip")} placement="top" open={tooltipVisible}>
<AccessibleButton className="mx_AuthBody_resend-button" kind="link" onClick={onResendClickFn}>
<RetryIcon className="mx_Icon mx_Icon_16" />
{_t("action|resend")}
Expand Down
2 changes: 1 addition & 1 deletion src/components/views/elements/AccessibleButton.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,7 @@ const AccessibleButton = forwardRef(function <T extends keyof JSX.IntrinsicEleme
if (title) {
return (
<Tooltip
label={title}
description={title}
caption={caption}
isTriggerInteractive={true}
placement={placement}
Expand Down
2 changes: 1 addition & 1 deletion src/components/views/elements/InfoTooltip.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ export default class InfoTooltip extends React.PureComponent<TooltipProps> {

// Tooltip are forced on the right for a more natural feel to them on info icons
return (
<Tooltip label={tooltip || title} placement="right">
<Tooltip description={tooltip || title} placement="right">
<div className={classNames("mx_InfoTooltip", className)} tabIndex={this.props.tabIndex ?? 0}>
<span className={classNames("mx_InfoTooltip_icon", iconClassName)} aria-label={title} />
{children}
Expand Down
9 changes: 6 additions & 3 deletions src/components/views/elements/PersistedElement.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ Please see LICENSE files in the repository root for full details.
import React, { MutableRefObject, ReactNode } from "react";
import ReactDOM from "react-dom";
import { isNullOrUndefined } from "matrix-js-sdk/src/utils";
import { TooltipProvider } from "@vector-im/compound-web";

import dis from "../../../dispatcher/dispatcher";
import MatrixClientContext from "../../../contexts/MatrixClientContext";
Expand Down Expand Up @@ -167,9 +168,11 @@ export default class PersistedElement extends React.Component<IProps> {
private renderApp(): void {
const content = (
<MatrixClientContext.Provider value={MatrixClientPeg.safeGet()}>
<div ref={this.collectChild} style={this.props.style}>
{this.props.children}
</div>
<TooltipProvider>
<div ref={this.collectChild} style={this.props.style}>
{this.props.children}
</div>
</TooltipProvider>
</MatrixClientContext.Provider>
);

Expand Down
2 changes: 1 addition & 1 deletion src/components/views/elements/Pill.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ export const Pill: React.FC<PillProps> = ({ type: propType, url, inMessage, room
<bdi>
<MatrixClientContext.Provider value={MatrixClientPeg.safeGet()}>
<Tooltip
label={resourceId ?? ""}
description={resourceId ?? ""}
open={resourceId ? undefined : false}
placement="right"
isTriggerInteractive={isAnchor}
Expand Down
2 changes: 1 addition & 1 deletion src/components/views/elements/RoomTopic.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ export default function RoomTopic({ room, className, ...props }: IProps): JSX.El
if (!body) return <div className={classNames(className, "mx_RoomTopic")} />;

return (
<Tooltip label={_t("room|read_topic")} disabled={disableTooltip}>
<Tooltip description={_t("room|read_topic")} disabled={disableTooltip}>
<div
{...props}
tabIndex={0}
Expand Down
2 changes: 1 addition & 1 deletion src/components/views/messages/MStickerBody.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ export default class MStickerBody extends MImageBody {

return {
placement: "right",
label: content.body,
description: content.body,
};
}

Expand Down
2 changes: 1 addition & 1 deletion src/components/views/messages/MessageTimestamp.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ export default class MessageTimestamp extends React.Component<IProps> {
}

return (
<Tooltip label={label} caption={caption}>
<Tooltip description={label} caption={caption}>
<span className="mx_MessageTimestamp" aria-hidden={true} aria-live="off">
{icon}
{timestamp}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ export default class ReactionsRowButtonTooltip extends React.PureComponent<Props
const caption = shortName ? _t("timeline|reactions|tooltip_caption", { shortName }) : undefined;

return (
<Tooltip label={formattedSenders} caption={caption} placement="right">
<Tooltip description={formattedSenders} caption={caption} placement="right">
{children}
</Tooltip>
);
Expand Down
7 changes: 6 additions & 1 deletion src/components/views/messages/TextualBody.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ Please see LICENSE files in the repository root for full details.
import React, { createRef, SyntheticEvent, MouseEvent } from "react";
import ReactDOM from "react-dom";
import { MsgType } from "matrix-js-sdk/src/matrix";
import { TooltipProvider } from "@vector-im/compound-web";

import * as HtmlUtils from "../../../HtmlUtils";
import { formatDate } from "../../../DateUtils";
Expand Down Expand Up @@ -335,7 +336,11 @@ export default class TextualBody extends React.Component<IBodyProps, IState> {

const reason = node.getAttribute("data-mx-spoiler") ?? undefined;
node.removeAttribute("data-mx-spoiler"); // we don't want to recurse
const spoiler = <Spoiler reason={reason} contentHtml={node.outerHTML} />;
const spoiler = (
<TooltipProvider>
<Spoiler reason={reason} contentHtml={node.outerHTML} />
</TooltipProvider>
);

ReactDOM.render(spoiler, spoilerContainer);
node.parentNode?.replaceChild(spoilerContainer, node);
Expand Down
4 changes: 1 addition & 3 deletions src/components/views/right_panel/RoomSummaryCard.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -373,9 +373,7 @@ const RoomSummaryCard: React.FC<IProps> = ({
Icon={FavouriteIcon}
label={_t("room|context_menu|favourite")}
checked={isFavorite}
onChange={() => tagRoom(room, DefaultTagID.Favourite)}
// XXX: https://github.com/element-hq/compound/issues/288
onSelect={() => {}}
onSelect={() => tagRoom(room, DefaultTagID.Favourite)}
/>
<MenuItem
Icon={UserAddIcon}
Expand Down
Loading

0 comments on commit 91e84f7

Please sign in to comment.