Skip to content
This repository has been archived by the owner on Sep 11, 2024. It is now read-only.

Add list functionality to rich text editor #9871

Merged
merged 39 commits into from
Jan 13, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
39 commits
Select commit Hold shift + click to select a range
be18bfd
add bullet list svg icon
Jan 5, 2023
ae37394
use new icon and add button
Jan 5, 2023
660b105
amend fill colour for icon
Jan 5, 2023
458e756
look at correct part of actionStates for styling
Jan 5, 2023
da8fdba
use .unorderedList method, not .orderedList
Jan 6, 2023
c2cff3e
style ul and ol to remove margin, use circle not disc
Jan 6, 2023
43cd30d
sort out line height and lhs padding
Jan 6, 2023
f051854
add numbered list and icon
Jan 6, 2023
20864e8
hide end of model linebreak, separate out ul styling
Jan 6, 2023
2f474fa
Change copy bullet => bulleted
Jan 6, 2023
cbbdce7
expand tests to include new buttons
Jan 6, 2023
fce7d3a
Merge branch 'develop' into alunturner/add-bullet-list
Jan 10, 2023
4e9d486
reformat tests, wip single failing refactored test
Jan 10, 2023
135d82c
rewrite final test
Jan 10, 2023
f4550d6
add i18n strings
Jan 10, 2023
2641fde
fix no implicit any errors, neaten up types
Jan 10, 2023
0d47acf
use yarn i18n to generate file
Jan 10, 2023
a3b2e9f
Merge remote-tracking branch 'origin/develop' into alunturner/add-bul…
Jan 10, 2023
e1926eb
rename to bulleted for consistency
Jan 10, 2023
10bff95
remove comment
Jan 10, 2023
ce7d478
remove unnecessary awaits
Jan 10, 2023
6be2f75
Merge branch 'develop' into alunturner/add-bullet-list
artcodespace Jan 10, 2023
186d69e
update import path
Jan 10, 2023
98dd6e6
make display: none only target last child of container
Jan 11, 2023
6b1e91c
Merge remote-tracking branch 'origin/develop' into alunturner/add-bul…
Jan 11, 2023
7421d69
amend CSS selector
Jan 11, 2023
ce881c1
Merge branch 'develop' into alunturner/add-bullet-list
artcodespace Jan 11, 2023
4677ee4
Merge branch 'develop' into alunturner/add-bullet-list
artcodespace Jan 11, 2023
421f805
use :is instead of repeating selectors
Jan 11, 2023
463268b
Merge branch 'develop' into alunturner/add-bullet-list
Jan 11, 2023
125c562
Merge branch 'develop' into alunturner/add-bullet-list
Jan 12, 2023
b2400f5
fix rounding issue for composer input
Jan 12, 2023
ee7ffa0
make timeline and composer use discs, not circles
Jan 12, 2023
e9962d0
fix shift enter not working
Jan 12, 2023
553a2c0
Merge branch 'develop' into alunturner/add-bullet-list
artcodespace Jan 12, 2023
07ed1ad
Merge remote-tracking branch 'origin/develop' into alunturner/add-bul…
Jan 12, 2023
a634d4d
use ! instead of === false
Jan 12, 2023
db0bf64
revert regression fix
Jan 12, 2023
bc5e6ef
Amend consecutive list display in timeline
Jan 13, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions res/css/views/rooms/_EventTile.pcss
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
}

Expand Down
14 changes: 14 additions & 0 deletions res/css/views/rooms/wysiwyg_composer/components/_Editor.pcss
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ limitations under the License.
}

.mx_WysiwygComposer_Editor_content {
line-height: $font-22px;
artcodespace marked this conversation as resolved.
Show resolved Hide resolved
white-space: pre-wrap;
word-wrap: break-word;
outline: none;
Expand All @@ -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 {
Expand Down
3 changes: 3 additions & 0 deletions res/img/element-icons/room/composer/bulleted_list.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
3 changes: 3 additions & 0 deletions res/img/element-icons/room/composer/numbered_list.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change necessary due to the increase in line height introduced by this PR


interface EditorProps {
disabled: boolean;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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")}
Expand Down
2 changes: 2 additions & 0 deletions src/i18n/strings/en_EN.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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");
}
});
});