Enhancement: Save image on CTRL+S (#30330)

* Save image on CTRL+S

* fixed cosmetic comments

* fixed test

* refactored out downloading functionality from buttons to useDownloadMedia hook

* ImageView CTRL+S use button component

* added CTRL+S test & lint

* removed forwardRef

* fix lint

* i18n

AUTHORS.rst

@@ -19,3 +19,6 @@ include:
 
 * Thom Cleary (https://github.com/thomcatdotrocks)
   Small update for tarball deployment
+
+* Alexander (https://github.com/ioalexander)
+  Save image on CTRL + S shortcut

src/accessibility/KeyboardShortcuts.ts

@@ -146,6 +146,7 @@ export enum KeyBindingAction {
     ArrowDown = "KeyBinding.arrowDown",
     Tab = "KeyBinding.tab",
     Comma = "KeyBinding.comma",
+    Save = "KeyBinding.save",
 
     /** Toggle visibility of hidden events */
     ToggleHiddenEventVisibility = "KeyBinding.toggleHiddenEventVisibility",
@@ -269,6 +270,7 @@ export const CATEGORIES: Record<CategoryName, ICategory> = {
             KeyBindingAction.ArrowRight,
             KeyBindingAction.ArrowDown,
             KeyBindingAction.Comma,
+            KeyBindingAction.Save,
         ],
     },
     [CategoryName.NAVIGATION]: {
@@ -621,6 +623,13 @@ export const KEYBOARD_SHORTCUTS: IKeyboardShortcuts = {
         },
         displayName: _td("keyboard|composer_redo"),
     },
+    [KeyBindingAction.Save]: {
+        default: {
+            key: Key.S,
+            ctrlOrCmdKey: true,
+        },
+        displayName: _td("keyboard|save"),
+    },
     [KeyBindingAction.PreviousVisitedRoomOrSpace]: {
         default: {
             metaKey: IS_MAC,

src/components/views/elements/ImageView.tsx

@@ -8,10 +8,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, { type JSX, createRef, type CSSProperties, useRef, useState, useMemo, useEffect } from "react";
+import React, { type JSX, createRef, type CSSProperties, useEffect } from "react";
 import FocusLock from "react-focus-lock";
-import { type MatrixEvent, parseErrorResponse } from "matrix-js-sdk/src/matrix";
+import { type MatrixEvent } from "matrix-js-sdk/src/matrix";
-import { logger } from "matrix-js-sdk/src/logger";
 
 import { _t } from "../../../languageHandler";
 import MemberAvatar from "../avatars/MemberAvatar";
@@ -31,11 +30,7 @@ import { KeyBindingAction } from "../../../accessibility/KeyboardShortcuts";
 import { getKeyBindingsManager } from "../../../KeyBindingsManager";
 import { presentableTextForFile } from "../../../utils/FileUtils";
 import AccessibleButton from "./AccessibleButton";
-import Modal from "../../../Modal";
+import { useDownloadMedia } from "../../../hooks/useDownloadMedia.ts";
-import ErrorDialog from "../dialogs/ErrorDialog";
-import { FileDownloader } from "../../../utils/FileDownloader";
-import { MediaEventHelper } from "../../../utils/MediaEventHelper.ts";
-import ModuleApi from "../../../modules/Api";
 
 // Max scale to keep gaps around the image
 const MAX_SCALE = 0.95;
@@ -123,6 +118,8 @@ export default class ImageView extends React.Component<IProps, IState> {
     private imageWrapper = createRef<HTMLDivElement>();
     private image = createRef<HTMLImageElement>();
 
+    private downloadFunction?: () => Promise<void>;
+
     private initX = 0;
     private initY = 0;
     private previousX = 0;
@@ -302,6 +299,13 @@ export default class ImageView extends React.Component<IProps, IState> {
                 ev.preventDefault();
                 this.props.onFinished();
                 break;
+            case KeyBindingAction.Save:
+                ev.preventDefault();
+                ev.stopPropagation();
+                if (this.downloadFunction) {
+                    this.downloadFunction();
+                }
+                break;
         }
     };
 
@@ -327,6 +331,10 @@ export default class ImageView extends React.Component<IProps, IState> {
         });
     };
 
+    private onDownloadFunctionReady = (download: () => Promise<void>): void => {
+        this.downloadFunction = download;
+    };
+
     private onPermalinkClicked = (ev: React.MouseEvent): void => {
         // This allows the permalink to be opened in a new tab/window or copied as
         // matrix.to, but also for it to enable routing within Element when clicked.
@@ -552,7 +560,12 @@ export default class ImageView extends React.Component<IProps, IState> {
                             title={_t("lightbox|rotate_right")}
                             onClick={this.onRotateClockwiseClick}
                         />
-                        <DownloadButton url={this.props.src} fileName={this.props.name} mxEvent={this.props.mxEvent} />
+                        <DownloadButton
+                            url={this.props.src}
+                            fileName={this.props.name}
+                            mxEvent={this.props.mxEvent}
+                            onDownloadReady={this.onDownloadFunctionReady}
+                        />
                         {contextMenuButton}
                         <AccessibleButton
                             className="mx_ImageView_button mx_ImageView_button_close"
@@ -585,99 +598,28 @@ export default class ImageView extends React.Component<IProps, IState> {
     }
 }
 
-function DownloadButton({
+interface DownloadButtonProps {
-    url,
-    fileName,
-    mxEvent,
-}: {
     url: string;
     fileName?: string;
     mxEvent?: MatrixEvent;
-}): JSX.Element | null {
+    onDownloadReady?: (download: () => Promise<void>) => void;
-    const downloader = useRef(new FileDownloader()).current;
+}
-    const [loading, setLoading] = useState(false);
+
-    const [canDownload, setCanDownload] = useState<boolean>(false);
+export const DownloadButton: React.FC<DownloadButtonProps> = ({ url, fileName, mxEvent, onDownloadReady }) => {
-    const blobRef = useRef<Blob>(undefined);
+    const { download, loading, canDownload } = useDownloadMedia(url, fileName, mxEvent);
-    const mediaEventHelper = useMemo(() => (mxEvent ? new MediaEventHelper(mxEvent) : undefined), [mxEvent]);
 
     useEffect(() => {
-        if (!mxEvent) {
+        if (onDownloadReady) onDownloadReady(download);
-            // If we have no event, we assume this is safe to download.
+    }, [download, onDownloadReady]);
-            setCanDownload(true);
-            return;
-        }
-        const hints = ModuleApi.customComponents.getHintsForMessage(mxEvent);
-        if (hints?.allowDownloadingMedia) {
-            // Disable downloading as soon as we know there is a hint.
-            setCanDownload(false);
-            hints
-                .allowDownloadingMedia()
-                .then((downloadable) => {
-                    setCanDownload(downloadable);
-                })
-                .catch((ex) => {
-                    logger.error(`Failed to check if media from ${mxEvent.getId()} could be downloaded`, ex);
-                    // Err on the side of safety.
-                    setCanDownload(false);
-                });
-        } else {
-            setCanDownload(true);
-        }
-    }, [mxEvent]);
 
-    function showError(e: unknown): void {
+    if (!canDownload) return null;
-        Modal.createDialog(ErrorDialog, {
-            title: _t("timeline|download_failed"),
-            description: (
-                <>
-                    <div>{_t("timeline|download_failed_description")}</div>
-                    <div>{e instanceof Error ? e.toString() : ""}</div>
-                </>
-            ),
-        });
-        setLoading(false);
-    }
-
-    const onDownloadClick = async (): Promise<void> => {
-        try {
-            if (loading) return;
-            setLoading(true);
-
-            if (blobRef.current) {
-                // Cheat and trigger a download, again.
-                return downloadBlob(blobRef.current);
-            }
-
-            const res = await fetch(url);
-            if (!res.ok) {
-                throw parseErrorResponse(res, await res.text());
-            }
-            const blob = await res.blob();
-            blobRef.current = blob;
-            await downloadBlob(blob);
-        } catch (e) {
-            showError(e);
-        }
-    };
-
-    async function downloadBlob(blob: Blob): Promise<void> {
-        await downloader.download({
-            blob,
-            name: mediaEventHelper?.fileName ?? fileName ?? _t("common|image"),
-        });
-        setLoading(false);
-    }
-
-    if (!canDownload) {
-        return null;
-    }
 
     return (
         <AccessibleButton
             className="mx_ImageView_button mx_ImageView_button_download"
             title={loading ? _t("timeline|download_action_downloading") : _t("action|download")}
-            onClick={onDownloadClick}
+            onClick={download}
             disabled={loading}
         />
     );
-}
+};

src/components/views/messages/DownloadActionButton.tsx

@@ -7,19 +7,15 @@ Please see LICENSE files in the repository root for full details.
 */
 
 import { type MatrixEvent } from "matrix-js-sdk/src/matrix";
-import React, { type JSX } from "react";
+import React, { type ReactElement, useMemo } from "react";
 import classNames from "classnames";
 import { DownloadIcon } from "@vector-im/compound-design-tokens/assets/web/icons";
-import { logger } from "matrix-js-sdk/src/logger";
 
 import { type MediaEventHelper } from "../../../utils/MediaEventHelper";
 import { RovingAccessibleButton } from "../../../accessibility/RovingTabIndex";
 import Spinner from "../elements/Spinner";
-import { _t, _td, type TranslationKey } from "../../../languageHandler";
+import { _t } from "../../../languageHandler";
-import { FileDownloader } from "../../../utils/FileDownloader";
+import { useDownloadMedia } from "../../../hooks/useDownloadMedia";
-import Modal from "../../../Modal";
-import ErrorDialog from "../dialogs/ErrorDialog";
-import ModuleApi from "../../../modules/Api";
 
 interface IProps {
     mxEvent: MatrixEvent;
@@ -30,121 +26,32 @@ interface IProps {
     mediaEventHelperGet: () => MediaEventHelper | undefined;
 }
 
-interface IState {
+export default function DownloadActionButton({ mxEvent, mediaEventHelperGet }: IProps): ReactElement | null {
-    canDownload: null | boolean;
+    const mediaEventHelper = useMemo(() => mediaEventHelperGet(), [mediaEventHelperGet]);
-    loading: boolean;
+    const downloadUrl = mediaEventHelper?.media.srcHttp ?? "";
-    blob?: Blob;
+    const fileName = mediaEventHelper?.fileName;
-    tooltip: TranslationKey;
+
-}
+    const { download, loading, canDownload } = useDownloadMedia(downloadUrl, fileName, mxEvent);
-
+
-export default class DownloadActionButton extends React.PureComponent<IProps, IState> {
+    if (!canDownload) return null;
-    private downloader = new FileDownloader();
+
-
+    const spinner = loading ? <Spinner w={18} h={18} /> : undefined;
-    public constructor(props: IProps) {
+    const classes = classNames({
-        super(props);
+        mx_MessageActionBar_iconButton: true,
-
+        mx_MessageActionBar_downloadButton: true,
-        const moduleHints = ModuleApi.customComponents.getHintsForMessage(props.mxEvent);
+        mx_MessageActionBar_downloadSpinnerButton: !!spinner,
-        const downloadState: Pick<IState, "canDownload"> = { canDownload: true };
+    });
-        if (moduleHints?.allowDownloadingMedia) {
+
-            downloadState.canDownload = null;
+    return (
-            moduleHints
+        <RovingAccessibleButton
-                .allowDownloadingMedia()
+            className={classes}
-                .then((canDownload) => {
+            title={loading ? _t("timeline|download_action_downloading") : _t("action|download")}
-                    this.setState({
+            onClick={download}
-                        canDownload: canDownload,
+            disabled={loading}
-                    });
+            placement="left"
-                })
+        >
-                .catch((ex) => {
+            <DownloadIcon />
-                    logger.error(`Failed to check if media from ${props.mxEvent.getId()} could be downloaded`, ex);
+            {spinner}
-                    this.setState({
+        </RovingAccessibleButton>
-                        canDownload: false,
+    );
-                    });
-                });
-        }
-
-        this.state = {
-            loading: false,
-            tooltip: _td("timeline|download_action_downloading"),
-            ...downloadState,
-        };
-    }
-
-    private onDownloadClick = async (): Promise<void> => {
-        try {
-            await this.doDownload();
-        } catch (e) {
-            Modal.createDialog(ErrorDialog, {
-                title: _t("timeline|download_failed"),
-                description: (
-                    <>
-                        <div>{_t("timeline|download_failed_description")}</div>
-                        <div>{e instanceof Error ? e.toString() : ""}</div>
-                    </>
-                ),
-            });
-            this.setState({ loading: false });
-        }
-    };
-
-    private async doDownload(): Promise<void> {
-        const mediaEventHelper = this.props.mediaEventHelperGet();
-        if (this.state.loading || !mediaEventHelper) return;
-
-        if (mediaEventHelper.media.isEncrypted) {
-            this.setState({ tooltip: _td("timeline|download_action_decrypting") });
-        }
-
-        this.setState({ loading: true });
-
-        if (this.state.blob) {
-            // Cheat and trigger a download, again.
-            return this.downloadBlob(this.state.blob);
-        }
-
-        const blob = await mediaEventHelper.sourceBlob.value;
-        this.setState({ blob });
-        await this.downloadBlob(blob);
-    }
-
-    private async downloadBlob(blob: Blob): Promise<void> {
-        await this.downloader.download({
-            blob,
-            name: this.props.mediaEventHelperGet()!.fileName,
-        });
-        this.setState({ loading: false });
-    }
-
-    public render(): React.ReactNode {
-        let spinner: JSX.Element | undefined;
-        if (this.state.loading) {
-            spinner = <Spinner w={18} h={18} />;
-        }
-
-        if (this.state.canDownload === null) {
-            spinner = <Spinner w={18} h={18} />;
-        }
-
-        if (this.state.canDownload === false) {
-            return null;
-        }
-
-        const classes = classNames({
-            mx_MessageActionBar_iconButton: true,
-            mx_MessageActionBar_downloadButton: true,
-            mx_MessageActionBar_downloadSpinnerButton: !!spinner,
-        });
-
-        return (
-            <RovingAccessibleButton
-                className={classes}
-                title={spinner ? _t(this.state.tooltip) : _t("action|download")}
-                onClick={this.onDownloadClick}
-                disabled={!!spinner}
-                placement="left"
-            >
-                <DownloadIcon />
-                {spinner}
-            </RovingAccessibleButton>
-        );
-    }
 }

src/hooks/useDownloadMedia.ts

@@ -0,0 +1,93 @@
+/*
+Copyright 2024 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 { parseErrorResponse } from "matrix-js-sdk/src/matrix";
+import { useRef, useState, useMemo, useEffect } from "react";
+import { type MatrixEvent } from "matrix-js-sdk/src/matrix";
+import { logger } from "matrix-js-sdk/src/logger";
+
+import ErrorDialog from "../components/views/dialogs/ErrorDialog";
+import { _t } from "../languageHandler";
+import Modal from "../Modal";
+import { FileDownloader } from "../utils/FileDownloader";
+import { MediaEventHelper } from "../utils/MediaEventHelper";
+import ModuleApi from "../modules/Api";
+
+export interface UseDownloadMediaReturn {
+    download: () => Promise<void>;
+    loading: boolean;
+    canDownload: boolean;
+}
+
+export function useDownloadMedia(url: string, fileName?: string, mxEvent?: MatrixEvent): UseDownloadMediaReturn {
+    const downloader = useRef(new FileDownloader()).current;
+    const blobRef = useRef<Blob>(null);
+    const [loading, setLoading] = useState(false);
+    const [canDownload, setCanDownload] = useState<boolean>(true);
+
+    const mediaEventHelper = useMemo(() => (mxEvent ? new MediaEventHelper(mxEvent) : undefined), [mxEvent]);
+
+    useEffect(() => {
+        if (!mxEvent) return;
+
+        const hints = ModuleApi.customComponents.getHintsForMessage(mxEvent);
+        if (hints?.allowDownloadingMedia) {
+            setCanDownload(false);
+            hints
+                .allowDownloadingMedia()
+                .then(setCanDownload)
+                .catch((err: any) => {
+                    logger.error(`Failed to check media download permission for ${mxEvent.event.event_id}`, err);
+
+                    setCanDownload(false);
+                });
+        } else {
+            setCanDownload(true);
+        }
+    }, [mxEvent]);
+
+    const download = async (): Promise<void> => {
+        if (loading) return;
+        try {
+            setLoading(true);
+
+            if (blobRef.current) {
+                return downloadBlob(blobRef.current);
+            }
+
+            const res = await fetch(url);
+            if (!res.ok) {
+                throw parseErrorResponse(res, await res.text());
+            }
+
+            const blob = await res.blob();
+            blobRef.current = blob;
+
+            await downloadBlob(blob);
+        } catch (e) {
+            showError(e);
+        }
+    };
+
+    const downloadBlob = async (blob: Blob): Promise<void> => {
+        await downloader.download({
+            blob,
+            name: mediaEventHelper?.fileName ?? fileName ?? _t("common|image"),
+        });
+        setLoading(false);
+    };
+
+    const showError = (e: unknown): void => {
+        Modal.createDialog(ErrorDialog, {
+            title: _t("timeline|download_failed"),
+            description: `${_t("timeline|download_failed_description")}\n\n${String(e)}`,
+        });
+        setLoading(false);
+    };
+
+    return { download, loading, canDownload };
+}

src/i18n/strings/en_EN.json

@@ -1450,6 +1450,7 @@
         "room_list_navigate_down": "Navigate down in the room list",
         "room_list_navigate_up": "Navigate up in the room list",
         "room_list_select_room": "Select room from the room list",
+        "save": "Save",
         "scroll_down_timeline": "Scroll down in the timeline",
         "scroll_up_timeline": "Scroll up in the timeline",
         "search": "Search (must be enabled)",
@@ -3370,7 +3371,6 @@
             "unable_to_decrypt": "Unable to decrypt message"
         },
         "disambiguated_profile": "%(displayName)s (%(matrixId)s)",
-        "download_action_decrypting": "Decrypting",
         "download_action_downloading": "Downloading",
         "download_failed": "Download failed",
         "download_failed_description": "An error occurred while downloading this file",

test/unit-tests/components/views/elements/ImageView-test.tsx

@@ -44,6 +44,28 @@ describe("<ImageView />", () => {
         expect(fetchMock).toHaveFetched("https://example.com/image.png");
     });
 
+    it("should start download on Ctrl+S", async () => {
+        fetchMock.get("https://example.com/image.png", "TESTFILE");
+
+        const { container } = render(
+            <ImageView src="https://example.com/image.png" name="filename.png" onFinished={jest.fn()} />,
+        );
+
+        const dialog = container.querySelector('[role="dialog"]') as HTMLElement;
+        dialog?.focus();
+
+        fireEvent.keyDown(dialog!, { key: "s", code: "KeyS", ctrlKey: true });
+
+        await waitFor(() => {
+            expect(mocked(FileDownloader).mock.instances[0].download).toHaveBeenCalledWith({
+                blob: expect.anything(),
+                name: "filename.png",
+            });
+        });
+
+        expect(fetchMock).toHaveFetched("https://example.com/image.png");
+    });
+
     it("should handle download errors", async () => {
         const modalSpy = jest.spyOn(Modal, "createDialog");
         fetchMock.get("https://example.com/image.png", { status: 500 });

test/unit-tests/components/views/settings/tabs/user/__snapshots__/KeyboardUserSettingsTab-test.tsx.snap

@@ -742,6 +742,26 @@ exports[`KeyboardUserSettingsTab renders list of keyboard shortcuts 1`] = `
                     </kbd>
                   </div>
                 </li>
+                <li
+                  class="mx_KeyboardShortcut_shortcutRow"
+                >
+                  Save
+                  <div
+                    class="mx_KeyboardShortcut"
+                  >
+                    <kbd>
+                       
+                      Ctrl
+                       
+                    </kbd>
+                    +
+                    <kbd>
+                       
+                      s
+                       
+                    </kbd>
+                  </div>
+                </li>
               </ul>
             </div>
           </div>