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

@@ -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;
+        }
     }
 }
 

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 {

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;

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")}

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",

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");
+        }
     });
 });