diff --git a/playwright/e2e/timeline/timeline.spec.ts b/playwright/e2e/timeline/timeline.spec.ts index 1f353e3e84..d6f791e50b 100644 --- a/playwright/e2e/timeline/timeline.spec.ts +++ b/playwright/e2e/timeline/timeline.spec.ts @@ -28,6 +28,8 @@ const NEW_AVATAR = fs.readFileSync("playwright/sample-files/element.png"); const OLD_NAME = "Alan"; const NEW_NAME = "Alan (away)"; +const VIDEO_FILE = fs.readFileSync("playwright/sample-files/5secvid.webm"); + const getEventTilesWithBodies = (page: Page): Locator => { return page.locator(".mx_EventTile").filter({ has: page.locator(".mx_EventTile_body") }); }; @@ -916,7 +918,27 @@ test.describe("Timeline", () => { await page.getByRole("button", { name: "Hide" }).click(); // Check that the image is now hidden. - await expect(page.getByRole("link", { name: "Show image" })).toBeVisible(); + await expect(page.getByRole("button", { name: "Show image" })).toBeVisible(); + }); + + test("should be able to hide a video", async ({ page, app, room, context }) => { + await app.viewRoomById(room.roomId); + const upload = await app.client.uploadContent(VIDEO_FILE, { name: "bbb.webm", type: "video/webm" }); + await app.client.sendEvent(room.roomId, null, "m.room.message" as EventType, { + msgtype: "m.video" as MsgType, + body: "bbb.webm", + url: upload.content_uri, + }); + + await app.timeline.scrollToBottom(); + const imgTile = page.locator(".mx_MVideoBody").first(); + await expect(imgTile).toBeVisible(); + await imgTile.hover(); + await page.getByRole("button", { name: "Hide" }).click(); + + // Check that the video is now hidden. + await expect(page.getByRole("button", { name: "Show video" })).toBeVisible(); + await expect(page.locator("video")).not.toBeVisible(); }); }); diff --git a/playwright/sample-files/5secvid.webm b/playwright/sample-files/5secvid.webm new file mode 100644 index 0000000000..ec5a695fa4 Binary files /dev/null and b/playwright/sample-files/5secvid.webm differ diff --git a/res/css/_components.pcss b/res/css/_components.pcss index 9df9dcb8cb..d622f69b84 100644 --- a/res/css/_components.pcss +++ b/res/css/_components.pcss @@ -227,6 +227,7 @@ @import "./views/messages/_DisambiguatedProfile.pcss"; @import "./views/messages/_EventTileBubble.pcss"; @import "./views/messages/_HiddenBody.pcss"; +@import "./views/messages/_HiddenMediaPlaceholder.pcss"; @import "./views/messages/_JumpToDatePicker.pcss"; @import "./views/messages/_LegacyCallEvent.pcss"; @import "./views/messages/_MEmoteBody.pcss"; diff --git a/res/css/views/messages/_HiddenMediaPlaceholder.pcss b/res/css/views/messages/_HiddenMediaPlaceholder.pcss new file mode 100644 index 0000000000..c7efe6ec7e --- /dev/null +++ b/res/css/views/messages/_HiddenMediaPlaceholder.pcss @@ -0,0 +1,29 @@ +.mx_HiddenMediaPlaceholder { + border: none; + width: 100%; + height: 100%; + inset: 0; + + /* To center the text in the middle of the frame */ + display: flex; + align-items: center; + justify-content: center; + text-align: center; + + cursor: pointer; + background-color: $header-panel-bg-color; + + > div { + color: $accent; + /* Icon alignment */ + display: flex; + > svg { + margin-top: auto; + margin-bottom: auto; + } + } +} + +.mx_EventTile:hover .mx_HiddenMediaPlaceholder { + background-color: $background; +} diff --git a/res/css/views/messages/_MImageBody.pcss b/res/css/views/messages/_MImageBody.pcss index 5942bb44b3..bb5f879568 100644 --- a/res/css/views/messages/_MImageBody.pcss +++ b/res/css/views/messages/_MImageBody.pcss @@ -79,39 +79,3 @@ Please see LICENSE files in the repository root for full details. color: $imagebody-giflabel-color; pointer-events: none; } - -.mx_HiddenImagePlaceholder { - position: absolute; - inset: 0; - - /* To center the text in the middle of the frame */ - display: flex; - align-items: center; - justify-content: center; - text-align: center; - - cursor: pointer; - background-color: $header-panel-bg-color; - - .mx_HiddenImagePlaceholder_button { - color: $accent; - - span.mx_HiddenImagePlaceholder_eye { - margin-right: 8px; - - background-color: $accent; - mask-image: url("$(res)/img/element-icons/eye.svg"); - display: inline-block; - width: 18px; - height: 14px; - } - - span:not(.mx_HiddenImagePlaceholder_eye) { - vertical-align: text-bottom; - } - } -} - -.mx_EventTile:hover .mx_HiddenImagePlaceholder { - background-color: $background; -} diff --git a/src/components/views/messages/HiddenMediaPlaceholder.tsx b/src/components/views/messages/HiddenMediaPlaceholder.tsx new file mode 100644 index 0000000000..16367ee05a --- /dev/null +++ b/src/components/views/messages/HiddenMediaPlaceholder.tsx @@ -0,0 +1,24 @@ +/* +Copyright 2025 New Vector Ltd. + +SPDX-License-Identifier: AGPL-3.0-only OR GPL-3.0-only OR LicenseRef-Element-Commercial +Please see LICENSE files in the repository root for full details. +*/ + +import React, { type PropsWithChildren, type MouseEventHandler } from "react"; +import { VisibilityOnIcon } from "@vector-im/compound-design-tokens/assets/web/icons"; + +interface IProps { + onClick: MouseEventHandler; +} + +export const HiddenMediaPlaceholder: React.FunctionComponent> = ({ onClick, children }) => { + return ( + + ); +}; diff --git a/src/components/views/messages/MImageBody.tsx b/src/components/views/messages/MImageBody.tsx index 3fa4f01bfd..15adb01dfc 100644 --- a/src/components/views/messages/MImageBody.tsx +++ b/src/components/views/messages/MImageBody.tsx @@ -33,6 +33,7 @@ import { presentableTextForFile } from "../../../utils/FileUtils"; import { createReconnectedListener } from "../../../utils/connection"; import MediaProcessingError from "./shared/MediaProcessingError"; import { DecryptError, DownloadError } from "../../../utils/DecryptFile"; +import { HiddenMediaPlaceholder } from "./HiddenMediaPlaceholder"; import { useMediaVisible } from "../../../hooks/useMediaVisible"; enum Placeholder { @@ -95,7 +96,7 @@ export class MImageBodyInner extends React.Component { if (ev.button === 0 && !ev.metaKey) { ev.preventDefault(); if (!this.props.mediaVisible) { - this.props.setMediaVisible?.(true); + this.props.setMediaVisible(true); return; } @@ -437,7 +438,11 @@ export class MImageBodyInner extends React.Component { if (!this.state.loadedImageDimensions) { let imageElement: JSX.Element; if (!this.props.mediaVisible) { - imageElement = ; + imageElement = ( + + {_t("timeline|m.image|show_image")} + + ); } else { imageElement = ( { } if (!this.props.mediaVisible) { - img = ; + img = ( +
+ + {_t("timeline|m.image|show_image")} + +
+ ); showPlaceholder = false; // because we're hiding the image, so don't show the placeholder. } @@ -563,7 +574,7 @@ export class MImageBodyInner extends React.Component { {/* HACK: This div fills out space while the image loads, to prevent scroll jumps */} - {!this.props.forExport && !this.state.imgLoaded && ( + {!this.props.forExport && !this.state.imgLoaded && !placeholder && (
)}
@@ -596,12 +607,6 @@ export class MImageBodyInner extends React.Component { {children} ); - } else if (!this.props.mediaVisible) { - return ( -
- {children} -
- ); } return children; } @@ -680,24 +685,7 @@ export class MImageBodyInner extends React.Component { } } -interface PlaceholderIProps { - maxWidth?: number; -} - -export class HiddenImagePlaceholder extends React.PureComponent { - public render(): React.ReactNode { - const maxWidth = this.props.maxWidth ? this.props.maxWidth + "px" : null; - return ( -
-
- - {_t("timeline|m.image|show_image")} -
-
- ); - } -} - +// Wrap MImageBody component so we can use a hook here. const MImageBody: React.FC = (props) => { const [mediaVisible, setVisible] = useMediaVisible(props.mxEvent.getId()!); return ; diff --git a/src/components/views/messages/MVideoBody.tsx b/src/components/views/messages/MVideoBody.tsx index 00c44b5170..be0f692fe8 100644 --- a/src/components/views/messages/MVideoBody.tsx +++ b/src/components/views/messages/MVideoBody.tsx @@ -21,6 +21,8 @@ import MFileBody from "./MFileBody"; import { type ImageSize, suggestedSize as suggestedVideoSize } from "../../../settings/enums/ImageSize"; import RoomContext, { TimelineRenderingType } from "../../../contexts/RoomContext"; import MediaProcessingError from "./shared/MediaProcessingError"; +import { HiddenMediaPlaceholder } from "./HiddenMediaPlaceholder"; +import { useMediaVisible } from "../../../hooks/useMediaVisible"; interface IState { decryptedUrl: string | null; @@ -32,7 +34,19 @@ interface IState { blurhashUrl: string | null; } -export default class MVideoBody extends React.PureComponent { +interface IProps extends IBodyProps { + /** + * Should the media be behind a preview. + */ + mediaVisible: boolean; + /** + * Set the visibility of the media event. + * @param visible Should the event be visible. + */ + setMediaVisible: (visible: boolean) => void; +} + +class MVideoBodyInner extends React.PureComponent { public static contextType = RoomContext; declare public context: React.ContextType; @@ -49,6 +63,10 @@ export default class MVideoBody extends React.PureComponent blurhashUrl: null, }; + private onClick = (): void => { + this.props.setMediaVisible(true); + }; + private getContentUrl(): string | undefined { const content = this.props.mxEvent.getContent(); // During export, the content url will point to the MSC, which will later point to a local url @@ -120,11 +138,7 @@ export default class MVideoBody extends React.PureComponent } } - public async componentDidMount(): Promise { - this.sizeWatcher = SettingsStore.watchSetting("Images.size", null, () => { - this.forceUpdate(); // we don't really have a reliable thing to update, so just update the whole thing - }); - + private async downloadVideo(): Promise { try { this.loadBlurhash(); } catch (e) { @@ -174,6 +188,23 @@ export default class MVideoBody extends React.PureComponent } } + public async componentDidMount(): Promise { + this.sizeWatcher = SettingsStore.watchSetting("Images.size", null, () => { + this.forceUpdate(); // we don't really have a reliable thing to update, so just update the whole thing + }); + + // Do not attempt to load the media if we do not want to show previews here. + if (this.props.mediaVisible) { + await this.downloadVideo(); + } + } + + public async componentDidUpdate(prevProps: Readonly): Promise { + if (!prevProps.mediaVisible && this.props.mediaVisible) { + await this.downloadVideo(); + } + } + public componentWillUnmount(): void { SettingsStore.unwatchSetting(this.sizeWatcher); } @@ -244,6 +275,22 @@ export default class MVideoBody extends React.PureComponent ); } + // Users may not even want to show a poster, so instead show a preview button. + if (!this.props.mediaVisible) { + return ( + +
+ + {_t("timeline|m.video|show_video")} + +
+
+ ); + } + // Important: If we aren't autoplaying and we haven't decrypted it yet, show a video with a poster. if (!this.props.forExport && content.file !== undefined && this.state.decryptedUrl === null && autoplay) { // Need to decrypt the attachment @@ -294,3 +341,11 @@ export default class MVideoBody extends React.PureComponent ); } } + +// Wrap MVideoBody component so we can use a hook here. +const MVideoBody: React.FC = (props) => { + const [mediaVisible, setVisible] = useMediaVisible(props.mxEvent.getId()!); + return ; +}; + +export default MVideoBody; diff --git a/src/components/views/messages/MessageActionBar.tsx b/src/components/views/messages/MessageActionBar.tsx index 0120947b31..0a48a1898c 100644 --- a/src/components/views/messages/MessageActionBar.tsx +++ b/src/components/views/messages/MessageActionBar.tsx @@ -536,9 +536,11 @@ export default class MessageActionBar extends React.PureComponent this.props.getTile()?.getMediaHelper?.()} key="download" />, - , ); } + if (MediaEventHelper.canHide(this.props.mxEvent)) { + toolbarOpts.splice(0, 0, ); + } } else if ( // Show thread icon even for deleted messages, but only within main timeline this.context.timelineRenderingType === TimelineRenderingType.Room && diff --git a/src/i18n/strings/en_EN.json b/src/i18n/strings/en_EN.json index 6130c2cada..9d723cbedd 100644 --- a/src/i18n/strings/en_EN.json +++ b/src/i18n/strings/en_EN.json @@ -3575,7 +3575,8 @@ }, "m.sticker": "%(senderDisplayName)s sent a sticker.", "m.video": { - "error_decrypting": "Error decrypting video" + "error_decrypting": "Error decrypting video", + "show_video": "Show video" }, "m.widget": { "added": "%(widgetName)s widget added by %(senderName)s", diff --git a/src/utils/MediaEventHelper.ts b/src/utils/MediaEventHelper.ts index 25339260f3..98cbe4da58 100644 --- a/src/utils/MediaEventHelper.ts +++ b/src/utils/MediaEventHelper.ts @@ -113,4 +113,18 @@ export class MediaEventHelper implements IDestroyable { // Finally, it's probably not media return false; } + + /** + * Determine if the media event in question supports being hidden in the timeline. + * @param event Any matrix event. + * @returns `true` if the media can be hidden, otherwise false. + */ + public static canHide(event: MatrixEvent): boolean { + if (!event) return false; + if (event.isRedacted()) return false; + const content = event.getContent(); + const hideTypes: string[] = [MsgType.Video, MsgType.Image]; + if (hideTypes.includes(content.msgtype!)) return true; + return false; + } } diff --git a/test/unit-tests/components/views/messages/MImageBody-test.tsx b/test/unit-tests/components/views/messages/MImageBody-test.tsx index 9d324f13e8..e22989fd47 100644 --- a/test/unit-tests/components/views/messages/MImageBody-test.tsx +++ b/test/unit-tests/components/views/messages/MImageBody-test.tsx @@ -7,7 +7,7 @@ Please see LICENSE files in the repository root for full details. */ import React, { act } from "react"; -import { fireEvent, render, screen, waitForElementToBeRemoved } from "jest-matrix-react"; +import { fireEvent, render, screen, waitFor, waitForElementToBeRemoved } from "jest-matrix-react"; import { EventType, getHttpUriForMxc, MatrixEvent, Room } from "matrix-js-sdk/src/matrix"; import fetchMock from "fetch-mock-jest"; import encrypt from "matrix-encrypt-attachment"; @@ -85,6 +85,10 @@ describe("", () => { fetchMock.mockReset(); }); + afterEach(() => { + mocked(encrypt.decryptAttachment).mockReset(); + }); + it("should show a thumbnail while image is being downloaded", async () => { fetchMock.getOnce(url, { status: 200 }); @@ -166,6 +170,8 @@ describe("", () => { />, ); + expect(screen.getByText("Show image")).toBeInTheDocument(); + expect(fetchMock).not.toHaveFetched(url); }); @@ -186,8 +192,12 @@ describe("", () => { expect(fetchMock).toHaveFetched(url); - // spinner while downloading image - expect(screen.getByRole("progressbar")).toBeInTheDocument(); + // Show image is asynchronous since it applies through a settings watcher hook, so + // be sure to wait here. + await waitFor(() => { + // spinner while downloading image + expect(screen.getByRole("progressbar")).toBeInTheDocument(); + }); }); }); diff --git a/test/unit-tests/components/views/messages/MVideoBody-test.tsx b/test/unit-tests/components/views/messages/MVideoBody-test.tsx index 0dd0c917dd..996d875e09 100644 --- a/test/unit-tests/components/views/messages/MVideoBody-test.tsx +++ b/test/unit-tests/components/views/messages/MVideoBody-test.tsx @@ -6,9 +6,9 @@ SPDX-License-Identifier: AGPL-3.0-only OR GPL-3.0-only OR LicenseRef-Element-Com Please see LICENSE files in the repository root for full details. */ -import React from "react"; +import React, { act } from "react"; import { EventType, getHttpUriForMxc, type IContent, MatrixEvent } from "matrix-js-sdk/src/matrix"; -import { render, type RenderResult } from "jest-matrix-react"; +import { fireEvent, render, screen, type RenderResult } from "jest-matrix-react"; import fetchMock from "fetch-mock-jest"; import MatrixClientContext from "../../../../../src/contexts/MatrixClientContext"; @@ -22,19 +22,22 @@ import { mockClientMethodsUser, } from "../../../../test-utils"; import MVideoBody from "../../../../../src/components/views/messages/MVideoBody"; +import type { IBodyProps } from "../../../../../src/components/views/messages/IBodyProps"; +import { SettingLevel } from "../../../../../src/settings/SettingLevel"; +import SettingsStore from "../../../../../src/settings/SettingsStore"; + +// Needed so we don't throw an error about failing to decrypt. +jest.mock("matrix-encrypt-attachment", () => ({ + decryptAttachment: jest.fn(), +})); describe("MVideoBody", () => { - it("does not crash when given a portrait image", () => { - // Check for an unreliable crash caused by a fractional-sized - // image dimension being used for a CanvasImageData. - const { asFragment } = makeMVideoBody(720, 1280); - expect(asFragment()).toMatchSnapshot(); - // If we get here, we did not crash. - }); + const userId = "@user:server"; + const deviceId = "DEADB33F"; - it("should show poster for encrypted media before downloading it", async () => { - const userId = "@user:server"; - const deviceId = "DEADB33F"; + const thumbUrl = "https://server/_matrix/media/v3/download/server/encrypted-poster"; + + beforeEach(() => { const cli = getMockClientWithEventEmitter({ ...mockClientMethodsUser(userId), ...mockClientMethodsServer(), @@ -49,40 +52,108 @@ describe("MVideoBody", () => { }, }), }); - const thumbUrl = "https://server/_matrix/media/v3/download/server/encrypted-poster"; - fetchMock.getOnce(thumbUrl, { status: 200 }); - // 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 encryptedMediaEvent = new MatrixEvent({ - room_id: "!room:server", - sender: userId, - type: EventType.RoomMessage, - content: { - body: "alt for a test video", - info: { - duration: 420, - w: 40, - h: 50, - thumbnail_file: { - url: "mxc://server/encrypted-poster", - }, - }, - file: { - url: "mxc://server/encrypted-image", + fetchMock.mockReset(); + }); + + const encryptedMediaEvent = new MatrixEvent({ + room_id: "!room:server", + sender: userId, + type: EventType.RoomMessage, + content: { + body: "alt for a test video", + info: { + duration: 420, + w: 40, + h: 50, + thumbnail_file: { + url: "mxc://server/encrypted-poster", }, }, - }); + file: { + url: "mxc://server/encrypted-image", + }, + }, + }); + it("does not crash when given a portrait image", () => { + // Check for an unreliable crash caused by a fractional-sized + // image dimension being used for a CanvasImageData. + const { asFragment } = makeMVideoBody(720, 1280); + expect(asFragment()).toMatchSnapshot(); + // If we get here, we did not crash. + }); + + it("should show poster for encrypted media before downloading it", async () => { + fetchMock.getOnce(thumbUrl, { status: 200 }); const { asFragment } = render( , ); expect(asFragment()).toMatchSnapshot(); }); + + describe("with video previews/thumbnails disabled", () => { + beforeEach(() => { + act(() => { + SettingsStore.setValue("showImages", null, SettingLevel.DEVICE, false); + }); + }); + + afterEach(() => { + act(() => { + SettingsStore.setValue( + "showImages", + null, + SettingLevel.DEVICE, + SettingsStore.getDefaultValue("showImages"), + ); + SettingsStore.setValue( + "showMediaEventIds", + null, + SettingLevel.DEVICE, + SettingsStore.getDefaultValue("showMediaEventIds"), + ); + }); + }); + + it("should not download video", async () => { + fetchMock.getOnce(thumbUrl, { status: 200 }); + + render( + , + ); + + expect(screen.getByText("Show video")).toBeInTheDocument(); + + expect(fetchMock).not.toHaveFetched(thumbUrl); + }); + + it("should render video poster after user consent", async () => { + fetchMock.getOnce(thumbUrl, { status: 200 }); + + render( + , + ); + + const placeholderButton = screen.getByRole("button", { name: "Show video" }); + + expect(placeholderButton).toBeInTheDocument(); + fireEvent.click(placeholderButton); + + expect(fetchMock).toHaveFetched(thumbUrl); + }); + }); }); function makeMVideoBody(w: number, h: number): RenderResult { @@ -109,7 +180,7 @@ function makeMVideoBody(w: number, h: number): RenderResult { content, }); - const defaultProps: MVideoBody["props"] = { + const defaultProps: IBodyProps = { mxEvent: event, highlights: [], highlightLink: "", diff --git a/test/unit-tests/components/views/messages/__snapshots__/MImageBody-test.tsx.snap b/test/unit-tests/components/views/messages/__snapshots__/MImageBody-test.tsx.snap index 3ba381641b..8b9950c681 100644 --- a/test/unit-tests/components/views/messages/__snapshots__/MImageBody-test.tsx.snap +++ b/test/unit-tests/components/views/messages/__snapshots__/MImageBody-test.tsx.snap @@ -41,9 +41,6 @@ exports[` should generate a thumbnail if one isn't included for ani GIF

-
@@ -77,9 +74,6 @@ exports[` should show a thumbnail while image is being downloaded 1
-