From 6052db1e8a91e07106946334c5b6e8de196e5f42 Mon Sep 17 00:00:00 2001
From: alunturner <56027671+alunturner@users.noreply.github.com>
Date: Fri, 13 Jan 2023 15:44:35 +0000
Subject: [PATCH] Add list functionality to rich text editor (#9871)

* adds buttons to toggle bulleted and numbered lists on and off
* adds icons for those buttons
* css changes to timeline display
* adds tests for the new buttons, refactors existing tests
---
 res/css/views/rooms/_EventTile.pcss           |  11 ++
 .../wysiwyg_composer/components/_Editor.pcss  |  14 ++
 .../room/composer/bulleted_list.svg           |   3 +
 .../room/composer/numbered_list.svg           |   3 +
 .../wysiwyg_composer/components/Editor.tsx    |   2 +-
 .../components/FormattingButtons.tsx          |  14 ++
 src/i18n/strings/en_EN.json                   |   2 +
 .../components/FormattingButtons-test.tsx     | 160 ++++++++++--------
 8 files changed, 142 insertions(+), 67 deletions(-)
 create mode 100644 res/img/element-icons/room/composer/bulleted_list.svg
 create mode 100644 res/img/element-icons/room/composer/numbered_list.svg

diff --git a/res/css/views/rooms/_EventTile.pcss b/res/css/views/rooms/_EventTile.pcss
index c7e857fc9cb..57bf2ce3c88 100644
--- a/res/css/views/rooms/_EventTile.pcss
+++ b/res/css/views/rooms/_EventTile.pcss
@@ -619,6 +619,17 @@ $left-gutter: 64px;
         ul ol {
             list-style-type: revert;
         }
+
+        /* Make list type disc to match rich text editor */
+        > ul {
+            list-style-type: disc;
+        }
+
+        /* Remove top and bottom margin for better consecutive list display */
+        > :is(ol, ul) {
+            margin-top: 0;
+            margin-bottom: 0;
+        }
     }
 }
 
diff --git a/res/css/views/rooms/wysiwyg_composer/components/_Editor.pcss b/res/css/views/rooms/wysiwyg_composer/components/_Editor.pcss
index d48476cfd7f..7ff3f11a500 100644
--- a/res/css/views/rooms/wysiwyg_composer/components/_Editor.pcss
+++ b/res/css/views/rooms/wysiwyg_composer/components/_Editor.pcss
@@ -25,6 +25,7 @@ limitations under the License.
     }
 
     .mx_WysiwygComposer_Editor_content {
+        line-height: $font-22px;
         white-space: pre-wrap;
         word-wrap: break-word;
         outline: none;
@@ -35,6 +36,19 @@ limitations under the License.
         .caretNode {
             user-select: all;
         }
+
+        ul,
+        ol {
+            margin-top: 0;
+            margin-bottom: 0;
+            padding-inline-start: $spacing-28;
+        }
+
+        // model output always includes a linebreak but we do not want the user
+        // to see it when writing input in lists
+        :is(ol, ul) + br:last-of-type {
+            display: none;
+        }
     }
 
     .mx_WysiwygComposer_Editor_content_placeholder::before {
diff --git a/res/img/element-icons/room/composer/bulleted_list.svg b/res/img/element-icons/room/composer/bulleted_list.svg
new file mode 100644
index 00000000000..828bb8ab038
--- /dev/null
+++ b/res/img/element-icons/room/composer/bulleted_list.svg
@@ -0,0 +1,3 @@
+<svg width="13" height="10" viewBox="0 0 13 10" fill="none" xmlns="http://www.w3.org/2000/svg">
+<path d="M1.66666 4C1.11332 4 0.666656 4.44667 0.666656 5C0.666656 5.55333 1.11332 6 1.66666 6C2.21999 6 2.66666 5.55333 2.66666 5C2.66666 4.44667 2.21999 4 1.66666 4ZM1.66666 0C1.11332 0 0.666656 0.446667 0.666656 1C0.666656 1.55333 1.11332 2 1.66666 2C2.21999 2 2.66666 1.55333 2.66666 1C2.66666 0.446667 2.21999 0 1.66666 0ZM1.66666 8C1.11332 8 0.666656 8.45333 0.666656 9C0.666656 9.54667 1.11999 10 1.66666 10C2.21332 10 2.66666 9.54667 2.66666 9C2.66666 8.45333 2.21999 8 1.66666 8ZM4.33332 9.66667H12.3333C12.7 9.66667 13 9.36667 13 9C13 8.63333 12.7 8.33333 12.3333 8.33333H4.33332C3.96666 8.33333 3.66666 8.63333 3.66666 9C3.66666 9.36667 3.96666 9.66667 4.33332 9.66667ZM4.33332 5.66667H12.3333C12.7 5.66667 13 5.36667 13 5C13 4.63333 12.7 4.33333 12.3333 4.33333H4.33332C3.96666 4.33333 3.66666 4.63333 3.66666 5C3.66666 5.36667 3.96666 5.66667 4.33332 5.66667ZM3.66666 1C3.66666 1.36667 3.96666 1.66667 4.33332 1.66667H12.3333C12.7 1.66667 13 1.36667 13 1C13 0.633333 12.7 0.333333 12.3333 0.333333H4.33332C3.96666 0.333333 3.66666 0.633333 3.66666 1Z" fill="currentColor"/>
+</svg>
diff --git a/res/img/element-icons/room/composer/numbered_list.svg b/res/img/element-icons/room/composer/numbered_list.svg
new file mode 100644
index 00000000000..46a5438f3f6
--- /dev/null
+++ b/res/img/element-icons/room/composer/numbered_list.svg
@@ -0,0 +1,3 @@
+<svg width="13" height="12" viewBox="0 0 13 12" fill="none" xmlns="http://www.w3.org/2000/svg">
+<path d="M4.33334 2.66663H12.3333C12.7 2.66663 13 2.36663 13 1.99996C13 1.63329 12.7 1.33329 12.3333 1.33329H4.33334C3.96668 1.33329 3.66668 1.63329 3.66668 1.99996C3.66668 2.36663 3.96668 2.66663 4.33334 2.66663ZM12.3333 9.33329H4.33334C3.96668 9.33329 3.66668 9.63329 3.66668 9.99996C3.66668 10.3666 3.96668 10.6666 4.33334 10.6666H12.3333C12.7 10.6666 13 10.3666 13 9.99996C13 9.63329 12.7 9.33329 12.3333 9.33329ZM12.3333 5.33329H4.33334C3.96668 5.33329 3.66668 5.63329 3.66668 5.99996C3.66668 6.36663 3.96668 6.66663 4.33334 6.66663H12.3333C12.7 6.66663 13 6.36663 13 5.99996C13 5.63329 12.7 5.33329 12.3333 5.33329ZM2.00001 8.66663H0.666677C0.48001 8.66663 0.333344 8.81329 0.333344 8.99996C0.333344 9.18663 0.48001 9.33329 0.666677 9.33329H1.66668V9.66663H1.33334C1.14668 9.66663 1.00001 9.81329 1.00001 9.99996C1.00001 10.1866 1.14668 10.3333 1.33334 10.3333H1.66668V10.6666H0.666677C0.48001 10.6666 0.333344 10.8133 0.333344 11C0.333344 11.1866 0.48001 11.3333 0.666677 11.3333H2.00001C2.18668 11.3333 2.33334 11.1866 2.33334 11V8.99996C2.33334 8.81329 2.18668 8.66663 2.00001 8.66663ZM0.666677 1.33329H1.00001V2.99996C1.00001 3.18663 1.14668 3.33329 1.33334 3.33329C1.52001 3.33329 1.66668 3.18663 1.66668 2.99996V0.999959C1.66668 0.813293 1.52001 0.666626 1.33334 0.666626H0.666677C0.48001 0.666626 0.333344 0.813293 0.333344 0.999959C0.333344 1.18663 0.48001 1.33329 0.666677 1.33329ZM2.00001 4.66663H0.666677C0.48001 4.66663 0.333344 4.81329 0.333344 4.99996C0.333344 5.18663 0.48001 5.33329 0.666677 5.33329H1.53334L0.413343 6.63996C0.36001 6.69996 0.333344 6.77996 0.333344 6.85329V6.99996C0.333344 7.18663 0.48001 7.33329 0.666677 7.33329H2.00001C2.18668 7.33329 2.33334 7.18663 2.33334 6.99996C2.33334 6.81329 2.18668 6.66663 2.00001 6.66663H1.13334L2.25334 5.35996C2.30668 5.29996 2.33334 5.21996 2.33334 5.14663V4.99996C2.33334 4.81329 2.18668 4.66663 2.00001 4.66663Z" fill="currentColor"/>
+</svg>
diff --git a/src/components/views/rooms/wysiwyg_composer/components/Editor.tsx b/src/components/views/rooms/wysiwyg_composer/components/Editor.tsx
index 215d3be84fb..43e46572d67 100644
--- a/src/components/views/rooms/wysiwyg_composer/components/Editor.tsx
+++ b/src/components/views/rooms/wysiwyg_composer/components/Editor.tsx
@@ -20,7 +20,7 @@ import React, { CSSProperties, forwardRef, memo, MutableRefObject, ReactNode } f
 import { useIsExpanded } from "../hooks/useIsExpanded";
 import { useSelection } from "../hooks/useSelection";
 
-const HEIGHT_BREAKING_POINT = 20;
+const HEIGHT_BREAKING_POINT = 24;
 
 interface EditorProps {
     disabled: boolean;
diff --git a/src/components/views/rooms/wysiwyg_composer/components/FormattingButtons.tsx b/src/components/views/rooms/wysiwyg_composer/components/FormattingButtons.tsx
index 1d3f44bee7e..7c1601b4416 100644
--- a/src/components/views/rooms/wysiwyg_composer/components/FormattingButtons.tsx
+++ b/src/components/views/rooms/wysiwyg_composer/components/FormattingButtons.tsx
@@ -24,6 +24,8 @@ import { Icon as UnderlineIcon } from "../../../../../../res/img/element-icons/r
 import { Icon as StrikeThroughIcon } from "../../../../../../res/img/element-icons/room/composer/strikethrough.svg";
 import { Icon as InlineCodeIcon } from "../../../../../../res/img/element-icons/room/composer/inline_code.svg";
 import { Icon as LinkIcon } from "../../../../../../res/img/element-icons/room/composer/link.svg";
+import { Icon as BulletedListIcon } from "../../../../../../res/img/element-icons/room/composer/bulleted_list.svg";
+import { Icon as NumberedListIcon } from "../../../../../../res/img/element-icons/room/composer/numbered_list.svg";
 import AccessibleTooltipButton from "../../../elements/AccessibleTooltipButton";
 import { Alignment } from "../../../elements/Tooltip";
 import { KeyboardShortcut } from "../../../settings/KeyboardShortcut";
@@ -109,6 +111,18 @@ export function FormattingButtons({ composer, actionStates }: FormattingButtonsP
                 onClick={() => composer.strikeThrough()}
                 icon={<StrikeThroughIcon className="mx_FormattingButtons_Icon" />}
             />
+            <Button
+                isActive={actionStates.unorderedList === "reversed"}
+                label={_td("Bulleted list")}
+                onClick={() => composer.unorderedList()}
+                icon={<BulletedListIcon className="mx_FormattingButtons_Icon" />}
+            />
+            <Button
+                isActive={actionStates.orderedList === "reversed"}
+                label={_td("Numbered list")}
+                onClick={() => composer.orderedList()}
+                icon={<NumberedListIcon className="mx_FormattingButtons_Icon" />}
+            />
             <Button
                 isActive={actionStates.inlineCode === "reversed"}
                 label={_td("Code")}
diff --git a/src/i18n/strings/en_EN.json b/src/i18n/strings/en_EN.json
index c502cb8f406..4da12e366bd 100644
--- a/src/i18n/strings/en_EN.json
+++ b/src/i18n/strings/en_EN.json
@@ -2135,6 +2135,8 @@
     "Stop recording": "Stop recording",
     "Italic": "Italic",
     "Underline": "Underline",
+    "Bulleted list": "Bulleted list",
+    "Numbered list": "Numbered list",
     "Code": "Code",
     "Link": "Link",
     "Edit link": "Edit link",
diff --git a/test/components/views/rooms/wysiwyg_composer/components/FormattingButtons-test.tsx b/test/components/views/rooms/wysiwyg_composer/components/FormattingButtons-test.tsx
index a467aa404e7..012090d5d89 100644
--- a/test/components/views/rooms/wysiwyg_composer/components/FormattingButtons-test.tsx
+++ b/test/components/views/rooms/wysiwyg_composer/components/FormattingButtons-test.tsx
@@ -17,90 +17,118 @@ limitations under the License.
 import React from "react";
 import { render, screen } from "@testing-library/react";
 import userEvent from "@testing-library/user-event";
-import { AllActionStates, FormattingFunctions } from "@matrix-org/matrix-wysiwyg";
+import { ActionState, ActionTypes, AllActionStates, FormattingFunctions } from "@matrix-org/matrix-wysiwyg";
 
 import { FormattingButtons } from "../../../../../../src/components/views/rooms/wysiwyg_composer/components/FormattingButtons";
 import * as LinkModal from "../../../../../../src/components/views/rooms/wysiwyg_composer/components/LinkModal";
 
-describe("FormattingButtons", () => {
-    const wysiwyg = {
-        bold: jest.fn(),
-        italic: jest.fn(),
-        underline: jest.fn(),
-        strikeThrough: jest.fn(),
-        inlineCode: jest.fn(),
-        link: jest.fn(),
-    } as unknown as FormattingFunctions;
-
-    const actionStates = {
-        bold: "reversed",
-        italic: "reversed",
-        underline: "enabled",
-        strikeThrough: "enabled",
-        inlineCode: "enabled",
-        link: "enabled",
-    } as AllActionStates;
+const mockWysiwyg = {
+    bold: jest.fn(),
+    italic: jest.fn(),
+    underline: jest.fn(),
+    strikeThrough: jest.fn(),
+    inlineCode: jest.fn(),
+    link: jest.fn(),
+    orderedList: jest.fn(),
+    unorderedList: jest.fn(),
+} as unknown as FormattingFunctions;
+
+const openLinkModalSpy = jest.spyOn(LinkModal, "openLinkModal");
+
+const testCases: Record<
+    Exclude<ActionTypes, "undo" | "redo" | "clear">,
+    { label: string; mockFormatFn: jest.Func | jest.SpyInstance }
+> = {
+    bold: { label: "Bold", mockFormatFn: mockWysiwyg.bold },
+    italic: { label: "Italic", mockFormatFn: mockWysiwyg.italic },
+    underline: { label: "Underline", mockFormatFn: mockWysiwyg.underline },
+    strikeThrough: { label: "Strikethrough", mockFormatFn: mockWysiwyg.strikeThrough },
+    inlineCode: { label: "Code", mockFormatFn: mockWysiwyg.inlineCode },
+    link: { label: "Link", mockFormatFn: openLinkModalSpy },
+    orderedList: { label: "Numbered list", mockFormatFn: mockWysiwyg.orderedList },
+    unorderedList: { label: "Bulleted list", mockFormatFn: mockWysiwyg.unorderedList },
+};
+
+const createActionStates = (state: ActionState): AllActionStates => {
+    return Object.fromEntries(Object.keys(testCases).map((testKey) => [testKey, state])) as AllActionStates;
+};
+
+const defaultActionStates = createActionStates("enabled");
+
+const renderComponent = (props = {}) => {
+    return render(<FormattingButtons composer={mockWysiwyg} actionStates={defaultActionStates} {...props} />);
+};
+
+const classes = {
+    active: "mx_FormattingButtons_active",
+    hover: "mx_FormattingButtons_Button_hover",
+};
 
+describe("FormattingButtons", () => {
     afterEach(() => {
         jest.resetAllMocks();
     });
 
-    it("Should have the correspond CSS classes", () => {
-        // When
-        render(<FormattingButtons composer={wysiwyg} actionStates={actionStates} />);
-
-        // Then
-        expect(screen.getByLabelText("Bold")).toHaveClass("mx_FormattingButtons_active");
-        expect(screen.getByLabelText("Italic")).toHaveClass("mx_FormattingButtons_active");
-        expect(screen.getByLabelText("Underline")).not.toHaveClass("mx_FormattingButtons_active");
-        expect(screen.getByLabelText("Strikethrough")).not.toHaveClass("mx_FormattingButtons_active");
-        expect(screen.getByLabelText("Code")).not.toHaveClass("mx_FormattingButtons_active");
-        expect(screen.getByLabelText("Link")).not.toHaveClass("mx_FormattingButtons_active");
+    it("Each button should not have active class when enabled", () => {
+        renderComponent();
+
+        Object.values(testCases).forEach(({ label }) => {
+            expect(screen.getByLabelText(label)).not.toHaveClass(classes.active);
+        });
+    });
+
+    it("Each button should have active class when reversed", () => {
+        const reversedActionStates = createActionStates("reversed");
+        renderComponent({ actionStates: reversedActionStates });
+
+        Object.values(testCases).forEach((testCase) => {
+            const { label } = testCase;
+            expect(screen.getByLabelText(label)).toHaveClass(classes.active);
+        });
     });
 
-    it("Should call wysiwyg function on button click", () => {
-        // When
-        const spy = jest.spyOn(LinkModal, "openLinkModal");
-        render(<FormattingButtons composer={wysiwyg} actionStates={actionStates} />);
-        screen.getByLabelText("Bold").click();
-        screen.getByLabelText("Italic").click();
-        screen.getByLabelText("Underline").click();
-        screen.getByLabelText("Strikethrough").click();
-        screen.getByLabelText("Code").click();
-        screen.getByLabelText("Link").click();
-
-        // Then
-        expect(wysiwyg.bold).toHaveBeenCalledTimes(1);
-        expect(wysiwyg.italic).toHaveBeenCalledTimes(1);
-        expect(wysiwyg.underline).toHaveBeenCalledTimes(1);
-        expect(wysiwyg.strikeThrough).toHaveBeenCalledTimes(1);
-        expect(wysiwyg.inlineCode).toHaveBeenCalledTimes(1);
-        expect(spy).toHaveBeenCalledTimes(1);
+    it("Should call wysiwyg function on button click", async () => {
+        renderComponent();
+
+        for (const testCase of Object.values(testCases)) {
+            const { label, mockFormatFn } = testCase;
+
+            screen.getByLabelText(label).click();
+            expect(mockFormatFn).toHaveBeenCalledTimes(1);
+        }
     });
 
-    it("Should display the tooltip on mouse over", async () => {
-        // When
-        const user = userEvent.setup();
-        render(<FormattingButtons composer={wysiwyg} actionStates={actionStates} />);
-        await user.hover(screen.getByLabelText("Bold"));
+    it("Each button should display the tooltip on mouse over", async () => {
+        renderComponent();
+
+        for (const testCase of Object.values(testCases)) {
+            const { label } = testCase;
 
-        // Then
-        expect(await screen.findByText("Bold")).toBeTruthy();
+            await userEvent.hover(screen.getByLabelText(label));
+            expect(await screen.findByText(label)).toBeTruthy();
+        }
     });
 
-    it("Should not have hover style when active", async () => {
-        // When
-        const user = userEvent.setup();
-        render(<FormattingButtons composer={wysiwyg} actionStates={actionStates} />);
-        await user.hover(screen.getByLabelText("Bold"));
+    it("Each button should have hover style when hovered and enabled", async () => {
+        renderComponent();
+
+        for (const testCase of Object.values(testCases)) {
+            const { label } = testCase;
+
+            await userEvent.hover(screen.getByLabelText(label));
+            expect(screen.getByLabelText(label)).toHaveClass("mx_FormattingButtons_Button_hover");
+        }
+    });
 
-        // Then
-        expect(screen.getByLabelText("Bold")).not.toHaveClass("mx_FormattingButtons_Button_hover");
+    it("Each button should not have hover style when hovered and reversed", async () => {
+        const reversedActionStates = createActionStates("reversed");
+        renderComponent({ actionStates: reversedActionStates });
 
-        // When
-        await user.hover(screen.getByLabelText("Underline"));
+        for (const testCase of Object.values(testCases)) {
+            const { label } = testCase;
 
-        // Then
-        expect(screen.getByLabelText("Underline")).toHaveClass("mx_FormattingButtons_Button_hover");
+            await userEvent.hover(screen.getByLabelText(label));
+            expect(screen.getByLabelText(label)).not.toHaveClass("mx_FormattingButtons_Button_hover");
+        }
     });
 });