Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RTE plain text mentions as pills #10852

Merged
merged 19 commits into from
May 16, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
14 changes: 8 additions & 6 deletions cypress/e2e/composer/composer.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -225,9 +225,10 @@ describe("Composer", () => {
});
// ...inserts the username into the composer
cy.findByRole("textbox").within(() => {
// TODO update this test when the mentions are inserted as pills, instead
// of as text
cy.findByText(otherUserName, { exact: false }).should("exist");
cy.findByText(otherUserName, { exact: false })
.should("exist")
.should("have.attr", "contenteditable", "false")
.should("have.attr", "data-mention-type", "user");
});

// Send the message to clear the composer
Expand All @@ -250,9 +251,10 @@ describe("Composer", () => {
// Selecting the autocomplete option using Enter inserts it into the composer
cy.findByRole("textbox").type(`{Enter}`);
cy.findByRole("textbox").within(() => {
// TODO update this test when the mentions are inserted as pills, instead
// of as text
cy.findByText(otherUserName, { exact: false }).should("exist");
cy.findByText(otherUserName, { exact: false })
.should("exist")
.should("have.attr", "contenteditable", "false")
.should("have.attr", "data-mention-type", "user");
});
});
});
Expand Down
4 changes: 4 additions & 0 deletions src/Typeguards.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,3 +17,7 @@ limitations under the License.
export function isNotNull<T>(arg: T): arg is Exclude<T, null> {
return arg !== null;
}

export function isNotUndefined<T>(arg: T): arg is Exclude<T, undefined> {
return arg !== undefined;
}
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import { IS_MAC, Key } from "../../../../../Keyboard";
import Autocomplete from "../../Autocomplete";
import { handleEventWithAutocomplete } from "./utils";
import { useSuggestion } from "./useSuggestion";
import { isNotNull, isNotUndefined } from "../../../../../Typeguards";

function isDivElement(target: EventTarget): target is HTMLDivElement {
return target instanceof HTMLDivElement;
Expand Down Expand Up @@ -65,7 +66,7 @@ export function usePlainTextListeners(
onInput(event: SyntheticEvent<HTMLDivElement, InputEvent | ClipboardEvent>): void;
onPaste(event: SyntheticEvent<HTMLDivElement, InputEvent | ClipboardEvent>): void;
onKeyDown(event: KeyboardEvent<HTMLDivElement>): void;
setContent(text: string): void;
setContent(text?: string): void;
handleMention: (link: string, text: string, attributes: Attributes) => void;
handleCommand: (text: string) => void;
onSelect: (event: SyntheticEvent<HTMLDivElement>) => void;
Expand All @@ -83,11 +84,18 @@ export function usePlainTextListeners(
}, [ref, onSend]);

const setText = useCallback(
(text: string) => {
setContent(text);
onChange?.(text);
(text?: string) => {
if (isNotUndefined(text)) {
setContent(text);
onChange?.(text);
} else if (isNotNull(ref) && isNotNull(ref.current)) {
// if called with no argument, read the current innerHTML from the ref
const currentRefContent = ref.current.innerHTML;
setContent(currentRefContent);
onChange?.(currentRefContent);
}
},
[onChange],
[onChange, ref],
);

// For separation of concerns, the suggestion handling is kept in a separate hook but is
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ limitations under the License.
import { Attributes, MappedSuggestion } from "@matrix-org/matrix-wysiwyg";
import { SyntheticEvent, useState } from "react";

import { isNotNull, isNotUndefined } from "../../../../../Typeguards";

/**
* Information about the current state of the `useSuggestion` hook.
*/
Expand Down Expand Up @@ -49,7 +51,7 @@ type SuggestionState = Suggestion | null;
*/
export function useSuggestion(
editorRef: React.RefObject<HTMLDivElement>,
setText: (text: string) => void,
setText: (text?: string) => void,
): {
handleMention: (href: string, displayName: string, attributes: Attributes) => void;
handleCommand: (text: string) => void;
Expand Down Expand Up @@ -144,7 +146,7 @@ export function processMention(
attributes: Attributes, // these will be used when formatting the link as a pill
andybalaam marked this conversation as resolved.
Show resolved Hide resolved
suggestionData: SuggestionState,
setSuggestionData: React.Dispatch<React.SetStateAction<SuggestionState>>,
setText: (text: string) => void,
setText: (text?: string) => void,
): void {
// if we do not have a suggestion, return early
if (suggestionData === null) {
Expand All @@ -153,18 +155,34 @@ export function processMention(

const { node } = suggestionData;

const textBeforeReplacement = node.textContent?.slice(0, suggestionData.startOffset) ?? "";
const textAfterReplacement = node.textContent?.slice(suggestionData.endOffset) ?? "";
// create an <a> element with the required attributes to allow us to interpret the mention as being a pill
const linkElement = document.createElement("a");
const linkTextNode = document.createTextNode(displayName);
linkElement.setAttribute("href", href);
linkElement.setAttribute("contenteditable", "false");
Object.entries(attributes).forEach(
([attr, value]) => isNotUndefined(value) && linkElement.setAttribute(attr, value),
);
linkElement.appendChild(linkTextNode);

// create text nodes to go before and after the link
const leadingTextNode = document.createTextNode(node.textContent?.slice(0, suggestionData.startOffset) || "\u200b");
const trailingTextNode = document.createTextNode(` ${node.textContent?.slice(suggestionData.endOffset) ?? ""}`);
andybalaam marked this conversation as resolved.
Show resolved Hide resolved

// now add the leading text node, link element and trailing text node before removing the node we are replacing
const parentNode = node.parentNode;
if (isNotNull(parentNode)) {
parentNode.insertBefore(leadingTextNode, node);
parentNode.insertBefore(linkElement, node);
parentNode.insertBefore(trailingTextNode, node);
parentNode.removeChild(node);
}

// TODO replace this markdown style text insertion with a pill representation
const newText = `[${displayName}](<${href}>) `;
const newCursorOffset = textBeforeReplacement.length + newText.length;
const newContent = textBeforeReplacement + newText + textAfterReplacement;
// move the selection to the trailing text node
document.getSelection()?.setBaseAndExtent(trailingTextNode, 1, trailingTextNode, 1);

// insert the new text, move the cursor, set the text state, clear the suggestion state
node.textContent = newContent;
document.getSelection()?.setBaseAndExtent(node, newCursorOffset, node, newCursorOffset);
setText(newContent);
// set the text content to be the innerHTML of the current editor ref and clear the suggestion state
setText();
setSuggestionData(null);
}

Expand All @@ -181,7 +199,7 @@ export function processCommand(
replacementText: string,
suggestionData: SuggestionState,
setSuggestionData: React.Dispatch<React.SetStateAction<SuggestionState>>,
setText: (text: string) => void,
setText: (text?: string) => void,
): void {
// if we do not have a suggestion, return early
if (suggestionData === null) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
/*
Copyright 2023 The Matrix.org Foundation C.I.C.

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

import { renderHook } from "@testing-library/react-hooks";
import { act } from "@testing-library/react";

import { usePlainTextListeners } from "../../../../../../src/components/views/rooms/wysiwyg_composer/hooks/usePlainTextListeners";

describe("setContent", () => {
it("calling with a string calls the onChange argument", () => {
const mockOnChange = jest.fn();
const { result } = renderHook(() => usePlainTextListeners("initialContent", mockOnChange));

const newContent = "new content";
act(() => {
result.current.setContent(newContent);
});

expect(mockOnChange).toHaveBeenCalledWith(newContent);
});

it("calling with no argument and no editor ref does not call onChange", () => {
const mockOnChange = jest.fn();
const { result } = renderHook(() => usePlainTextListeners("initialContent", mockOnChange));

act(() => {
result.current.setContent();
});

expect(mockOnChange).not.toHaveBeenCalled();
});

it("calling with no argument and a valid editor ref calls onChange with the editorRef innerHTML", () => {
const mockOnChange = jest.fn();

// create a div to represent the editor and append some content
const mockEditor = document.createElement("div");
const mockEditorText = "some text content";
const textNode = document.createTextNode(mockEditorText);
mockEditor.appendChild(textNode);

const { result } = renderHook(() => usePlainTextListeners("initialContent", mockOnChange));

// @ts-ignore in order to allow us to reassign the ref without complaint
result.current.ref.current = mockEditor;

act(() => {
result.current.setContent();
});

expect(mockOnChange).toHaveBeenCalledWith(mockEditor.innerHTML);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -78,34 +78,42 @@ describe("processMention", () => {
expect(mockSetText).not.toHaveBeenCalled();
});

it("can insert a mention into an empty text node", () => {
// make an empty text node, set the cursor inside it and then append to the document
const textNode = document.createTextNode("");
document.body.appendChild(textNode);
document.getSelection()?.setBaseAndExtent(textNode, 0, textNode, 0);
it("can insert a mention into a text node", () => {
// make a text node and an editor div, set the cursor inside the text node and then
// append node to editor, then editor to document
const textNode = document.createTextNode("@a");
const mockEditor = document.createElement("div");
mockEditor.appendChild(textNode);
document.body.appendChild(mockEditor);
document.getSelection()?.setBaseAndExtent(textNode, 1, textNode, 1);

// call the util function
const href = "href";
const displayName = "displayName";
const mockSetSuggestion = jest.fn();
const mockSetSuggestionData = jest.fn();
const mockSetText = jest.fn();
processMention(
href,
displayName,
{},
{ node: textNode, startOffset: 0, endOffset: 0 } as unknown as Suggestion,
mockSetSuggestion,
{ "data-test-attribute": "test" },
{ node: textNode, startOffset: 0, endOffset: 2 } as unknown as Suggestion,
mockSetSuggestionData,
mockSetText,
);

// placeholder testing for the changed content - these tests will all be changed
// when the mention is inserted as an <a> tagfs
const { textContent } = textNode;
expect(textContent!.includes(href)).toBe(true);
expect(textContent!.includes(displayName)).toBe(true);
// check that the editor has a single child
expect(mockEditor.children).toHaveLength(1);
const linkElement = mockEditor.firstElementChild as HTMLElement;

expect(mockSetText).toHaveBeenCalledWith(expect.stringContaining(displayName));
expect(mockSetSuggestion).toHaveBeenCalledWith(null);
// and that the child is an <a> tag with the expected attributes and content
expect(linkElement).toBeInstanceOf(HTMLAnchorElement);
expect(linkElement).toHaveAttribute(href, href);
expect(linkElement).toHaveAttribute("contenteditable", "false");
expect(linkElement).toHaveAttribute("data-test-attribute", "test");
expect(linkElement.textContent).toBe(displayName);

expect(mockSetText).toHaveBeenCalledWith();
expect(mockSetSuggestionData).toHaveBeenCalledWith(null);
});
});

Expand Down
Loading