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

* Update design of files list in right panel Signed-off-by: Michael Telatynski <[email protected]> * Make i18n script happier Signed-off-by: Michael Telatynski <[email protected]> * Improve coverage Signed-off-by: Michael Telatynski <[email protected]> * Improve coverage Signed-off-by: Michael Telatynski <[email protected]> * Improve coverage Signed-off-by: Michael Telatynski <[email protected]> * Discard changes to src/components/structures/MessagePanel.tsx * Iterate Signed-off-by: Michael Telatynski <[email protected]> * Iterate Signed-off-by: Michael Telatynski <[email protected]> * Iterate Signed-off-by: Michael Telatynski <[email protected]> * Update tests Signed-off-by: Michael Telatynski <[email protected]> * Fix flaky screenshot test Signed-off-by: Michael Telatynski <[email protected]> * Update screenshot Signed-off-by: Michael Telatynski <[email protected]> --------- Signed-off-by: Michael Telatynski <[email protected]>
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";