Improve switching between rich and plain editing modes

package.json

@@ -57,7 +57,7 @@
     "dependencies": {
         "@babel/runtime": "^7.12.5",
         "@matrix-org/analytics-events": "^0.3.0",
-        "@matrix-org/matrix-wysiwyg": "^0.9.0",
+        "@matrix-org/matrix-wysiwyg": "^0.12.0",
         "@matrix-org/react-sdk-module-api": "^0.0.3",
         "@sentry/browser": "^7.0.0",
         "@sentry/tracing": "^7.0.0",

src/components/views/rooms/MessageComposer.tsx

@@ -15,6 +15,7 @@ limitations under the License.
 */
 
 import React, { createRef, ReactNode } from "react";
+import { richToPlain, plainToRich } from "@matrix-org/matrix-wysiwyg";
 import classNames from "classnames";
 import { IEventRelation, MatrixEvent } from "matrix-js-sdk/src/models/event";
 import { Room } from "matrix-js-sdk/src/models/room";
@@ -57,7 +58,6 @@ import { VoiceMessageRecording } from "../../../audio/VoiceMessageRecording";
 import { VoiceBroadcastRecordingsStore } from "../../../voice-broadcast";
 import { SendWysiwygComposer, sendMessage } from "./wysiwyg_composer/";
 import { MatrixClientProps, withMatrixClientHOC } from "../../../contexts/MatrixClientContext";
-import { htmlToPlainText } from "../../../utils/room/htmlToPlaintext";
 import { setUpVoiceBroadcastPreRecording } from "../../../voice-broadcast/utils/setUpVoiceBroadcastPreRecording";
 import { SdkContextClass } from "../../../contexts/SDKContext";
 
@@ -334,7 +334,7 @@ export class MessageComposer extends React.Component<IProps, IState> {
 
         if (this.state.isWysiwygLabEnabled) {
             const { permalinkCreator, relation, replyToEvent } = this.props;
-            sendMessage(this.state.composerContent, this.state.isRichTextEnabled, {
+            await sendMessage(this.state.composerContent, this.state.isRichTextEnabled, {
                 mxClient: this.props.mxClient,
                 roomContext: this.context,
                 permalinkCreator,
@@ -359,14 +359,17 @@ export class MessageComposer extends React.Component<IProps, IState> {
         });
     };
 
-    private onRichTextToggle = () => {
-        this.setState((state) => ({
-            isRichTextEnabled: !state.isRichTextEnabled,
-            initialComposerContent: !state.isRichTextEnabled
-                ? state.composerContent
-                : // TODO when available use rust model plain text
-                  htmlToPlainText(state.composerContent),
-        }));
+    private onRichTextToggle = async () => {
+        const { isRichTextEnabled, composerContent } = this.state;
+        const convertedContent = isRichTextEnabled
+            ? await richToPlain(composerContent)
+            : await plainToRich(composerContent);
+
+        this.setState({
+            isRichTextEnabled: !isRichTextEnabled,
+            composerContent: convertedContent,
+            initialComposerContent: convertedContent,
+        });
     };
 
     private onVoiceStoreUpdate = () => {

src/components/views/rooms/wysiwyg_composer/hooks/usePlainTextListeners.ts

@@ -17,11 +17,22 @@ limitations under the License.
 import { KeyboardEvent, SyntheticEvent, useCallback, useRef, useState } from "react";
 
 import { useSettingValue } from "../../../../../hooks/useSettings";
+import { IS_MAC, Key } from '../../../../../Keyboard';
 
 function isDivElement(target: EventTarget): target is HTMLDivElement {
     return target instanceof HTMLDivElement;
 }
 
+// Hitting enter inside the editor inserts an editable div, initially containing a <br />
+// For correct display, first replace this pattern with a newline character and then remove divs
+// noting that they are used to delimit paragraphs
+function amendInnerHtml(text: string) {
+    return text
+        .replace(/<div><br><\/div>/g, "\n") // this is pressing enter then not typing
+        .replace(/<div>/g, "\n") // this is from pressing enter, then typing inside the div
+        .replace(/<\/div>/g, "");
+}
+
 export function usePlainTextListeners(
     initialContent?: string,
     onChange?: (content: string) => void,
@@ -44,25 +55,39 @@ export function usePlainTextListeners(
         [onChange],
     );
 
+    const enterShouldSend = !useSettingValue<boolean>("MessageComposerInput.ctrlEnterToSend");
     const onInput = useCallback(
         (event: SyntheticEvent<HTMLDivElement, InputEvent | ClipboardEvent>) => {
             if (isDivElement(event.target)) {
-                setText(event.target.innerHTML);
+                // if enterShouldSend, we do not need to amend the html before setting text
+                const newInnerHTML = enterShouldSend ? event.target.innerHTML : amendInnerHtml(event.target.innerHTML);
+                setText(newInnerHTML);
             }
         },
-        [setText],
+        [setText, enterShouldSend],
     );
 
-    const isCtrlEnter = useSettingValue<boolean>("MessageComposerInput.ctrlEnterToSend");
     const onKeyDown = useCallback(
         (event: KeyboardEvent<HTMLDivElement>) => {
-            if (event.key === "Enter" && !event.shiftKey && (!isCtrlEnter || (isCtrlEnter && event.ctrlKey))) {
-                event.preventDefault();
-                event.stopPropagation();
-                send();
+            if (event.key === Key.ENTER) {
+                const sendModifierIsPressed = IS_MAC ? event.metaKey : event.ctrlKey;
+
+                // if enter should send, send if the user is not pushing shift
+                if (enterShouldSend && !event.shiftKey) {
+                    event.preventDefault();
+                    event.stopPropagation();
+                    send();
+                }
+
+                // if enter should not send, send only if the user is pushing ctrl/cmd
+                if (!enterShouldSend && sendModifierIsPressed) {
+                    event.preventDefault();
+                    event.stopPropagation();
+                    send();
+                }
             }
         },
-        [isCtrlEnter, send],
+        [enterShouldSend, send],
     );
 
     return { ref, onInput, onPaste: onInput, onKeyDown, content, setContent: setText };

src/components/views/rooms/wysiwyg_composer/utils/createMessageContent.ts

@@ -14,13 +14,12 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
+import { richToPlain, plainToRich } from "@matrix-org/matrix-wysiwyg";
 import { IContent, IEventRelation, MatrixEvent, MsgType } from "matrix-js-sdk/src/matrix";
 
-import { htmlSerializeFromMdIfNeeded } from "../../../../../editor/serialize";
 import SettingsStore from "../../../../../settings/SettingsStore";
 import { RoomPermalinkCreator } from "../../../../../utils/permalinks/Permalinks";
 import { addReplyToMessageContent } from "../../../../../utils/Reply";
-import { htmlToPlainText } from "../../../../../utils/room/htmlToPlaintext";
 
 // Merges favouring the given relation
 function attachRelation(content: IContent, relation?: IEventRelation): void {
@@ -62,7 +61,7 @@ interface CreateMessageContentParams {
     editedEvent?: MatrixEvent;
 }
 
-export function createMessageContent(
+export async function createMessageContent(
     message: string,
     isHTML: boolean,
     {
@@ -72,7 +71,7 @@ export function createMessageContent(
         includeReplyLegacyFallback = true,
         editedEvent,
     }: CreateMessageContentParams,
-): IContent {
+): Promise<IContent> {
     // TODO emote ?
 
     const isEditing = Boolean(editedEvent);
@@ -90,15 +89,15 @@ export function createMessageContent(
 
     // const body = textSerialize(model);
 
-    // TODO remove this ugly hack for replace br tag
-    const body = (isHTML && htmlToPlainText(message)) || message.replace(/<br>/g, "\n");
+    // if we're editing rich text, the message content is pure html
+    // BUT if we're not, the message content will be plain text
+    const body = isHTML ? await richToPlain(message) : message;
     const bodyPrefix = (isReplyAndEditing && getTextReplyFallback(editedEvent)) || "";
     const formattedBodyPrefix = (isReplyAndEditing && getHtmlReplyFallback(editedEvent)) || "";
 
     const content: IContent = {
         // TODO emote
         msgtype: MsgType.Text,
-        // TODO when available, use HTML --> Plain text conversion from wysiwyg rust model
         body: isEditing ? `${bodyPrefix} * ${body}` : body,
     };
 
@@ -108,8 +107,8 @@ export function createMessageContent(
     const formattedBody = isHTML
         ? message
         : isMarkdownEnabled
-        ? htmlSerializeFromMdIfNeeded(message, { forceHTML: isReply })
-        : null;
+            ? await plainToRich(message)
+            : null;
 
     if (formattedBody) {
         content.format = "org.matrix.custom.html";

src/components/views/rooms/wysiwyg_composer/utils/message.ts

@@ -43,7 +43,11 @@ interface SendMessageParams {
     includeReplyLegacyFallback?: boolean;
 }
 
-export function sendMessage(message: string, isHTML: boolean, { roomContext, mxClient, ...params }: SendMessageParams) {
+export async function sendMessage(
+    message: string,
+    isHTML: boolean,
+    { roomContext, mxClient, ...params }: SendMessageParams,
+) {
     const { relation, replyToEvent } = params;
     const { room } = roomContext;
     const { roomId } = room;
@@ -72,7 +76,7 @@ export function sendMessage(message: string, isHTML: boolean, { roomContext, mxC
     // TODO quick reaction
 
     if (!content) {
-        content = createMessageContent(message, isHTML, params);
+        content = await createMessageContent(message, isHTML, params);
     }
 
     // don't bother sending an empty message

test/components/views/rooms/wysiwyg_composer/components/PlainTextComposer-test.tsx

@@ -19,6 +19,22 @@ import { render, screen } from "@testing-library/react";
 import userEvent from "@testing-library/user-event";
 
 import { PlainTextComposer } from "../../../../../../src/components/views/rooms/wysiwyg_composer/components/PlainTextComposer";
+import * as mockUseSettingsHook from "../../../../../../src/hooks/useSettings";
+import * as mockKeyboard from "../../../../../../src/Keyboard";
+
+jest.mock("../../../../../../src/hooks/useSettings");
+
+jest.mock("../../../../../../src/hooks/useSettings");
+
+beforeEach(() => {
+    // defaults for these tests are:
+    // ctrlEnterToSend is false
+    mockUseSettingsHook.useSettingValue = jest.fn().mockReturnValue(false);
+    // platform is not mac
+    mockKeyboard.IS_MAC = false;
+});
+
+
 
 describe("PlainTextComposer", () => {
     const customRender = (
@@ -64,7 +80,7 @@ describe("PlainTextComposer", () => {
         expect(onChange).toBeCalledWith(content);
     });
 
-    it("Should call onSend when Enter is pressed", async () => {
+    it("Should call onSend when Enter is pressed when ctrlEnterToSend is false", async () => {
         //When
         const onSend = jest.fn();
         customRender(jest.fn(), onSend);
@@ -74,6 +90,127 @@ describe("PlainTextComposer", () => {
         expect(onSend).toBeCalledTimes(1);
     });
 
+    it("Should not call onSend when Enter is pressed when ctrlEnterToSend is true", async () => {
+        //When
+        mockUseSettingsHook.useSettingValue.mockReturnValue(true);
+        const onSend = jest.fn();
+        customRender(jest.fn(), onSend);
+        await userEvent.type(screen.getByRole("textbox"), "{enter}");
+
+        // Then it does not send a message
+        expect(onSend).toBeCalledTimes(0);
+    });
+
+    it("Should only call onSend when ctrl+enter is pressed when ctrlEnterToSend is true on windows", async () => {
+        //When
+        mockUseSettingsHook.useSettingValue.mockReturnValue(true);
+
+        const onSend = jest.fn();
+        customRender(jest.fn(), onSend);
+        const textBox = screen.getByRole("textbox");
+        await userEvent.type(textBox, "hello");
+
+        // Then it does NOT send a message on enter
+        await userEvent.type(textBox, "{enter}");
+        expect(onSend).toBeCalledTimes(0);
+
+        // Then it does NOT send a message on windows+enter
+        await userEvent.type(textBox, "{meta>}{enter}{meta/}");
+        expect(onSend).toBeCalledTimes(0);
+
+        // Then it does send a message on ctrl+enter
+        await userEvent.type(textBox, "{control>}{enter}{control/}");
+        expect(onSend).toBeCalledTimes(1);
+    });
+
+    it("Should only call onSend when cmd+enter is pressed when ctrlEnterToSend is true on mac", async () => {
+        //When
+        mockUseSettingsHook.useSettingValue.mockReturnValue(true);
+        mockKeyboard.IS_MAC = true;
+
+        const onSend = jest.fn();
+        customRender(jest.fn(), onSend);
+        const textBox = screen.getByRole("textbox");
+        await userEvent.type(textBox, "hello");
+
+        // Then it does NOT send a message on enter
+        await userEvent.type(textBox, "{enter}");
+        expect(onSend).toBeCalledTimes(0);
+
+        // Then it does NOT send a message on ctrl+enter
+        await userEvent.type(textBox, "{control>}{enter}{control/}");
+        expect(onSend).toBeCalledTimes(0);
+
+        // Then it does send a message on cmd+enter
+        await userEvent.type(textBox, "{meta>}{enter}{meta/}");
+        expect(onSend).toBeCalledTimes(1);
+    });
+
+    it("Should insert a newline character when shift enter is pressed when ctrlEnterToSend is false", async () => {
+        //When
+        const onSend = jest.fn();
+        customRender(jest.fn(), onSend);
+        const textBox = screen.getByRole("textbox");
+        const inputWithShiftEnter = "new{Shift>}{enter}{/Shift}line";
+        const expectedInnerHtml = "new\nline";
+
+        await userEvent.click(textBox);
+        await userEvent.type(textBox, inputWithShiftEnter);
+
+        // Then it does not send a message, but inserts a newline character
+        expect(onSend).toBeCalledTimes(0);
+        expect(textBox.innerHTML).toBe(expectedInnerHtml);
+    });
+
+    it("Should insert a newline character when shift enter is pressed when ctrlEnterToSend is true", async () => {
+        //When
+        mockUseSettingsHook.useSettingValue.mockReturnValue(true);
+        const onSend = jest.fn();
+        customRender(jest.fn(), onSend);
+        const textBox = screen.getByRole("textbox");
+        const keyboardInput = "new{Shift>}{enter}{/Shift}line";
+        const expectedInnerHtml = "new\nline";
+
+        await userEvent.click(textBox);
+        await userEvent.type(textBox, keyboardInput);
+
+        // Then it does not send a message, but inserts a newline character
+        expect(onSend).toBeCalledTimes(0);
+        expect(textBox.innerHTML).toBe(expectedInnerHtml);
+    });
+
+    it("Should not insert div and br tags when enter is pressed when ctrlEnterToSend is true", async () => {
+        //When
+        mockUseSettingsHook.useSettingValue.mockReturnValue(true);
+        const onSend = jest.fn();
+        customRender(jest.fn(), onSend);
+        const textBox = screen.getByRole("textbox");
+        const enterThenTypeHtml = "<div>hello</div";
+
+        await userEvent.click(textBox);
+        await userEvent.type(textBox, "{enter}hello");
+
+        // Then it does not send a message, but inserts a newline character
+        expect(onSend).toBeCalledTimes(0);
+        expect(textBox).not.toContainHTML(enterThenTypeHtml);
+    });
+
+    it("Should not insert div tags when enter is pressed then user types more when ctrlEnterToSend is true", async () => {
+        //When
+        mockUseSettingsHook.useSettingValue.mockReturnValue(true);
+        const onSend = jest.fn();
+        customRender(jest.fn(), onSend);
+        const textBox = screen.getByRole("textbox");
+        const defaultEnterHtml = "<div><br></div";
+
+        await userEvent.click(textBox);
+        await userEvent.type(textBox, "{enter}");
+
+        // Then it does not send a message, but inserts a newline character
+        expect(onSend).toBeCalledTimes(0);
+        expect(textBox).not.toContainHTML(defaultEnterHtml);
+    });
+
     it("Should clear textbox content when clear is called", async () => {
         //When
         let composer;