diff --git a/playwright/e2e/right-panel/file-panel.spec.ts b/playwright/e2e/right-panel/file-panel.spec.ts index b508222f06..1cb39aad25 100644 --- 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/playwright/snapshots/audio-player/audio-player.spec.ts/Selected-EventTile-of-audio-player-with-a-reply-bubble-layout-linux.png b/playwright/snapshots/audio-player/audio-player.spec.ts/Selected-EventTile-of-audio-player-with-a-reply-bubble-layout-linux.png index bc3f87e221..61051aa939 100644 Binary files a/playwright/snapshots/audio-player/audio-player.spec.ts/Selected-EventTile-of-audio-player-with-a-reply-bubble-layout-linux.png and b/playwright/snapshots/audio-player/audio-player.spec.ts/Selected-EventTile-of-audio-player-with-a-reply-bubble-layout-linux.png differ diff --git a/playwright/snapshots/audio-player/audio-player.spec.ts/Selected-EventTile-of-audio-player-with-a-reply-chain-bubble-layout-linux.png b/playwright/snapshots/audio-player/audio-player.spec.ts/Selected-EventTile-of-audio-player-with-a-reply-chain-bubble-layout-linux.png index 88f8e1982d..3dded6ef82 100644 Binary files a/playwright/snapshots/audio-player/audio-player.spec.ts/Selected-EventTile-of-audio-player-with-a-reply-chain-bubble-layout-linux.png and b/playwright/snapshots/audio-player/audio-player.spec.ts/Selected-EventTile-of-audio-player-with-a-reply-chain-bubble-layout-linux.png differ diff --git a/playwright/snapshots/audio-player/audio-player.spec.ts/Selected-EventTile-of-audio-player-with-a-reply-chain-group-layout-linux.png b/playwright/snapshots/audio-player/audio-player.spec.ts/Selected-EventTile-of-audio-player-with-a-reply-chain-group-layout-linux.png index 834135a09b..6866b16475 100644 Binary files a/playwright/snapshots/audio-player/audio-player.spec.ts/Selected-EventTile-of-audio-player-with-a-reply-chain-group-layout-linux.png and b/playwright/snapshots/audio-player/audio-player.spec.ts/Selected-EventTile-of-audio-player-with-a-reply-chain-group-layout-linux.png differ diff --git a/playwright/snapshots/audio-player/audio-player.spec.ts/Selected-EventTile-of-audio-player-with-a-reply-chain-irc-layout-linux.png b/playwright/snapshots/audio-player/audio-player.spec.ts/Selected-EventTile-of-audio-player-with-a-reply-chain-irc-layout-linux.png index d87f2da4ba..a340f5c104 100644 Binary files a/playwright/snapshots/audio-player/audio-player.spec.ts/Selected-EventTile-of-audio-player-with-a-reply-chain-irc-layout-linux.png and b/playwright/snapshots/audio-player/audio-player.spec.ts/Selected-EventTile-of-audio-player-with-a-reply-chain-irc-layout-linux.png differ diff --git a/playwright/snapshots/audio-player/audio-player.spec.ts/Selected-EventTile-of-audio-player-with-a-reply-group-layout-linux.png b/playwright/snapshots/audio-player/audio-player.spec.ts/Selected-EventTile-of-audio-player-with-a-reply-group-layout-linux.png index 249f4dabfb..80927840ae 100644 Binary files a/playwright/snapshots/audio-player/audio-player.spec.ts/Selected-EventTile-of-audio-player-with-a-reply-group-layout-linux.png and b/playwright/snapshots/audio-player/audio-player.spec.ts/Selected-EventTile-of-audio-player-with-a-reply-group-layout-linux.png differ diff --git a/playwright/snapshots/audio-player/audio-player.spec.ts/Selected-EventTile-of-audio-player-with-a-reply-irc-layout-linux.png b/playwright/snapshots/audio-player/audio-player.spec.ts/Selected-EventTile-of-audio-player-with-a-reply-irc-layout-linux.png index d057007ec2..346642f84d 100644 Binary files a/playwright/snapshots/audio-player/audio-player.spec.ts/Selected-EventTile-of-audio-player-with-a-reply-irc-layout-linux.png and b/playwright/snapshots/audio-player/audio-player.spec.ts/Selected-EventTile-of-audio-player-with-a-reply-irc-layout-linux.png differ 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 index 1f24df116e..8b248e5240 100644 Binary files a/playwright/snapshots/right-panel/file-panel.spec.ts/file-tiles-list-linux.png and b/playwright/snapshots/right-panel/file-panel.spec.ts/file-tiles-list-linux.png differ diff --git a/res/css/structures/_FilePanel.pcss b/res/css/structures/_FilePanel.pcss index d9310c4c1f..ba6a623576 100644 --- 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 { + border-top: 1px solid var(--cpd-color-gray-400); + padding-top: var(--cpd-space-6x); + } + .mx_EventTile_line { padding-inline-start: 0; } - .mx_MFileBody { - line-height: 2.4rem; - } - .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 index cdc0f062a7..2ce13320ef 100644 --- 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 index cc3d10f2b8..97b8bc9c02 100644 --- a/src/components/structures/MessagePanel.tsx +++ b/src/components/structures/MessagePanel.tsx @@ -477,6 +477,8 @@ export default class MessagePanel extends React.Component { } 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 index f984575e3f..549c6ed251 100644 --- 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 { 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 { } 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 { 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 { 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 { ); + showDownloadLink = false; } if (this.props.forExport) { @@ -212,12 +223,6 @@ export default class MFileBody extends React.Component { ); } - 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 { {placeholder} {showDownloadLink && (
- - {_t("timeline|m.file|decrypt_label", { text: this.linkText })} - +
)} @@ -254,14 +259,13 @@ export default class MFileBody extends React.Component {
{/* - * 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 */} - +
{/* TODO: Move iframe (and dummy link) into FileDownloader. @@ -283,7 +287,10 @@ export default class MFileBody extends React.Component { ); } else if (contentUrl) { - const downloadProps: AllHTMLAttributes = { + const downloadProps: Pick< + AllHTMLAttributes, + "target" | "rel" | "href" | "onClick" | "download" + > = { target: "_blank", rel: "noreferrer noopener", @@ -332,25 +339,18 @@ export default class MFileBody extends React.Component { {placeholder} {showDownloadLink && (
- - - {_t("timeline|m.file|download_label", { text: this.linkText })} - - {this.context.timelineRenderingType === TimelineRenderingType.File && ( -
- {this.content.info?.size ? fileSize(this.content.info.size) : ""} -
- )} +
)} ); } else { - const extra = this.linkText ? ": " + this.linkText : ""; return ( {placeholder} - {_t("timeline|m.file|error_invalid", { extra: extra })} + {_t("timeline|m.file|error_invalid")} ); } diff --git a/src/components/views/rooms/EventTile.tsx b/src/components/views/rooms/EventTile.tsx index 9f7290b68b..813e5de40c 100644 --- a/src/components/views/rooms/EventTile.tsx +++ b/src/components/views/rooms/EventTile.tsx @@ -1024,6 +1024,9 @@ export class UnwrappedEventTile extends React.Component // 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 "data-scroll-tokens": scrollToken, }, [ + +
+ {avatar} + {sender} + {timestamp} +
+ ,
{this.renderContextMenu()} {renderTile( @@ -1371,17 +1386,6 @@ export class UnwrappedEventTile extends React.Component this.context.showHiddenEvents, )}
, - -
- {sender} - {timestamp} -
- , ], ); } diff --git a/src/i18n/strings/en_EN.json b/src/i18n/strings/en_EN.json index 4e2a0c01d5..77fecde80e 100644 --- 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 index 666254d2a1..ceedf5e27a 100644 --- 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"; diff --git a/src/utils/FileUtils.ts b/src/utils/FileUtils.ts index 37fd181900..2fb095d53c 100644 --- a/src/utils/FileUtils.ts +++ b/src/utils/FileUtils.ts @@ -21,6 +21,17 @@ import { MediaEventContent } from "matrix-js-sdk/src/types"; import { _t } from "../languageHandler"; +export function downloadLabelForFile(content: MediaEventContent, withSize = true): string { + let text = _t("action|download"); + + if (content.info?.size && withSize) { + // If we know the size of the file then add it as human-readable string to the end of the link text + // so that the user knows how big a file they are downloading. + text += " (" + fileSize(content.info.size, { base: 2, standard: "jedec" }) + ")"; + } + return text; +} + /** * Extracts a human-readable label for the file attachment to use as * link text. diff --git a/test/components/views/messages/MFileBody-test.tsx b/test/components/views/messages/MFileBody-test.tsx new file mode 100644 index 0000000000..2d8462a02c --- /dev/null +++ b/test/components/views/messages/MFileBody-test.tsx @@ -0,0 +1,88 @@ +/* +Copyright 2024 New Vector Ltd. + +SPDX-License-Identifier: AGPL-3.0-only OR GPL-3.0-only +Please see LICENSE files in the repository root for full details. +*/ + +import React from "react"; +import { render } from "jest-matrix-react"; +import { EventType, getHttpUriForMxc, MatrixEvent, Room } from "matrix-js-sdk/src/matrix"; + +import { RoomPermalinkCreator } from "../../../../src/utils/permalinks/Permalinks"; +import { + getMockClientWithEventEmitter, + mockClientMethodsCrypto, + mockClientMethodsDevice, + mockClientMethodsServer, + mockClientMethodsUser, +} from "../../../test-utils"; +import { MediaEventHelper } from "../../../../src/utils/MediaEventHelper"; +import SettingsStore from "../../../../src/settings/SettingsStore"; +import MFileBody from "../../../../src/components/views/messages/MFileBody.tsx"; +import RoomContext, { TimelineRenderingType } from "../../../../src/contexts/RoomContext.ts"; + +jest.mock("matrix-encrypt-attachment", () => ({ + decryptAttachment: jest.fn(), +})); + +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, + }, + }), + }); + // 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: "alt for a image", + msgtype: "m.image", + url: "mxc://server/image", + }, + }); + + const props = { + onHeightChanged: jest.fn(), + onMessageAllowed: jest.fn(), + permalinkCreator: new RoomPermalinkCreator(new Room(mediaEvent.getRoomId()!, cli, cli.getUserId()!)), + }; + + beforeEach(() => { + jest.spyOn(SettingsStore, "getValue").mockRestore(); + }); + + it("should show a download button in file rendering type", async () => { + const { container, getByRole } = render( + + + , + ); + + expect(getByRole("link", { name: "Download" })).toBeInTheDocument(); + expect(container).toMatchSnapshot(); + }); +}); diff --git a/test/components/views/messages/__snapshots__/MFileBody-test.tsx.snap b/test/components/views/messages/__snapshots__/MFileBody-test.tsx.snap new file mode 100644 index 0000000000..d15b7f454e --- /dev/null +++ b/test/components/views/messages/__snapshots__/MFileBody-test.tsx.snap @@ -0,0 +1,39 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[` should show a download button in file rendering type 1`] = ` + +`; diff --git a/test/components/views/rooms/EventTile-test.tsx b/test/components/views/rooms/EventTile-test.tsx index 051cdbcf62..d53e37a5f4 100644 --- a/test/components/views/rooms/EventTile-test.tsx +++ b/test/components/views/rooms/EventTile-test.tsx @@ -169,6 +169,15 @@ describe("EventTile", () => { }); }); + describe("EventTile renderingType: File", () => { + it("should not display the pinned message badge", async () => { + jest.spyOn(PinningUtils, "isPinned").mockReturnValue(true); + getComponent({}, TimelineRenderingType.File); + + expect(screen.queryByText("Pinned message")).not.toBeInTheDocument(); + }); + }); + describe("EventTile renderingType: default", () => { it.each([[Layout.Group], [Layout.Bubble], [Layout.IRC]])( "should display the pinned message badge", diff --git a/test/utils/FileUtils-test.ts b/test/utils/FileUtils-test.ts new file mode 100644 index 0000000000..a8c447161b --- /dev/null +++ b/test/utils/FileUtils-test.ts @@ -0,0 +1,60 @@ +/* +Copyright 2024 New Vector Ltd. + +SPDX-License-Identifier: AGPL-3.0-only OR GPL-3.0-only +Please see LICENSE files in the repository root for full details. +*/ + +import { MediaEventContent } from "matrix-js-sdk/src/types"; + +import { downloadLabelForFile } from "../../src/utils/FileUtils.ts"; + +describe("FileUtils", () => { + describe("downloadLabelForFile", () => { + it.each([ + [ + "File with size", + { + input: { + msgtype: "m.file", + body: "Test", + info: { + size: 102434566, + }, + } as MediaEventContent, + output: "Download (97.69 MB)", + }, + ], + [ + "Image", + { + input: { + msgtype: "m.image", + body: "Test", + } as MediaEventContent, + output: "Download", + }, + ], + [ + "Video", + { + input: { + msgtype: "m.video", + body: "Test", + } as MediaEventContent, + output: "Download", + }, + ], + [ + "Audio", + { + input: { + msgtype: "m.audio", + body: "Test", + } as MediaEventContent, + output: "Download", + }, + ], + ])("should correctly label %s", (_d, { input, output }) => expect(downloadLabelForFile(input)).toBe(output)); + }); +});