Update design of files list in right panel (#144)

* Update design of files list in right panel Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> * Make i18n script happier Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> * Improve coverage Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> * Improve coverage Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> * Improve coverage Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> * Discard changes to src/components/structures/MessagePanel.tsx * Iterate Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> * Iterate Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> * Iterate Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> * Update tests Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> * Fix flaky screenshot test Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> * Update screenshot Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --------- Signed-off-by: Michael Telatynski <7t3chguy@gmail.com>
element-hq · Oct 14, 2024 · 0750625 · 0750625
1 parent d770e2a
commit 0750625
Show file tree

Hide file tree

Showing 20 changed files with 276 additions and 107 deletions.
diff --git a/playwright/e2e/right-panel/file-panel.spec.ts b/playwright/e2e/right-panel/file-panel.spec.ts
@@ -84,7 +84,7 @@ test.describe("FilePanel", () => {
             await expect(filePanelMessageList.locator(".mx_EventTile")).toHaveCount(3);
 
             // Assert that the download links are rendered
-            await expect(filePanelMessageList.locator(".mx_MFileBody_download")).toHaveCount(3);
+            await expect(filePanelMessageList.locator(".mx_MFileBody_download,.mx_MFileBody_info")).toHaveCount(3);
 
             // Assert that the sender of the files is rendered on all of the tiles
             await expect(filePanelMessageList.getByText(NAME)).toHaveCount(3);
@@ -176,8 +176,7 @@ test.describe("FilePanel", () => {
             // Assert that the file size is displayed in kibibytes, not kilobytes (1000 bytes)
             // See: https://github.com/vector-im/element-web/issues/24866
             await expect(tile.locator(".mx_MFileBody_info_filename", { hasText: size })).toBeVisible();
-            await expect(tile.locator(".mx_MFileBody_download a", { hasText: size })).toBeVisible();
-            await expect(tile.locator(".mx_MFileBody_download .mx_MImageBody_size", { hasText: size })).toBeVisible();
+            await expect(tile.locator(".mx_MFileBody_info", { hasText: size })).toBeVisible();
         });
     });
 

diff --git a/...spec.ts/Selected-EventTile-of-audio-player-with-a-reply-bubble-layout-linux.png b/...spec.ts/Selected-EventTile-of-audio-player-with-a-reply-bubble-layout-linux.png
diff --git a/...s/Selected-EventTile-of-audio-player-with-a-reply-chain-bubble-layout-linux.png b/...s/Selected-EventTile-of-audio-player-with-a-reply-chain-bubble-layout-linux.png
diff --git a/...ts/Selected-EventTile-of-audio-player-with-a-reply-chain-group-layout-linux.png b/...ts/Selected-EventTile-of-audio-player-with-a-reply-chain-group-layout-linux.png
diff --git a/...c.ts/Selected-EventTile-of-audio-player-with-a-reply-chain-irc-layout-linux.png b/...c.ts/Selected-EventTile-of-audio-player-with-a-reply-chain-irc-layout-linux.png
diff --git a/....spec.ts/Selected-EventTile-of-audio-player-with-a-reply-group-layout-linux.png b/....spec.ts/Selected-EventTile-of-audio-player-with-a-reply-group-layout-linux.png
diff --git a/...er.spec.ts/Selected-EventTile-of-audio-player-with-a-reply-irc-layout-linux.png b/...er.spec.ts/Selected-EventTile-of-audio-player-with-a-reply-irc-layout-linux.png
diff --git a/playwright/snapshots/right-panel/file-panel.spec.ts/file-tiles-list-linux.png b/playwright/snapshots/right-panel/file-panel.spec.ts/file-tiles-list-linux.png
diff --git a/res/css/structures/_FilePanel.pcss b/res/css/structures/_FilePanel.pcss
@@ -20,70 +20,51 @@ Please see LICENSE files in the repository root for full details.
 
     .mx_RoomView_MessageList {
         width: 100%;
-
-        h2 {
-            display: none;
-        }
+        gap: var(--cpd-space-6x);
     }
 
     /* FIXME: rather than having EventTile's default CSS be for MessagePanel,
    we should make EventTile a base CSS class and customise it specifically
    for usage in {Message,File,Notification}Panel. */
 
-    .mx_EventTile_avatar {
-        display: none;
-    }
-
     /* Overrides for the attachment body tiles */
     .mx_EventTile {
         word-break: break-word;
-        margin-top: 10px;
         padding-top: 0;
 
-        .mx_EventTile_line {
-            padding-inline-start: 0;
+        & + .mx_EventTile {
+            border-top: 1px solid var(--cpd-color-gray-400);
+            padding-top: var(--cpd-space-6x);
         }
 
-        .mx_MFileBody {
-            line-height: 2.4rem;
+        .mx_EventTile_line {
+            padding-inline-start: 0;
         }
 
         .mx_MFileBody_download {
-            padding-top: $spacing-8;
-            display: flex;
-            justify-content: space-between;
-            font: var(--cpd-font-body-md-regular);
-            color: $event-timestamp-color;
-
-            .mx_MImageBody_size {
-                font: var(--cpd-font-body-md-regular);
-                text-align: right;
-                white-space: nowrap;
-            }
-        }
-
-        .mx_MFileBody_downloadLink {
-            flex: 1 1 auto;
-            color: $light-fg-color;
+            margin-top: var(--cpd-space-4x);
         }
 
         /* anchor link as wrapper */
         .mx_EventTile_senderDetailsLink {
             text-decoration: none;
+            margin-bottom: var(--cpd-space-1x);
+            display: block;
 
             .mx_EventTile_senderDetails {
                 display: flex;
-                justify-content: space-between;
                 margin-top: -2px;
+                gap: var(--cpd-space-2x);
+                align-items: center;
 
                 .mx_DisambiguatedProfile {
                     color: $event-timestamp-color; /* for ellipsis. Color of displayName and mxid is inherited */
                 }
 
                 .mx_MessageTimestamp {
-                    text-align: right;
-                    color: $secondary-content;
-                    font: var(--cpd-font-body-sm-regular);
+                    margin-left: auto;
+                    font: var(--cpd-font-body-xs-regular);
+                    color: var(--cpd-color-text-secondary);
                 }
             }
         }

diff --git a/res/css/views/messages/_MFileBody.pcss b/res/css/views/messages/_MFileBody.pcss
@@ -8,25 +8,7 @@ Please see LICENSE files in the repository root for full details.
 
 .mx_MFileBody_download {
     color: $accent;
-
-    .mx_MFileBody_download_icon {
-        /* 12px instead of 14px to better match surrounding font size */
-        width: 12px;
-        height: 12px;
-        mask-size: 12px;
-
-        mask-position: center;
-        mask-repeat: no-repeat;
-        mask-image: url("$(res)/img/download.svg");
-        background-color: $accent;
-        display: inline-block;
-    }
-}
-
-.mx_MFileBody_download a {
-    color: $accent;
-    text-decoration: none;
-    cursor: pointer;
+    height: var(--cpd-space-9x);
 }
 
 .mx_MFileBody_download object {
@@ -43,12 +25,6 @@ Please see LICENSE files in the repository root for full details.
     padding: 0px;
     border: none;
     width: 100%;
-    /* Set the height of the iframe to be 1 line of text.
-     * Iframes don't automatically size themselves to fit their content.
-     * So either we have to fix the height of the iframe using CSS or
-     * use javascript's cross-origin postMessage API to communicate how
-     * big the content of the iframe is. */
-    height: 1.5em;
 }
 
 .mx_MFileBody_info {
@@ -81,6 +57,8 @@ Please see LICENSE files in the repository root for full details.
     }
 
     .mx_MFileBody_info_filename {
+        font: var(--cpd-font-body-md-regular);
+        color: var(--cpd-color-text-primary);
         text-overflow: ellipsis;
         overflow: hidden;
         white-space: nowrap;

diff --git a/src/components/structures/MessagePanel.tsx b/src/components/structures/MessagePanel.tsx
@@ -477,6 +477,8 @@ export default class MessagePanel extends React.Component<IProps, IState> {
     }
 
     public readMarkerForEvent(eventId: string, isLastEvent: boolean): ReactNode {
+        if (this.context.timelineRenderingType === TimelineRenderingType.File) return null;
+
         const visible = !isLastEvent && this.props.readMarkerVisible;
 
         if (this.props.readMarkerEventId === eventId) {

diff --git a/src/components/views/messages/MFileBody.tsx b/src/components/views/messages/MFileBody.tsx
@@ -9,13 +9,15 @@ Please see LICENSE files in the repository root for full details.
 import React, { AllHTMLAttributes, createRef } from "react";
 import { logger } from "matrix-js-sdk/src/logger";
 import { MediaEventContent } from "matrix-js-sdk/src/types";
+import { Button } from "@vector-im/compound-web";
+import { DownloadIcon } from "@vector-im/compound-design-tokens/assets/web/icons";
 
 import { _t } from "../../../languageHandler";
 import Modal from "../../../Modal";
 import AccessibleButton from "../elements/AccessibleButton";
 import { mediaFromContent } from "../../../customisations/Media";
 import ErrorDialog from "../dialogs/ErrorDialog";
-import { fileSize, presentableTextForFile } from "../../../utils/FileUtils";
+import { downloadLabelForFile, presentableTextForFile } from "../../../utils/FileUtils";
 import { IBodyProps } from "./IBodyProps";
 import { FileDownloader } from "../../../utils/FileDownloader";
 import TextWithTooltip from "../elements/TextWithTooltip";
@@ -26,7 +28,9 @@ export let DOWNLOAD_ICON_URL: string; // cached copy of the download.svg asset f
 async function cacheDownloadIcon(): Promise<void> {
     if (DOWNLOAD_ICON_URL) return; // cached already
     // eslint-disable-next-line @typescript-eslint/no-var-requires
-    const svg = await fetch(require("../../../../res/img/download.svg").default).then((r) => r.text());
+    const svg = await fetch(require("@vector-im/compound-design-tokens/icons/download.svg").default).then((r) =>
+        r.text(),
+    );
     DOWNLOAD_ICON_URL = "data:image/svg+xml;base64," + window.btoa(svg);
 }
 
@@ -125,7 +129,7 @@ export default class MFileBody extends React.Component<IProps, IState> {
     }
 
     private get linkText(): string {
-        return presentableTextForFile(this.content);
+        return downloadLabelForFile(this.content, true);
     }
 
     private downloadFile(fileName: string, text: string): void {
@@ -138,7 +142,7 @@ export default class MFileBody extends React.Component<IProps, IState> {
                 imgSrc: DOWNLOAD_ICON_URL,
                 imgStyle: null,
                 style: computedStyle(this.dummyLink.current),
-                textContent: _t("timeline|m.file|download_label", { text }),
+                textContent: text,
             },
         });
     }
@@ -188,6 +192,12 @@ export default class MFileBody extends React.Component<IProps, IState> {
         const contentFileSize = this.content.info ? this.content.info.size : null;
         const fileType = this.content.info?.mimetype ?? "application/octet-stream";
 
+        let showDownloadLink =
+            !this.props.showGenericPlaceholder ||
+            (this.context.timelineRenderingType !== TimelineRenderingType.Room &&
+                this.context.timelineRenderingType !== TimelineRenderingType.Search &&
+                this.context.timelineRenderingType !== TimelineRenderingType.Pinned);
+
         let placeholder: React.ReactNode = null;
         if (this.props.showGenericPlaceholder) {
             placeholder = (
@@ -200,6 +210,7 @@ export default class MFileBody extends React.Component<IProps, IState> {
                     </TextWithTooltip>
                 </AccessibleButton>
             );
+            showDownloadLink = false;
         }
 
         if (this.props.forExport) {
@@ -212,12 +223,6 @@ export default class MFileBody extends React.Component<IProps, IState> {
             );
         }
 
-        let showDownloadLink =
-            !this.props.showGenericPlaceholder ||
-            (this.context.timelineRenderingType !== TimelineRenderingType.Room &&
-                this.context.timelineRenderingType !== TimelineRenderingType.Search &&
-                this.context.timelineRenderingType !== TimelineRenderingType.Pinned);
-
         if (this.context.timelineRenderingType === TimelineRenderingType.Thread) {
             showDownloadLink = false;
         }
@@ -235,9 +240,9 @@ export default class MFileBody extends React.Component<IProps, IState> {
                         {placeholder}
                         {showDownloadLink && (
                             <div className="mx_MFileBody_download">
-                                <AccessibleButton onClick={this.decryptFile}>
-                                    {_t("timeline|m.file|decrypt_label", { text: this.linkText })}
-                                </AccessibleButton>
+                                <Button size="sm" kind="secondary" Icon={DownloadIcon} onClick={this.decryptFile}>
+                                    {this.linkText}
+                                </Button>
                             </div>
                         )}
                     </span>
@@ -254,14 +259,13 @@ export default class MFileBody extends React.Component<IProps, IState> {
                         <div className="mx_MFileBody_download">
                             <div aria-hidden style={{ display: "none" }}>
                                 {/*
-                                 * Add dummy copy of the "a" tag
-                                 * We'll use it to learn how the download link
+                                 * Add dummy copy of the button
+                                 * We'll use it to learn how the download button
                                  * would have been styled if it was rendered inline.
                                  */}
                                 {/* this violates multiple eslint rules
                             so ignore it completely */}
-                                {/* eslint-disable-next-line */}
-                                <a ref={this.dummyLink} />
+                                <Button size="sm" kind="secondary" Icon={DownloadIcon} as="a" ref={this.dummyLink} />
                             </div>
                             {/*
                             TODO: Move iframe (and dummy link) into FileDownloader.
@@ -283,7 +287,10 @@ export default class MFileBody extends React.Component<IProps, IState> {
                 </span>
             );
         } else if (contentUrl) {
-            const downloadProps: AllHTMLAttributes<HTMLAnchorElement> = {
+            const downloadProps: Pick<
+                AllHTMLAttributes<HTMLAnchorElement>,
+                "target" | "rel" | "href" | "onClick" | "download"
+            > = {
                 target: "_blank",
                 rel: "noreferrer noopener",
 
@@ -332,25 +339,18 @@ export default class MFileBody extends React.Component<IProps, IState> {
                     {placeholder}
                     {showDownloadLink && (
                         <div className="mx_MFileBody_download">
-                            <a {...downloadProps}>
-                                <span className="mx_MFileBody_download_icon" />
-                                {_t("timeline|m.file|download_label", { text: this.linkText })}
-                            </a>
-                            {this.context.timelineRenderingType === TimelineRenderingType.File && (
-                                <div className="mx_MImageBody_size">
-                                    {this.content.info?.size ? fileSize(this.content.info.size) : ""}
-                                </div>
-                            )}
+                            <Button size="sm" kind="secondary" Icon={DownloadIcon} as="a" {...downloadProps}>
+                                {this.linkText}
+                            </Button>
                         </div>
                     )}
                 </span>
             );
         } else {
-            const extra = this.linkText ? ": " + this.linkText : "";
             return (
                 <span className="mx_MFileBody">
                     {placeholder}
-                    {_t("timeline|m.file|error_invalid", { extra: extra })}
+                    {_t("timeline|m.file|error_invalid")}
                 </span>
             );
         }

diff --git a/src/components/views/rooms/EventTile.tsx b/src/components/views/rooms/EventTile.tsx
@@ -1024,6 +1024,9 @@ export class UnwrappedEventTile extends React.Component<EventTileProps, IState>
             // no avatar or sender profile for continuation messages and call tiles
             avatarSize = null;
             needsSenderProfile = false;
+        } else if (this.context.timelineRenderingType === TimelineRenderingType.File) {
+            avatarSize = "20px";
+            needsSenderProfile = true;
         } else {
             avatarSize = "30px";
             needsSenderProfile = true;
@@ -1351,6 +1354,18 @@ export class UnwrappedEventTile extends React.Component<EventTileProps, IState>
                         "data-scroll-tokens": scrollToken,
                     },
                     [
+                        <a
+                            className="mx_EventTile_senderDetailsLink"
+                            key="mx_EventTile_senderDetailsLink"
+                            href={permalink}
+                            onClick={this.onPermalinkClicked}
+                        >
+                            <div className="mx_EventTile_senderDetails" onContextMenu={this.onTimestampContextMenu}>
+                                {avatar}
+                                {sender}
+                                {timestamp}
+                            </div>
+                        </a>,
                         <div className={lineClasses} key="mx_EventTile_line" onContextMenu={this.onContextMenu}>
                             {this.renderContextMenu()}
                             {renderTile(
@@ -1371,17 +1386,6 @@ export class UnwrappedEventTile extends React.Component<EventTileProps, IState>
                                 this.context.showHiddenEvents,
                             )}
                         </div>,
-                        <a
-                            className="mx_EventTile_senderDetailsLink"
-                            key="mx_EventTile_senderDetailsLink"
-                            href={permalink}
-                            onClick={this.onPermalinkClicked}
-                        >
-                            <div className="mx_EventTile_senderDetails" onContextMenu={this.onTimestampContextMenu}>
-                                {sender}
-                                {timestamp}
-                            </div>
-                        </a>,
                     ],
                 );
             }

diff --git a/src/i18n/strings/en_EN.json b/src/i18n/strings/en_EN.json
@@ -3328,10 +3328,8 @@
             "voice_call_unsupported": "%(senderName)s placed a voice call. (not supported by this browser)"
         },
         "m.file": {
-            "decrypt_label": "Decrypt %(text)s",
-            "download_label": "Download %(text)s",
             "error_decrypting": "Error decrypting attachment",
-            "error_invalid": "Invalid file%(extra)s"
+            "error_invalid": "Invalid file"
         },
         "m.image": {
             "error": "Unable to show image due to error",

diff --git a/src/usercontent/index.ts b/src/usercontent/index.ts
@@ -36,9 +36,9 @@ function remoteRender(event: MessageEvent): void {
         // @ts-ignore
         img.style = data.imgStyle;
     } else {
-        img.style.width = "12px";
-        img.style.height = "12px";
-        img.style.webkitMaskSize = "12px";
+        img.style.width = "20px";
+        img.style.height = "20px";
+        img.style.webkitMaskSize = "20px";
         img.style.webkitMaskPosition = "center";
         img.style.webkitMaskRepeat = "no-repeat";
         img.style.display = "inline-block";