Add list functionality to rich text editor (matrix-org#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

Author: artcodespace

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 {

res/img/element-icons/room/composer/bulleted_list.svg

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

res/img/element-icons/room/composer/numbered_list.svg

@@ -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/components/views/rooms/wysiwyg_composer/components/Editor.tsx

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

src/components/views/rooms/wysiwyg_composer/components/FormattingButtons.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");
+ }
  });
 });