diff --git a/cypress/e2e/composer/composer.spec.ts b/cypress/e2e/composer/composer.spec.ts index 1013885b3d7..2b49b5e32e0 100644 --- a/cypress/e2e/composer/composer.spec.ts +++ b/cypress/e2e/composer/composer.spec.ts @@ -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 @@ -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"); }); }); }); diff --git a/src/Typeguards.ts b/src/Typeguards.ts index 78869097c60..397cbbd29b1 100644 --- a/src/Typeguards.ts +++ b/src/Typeguards.ts @@ -17,3 +17,7 @@ limitations under the License. export function isNotNull(arg: T): arg is Exclude { return arg !== null; } + +export function isNotUndefined(arg: T): arg is Exclude { + return arg !== undefined; +} diff --git a/src/components/views/rooms/wysiwyg_composer/hooks/usePlainTextListeners.ts b/src/components/views/rooms/wysiwyg_composer/hooks/usePlainTextListeners.ts index 8369d2684fb..2bccfc444ae 100644 --- a/src/components/views/rooms/wysiwyg_composer/hooks/usePlainTextListeners.ts +++ b/src/components/views/rooms/wysiwyg_composer/hooks/usePlainTextListeners.ts @@ -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; @@ -65,7 +66,7 @@ export function usePlainTextListeners( onInput(event: SyntheticEvent): void; onPaste(event: SyntheticEvent): void; onKeyDown(event: KeyboardEvent): void; - setContent(text: string): void; + setContent(text?: string): void; handleMention: (link: string, text: string, attributes: Attributes) => void; handleCommand: (text: string) => void; onSelect: (event: SyntheticEvent) => void; @@ -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 diff --git a/src/components/views/rooms/wysiwyg_composer/hooks/useSuggestion.ts b/src/components/views/rooms/wysiwyg_composer/hooks/useSuggestion.ts index e8d1833cc89..3e514f9e27d 100644 --- a/src/components/views/rooms/wysiwyg_composer/hooks/useSuggestion.ts +++ b/src/components/views/rooms/wysiwyg_composer/hooks/useSuggestion.ts @@ -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. */ @@ -49,7 +51,7 @@ type SuggestionState = Suggestion | null; */ export function useSuggestion( editorRef: React.RefObject, - setText: (text: string) => void, + setText: (text?: string) => void, ): { handleMention: (href: string, displayName: string, attributes: Attributes) => void; handleCommand: (text: string) => void; @@ -144,7 +146,7 @@ export function processMention( attributes: Attributes, // these will be used when formatting the link as a pill suggestionData: SuggestionState, setSuggestionData: React.Dispatch>, - setText: (text: string) => void, + setText: (text?: string) => void, ): void { // if we do not have a suggestion, return early if (suggestionData === null) { @@ -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 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) ?? ""}`); + + // 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); } @@ -181,7 +199,7 @@ export function processCommand( replacementText: string, suggestionData: SuggestionState, setSuggestionData: React.Dispatch>, - setText: (text: string) => void, + setText: (text?: string) => void, ): void { // if we do not have a suggestion, return early if (suggestionData === null) { diff --git a/test/components/views/rooms/wysiwyg_composer/hooks/usePlainTextListeners-test.tsx b/test/components/views/rooms/wysiwyg_composer/hooks/usePlainTextListeners-test.tsx new file mode 100644 index 00000000000..779cca73334 --- /dev/null +++ b/test/components/views/rooms/wysiwyg_composer/hooks/usePlainTextListeners-test.tsx @@ -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); + }); +}); diff --git a/test/components/views/rooms/wysiwyg_composer/hooks/useSuggestion-test.tsx b/test/components/views/rooms/wysiwyg_composer/hooks/useSuggestion-test.tsx index 568bc613466..11f545ffda4 100644 --- a/test/components/views/rooms/wysiwyg_composer/hooks/useSuggestion-test.tsx +++ b/test/components/views/rooms/wysiwyg_composer/hooks/useSuggestion-test.tsx @@ -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 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 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); }); });