From a691e634b02f21b875de6475cb1d464754ec853e Mon Sep 17 00:00:00 2001
From: Florian Duros <florianduros@element.io>
Date: Wed, 11 Jan 2023 11:10:55 +0100
Subject: [PATCH] Add edit and remove actions to link in RTE (#9864)

Add edit and remove actions to link in RTE
---
 package.json                                  |   2 +-
 .../components/_LinkModal.pcss                |  28 ++++-
 src/components/views/elements/Field.tsx       |   2 +-
 .../components/FormattingButtons.tsx          |   2 +-
 .../wysiwyg_composer/components/LinkModal.tsx | 119 ++++++++++++------
 src/i18n/strings/en_EN.json                   |   1 +
 test/components/views/elements/Field-test.tsx |  54 ++++++++
 .../components/LinkModal-test.tsx             |  51 ++++++--
 yarn.lock                                     |   8 +-
 9 files changed, 209 insertions(+), 58 deletions(-)
 create mode 100644 test/components/views/elements/Field-test.tsx

diff --git a/package.json b/package.json
index 37f8a2d895..cf00952f23 100644
--- a/package.json
+++ b/package.json
@@ -57,7 +57,7 @@
     "dependencies": {
         "@babel/runtime": "^7.12.5",
         "@matrix-org/analytics-events": "^0.3.0",
-        "@matrix-org/matrix-wysiwyg": "^0.13.0",
+        "@matrix-org/matrix-wysiwyg": "^0.14.0",
         "@matrix-org/react-sdk-module-api": "^0.0.3",
         "@sentry/browser": "^7.0.0",
         "@sentry/tracing": "^7.0.0",
diff --git a/res/css/views/rooms/wysiwyg_composer/components/_LinkModal.pcss b/res/css/views/rooms/wysiwyg_composer/components/_LinkModal.pcss
index 5bd02cc245..b5c1c04d57 100644
--- a/res/css/views/rooms/wysiwyg_composer/components/_LinkModal.pcss
+++ b/res/css/views/rooms/wysiwyg_composer/components/_LinkModal.pcss
@@ -16,14 +16,32 @@ limitations under the License.
 
 .mx_LinkModal {
     padding: $spacing-32;
-
-    .mx_Dialog_content {
-        margin-top: 30px;
-        margin-bottom: 42px;
-    }
+    max-width: 600px;
+    height: 341px;
+    box-sizing: border-box;
+    display: flex;
+    flex-direction: column;
 
     .mx_LinkModal_content {
         display: flex;
         flex-direction: column;
+        flex: 1;
+        gap: $spacing-8;
+        margin-top: 7px;
+
+        .mx_LinkModal_Field {
+            flex: initial;
+            height: 40px;
+        }
+
+        .mx_LinkModal_buttons {
+            display: flex;
+            flex: 1;
+            align-items: flex-end;
+
+            .mx_Dialog_buttons {
+                display: inline-block;
+            }
+        }
     }
 }
diff --git a/src/components/views/elements/Field.tsx b/src/components/views/elements/Field.tsx
index 31a6e88897..295cdf99a9 100644
--- a/src/components/views/elements/Field.tsx
+++ b/src/components/views/elements/Field.tsx
@@ -262,7 +262,7 @@ export default class Field extends React.PureComponent<PropShapes, IState> {
 
         this.inputRef = inputRef || React.createRef();
 
-        inputProps.placeholder = inputProps.placeholder || inputProps.label;
+        inputProps.placeholder = inputProps.placeholder ?? inputProps.label;
         inputProps.id = this.id; // this overwrites the id from props
 
         inputProps.onFocus = this.onFocus;
diff --git a/src/components/views/rooms/wysiwyg_composer/components/FormattingButtons.tsx b/src/components/views/rooms/wysiwyg_composer/components/FormattingButtons.tsx
index d2cafb1198..6f85328ba5 100644
--- a/src/components/views/rooms/wysiwyg_composer/components/FormattingButtons.tsx
+++ b/src/components/views/rooms/wysiwyg_composer/components/FormattingButtons.tsx
@@ -120,7 +120,7 @@ export function FormattingButtons({ composer, actionStates }: FormattingButtonsP
             <Button
                 isActive={actionStates.link === "reversed"}
                 label={_td("Link")}
-                onClick={() => openLinkModal(composer, composerContext)}
+                onClick={() => openLinkModal(composer, composerContext, actionStates.link === "reversed")}
                 icon={<LinkIcon className="mx_FormattingButtons_Icon" />}
             />
         </div>
diff --git a/src/components/views/rooms/wysiwyg_composer/components/LinkModal.tsx b/src/components/views/rooms/wysiwyg_composer/components/LinkModal.tsx
index 2dcfc43ead..0e41d8baa4 100644
--- a/src/components/views/rooms/wysiwyg_composer/components/LinkModal.tsx
+++ b/src/components/views/rooms/wysiwyg_composer/components/LinkModal.tsx
@@ -17,17 +17,28 @@ limitations under the License.
 import { FormattingFunctions } from "@matrix-org/matrix-wysiwyg";
 import React, { ChangeEvent, useState } from "react";
 
-import { _td } from "../../../../../languageHandler";
+import { _t } from "../../../../../languageHandler";
 import Modal from "../../../../../Modal";
-import QuestionDialog from "../../../dialogs/QuestionDialog";
 import Field from "../../../elements/Field";
 import { ComposerContextState } from "../ComposerContext";
 import { isSelectionEmpty, setSelection } from "../utils/selection";
+import BaseDialog from "../../../dialogs/BaseDialog";
+import DialogButtons from "../../../elements/DialogButtons";
 
-export function openLinkModal(composer: FormattingFunctions, composerContext: ComposerContextState) {
+export function openLinkModal(
+    composer: FormattingFunctions,
+    composerContext: ComposerContextState,
+    isEditing: boolean,
+) {
     const modal = Modal.createDialog(
         LinkModal,
-        { composerContext, composer, onClose: () => modal.close(), isTextEnabled: isSelectionEmpty() },
+        {
+            composerContext,
+            composer,
+            onClose: () => modal.close(),
+            isTextEnabled: isSelectionEmpty(),
+            isEditing,
+        },
         "mx_CompoundDialog",
         false,
         true,
@@ -43,48 +54,86 @@ interface LinkModalProps {
     isTextEnabled: boolean;
     onClose: () => void;
     composerContext: ComposerContextState;
+    isEditing: boolean;
 }
 
-export function LinkModal({ composer, isTextEnabled, onClose, composerContext }: LinkModalProps) {
-    const [fields, setFields] = useState({ text: "", link: "" });
-    const isSaveDisabled = (isTextEnabled && isEmpty(fields.text)) || isEmpty(fields.link);
+export function LinkModal({ composer, isTextEnabled, onClose, composerContext, isEditing }: LinkModalProps) {
+    const [hasLinkChanged, setHasLinkChanged] = useState(false);
+    const [fields, setFields] = useState({ text: "", link: isEditing ? composer.getLink() : "" });
+    const hasText = !isEditing && isTextEnabled;
+    const isSaveDisabled = !hasLinkChanged || (hasText && isEmpty(fields.text)) || isEmpty(fields.link);
 
     return (
-        <QuestionDialog
+        <BaseDialog
             className="mx_LinkModal"
-            title={_td("Create a link")}
-            button={_td("Save")}
-            buttonDisabled={isSaveDisabled}
-            hasCancelButton={true}
-            onFinished={async (isClickOnSave: boolean) => {
-                if (isClickOnSave) {
+            title={isEditing ? _t("Edit link") : _t("Create a link")}
+            hasCancel={true}
+            onFinished={onClose}
+        >
+            <form
+                className="mx_LinkModal_content"
+                onSubmit={async (evt) => {
+                    evt.preventDefault();
+                    evt.stopPropagation();
+
+                    onClose();
+
+                    // When submitting is done when pressing enter when the link field has the focus,
+                    // The link field is getting back the focus (due to react-focus-lock)
+                    // So we are waiting that the focus stuff is done to play with the composer selection
+                    await new Promise((resolve) => setTimeout(resolve, 0));
+
                     await setSelection(composerContext.selection);
                     composer.link(fields.link, isTextEnabled ? fields.text : undefined);
-                }
-                onClose();
-            }}
-            description={
-                <div className="mx_LinkModal_content">
-                    {isTextEnabled && (
-                        <Field
-                            autoFocus={true}
-                            label={_td("Text")}
-                            value={fields.text}
-                            onChange={(e: ChangeEvent<HTMLInputElement>) =>
-                                setFields((fields) => ({ ...fields, text: e.target.value }))
-                            }
-                        />
-                    )}
+                }}
+            >
+                {hasText && (
                     <Field
-                        autoFocus={!isTextEnabled}
-                        label={_td("Link")}
-                        value={fields.link}
+                        required={true}
+                        autoFocus={true}
+                        label={_t("Text")}
+                        value={fields.text}
+                        className="mx_LinkModal_Field"
+                        placeholder=""
                         onChange={(e: ChangeEvent<HTMLInputElement>) =>
-                            setFields((fields) => ({ ...fields, link: e.target.value }))
+                            setFields((fields) => ({ ...fields, text: e.target.value }))
                         }
                     />
+                )}
+                <Field
+                    required={true}
+                    autoFocus={!hasText}
+                    label={_t("Link")}
+                    value={fields.link}
+                    className="mx_LinkModal_Field"
+                    placeholder=""
+                    onChange={(e: ChangeEvent<HTMLInputElement>) => {
+                        setFields((fields) => ({ ...fields, link: e.target.value }));
+                        setHasLinkChanged(true);
+                    }}
+                />
+
+                <div className="mx_LinkModal_buttons">
+                    {isEditing && (
+                        <button
+                            type="button"
+                            className="danger"
+                            onClick={() => {
+                                composer.removeLinks();
+                                onClose();
+                            }}
+                        >
+                            {_t("Remove")}
+                        </button>
+                    )}
+                    <DialogButtons
+                        primaryButton={_t("Save")}
+                        primaryDisabled={isSaveDisabled}
+                        primaryIsSubmit={true}
+                        onCancel={onClose}
+                    />
                 </div>
-            }
-        />
+            </form>
+        </BaseDialog>
     );
 }
diff --git a/src/i18n/strings/en_EN.json b/src/i18n/strings/en_EN.json
index 16acf9b8dc..bc876ff668 100644
--- a/src/i18n/strings/en_EN.json
+++ b/src/i18n/strings/en_EN.json
@@ -2136,6 +2136,7 @@
     "Underline": "Underline",
     "Code": "Code",
     "Link": "Link",
+    "Edit link": "Edit link",
     "Create a link": "Create a link",
     "Text": "Text",
     "Message Actions": "Message Actions",
diff --git a/test/components/views/elements/Field-test.tsx b/test/components/views/elements/Field-test.tsx
new file mode 100644
index 0000000000..199f1c2557
--- /dev/null
+++ b/test/components/views/elements/Field-test.tsx
@@ -0,0 +1,54 @@
+/*
+Copyright 2023 The Matrix.org Foundation C.I.C.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+import React from "react";
+import { render, screen } from "@testing-library/react";
+
+import Field from "../../../../src/components/views/elements/Field";
+
+describe("Field", () => {
+    describe("Placeholder", () => {
+        it("Should display a placeholder", async () => {
+            // When
+            const { rerender } = render(<Field value="" placeholder="my placeholder" />);
+
+            // Then
+            expect(screen.getByRole("textbox")).toHaveAttribute("placeholder", "my placeholder");
+
+            // When
+            rerender(<Field value="" placeholder="" />);
+
+            // Then
+            expect(screen.getByRole("textbox")).toHaveAttribute("placeholder", "");
+        });
+
+        it("Should display label as placeholder", async () => {
+            // When
+            render(<Field value="" label="my label" />);
+
+            // Then
+            expect(screen.getByRole("textbox")).toHaveAttribute("placeholder", "my label");
+        });
+
+        it("Should not display a placeholder", async () => {
+            // When
+            render(<Field value="" />);
+
+            // Then
+            expect(screen.getByRole("textbox")).not.toHaveAttribute("placeholder", "my placeholder");
+        });
+    });
+});
diff --git a/test/components/views/rooms/wysiwyg_composer/components/LinkModal-test.tsx b/test/components/views/rooms/wysiwyg_composer/components/LinkModal-test.tsx
index c2fd1aeff2..527f7e7b3d 100644
--- a/test/components/views/rooms/wysiwyg_composer/components/LinkModal-test.tsx
+++ b/test/components/views/rooms/wysiwyg_composer/components/LinkModal-test.tsx
@@ -27,6 +27,8 @@ import { SubSelection } from "../../../../../../src/components/views/rooms/wysiw
 describe("LinkModal", () => {
     const formattingFunctions = {
         link: jest.fn(),
+        removeLinks: jest.fn(),
+        getLink: jest.fn().mockReturnValue("my initial content"),
     } as unknown as FormattingFunctions;
     const defaultValue: SubSelection = {
         focusNode: null,
@@ -35,13 +37,14 @@ describe("LinkModal", () => {
         anchorOffset: 4,
     };
 
-    const customRender = (isTextEnabled: boolean, onClose: () => void) => {
+    const customRender = (isTextEnabled: boolean, onClose: () => void, isEditing = false) => {
         return render(
             <LinkModal
                 composer={formattingFunctions}
                 isTextEnabled={isTextEnabled}
                 onClose={onClose}
                 composerContext={{ selection: defaultValue }}
+                isEditing={isEditing}
             />,
         );
     };
@@ -75,13 +78,13 @@ describe("LinkModal", () => {
         // When
         jest.useFakeTimers();
         screen.getByText("Save").click();
+        jest.runAllTimers();
 
         // Then
-        expect(selectionSpy).toHaveBeenCalledWith(defaultValue);
-        await waitFor(() => expect(onClose).toBeCalledTimes(1));
-
-        // When
-        jest.runAllTimers();
+        await waitFor(() => {
+            expect(selectionSpy).toHaveBeenCalledWith(defaultValue);
+            expect(onClose).toBeCalledTimes(1);
+        });
 
         // Then
         expect(formattingFunctions.link).toHaveBeenCalledWith("l", undefined);
@@ -118,15 +121,41 @@ describe("LinkModal", () => {
         // When
         jest.useFakeTimers();
         screen.getByText("Save").click();
+        jest.runAllTimers();
 
         // Then
-        expect(selectionSpy).toHaveBeenCalledWith(defaultValue);
-        await waitFor(() => expect(onClose).toBeCalledTimes(1));
-
-        // When
-        jest.runAllTimers();
+        await waitFor(() => {
+            expect(selectionSpy).toHaveBeenCalledWith(defaultValue);
+            expect(onClose).toBeCalledTimes(1);
+        });
 
         // Then
         expect(formattingFunctions.link).toHaveBeenCalledWith("l", "t");
     });
+
+    it("Should remove the link", async () => {
+        // When
+        const onClose = jest.fn();
+        customRender(true, onClose, true);
+        await userEvent.click(screen.getByText("Remove"));
+
+        // Then
+        expect(formattingFunctions.removeLinks).toHaveBeenCalledTimes(1);
+        expect(onClose).toBeCalledTimes(1);
+    });
+
+    it("Should display the link in editing", async () => {
+        // When
+        customRender(true, jest.fn(), true);
+
+        // Then
+        expect(screen.getByLabelText("Link")).toContainHTML("my initial content");
+        expect(screen.getByText("Save")).toBeDisabled();
+
+        // When
+        await userEvent.type(screen.getByLabelText("Link"), "l");
+
+        // Then
+        await waitFor(() => expect(screen.getByText("Save")).toBeEnabled());
+    });
 });
diff --git a/yarn.lock b/yarn.lock
index c9c01780cd..ee1874ea43 100644
--- a/yarn.lock
+++ b/yarn.lock
@@ -1525,10 +1525,10 @@
   resolved "https://registry.yarnpkg.com/@matrix-org/matrix-sdk-crypto-js/-/matrix-sdk-crypto-js-0.1.0-alpha.2.tgz#a09d0fea858e817da971a3c9f904632ef7b49eb6"
   integrity sha512-oVkBCh9YP7H9i4gAoQbZzswniczfo/aIptNa4dxRi4Ff9lSvUCFv6Hvzi7C+90c0/PWZLXjIDTIAWZYmwyd2fA==
 
-"@matrix-org/matrix-wysiwyg@^0.13.0":
-  version "0.13.0"
-  resolved "https://registry.yarnpkg.com/@matrix-org/matrix-wysiwyg/-/matrix-wysiwyg-0.13.0.tgz#e643df4e13cdc5dbf9285740bc0ce2aef9873c16"
-  integrity sha512-MCeTj4hkl0snjlygd1v+mEEOgaN6agyjAVjJEbvEvP/BaYaDiPEXMTDaRQrcUt3OIY53UNhm1DDEn4yPTn83Jg==
+"@matrix-org/matrix-wysiwyg@^0.14.0":
+  version "0.14.0"
+  resolved "https://registry.yarnpkg.com/@matrix-org/matrix-wysiwyg/-/matrix-wysiwyg-0.14.0.tgz#359fabf5af403b3f128fe6ede3bff9754a9e18c4"
+  integrity sha512-iSwIR7kS/zwAzy/8S5cUMv2aceoJl/vIGhqmY9hSU0gVyzmsyaVnx00uNMvVDBUFiiPT2gonN8R3+dxg58TPaQ==
 
 "@matrix-org/olm@https://gitlab.matrix.org/api/v4/projects/27/packages/npm/@matrix-org/olm/-/@matrix-org/olm-3.2.14.tgz":
   version "3.2.14"