From b305cc934bff4c62f5813be19a79ba649cf9ea97 Mon Sep 17 00:00:00 2001 From: Alun Turner Date: Tue, 11 Apr 2023 12:39:41 +0100 Subject: [PATCH 01/61] add the handlers for when autocomplete is open plus rough / handling --- .../hooks/usePlainTextListeners.ts | 78 ++++++++++++++++++- 1 file changed, 75 insertions(+), 3 deletions(-) diff --git a/src/components/views/rooms/wysiwyg_composer/hooks/usePlainTextListeners.ts b/src/components/views/rooms/wysiwyg_composer/hooks/usePlainTextListeners.ts index f8b045ad657..cc8746ddf23 100644 --- a/src/components/views/rooms/wysiwyg_composer/hooks/usePlainTextListeners.ts +++ b/src/components/views/rooms/wysiwyg_composer/hooks/usePlainTextListeners.ts @@ -14,10 +14,14 @@ See the License for the specific language governing permissions and limitations under the License. */ -import { KeyboardEvent, RefObject, SyntheticEvent, useCallback, useRef, useState } from "react"; +import { MappedSuggestion } from "@matrix-org/matrix-wysiwyg"; +import { KeyboardEvent, RefObject, SyntheticEvent, useCallback, useRef, useEffect, useState } from "react"; import { useSettingValue } from "../../../../../hooks/useSettings"; import { IS_MAC, Key } from "../../../../../Keyboard"; +import Autocomplete from "../../Autocomplete"; +import { getKeyBindingsManager } from "../../../../../KeyBindingsManager"; +import { KeyBindingAction } from "../../../../../accessibility/KeyboardShortcuts"; function isDivElement(target: EventTarget): target is HTMLDivElement { return target instanceof HTMLDivElement; @@ -33,7 +37,10 @@ function amendInnerHtml(text: string): string { .replace(/<\/div>/g, ""); } +const emptySuggestion = { keyChar: "", text: "", type: "unknown" } as const; + export function usePlainTextListeners( + autocompleteRef: React.RefObject, initialContent?: string, onChange?: (content: string) => void, onSend?: () => void, @@ -44,9 +51,12 @@ export function usePlainTextListeners( onPaste(event: SyntheticEvent): void; onKeyDown(event: KeyboardEvent): void; setContent(text: string): void; + suggestion: MappedSuggestion; } { const ref = useRef(null); const [content, setContent] = useState(initialContent); + const [suggestion, setSuggestion] = useState(emptySuggestion); + const send = useCallback(() => { if (ref.current) { ref.current.innerHTML = ""; @@ -76,6 +86,46 @@ export function usePlainTextListeners( const onKeyDown = useCallback( (event: KeyboardEvent) => { + // WIP drop in the autocomplete handling + const autocompleteIsOpen = autocompleteRef?.current && !autocompleteRef.current.state.hide; + + // we need autocomplete to take priority when it is open for using enter to select + if (autocompleteIsOpen) { + let handled = false; + const autocompleteAction = getKeyBindingsManager().getAutocompleteAction(event); + const component = autocompleteRef.current; + if (component && component.countCompletions() > 0) { + switch (autocompleteAction) { + case KeyBindingAction.ForceCompleteAutocomplete: + case KeyBindingAction.CompleteAutocomplete: + autocompleteRef.current.onConfirmCompletion(); + handled = true; + break; + case KeyBindingAction.PrevSelectionInAutocomplete: + autocompleteRef.current.moveSelection(-1); + handled = true; + break; + case KeyBindingAction.NextSelectionInAutocomplete: + autocompleteRef.current.moveSelection(1); + handled = true; + break; + case KeyBindingAction.CancelAutocomplete: + autocompleteRef.current.onEscape(event as {} as React.KeyboardEvent); + handled = true; + break; + default: + break; // don't return anything, allow event to pass through + } + } + + if (handled) { + event.preventDefault(); + event.stopPropagation(); + return; + } + } + + // resume regular flow if (event.key === Key.ENTER) { // TODO use getKeyBindingsManager().getMessageComposerAction(event) like in useInputEventProcessor const sendModifierIsPressed = IS_MAC ? event.metaKey : event.ctrlKey; @@ -95,8 +145,30 @@ export function usePlainTextListeners( } } }, - [enterShouldSend, send], + [autocompleteRef, enterShouldSend, send], ); - return { ref, onInput, onPaste: onInput, onKeyDown, content, setContent: setText }; + useEffect(() => { + // do we need to do this onSelect? Initial thought was yes, but perhaps we can actually just + // do checks on input? onInput would probably be easier to be honest, but then with the initial + // implementation this wouldn't allow us to + // when there's a selection change, we need to figure out if we are doing some + // sort of suggestion + + // lets do slash first, probably more straightforward + if (content && content.startsWith("/") && !content.startsWith("//")) { + // then we have a command + setSuggestion({ keyChar: "/", text: content.slice(1), type: "command" }); + } + }, [content]); + + return { + ref, + onInput, + onPaste: onInput, + onKeyDown, + content, + setContent: setText, + suggestion, + }; } From f092fb1c402e5dd5f0d76acc192404b2d827a597 Mon Sep 17 00:00:00 2001 From: Alun Turner Date: Tue, 11 Apr 2023 12:39:52 +0100 Subject: [PATCH 02/61] hack in using the wysiwyg autocomplete --- .../components/PlainTextComposer.tsx | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/src/components/views/rooms/wysiwyg_composer/components/PlainTextComposer.tsx b/src/components/views/rooms/wysiwyg_composer/components/PlainTextComposer.tsx index d68c5260641..3c2a37d8696 100644 --- a/src/components/views/rooms/wysiwyg_composer/components/PlainTextComposer.tsx +++ b/src/components/views/rooms/wysiwyg_composer/components/PlainTextComposer.tsx @@ -15,7 +15,7 @@ limitations under the License. */ import classNames from "classnames"; -import React, { MutableRefObject, ReactNode } from "react"; +import React, { MutableRefObject, ReactNode, useRef } from "react"; import { useComposerFunctions } from "../hooks/useComposerFunctions"; import { useIsFocused } from "../hooks/useIsFocused"; @@ -24,6 +24,8 @@ import { usePlainTextListeners } from "../hooks/usePlainTextListeners"; import { useSetCursorPosition } from "../hooks/useSetCursorPosition"; import { ComposerFunctions } from "../types"; import { Editor } from "./Editor"; +import Autocomplete from "../../Autocomplete"; +import { WysiwygAutocomplete } from "./WysiwygAutocomplete"; interface PlainTextComposerProps { disabled?: boolean; @@ -48,7 +50,13 @@ export function PlainTextComposer({ leftComponent, rightComponent, }: PlainTextComposerProps): JSX.Element { - const { ref, onInput, onPaste, onKeyDown, content, setContent } = usePlainTextListeners( + // WIP - hack in an autocomplete implementation + const autocompleteRef = useRef(null); + const handleMention = (): void => {}; + const handleCommand = (): void => {}; + + const { ref, onInput, onPaste, onKeyDown, content, setContent, suggestion } = usePlainTextListeners( + autocompleteRef, initialContent, onChange, onSend, @@ -69,6 +77,12 @@ export function PlainTextComposer({ onPaste={onPaste} onKeyDown={onKeyDown} > + Date: Tue, 11 Apr 2023 14:04:09 +0100 Subject: [PATCH 03/61] switch to using onSelect for the behaviour --- .../components/PlainTextComposer.tsx | 3 +- .../hooks/usePlainTextListeners.ts | 69 +++++++++++++++---- 2 files changed, 58 insertions(+), 14 deletions(-) diff --git a/src/components/views/rooms/wysiwyg_composer/components/PlainTextComposer.tsx b/src/components/views/rooms/wysiwyg_composer/components/PlainTextComposer.tsx index 3c2a37d8696..cd43304ebab 100644 --- a/src/components/views/rooms/wysiwyg_composer/components/PlainTextComposer.tsx +++ b/src/components/views/rooms/wysiwyg_composer/components/PlainTextComposer.tsx @@ -55,7 +55,7 @@ export function PlainTextComposer({ const handleMention = (): void => {}; const handleCommand = (): void => {}; - const { ref, onInput, onPaste, onKeyDown, content, setContent, suggestion } = usePlainTextListeners( + const { ref, onInput, onPaste, onKeyDown, content, setContent, suggestion, onSelect } = usePlainTextListeners( autocompleteRef, initialContent, onChange, @@ -76,6 +76,7 @@ export function PlainTextComposer({ onInput={onInput} onPaste={onPaste} onKeyDown={onKeyDown} + onSelect={onSelect} > , @@ -52,10 +55,12 @@ export function usePlainTextListeners( onKeyDown(event: KeyboardEvent): void; setContent(text: string): void; suggestion: MappedSuggestion; + onSelect(): void; } { const ref = useRef(null); const [content, setContent] = useState(initialContent); const [suggestion, setSuggestion] = useState(emptySuggestion); + const [suggestionNodeInfo, setSuggestionNodeInfo] = useState(emptySuggestionCandidate); const send = useCallback(() => { if (ref.current) { @@ -148,19 +153,56 @@ export function usePlainTextListeners( [autocompleteRef, enterShouldSend, send], ); - useEffect(() => { - // do we need to do this onSelect? Initial thought was yes, but perhaps we can actually just - // do checks on input? onInput would probably be easier to be honest, but then with the initial - // implementation this wouldn't allow us to - // when there's a selection change, we need to figure out if we are doing some - // sort of suggestion - - // lets do slash first, probably more straightforward - if (content && content.startsWith("/") && !content.startsWith("//")) { - // then we have a command - setSuggestion({ keyChar: "/", text: content.slice(1), type: "command" }); + // do for slash commands first + const onSelect = (): void => { + // whenever there's a change in selection, we're going to have to do some magic + const s = document.getSelection(); + + // if we have a cursor inside a text node, then we're potentially interested in the text content + if (s && s.isCollapsed && s.anchorNode?.nodeName === "#text" && ref.current) { + // first check is that the text node is the first text node of the editor as we can also have + //

tags in the markup + const firstTextNode = document.createNodeIterator(ref.current, NodeFilter.SHOW_TEXT).nextNode(); + const isFirstTextNode = s.anchorNode === firstTextNode; + const textContent = s.anchorNode.textContent; + + // if we're not in the first text node or we have no text content, return + if (!isFirstTextNode || textContent === null) { + return; + } + + // it's a command if: it starts with /, not //, then has letters all the way up to the end of the textContent + const commandRegex = /^\/{1}(\w*)$/; + const commandMatches = textContent.match(commandRegex); + + // if we don't have a command, clear the suggestion state and return + if (commandMatches === null) { + if (suggestionNodeInfo.node !== null) { + setSuggestionNodeInfo(emptySuggestionCandidate); + setSuggestion(emptySuggestion); + } + return; + } + + // but if we do have some matches, use that to populate the suggestion state + setSuggestionNodeInfo({ node: s.anchorNode, startOffset: 0, endOffset: textContent.length }); + setSuggestion({ keyChar: "/", type: "command", text: commandMatches[1] }); } - }, [content]); + }; + + // useEffect(() => { + // // do we need to do this onSelect? Initial thought was yes, but perhaps we can actually just + // // do checks on input? onInput would probably be easier to be honest, but then with the initial + // // implementation this wouldn't allow us to + // // when there's a selection change, we need to figure out if we are doing some + // // sort of suggestion + + // // lets do slash first, probably more straightforward + // if (content && content.startsWith("/") && !content.startsWith("//")) { + // // then we have a command + // setSuggestion({ keyChar: "/", text: content.slice(1), type: "command" }); + // } + // }, [content]); return { ref, @@ -170,5 +212,6 @@ export function usePlainTextListeners( content, setContent: setText, suggestion, + onSelect, }; } From d3000501a10830d8dadf9846eb66dc9044194863 Mon Sep 17 00:00:00 2001 From: Alun Turner Date: Tue, 11 Apr 2023 14:21:41 +0100 Subject: [PATCH 04/61] expand comment --- .../rooms/wysiwyg_composer/hooks/usePlainTextListeners.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/components/views/rooms/wysiwyg_composer/hooks/usePlainTextListeners.ts b/src/components/views/rooms/wysiwyg_composer/hooks/usePlainTextListeners.ts index 78097ca594a..9281dae4188 100644 --- a/src/components/views/rooms/wysiwyg_composer/hooks/usePlainTextListeners.ts +++ b/src/components/views/rooms/wysiwyg_composer/hooks/usePlainTextListeners.ts @@ -171,7 +171,9 @@ export function usePlainTextListeners( return; } - // it's a command if: it starts with /, not //, then has letters all the way up to the end of the textContent + // it's a command if: it is the first textnode, it starts with /, not //, then has letters all the way up to + // the end of the textcontent - nb think this last assumption may give us some behavioural inconsistency + // between the rust model and this, but it's a decent starting point const commandRegex = /^\/{1}(\w*)$/; const commandMatches = textContent.match(commandRegex); From 092fd71a138529e493b487094f015a62a106d7e4 Mon Sep 17 00:00:00 2001 From: Alun Turner Date: Tue, 11 Apr 2023 14:44:02 +0100 Subject: [PATCH 05/61] add a handle command function to replace text --- .../components/PlainTextComposer.tsx | 34 +++++++++++++++---- .../hooks/usePlainTextListeners.ts | 25 +++++--------- 2 files changed, 36 insertions(+), 23 deletions(-) diff --git a/src/components/views/rooms/wysiwyg_composer/components/PlainTextComposer.tsx b/src/components/views/rooms/wysiwyg_composer/components/PlainTextComposer.tsx index cd43304ebab..7a96baebcd1 100644 --- a/src/components/views/rooms/wysiwyg_composer/components/PlainTextComposer.tsx +++ b/src/components/views/rooms/wysiwyg_composer/components/PlainTextComposer.tsx @@ -52,15 +52,35 @@ export function PlainTextComposer({ }: PlainTextComposerProps): JSX.Element { // WIP - hack in an autocomplete implementation const autocompleteRef = useRef(null); + + const { + ref, + onInput, + onPaste, + onKeyDown, + content, + setContent, + suggestion, + onSelect, + clearSuggestions, + suggestionNodeInfo, + } = usePlainTextListeners(autocompleteRef, initialContent, onChange, onSend); + const handleMention = (): void => {}; - const handleCommand = (): void => {}; + const handleCommand = (replacementText: string): void => { + // if this gets triggered, then we have a command on the page, so what we want to do is + // manually amend the html text content with the stored state + const { node, startOffset, endOffset } = suggestionNodeInfo; + if (node === null || node.textContent === null) return; + + // for a command we know we're starting at the beginning, so it's a bit easier + const newContent = `${replacementText} `; + node.textContent = newContent; // note the trailing space + // then set the cursor to the end of the node before clearing the suggestion + document.getSelection()?.setBaseAndExtent(node, newContent.length, node, newContent.length); + clearSuggestions(); + }; - const { ref, onInput, onPaste, onKeyDown, content, setContent, suggestion, onSelect } = usePlainTextListeners( - autocompleteRef, - initialContent, - onChange, - onSend, - ); const composerFunctions = useComposerFunctions(ref, setContent); usePlainTextInitialization(initialContent, ref); useSetCursorPosition(disabled, ref); diff --git a/src/components/views/rooms/wysiwyg_composer/hooks/usePlainTextListeners.ts b/src/components/views/rooms/wysiwyg_composer/hooks/usePlainTextListeners.ts index 9281dae4188..816b97e6f72 100644 --- a/src/components/views/rooms/wysiwyg_composer/hooks/usePlainTextListeners.ts +++ b/src/components/views/rooms/wysiwyg_composer/hooks/usePlainTextListeners.ts @@ -22,7 +22,6 @@ import { IS_MAC, Key } from "../../../../../Keyboard"; import Autocomplete from "../../Autocomplete"; import { getKeyBindingsManager } from "../../../../../KeyBindingsManager"; import { KeyBindingAction } from "../../../../../accessibility/KeyboardShortcuts"; -import { first } from "cheerio/lib/api/traversing"; function isDivElement(target: EventTarget): target is HTMLDivElement { return target instanceof HTMLDivElement; @@ -56,6 +55,8 @@ export function usePlainTextListeners( setContent(text: string): void; suggestion: MappedSuggestion; onSelect(): void; + clearSuggestions(): void; + suggestionNodeInfo: SuggestionCandidate; } { const ref = useRef(null); const [content, setContent] = useState(initialContent); @@ -180,8 +181,7 @@ export function usePlainTextListeners( // if we don't have a command, clear the suggestion state and return if (commandMatches === null) { if (suggestionNodeInfo.node !== null) { - setSuggestionNodeInfo(emptySuggestionCandidate); - setSuggestion(emptySuggestion); + clearSuggestions(); } return; } @@ -192,19 +192,10 @@ export function usePlainTextListeners( } }; - // useEffect(() => { - // // do we need to do this onSelect? Initial thought was yes, but perhaps we can actually just - // // do checks on input? onInput would probably be easier to be honest, but then with the initial - // // implementation this wouldn't allow us to - // // when there's a selection change, we need to figure out if we are doing some - // // sort of suggestion - - // // lets do slash first, probably more straightforward - // if (content && content.startsWith("/") && !content.startsWith("//")) { - // // then we have a command - // setSuggestion({ keyChar: "/", text: content.slice(1), type: "command" }); - // } - // }, [content]); + const clearSuggestions = (): void => { + setSuggestion(emptySuggestion); + setSuggestionNodeInfo(emptySuggestionCandidate); + }; return { ref, @@ -215,5 +206,7 @@ export function usePlainTextListeners( setContent: setText, suggestion, onSelect, + clearSuggestions, + suggestionNodeInfo, }; } From 9f2e7f12f5210566933564a73f6b7034b9787f45 Mon Sep 17 00:00:00 2001 From: Alun Turner Date: Tue, 11 Apr 2023 15:10:19 +0100 Subject: [PATCH 06/61] add event firing step --- .../components/PlainTextComposer.tsx | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/components/views/rooms/wysiwyg_composer/components/PlainTextComposer.tsx b/src/components/views/rooms/wysiwyg_composer/components/PlainTextComposer.tsx index 7a96baebcd1..6fcc387c1fc 100644 --- a/src/components/views/rooms/wysiwyg_composer/components/PlainTextComposer.tsx +++ b/src/components/views/rooms/wysiwyg_composer/components/PlainTextComposer.tsx @@ -71,13 +71,17 @@ export function PlainTextComposer({ // if this gets triggered, then we have a command on the page, so what we want to do is // manually amend the html text content with the stored state const { node, startOffset, endOffset } = suggestionNodeInfo; - if (node === null || node.textContent === null) return; + if (ref.current === null || node === null || node.textContent === null) return; // for a command we know we're starting at the beginning, so it's a bit easier - const newContent = `${replacementText} `; - node.textContent = newContent; // note the trailing space - // then set the cursor to the end of the node before clearing the suggestion + const newContent = `${replacementText} `; // note the trailing space + node.textContent = newContent; + + // then set the cursor to the end of the node, fire an inputEvent (updates + // the hook state), clear the suggestions from state document.getSelection()?.setBaseAndExtent(node, newContent.length, node, newContent.length); + const event = new Event("inputEvent"); + ref.current.dispatchEvent(event); clearSuggestions(); }; From 47d0d531efe624e5ed54af24b67e4abbdffec8ec Mon Sep 17 00:00:00 2001 From: Alun Turner Date: Thu, 13 Apr 2023 10:15:49 +0100 Subject: [PATCH 07/61] fix TS errors for RefObject --- .../rooms/wysiwyg_composer/components/PlainTextComposer.tsx | 2 +- .../views/rooms/wysiwyg_composer/hooks/usePlainTextListeners.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/components/views/rooms/wysiwyg_composer/components/PlainTextComposer.tsx b/src/components/views/rooms/wysiwyg_composer/components/PlainTextComposer.tsx index 6fcc387c1fc..f2c12bd987d 100644 --- a/src/components/views/rooms/wysiwyg_composer/components/PlainTextComposer.tsx +++ b/src/components/views/rooms/wysiwyg_composer/components/PlainTextComposer.tsx @@ -70,7 +70,7 @@ export function PlainTextComposer({ const handleCommand = (replacementText: string): void => { // if this gets triggered, then we have a command on the page, so what we want to do is // manually amend the html text content with the stored state - const { node, startOffset, endOffset } = suggestionNodeInfo; + const { node /* startOffset, endOffset */ } = suggestionNodeInfo; if (ref.current === null || node === null || node.textContent === null) return; // for a command we know we're starting at the beginning, so it's a bit easier diff --git a/src/components/views/rooms/wysiwyg_composer/hooks/usePlainTextListeners.ts b/src/components/views/rooms/wysiwyg_composer/hooks/usePlainTextListeners.ts index 816b97e6f72..8f22e7bb195 100644 --- a/src/components/views/rooms/wysiwyg_composer/hooks/usePlainTextListeners.ts +++ b/src/components/views/rooms/wysiwyg_composer/hooks/usePlainTextListeners.ts @@ -47,7 +47,7 @@ export function usePlainTextListeners( onChange?: (content: string) => void, onSend?: () => void, ): { - ref: RefObject; + ref: RefObject; content?: string; onInput(event: SyntheticEvent): void; onPaste(event: SyntheticEvent): void; From ac776b43324180c01f3fc53c0a3db412779e5810 Mon Sep 17 00:00:00 2001 From: Alun Turner Date: Thu, 13 Apr 2023 10:29:20 +0100 Subject: [PATCH 08/61] extract common functionality to new util --- .../rooms/wysiwyg_composer/hooks/utils.ts | 56 ++++++++++++++++++- 1 file changed, 55 insertions(+), 1 deletion(-) diff --git a/src/components/views/rooms/wysiwyg_composer/hooks/utils.ts b/src/components/views/rooms/wysiwyg_composer/hooks/utils.ts index 83a18fb55b8..ea13c4dc8c4 100644 --- a/src/components/views/rooms/wysiwyg_composer/hooks/utils.ts +++ b/src/components/views/rooms/wysiwyg_composer/hooks/utils.ts @@ -14,10 +14,13 @@ See the License for the specific language governing permissions and limitations under the License. */ -import { MutableRefObject } from "react"; +import { MutableRefObject, RefObject, KeyboardEvent } from "react"; import { TimelineRenderingType } from "../../../../../contexts/RoomContext"; import { IRoomState } from "../../../../structures/RoomView"; +import Autocomplete from "../../Autocomplete"; +import { getKeyBindingsManager } from "../../../../../KeyBindingsManager"; +import { KeyBindingAction } from "../../../../../accessibility/KeyboardShortcuts"; export function focusComposer( composerElement: MutableRefObject, @@ -51,3 +54,54 @@ export function setCursorPositionAtTheEnd(element: HTMLElement): void { element.focus(); } + +/** + * When the autocomplete modal is open we need to be able to properly + * handle events that are dispatched. This allows the user to move the selection + * in the autocomplete and select using enter. + * + * @param autocompleteRef - a ref to the autocomplete of interest + * @param event - the keyboard event that has been dispatched + * @returns boolean - whether or not the autocomplete has handled the event + */ +export function handleEventWithAutocomplete( + autocompleteRef: RefObject, + event: KeyboardEvent, +): boolean { + const autocompleteIsOpen = autocompleteRef?.current && !autocompleteRef.current.state.hide; + let handled = false; + + if (autocompleteIsOpen) { + const autocompleteAction = getKeyBindingsManager().getAutocompleteAction(event); + const component = autocompleteRef.current; + if (component && component.countCompletions() > 0) { + switch (autocompleteAction) { + case KeyBindingAction.ForceCompleteAutocomplete: + case KeyBindingAction.CompleteAutocomplete: + autocompleteRef.current.onConfirmCompletion(); + handled = true; + break; + case KeyBindingAction.PrevSelectionInAutocomplete: + autocompleteRef.current.moveSelection(-1); + handled = true; + break; + case KeyBindingAction.NextSelectionInAutocomplete: + autocompleteRef.current.moveSelection(1); + handled = true; + break; + case KeyBindingAction.CancelAutocomplete: + autocompleteRef.current.onEscape(event as {} as React.KeyboardEvent); + handled = true; + break; + default: + break; // don't return anything, allow event to pass through + } + } + + if (handled) { + event.preventDefault(); + event.stopPropagation(); + } + } + return handled; +} From 7de5f45b230c50180c994c69bc7f4fbd9fce4ee3 Mon Sep 17 00:00:00 2001 From: Alun Turner Date: Thu, 13 Apr 2023 10:32:18 +0100 Subject: [PATCH 09/61] use util for plain text mode --- .../hooks/usePlainTextListeners.ts | 41 ++----------------- 1 file changed, 4 insertions(+), 37 deletions(-) diff --git a/src/components/views/rooms/wysiwyg_composer/hooks/usePlainTextListeners.ts b/src/components/views/rooms/wysiwyg_composer/hooks/usePlainTextListeners.ts index 8f22e7bb195..24f5231f95c 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 { getKeyBindingsManager } from "../../../../../KeyBindingsManager"; import { KeyBindingAction } from "../../../../../accessibility/KeyboardShortcuts"; +import { handleEventWithAutocomplete } from "./utils"; function isDivElement(target: EventTarget): target is HTMLDivElement { return target instanceof HTMLDivElement; @@ -92,43 +93,9 @@ export function usePlainTextListeners( const onKeyDown = useCallback( (event: KeyboardEvent) => { - // WIP drop in the autocomplete handling - const autocompleteIsOpen = autocompleteRef?.current && !autocompleteRef.current.state.hide; - - // we need autocomplete to take priority when it is open for using enter to select - if (autocompleteIsOpen) { - let handled = false; - const autocompleteAction = getKeyBindingsManager().getAutocompleteAction(event); - const component = autocompleteRef.current; - if (component && component.countCompletions() > 0) { - switch (autocompleteAction) { - case KeyBindingAction.ForceCompleteAutocomplete: - case KeyBindingAction.CompleteAutocomplete: - autocompleteRef.current.onConfirmCompletion(); - handled = true; - break; - case KeyBindingAction.PrevSelectionInAutocomplete: - autocompleteRef.current.moveSelection(-1); - handled = true; - break; - case KeyBindingAction.NextSelectionInAutocomplete: - autocompleteRef.current.moveSelection(1); - handled = true; - break; - case KeyBindingAction.CancelAutocomplete: - autocompleteRef.current.onEscape(event as {} as React.KeyboardEvent); - handled = true; - break; - default: - break; // don't return anything, allow event to pass through - } - } - - if (handled) { - event.preventDefault(); - event.stopPropagation(); - return; - } + const isHandledByAutocomplete = handleEventWithAutocomplete(autocompleteRef, event); + if (isHandledByAutocomplete) { + return; } // resume regular flow From 5f89098f0ae2fa912e8ae006e305a50133ece1be Mon Sep 17 00:00:00 2001 From: Alun Turner Date: Thu, 13 Apr 2023 10:32:28 +0100 Subject: [PATCH 10/61] use util for rich text mode --- .../hooks/useInputEventProcessor.ts | 39 ++----------------- 1 file changed, 4 insertions(+), 35 deletions(-) diff --git a/src/components/views/rooms/wysiwyg_composer/hooks/useInputEventProcessor.ts b/src/components/views/rooms/wysiwyg_composer/hooks/useInputEventProcessor.ts index cf3e7d6be15..a16d090bf27 100644 --- a/src/components/views/rooms/wysiwyg_composer/hooks/useInputEventProcessor.ts +++ b/src/components/views/rooms/wysiwyg_composer/hooks/useInputEventProcessor.ts @@ -33,6 +33,7 @@ import { isCaretAtEnd, isCaretAtStart } from "../utils/selection"; import { getEventsFromEditorStateTransfer, getEventsFromRoom } from "../utils/event"; import { endEditing } from "../utils/editing"; import Autocomplete from "../../Autocomplete"; +import { handleEventWithAutocomplete } from "./utils"; export function useInputEventProcessor( onSend: () => void, @@ -99,42 +100,10 @@ function handleKeyboardEvent( const isEditorModified = isEditing ? initialContent !== composer.content() : composer.content().length !== 0; const action = getKeyBindingsManager().getMessageComposerAction(event); - const autocompleteIsOpen = autocompleteRef?.current && !autocompleteRef.current.state.hide; - // we need autocomplete to take priority when it is open for using enter to select - if (autocompleteIsOpen) { - let handled = false; - const autocompleteAction = getKeyBindingsManager().getAutocompleteAction(event); - const component = autocompleteRef.current; - if (component && component.countCompletions() > 0) { - switch (autocompleteAction) { - case KeyBindingAction.ForceCompleteAutocomplete: - case KeyBindingAction.CompleteAutocomplete: - autocompleteRef.current.onConfirmCompletion(); - handled = true; - break; - case KeyBindingAction.PrevSelectionInAutocomplete: - autocompleteRef.current.moveSelection(-1); - handled = true; - break; - case KeyBindingAction.NextSelectionInAutocomplete: - autocompleteRef.current.moveSelection(1); - handled = true; - break; - case KeyBindingAction.CancelAutocomplete: - autocompleteRef.current.onEscape(event as {} as React.KeyboardEvent); - handled = true; - break; - default: - break; // don't return anything, allow event to pass through - } - } - - if (handled) { - event.preventDefault(); - event.stopPropagation(); - return event; - } + const isHandledByAutocomplete = handleEventWithAutocomplete(autocompleteRef, event); + if (isHandledByAutocomplete) { + return event; } switch (action) { From ba8999cdb9a851136d5d115527fd9450a735799c Mon Sep 17 00:00:00 2001 From: Alun Turner Date: Thu, 13 Apr 2023 10:49:58 +0100 Subject: [PATCH 11/61] remove unused imports --- .../views/rooms/wysiwyg_composer/hooks/usePlainTextListeners.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/components/views/rooms/wysiwyg_composer/hooks/usePlainTextListeners.ts b/src/components/views/rooms/wysiwyg_composer/hooks/usePlainTextListeners.ts index 24f5231f95c..c0c1ac17f37 100644 --- a/src/components/views/rooms/wysiwyg_composer/hooks/usePlainTextListeners.ts +++ b/src/components/views/rooms/wysiwyg_composer/hooks/usePlainTextListeners.ts @@ -20,8 +20,6 @@ import { KeyboardEvent, RefObject, SyntheticEvent, useCallback, useRef, useState import { useSettingValue } from "../../../../../hooks/useSettings"; import { IS_MAC, Key } from "../../../../../Keyboard"; import Autocomplete from "../../Autocomplete"; -import { getKeyBindingsManager } from "../../../../../KeyBindingsManager"; -import { KeyBindingAction } from "../../../../../accessibility/KeyboardShortcuts"; import { handleEventWithAutocomplete } from "./utils"; function isDivElement(target: EventTarget): target is HTMLDivElement { From 46f53098111bc1498fb89591f0e5e4c129bc0e1d Mon Sep 17 00:00:00 2001 From: Alun Turner Date: Thu, 13 Apr 2023 10:50:17 +0100 Subject: [PATCH 12/61] make util able to handle either type of keyboard event --- src/components/views/rooms/wysiwyg_composer/hooks/utils.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/components/views/rooms/wysiwyg_composer/hooks/utils.ts b/src/components/views/rooms/wysiwyg_composer/hooks/utils.ts index ea13c4dc8c4..35febe6b778 100644 --- a/src/components/views/rooms/wysiwyg_composer/hooks/utils.ts +++ b/src/components/views/rooms/wysiwyg_composer/hooks/utils.ts @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -import { MutableRefObject, RefObject, KeyboardEvent } from "react"; +import { MutableRefObject, RefObject } from "react"; import { TimelineRenderingType } from "../../../../../contexts/RoomContext"; import { IRoomState } from "../../../../structures/RoomView"; @@ -66,7 +66,8 @@ export function setCursorPositionAtTheEnd(element: HTMLElement): void { */ export function handleEventWithAutocomplete( autocompleteRef: RefObject, - event: KeyboardEvent, + // we get a React Keyboard event from plain text, a Keyboard Event from the rich text + event: KeyboardEvent | React.KeyboardEvent, ): boolean { const autocompleteIsOpen = autocompleteRef?.current && !autocompleteRef.current.state.hide; let handled = false; From f857c14f63aac573d20cee44b1b0fde3c244f96b Mon Sep 17 00:00:00 2001 From: Alun Turner Date: Thu, 13 Apr 2023 10:54:08 +0100 Subject: [PATCH 13/61] fix TS error for mxClient --- .../rooms/wysiwyg_composer/hooks/useInputEventProcessor.ts | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/components/views/rooms/wysiwyg_composer/hooks/useInputEventProcessor.ts b/src/components/views/rooms/wysiwyg_composer/hooks/useInputEventProcessor.ts index a16d090bf27..9fcad115f2c 100644 --- a/src/components/views/rooms/wysiwyg_composer/hooks/useInputEventProcessor.ts +++ b/src/components/views/rooms/wysiwyg_composer/hooks/useInputEventProcessor.ts @@ -92,7 +92,7 @@ function handleKeyboardEvent( editor: HTMLElement, roomContext: IRoomState, composerContext: ComposerContextState, - mxClient: MatrixClient, + mxClient: MatrixClient | undefined, autocompleteRef: React.RefObject, ): KeyboardEvent | null { const { editorStateTransfer } = composerContext; @@ -106,6 +106,11 @@ function handleKeyboardEvent( return event; } + // taking the client from context gives us an client | undefined type, narrow it down + if (mxClient === undefined) { + return null; + } + switch (action) { case KeyBindingAction.SendMessage: send(); From 26baf9714a28dd132b2e5ba4cc3d471087dbd23b Mon Sep 17 00:00:00 2001 From: Alun Turner Date: Thu, 13 Apr 2023 11:25:01 +0100 Subject: [PATCH 14/61] lift all new code into main component prior to extracting to custom hook --- .../components/PlainTextComposer.tsx | 73 +++++++++++++++---- .../hooks/usePlainTextListeners.ts | 58 --------------- 2 files changed, 60 insertions(+), 71 deletions(-) diff --git a/src/components/views/rooms/wysiwyg_composer/components/PlainTextComposer.tsx b/src/components/views/rooms/wysiwyg_composer/components/PlainTextComposer.tsx index f2c12bd987d..c7dffcf7c15 100644 --- a/src/components/views/rooms/wysiwyg_composer/components/PlainTextComposer.tsx +++ b/src/components/views/rooms/wysiwyg_composer/components/PlainTextComposer.tsx @@ -15,7 +15,8 @@ limitations under the License. */ import classNames from "classnames"; -import React, { MutableRefObject, ReactNode, useRef } from "react"; +import { MappedSuggestion } from "@matrix-org/matrix-wysiwyg"; +import React, { MutableRefObject, ReactNode, useRef, useState } from "react"; import { useComposerFunctions } from "../hooks/useComposerFunctions"; import { useIsFocused } from "../hooks/useIsFocused"; @@ -39,6 +40,11 @@ interface PlainTextComposerProps { children?: (ref: MutableRefObject, composerFunctions: ComposerFunctions) => ReactNode; } +// WIP suggestion stuff, this needs tidying to be a single piece of state +const emptySuggestion = { keyChar: "", text: "", type: "unknown" } as const; +type SuggestionCandidate = { node: Node | null; startOffset: number; endOffset: number }; +const emptySuggestionCandidate: SuggestionCandidate = { node: null, startOffset: NaN, endOffset: NaN }; + export function PlainTextComposer({ className, disabled = false, @@ -53,20 +59,16 @@ export function PlainTextComposer({ // WIP - hack in an autocomplete implementation const autocompleteRef = useRef(null); - const { - ref, - onInput, - onPaste, - onKeyDown, - content, - setContent, - suggestion, - onSelect, - clearSuggestions, - suggestionNodeInfo, - } = usePlainTextListeners(autocompleteRef, initialContent, onChange, onSend); + // WIP - may become a custom hook in keeping with the rest of the design + const [suggestion, setSuggestion] = useState(emptySuggestion); + const [suggestionNodeInfo, setSuggestionNodeInfo] = useState(emptySuggestionCandidate); + const clearSuggestions = (): void => { + setSuggestion(emptySuggestion); + setSuggestionNodeInfo(emptySuggestionCandidate); + }; const handleMention = (): void => {}; + const handleCommand = (replacementText: string): void => { // if this gets triggered, then we have a command on the page, so what we want to do is // manually amend the html text content with the stored state @@ -85,6 +87,51 @@ export function PlainTextComposer({ clearSuggestions(); }; + // do for slash commands first + const onSelect = (): void => { + // whenever there's a change in selection, we're going to have to do some magic + const s = document.getSelection(); + + // if we have a cursor inside a text node, then we're potentially interested in the text content + if (s && s.isCollapsed && s.anchorNode?.nodeName === "#text" && ref.current) { + // first check is that the text node is the first text node of the editor as we can also have + //

tags in the markup + const firstTextNode = document.createNodeIterator(ref.current, NodeFilter.SHOW_TEXT).nextNode(); + const isFirstTextNode = s.anchorNode === firstTextNode; + const textContent = s.anchorNode.textContent; + + // if we're not in the first text node or we have no text content, return + if (!isFirstTextNode || textContent === null) { + return; + } + + // it's a command if: it is the first textnode, it starts with /, not //, then has letters all the way up to + // the end of the textcontent - nb think this last assumption may give us some behavioural inconsistency + // between the rust model and this, but it's a decent starting point + const commandRegex = /^\/{1}(\w*)$/; + const commandMatches = textContent.match(commandRegex); + + // if we don't have a command, clear the suggestion state and return + if (commandMatches === null) { + if (suggestionNodeInfo.node !== null) { + clearSuggestions(); + } + return; + } + + // but if we do have some matches, use that to populate the suggestion state + setSuggestionNodeInfo({ node: s.anchorNode, startOffset: 0, endOffset: textContent.length }); + setSuggestion({ keyChar: "/", type: "command", text: commandMatches[1] }); + } + }; + + const { ref, onInput, onPaste, onKeyDown, content, setContent } = usePlainTextListeners( + autocompleteRef, + initialContent, + onChange, + onSend, + ); + const composerFunctions = useComposerFunctions(ref, setContent); usePlainTextInitialization(initialContent, ref); useSetCursorPosition(disabled, ref); diff --git a/src/components/views/rooms/wysiwyg_composer/hooks/usePlainTextListeners.ts b/src/components/views/rooms/wysiwyg_composer/hooks/usePlainTextListeners.ts index c0c1ac17f37..a8a86ce5c2e 100644 --- a/src/components/views/rooms/wysiwyg_composer/hooks/usePlainTextListeners.ts +++ b/src/components/views/rooms/wysiwyg_composer/hooks/usePlainTextListeners.ts @@ -14,7 +14,6 @@ See the License for the specific language governing permissions and limitations under the License. */ -import { MappedSuggestion } from "@matrix-org/matrix-wysiwyg"; import { KeyboardEvent, RefObject, SyntheticEvent, useCallback, useRef, useState } from "react"; import { useSettingValue } from "../../../../../hooks/useSettings"; @@ -36,10 +35,6 @@ function amendInnerHtml(text: string): string { .replace(/<\/div>/g, ""); } -const emptySuggestion = { keyChar: "", text: "", type: "unknown" } as const; -type SuggestionCandidate = { node: Node | null; startOffset: number; endOffset: number }; -const emptySuggestionCandidate: SuggestionCandidate = { node: null, startOffset: NaN, endOffset: NaN }; - export function usePlainTextListeners( autocompleteRef: React.RefObject, initialContent?: string, @@ -52,15 +47,9 @@ export function usePlainTextListeners( onPaste(event: SyntheticEvent): void; onKeyDown(event: KeyboardEvent): void; setContent(text: string): void; - suggestion: MappedSuggestion; - onSelect(): void; - clearSuggestions(): void; - suggestionNodeInfo: SuggestionCandidate; } { const ref = useRef(null); const [content, setContent] = useState(initialContent); - const [suggestion, setSuggestion] = useState(emptySuggestion); - const [suggestionNodeInfo, setSuggestionNodeInfo] = useState(emptySuggestionCandidate); const send = useCallback(() => { if (ref.current) { @@ -119,49 +108,6 @@ export function usePlainTextListeners( [autocompleteRef, enterShouldSend, send], ); - // do for slash commands first - const onSelect = (): void => { - // whenever there's a change in selection, we're going to have to do some magic - const s = document.getSelection(); - - // if we have a cursor inside a text node, then we're potentially interested in the text content - if (s && s.isCollapsed && s.anchorNode?.nodeName === "#text" && ref.current) { - // first check is that the text node is the first text node of the editor as we can also have - //

tags in the markup - const firstTextNode = document.createNodeIterator(ref.current, NodeFilter.SHOW_TEXT).nextNode(); - const isFirstTextNode = s.anchorNode === firstTextNode; - const textContent = s.anchorNode.textContent; - - // if we're not in the first text node or we have no text content, return - if (!isFirstTextNode || textContent === null) { - return; - } - - // it's a command if: it is the first textnode, it starts with /, not //, then has letters all the way up to - // the end of the textcontent - nb think this last assumption may give us some behavioural inconsistency - // between the rust model and this, but it's a decent starting point - const commandRegex = /^\/{1}(\w*)$/; - const commandMatches = textContent.match(commandRegex); - - // if we don't have a command, clear the suggestion state and return - if (commandMatches === null) { - if (suggestionNodeInfo.node !== null) { - clearSuggestions(); - } - return; - } - - // but if we do have some matches, use that to populate the suggestion state - setSuggestionNodeInfo({ node: s.anchorNode, startOffset: 0, endOffset: textContent.length }); - setSuggestion({ keyChar: "/", type: "command", text: commandMatches[1] }); - } - }; - - const clearSuggestions = (): void => { - setSuggestion(emptySuggestion); - setSuggestionNodeInfo(emptySuggestionCandidate); - }; - return { ref, onInput, @@ -169,9 +115,5 @@ export function usePlainTextListeners( onKeyDown, content, setContent: setText, - suggestion, - onSelect, - clearSuggestions, - suggestionNodeInfo, }; } From 380e26541d17415c3f213347276bf8fbd9c574f2 Mon Sep 17 00:00:00 2001 From: Alun Turner Date: Thu, 13 Apr 2023 14:08:59 +0100 Subject: [PATCH 15/61] shift logic into custom hook --- .../components/PlainTextComposer.tsx | 77 +----------- .../wysiwyg_composer/hooks/useSuggestion.ts | 117 ++++++++++++++++++ .../components/PlainTextComposer-test.tsx | 27 +++- 3 files changed, 147 insertions(+), 74 deletions(-) create mode 100644 src/components/views/rooms/wysiwyg_composer/hooks/useSuggestion.ts diff --git a/src/components/views/rooms/wysiwyg_composer/components/PlainTextComposer.tsx b/src/components/views/rooms/wysiwyg_composer/components/PlainTextComposer.tsx index c7dffcf7c15..8a9756685de 100644 --- a/src/components/views/rooms/wysiwyg_composer/components/PlainTextComposer.tsx +++ b/src/components/views/rooms/wysiwyg_composer/components/PlainTextComposer.tsx @@ -15,8 +15,7 @@ limitations under the License. */ import classNames from "classnames"; -import { MappedSuggestion } from "@matrix-org/matrix-wysiwyg"; -import React, { MutableRefObject, ReactNode, useRef, useState } from "react"; +import React, { MutableRefObject, ReactNode, useRef } from "react"; import { useComposerFunctions } from "../hooks/useComposerFunctions"; import { useIsFocused } from "../hooks/useIsFocused"; @@ -27,6 +26,7 @@ import { ComposerFunctions } from "../types"; import { Editor } from "./Editor"; import Autocomplete from "../../Autocomplete"; import { WysiwygAutocomplete } from "./WysiwygAutocomplete"; +import { useSuggestion } from "../hooks/useSuggestion"; interface PlainTextComposerProps { disabled?: boolean; @@ -40,11 +40,6 @@ interface PlainTextComposerProps { children?: (ref: MutableRefObject, composerFunctions: ComposerFunctions) => ReactNode; } -// WIP suggestion stuff, this needs tidying to be a single piece of state -const emptySuggestion = { keyChar: "", text: "", type: "unknown" } as const; -type SuggestionCandidate = { node: Node | null; startOffset: number; endOffset: number }; -const emptySuggestionCandidate: SuggestionCandidate = { node: null, startOffset: NaN, endOffset: NaN }; - export function PlainTextComposer({ className, disabled = false, @@ -59,72 +54,6 @@ export function PlainTextComposer({ // WIP - hack in an autocomplete implementation const autocompleteRef = useRef(null); - // WIP - may become a custom hook in keeping with the rest of the design - const [suggestion, setSuggestion] = useState(emptySuggestion); - const [suggestionNodeInfo, setSuggestionNodeInfo] = useState(emptySuggestionCandidate); - const clearSuggestions = (): void => { - setSuggestion(emptySuggestion); - setSuggestionNodeInfo(emptySuggestionCandidate); - }; - - const handleMention = (): void => {}; - - const handleCommand = (replacementText: string): void => { - // if this gets triggered, then we have a command on the page, so what we want to do is - // manually amend the html text content with the stored state - const { node /* startOffset, endOffset */ } = suggestionNodeInfo; - if (ref.current === null || node === null || node.textContent === null) return; - - // for a command we know we're starting at the beginning, so it's a bit easier - const newContent = `${replacementText} `; // note the trailing space - node.textContent = newContent; - - // then set the cursor to the end of the node, fire an inputEvent (updates - // the hook state), clear the suggestions from state - document.getSelection()?.setBaseAndExtent(node, newContent.length, node, newContent.length); - const event = new Event("inputEvent"); - ref.current.dispatchEvent(event); - clearSuggestions(); - }; - - // do for slash commands first - const onSelect = (): void => { - // whenever there's a change in selection, we're going to have to do some magic - const s = document.getSelection(); - - // if we have a cursor inside a text node, then we're potentially interested in the text content - if (s && s.isCollapsed && s.anchorNode?.nodeName === "#text" && ref.current) { - // first check is that the text node is the first text node of the editor as we can also have - //

tags in the markup - const firstTextNode = document.createNodeIterator(ref.current, NodeFilter.SHOW_TEXT).nextNode(); - const isFirstTextNode = s.anchorNode === firstTextNode; - const textContent = s.anchorNode.textContent; - - // if we're not in the first text node or we have no text content, return - if (!isFirstTextNode || textContent === null) { - return; - } - - // it's a command if: it is the first textnode, it starts with /, not //, then has letters all the way up to - // the end of the textcontent - nb think this last assumption may give us some behavioural inconsistency - // between the rust model and this, but it's a decent starting point - const commandRegex = /^\/{1}(\w*)$/; - const commandMatches = textContent.match(commandRegex); - - // if we don't have a command, clear the suggestion state and return - if (commandMatches === null) { - if (suggestionNodeInfo.node !== null) { - clearSuggestions(); - } - return; - } - - // but if we do have some matches, use that to populate the suggestion state - setSuggestionNodeInfo({ node: s.anchorNode, startOffset: 0, endOffset: textContent.length }); - setSuggestion({ keyChar: "/", type: "command", text: commandMatches[1] }); - } - }; - const { ref, onInput, onPaste, onKeyDown, content, setContent } = usePlainTextListeners( autocompleteRef, initialContent, @@ -132,6 +61,8 @@ export function PlainTextComposer({ onSend, ); + const { suggestion, onSelect, handleCommand, handleMention } = useSuggestion(ref, autocompleteRef); + const composerFunctions = useComposerFunctions(ref, setContent); usePlainTextInitialization(initialContent, ref); useSetCursorPosition(disabled, ref); diff --git a/src/components/views/rooms/wysiwyg_composer/hooks/useSuggestion.ts b/src/components/views/rooms/wysiwyg_composer/hooks/useSuggestion.ts new file mode 100644 index 00000000000..0bbf551c61a --- /dev/null +++ b/src/components/views/rooms/wysiwyg_composer/hooks/useSuggestion.ts @@ -0,0 +1,117 @@ +/* +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 { Attributes, MappedSuggestion, SuggestionChar } from "@matrix-org/matrix-wysiwyg"; +import { SyntheticEvent, useState } from "react"; + +// WIP suggestion stuff, this needs tidying to be a single piece of state +// try to make this like the rust model for consistency, in Rust: +/** + * pub struct SuggestionPattern { + * pub key: PatternKey, // translate to keyChar so we're not using an enum + * pub text: String, + * pub start: usize, // use startOffset as we'll track the node + * pub end: usize, // use endOffset as we'll track the node + * } + */ +// but we also need to know which node we're looking at +// TODO shift over to using this type and the refactoring it will require, but get tests written first to +// give a safety net for the refactoring + +type PlainTextSuggestionPattern = { + keyChar: SuggestionChar; + text: string; + node: Node; + startOffset: number; + endOffset: number; +} | null; +const emptySuggestion: MappedSuggestion = { keyChar: "", text: "", type: "unknown" }; +type SuggestionCandidate = { node: Node | null; startOffset: number; endOffset: number }; +const emptySuggestionCandidate: SuggestionCandidate = { node: null, startOffset: NaN, endOffset: NaN }; + +export function useSuggestion(editorRef: React.RefObject): { + suggestion: MappedSuggestion; + handleMention: (link: string, text: string, attributes: Attributes) => void; + handleCommand: (text: string) => void; + onSelect: (event: SyntheticEvent) => void; +} { + // WIP - may become a custom hook in keeping with the rest of the design + const [suggestion, setSuggestion] = useState(emptySuggestion); + const [suggestionNodeInfo, setSuggestionNodeInfo] = useState(emptySuggestionCandidate); + const clearSuggestions = (): void => { + setSuggestion(emptySuggestion); + setSuggestionNodeInfo(emptySuggestionCandidate); + }; + // TODO handle the mentions (@user, #room etc) + const handleMention = (): void => {}; + + const handleCommand = (replacementText: string): void => { + // if this gets triggered, then we have a command on the page, so what we want to do is + // manually amend the html text content with the stored state + const { node /* startOffset, endOffset */ } = suggestionNodeInfo; + if (editorRef.current === null || node === null || node.textContent === null) return; + + // for a command we know we're starting at the beginning, so it's a bit easier + const newContent = `${replacementText} `; // note the trailing space + node.textContent = newContent; + + // then set the cursor to the end of the node, fire an inputEvent (updates + // the hook state), clear the suggestions from state + document.getSelection()?.setBaseAndExtent(node, newContent.length, node, newContent.length); + const event = new Event("inputEvent"); + editorRef.current.dispatchEvent(event); + clearSuggestions(); + }; + // do for slash commands first + const onSelect = (e): void => { + // whenever there's a change in selection, we're going to have to do some magic + const s = document.getSelection(); + + // if we have a cursor inside a text node, then we're potentially interested in the text content + if (s && s.isCollapsed && s.anchorNode?.nodeName === "#text" && editorRef.current) { + // first check is that the text node is the first text node of the editor as we can also have + //

tags in the markup + const firstTextNode = document.createNodeIterator(editorRef.current, NodeFilter.SHOW_TEXT).nextNode(); + const isFirstTextNode = s.anchorNode === firstTextNode; + const textContent = s.anchorNode.textContent; + + // if we're not in the first text node or we have no text content, return + if (!isFirstTextNode || textContent === null) { + return; + } + + // it's a command if: it is the first textnode, it starts with /, not //, then has letters all the way up to + // the end of the textcontent - nb think this last assumption may give us some behavioural inconsistency + // between the rust model and this, but it's a decent starting point + const commandRegex = /^\/{1}(\w*)$/; + const commandMatches = textContent.match(commandRegex); + + // if we don't have a command, clear the suggestion state and return + if (commandMatches === null) { + if (suggestionNodeInfo.node !== null) { + clearSuggestions(); + } + return; + } + + // but if we do have some matches, use that to populate the suggestion state + setSuggestionNodeInfo({ node: s.anchorNode, startOffset: 0, endOffset: textContent.length }); + setSuggestion({ keyChar: "/", type: "command", text: commandMatches[1] }); + } + }; + + return { suggestion, handleCommand, handleMention, onSelect }; +} diff --git a/test/components/views/rooms/wysiwyg_composer/components/PlainTextComposer-test.tsx b/test/components/views/rooms/wysiwyg_composer/components/PlainTextComposer-test.tsx index e3b1b2fce19..7af2208fc26 100644 --- a/test/components/views/rooms/wysiwyg_composer/components/PlainTextComposer-test.tsx +++ b/test/components/views/rooms/wysiwyg_composer/components/PlainTextComposer-test.tsx @@ -15,12 +15,20 @@ limitations under the License. */ import React from "react"; -import { act, render, screen } from "@testing-library/react"; +import { act, fireEvent, render, screen, waitFor } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; import { PlainTextComposer } from "../../../../../../src/components/views/rooms/wysiwyg_composer/components/PlainTextComposer"; import * as mockUseSettingsHook from "../../../../../../src/hooks/useSettings"; import * as mockKeyboard from "../../../../../../src/Keyboard"; +import { createMocks } from "../utils"; +import MatrixClientContext from "../../../../../../src/contexts/MatrixClientContext"; +import RoomContext from "../../../../../../src/contexts/RoomContext"; +import defaultDispatcher from "../../../../../../src/dispatcher/dispatcher"; +import Autocompleter, { ICompletion } from "../../../../../../src/autocomplete/Autocompleter"; +import AutocompleteProvider from "../../../../../../src/autocomplete/AutocompleteProvider"; +import * as Permalinks from "../../../../../../src/utils/permalinks/Permalinks"; +import { PermalinkParts } from "../../../../../../src/utils/permalinks/PermalinkConstructor"; describe("PlainTextComposer", () => { const customRender = ( @@ -271,4 +279,21 @@ describe("PlainTextComposer", () => { jest.useRealTimers(); (global.ResizeObserver as jest.Mock).mockRestore(); }); + + it("Should not render if not wrapped in room context", () => { + customRender(); + expect(screen.queryByTestId("autocomplete-wrapper")).not.toBeInTheDocument(); + }); + + it("Should render if wrapped in room context", () => { + const { defaultRoomContext } = createMocks(); + + render( + + + , + ); + + expect(screen.getByTestId("autocomplete-wrapper")).toBeInTheDocument(); + }); }); From e0fa7dd98ed66facfe0dfb188d2b504423560292 Mon Sep 17 00:00:00 2001 From: Alun Turner Date: Thu, 13 Apr 2023 14:10:21 +0100 Subject: [PATCH 16/61] rename ref to editorRef for clarity --- .../components/PlainTextComposer.tsx | 26 ++++++++++--------- 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/src/components/views/rooms/wysiwyg_composer/components/PlainTextComposer.tsx b/src/components/views/rooms/wysiwyg_composer/components/PlainTextComposer.tsx index 8a9756685de..8843e5c02f5 100644 --- a/src/components/views/rooms/wysiwyg_composer/components/PlainTextComposer.tsx +++ b/src/components/views/rooms/wysiwyg_composer/components/PlainTextComposer.tsx @@ -54,18 +54,20 @@ export function PlainTextComposer({ // WIP - hack in an autocomplete implementation const autocompleteRef = useRef(null); - const { ref, onInput, onPaste, onKeyDown, content, setContent } = usePlainTextListeners( - autocompleteRef, - initialContent, - onChange, - onSend, - ); + const { + ref: editorRef, + onInput, + onPaste, + onKeyDown, + content, + setContent, + } = usePlainTextListeners(autocompleteRef, initialContent, onChange, onSend); - const { suggestion, onSelect, handleCommand, handleMention } = useSuggestion(ref, autocompleteRef); + const { suggestion, onSelect, handleCommand, handleMention } = useSuggestion(editorRef); - const composerFunctions = useComposerFunctions(ref, setContent); - usePlainTextInitialization(initialContent, ref); - useSetCursorPosition(disabled, ref); + const composerFunctions = useComposerFunctions(editorRef, setContent); + usePlainTextInitialization(initialContent, editorRef); + useSetCursorPosition(disabled, editorRef); const { isFocused, onFocus } = useIsFocused(); const computedPlaceholder = (!content && placeholder) || undefined; @@ -87,13 +89,13 @@ export function PlainTextComposer({ handleCommand={handleCommand} /> - {children?.(ref, composerFunctions)} + {children?.(editorRef, composerFunctions)} ); } From 76f46b42f6ef46c6856f873746a2c744746914cf Mon Sep 17 00:00:00 2001 From: Alun Turner Date: Thu, 13 Apr 2023 14:33:15 +0100 Subject: [PATCH 17/61] remove comment --- .../rooms/wysiwyg_composer/components/PlainTextComposer.tsx | 1 - 1 file changed, 1 deletion(-) diff --git a/src/components/views/rooms/wysiwyg_composer/components/PlainTextComposer.tsx b/src/components/views/rooms/wysiwyg_composer/components/PlainTextComposer.tsx index 8843e5c02f5..17972a5fa54 100644 --- a/src/components/views/rooms/wysiwyg_composer/components/PlainTextComposer.tsx +++ b/src/components/views/rooms/wysiwyg_composer/components/PlainTextComposer.tsx @@ -51,7 +51,6 @@ export function PlainTextComposer({ leftComponent, rightComponent, }: PlainTextComposerProps): JSX.Element { - // WIP - hack in an autocomplete implementation const autocompleteRef = useRef(null); const { From 5d971b3d2cf26ef07a4560d10eee6a95af39dd69 Mon Sep 17 00:00:00 2001 From: Alun Turner Date: Fri, 14 Apr 2023 11:02:11 +0100 Subject: [PATCH 18/61] try to add cypress test for behaviour --- cypress/e2e/composer/composer.spec.ts | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/cypress/e2e/composer/composer.spec.ts b/cypress/e2e/composer/composer.spec.ts index 0ed67334169..0a64c8f722e 100644 --- a/cypress/e2e/composer/composer.spec.ts +++ b/cypress/e2e/composer/composer.spec.ts @@ -117,6 +117,22 @@ describe("Composer", () => { cy.viewRoomByName("Composing Room"); }); + it("autocomplete works for slash commands", () => { + // Type a message + cy.get("div[contenteditable=true]").type("/spo"); + + // Check that the autocomplete option is visibile and click it + cy.findByRole("presentation").should("be.visible"); + cy.findByText("/spoiler").click(); + + // this should close the autocomplete and have completed with the + // expected text + cy.findByRole("presentation").should("not.be.visible"); + cy.findByRole("textbox").within(() => { + cy.findByText("/spoiler").should("exist"); + }); + }); + it("sends a message when you click send or press Enter", () => { // Type a message cy.get("div[contenteditable=true]").type("my message 0"); From 14fd02bdd0a753458ff5b6b51d62849730adf2e4 Mon Sep 17 00:00:00 2001 From: Alun Turner Date: Fri, 14 Apr 2023 11:04:58 +0100 Subject: [PATCH 19/61] remove unused imports --- .../components/PlainTextComposer-test.tsx | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/test/components/views/rooms/wysiwyg_composer/components/PlainTextComposer-test.tsx b/test/components/views/rooms/wysiwyg_composer/components/PlainTextComposer-test.tsx index 7af2208fc26..01ba55cbef9 100644 --- a/test/components/views/rooms/wysiwyg_composer/components/PlainTextComposer-test.tsx +++ b/test/components/views/rooms/wysiwyg_composer/components/PlainTextComposer-test.tsx @@ -15,20 +15,14 @@ limitations under the License. */ import React from "react"; -import { act, fireEvent, render, screen, waitFor } from "@testing-library/react"; +import { act, render, screen } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; import { PlainTextComposer } from "../../../../../../src/components/views/rooms/wysiwyg_composer/components/PlainTextComposer"; import * as mockUseSettingsHook from "../../../../../../src/hooks/useSettings"; import * as mockKeyboard from "../../../../../../src/Keyboard"; import { createMocks } from "../utils"; -import MatrixClientContext from "../../../../../../src/contexts/MatrixClientContext"; import RoomContext from "../../../../../../src/contexts/RoomContext"; -import defaultDispatcher from "../../../../../../src/dispatcher/dispatcher"; -import Autocompleter, { ICompletion } from "../../../../../../src/autocomplete/Autocompleter"; -import AutocompleteProvider from "../../../../../../src/autocomplete/AutocompleteProvider"; -import * as Permalinks from "../../../../../../src/utils/permalinks/Permalinks"; -import { PermalinkParts } from "../../../../../../src/utils/permalinks/PermalinkConstructor"; describe("PlainTextComposer", () => { const customRender = ( From e08f45027aca6c625e3b0a52d3e774a434c98ba3 Mon Sep 17 00:00:00 2001 From: Alun Turner Date: Fri, 14 Apr 2023 11:13:09 +0100 Subject: [PATCH 20/61] fix various lint/TS errors for CI --- .../wysiwyg_composer/hooks/useSuggestion.ts | 20 +++++++++---------- .../components/PlainTextComposer-test.tsx | 2 +- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/src/components/views/rooms/wysiwyg_composer/hooks/useSuggestion.ts b/src/components/views/rooms/wysiwyg_composer/hooks/useSuggestion.ts index 0bbf551c61a..a93c4b2640c 100644 --- a/src/components/views/rooms/wysiwyg_composer/hooks/useSuggestion.ts +++ b/src/components/views/rooms/wysiwyg_composer/hooks/useSuggestion.ts @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -import { Attributes, MappedSuggestion, SuggestionChar } from "@matrix-org/matrix-wysiwyg"; +import { Attributes, MappedSuggestion /* SuggestionChar */ } from "@matrix-org/matrix-wysiwyg"; import { SyntheticEvent, useState } from "react"; // WIP suggestion stuff, this needs tidying to be a single piece of state @@ -31,13 +31,13 @@ import { SyntheticEvent, useState } from "react"; // TODO shift over to using this type and the refactoring it will require, but get tests written first to // give a safety net for the refactoring -type PlainTextSuggestionPattern = { - keyChar: SuggestionChar; - text: string; - node: Node; - startOffset: number; - endOffset: number; -} | null; +// type PlainTextSuggestionPattern = { +// keyChar: SuggestionChar; +// text: string; +// node: Node; +// startOffset: number; +// endOffset: number; +// } | null; const emptySuggestion: MappedSuggestion = { keyChar: "", text: "", type: "unknown" }; type SuggestionCandidate = { node: Node | null; startOffset: number; endOffset: number }; const emptySuggestionCandidate: SuggestionCandidate = { node: null, startOffset: NaN, endOffset: NaN }; @@ -75,8 +75,8 @@ export function useSuggestion(editorRef: React.RefObject): { editorRef.current.dispatchEvent(event); clearSuggestions(); }; - // do for slash commands first - const onSelect = (e): void => { + // do for slash commands first, should probably use the event somehow + const onSelect = (): void => { // whenever there's a change in selection, we're going to have to do some magic const s = document.getSelection(); diff --git a/test/components/views/rooms/wysiwyg_composer/components/PlainTextComposer-test.tsx b/test/components/views/rooms/wysiwyg_composer/components/PlainTextComposer-test.tsx index 01ba55cbef9..9277ecb16c7 100644 --- a/test/components/views/rooms/wysiwyg_composer/components/PlainTextComposer-test.tsx +++ b/test/components/views/rooms/wysiwyg_composer/components/PlainTextComposer-test.tsx @@ -284,7 +284,7 @@ describe("PlainTextComposer", () => { render( - + , ); From 6a40aa6f15bca27de8b05b3d75709f160b81d2e8 Mon Sep 17 00:00:00 2001 From: Alun Turner Date: Fri, 14 Apr 2023 12:08:38 +0100 Subject: [PATCH 21/61] update cypress test --- cypress/e2e/composer/composer.spec.ts | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/cypress/e2e/composer/composer.spec.ts b/cypress/e2e/composer/composer.spec.ts index 0a64c8f722e..513c4f47b71 100644 --- a/cypress/e2e/composer/composer.spec.ts +++ b/cypress/e2e/composer/composer.spec.ts @@ -117,17 +117,18 @@ describe("Composer", () => { cy.viewRoomByName("Composing Room"); }); - it("autocomplete works for slash commands", () => { + it.only("autocomplete works for slash commands", () => { // Type a message cy.get("div[contenteditable=true]").type("/spo"); - // Check that the autocomplete option is visibile and click it - cy.findByRole("presentation").should("be.visible"); - cy.findByText("/spoiler").click(); + // Check that the autocomplete option is visible and click it + cy.findByTestId("autocomplete-wrapper").within(() => { + cy.findByText("/spoiler").click(); + }); // this should close the autocomplete and have completed with the // expected text - cy.findByRole("presentation").should("not.be.visible"); + cy.findByTestId("autocomplete-wrapper").should("not.be.visible"); cy.findByRole("textbox").within(() => { cy.findByText("/spoiler").should("exist"); }); From 69a6ca49607e1d4537e4dd7f19bdd3c2cd41d977 Mon Sep 17 00:00:00 2001 From: Alun Turner Date: Fri, 14 Apr 2023 13:05:34 +0100 Subject: [PATCH 22/61] add test for pressing escape to close autocomplete --- .../components/WysiwygComposer-test.tsx | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/test/components/views/rooms/wysiwyg_composer/components/WysiwygComposer-test.tsx b/test/components/views/rooms/wysiwyg_composer/components/WysiwygComposer-test.tsx index 3f9694e2a3c..0263ee22b85 100644 --- a/test/components/views/rooms/wysiwyg_composer/components/WysiwygComposer-test.tsx +++ b/test/components/views/rooms/wysiwyg_composer/components/WysiwygComposer-test.tsx @@ -278,6 +278,18 @@ describe("WysiwygComposer", () => { expect(screen.getByRole("link", { name: mockCompletions[0].completion })).toBeInTheDocument(); }); + it("pressing escape closes the autocomplete", async () => { + await insertMentionInput(); + + // press enter + await userEvent.keyboard("{Escape}"); + + // check that it closes the autocomplete + await waitFor(() => { + expect(screen.queryByRole("presentation")).not.toBeInTheDocument(); + }); + }); + it("clicking on a mention in the composer dispatches the correct action", async () => { await insertMentionInput(); From c092b3e52753b48577350baf62dbe38d6952e34e Mon Sep 17 00:00:00 2001 From: Alun Turner Date: Fri, 14 Apr 2023 13:31:10 +0100 Subject: [PATCH 23/61] expand cypress tests --- cypress/e2e/composer/composer.spec.ts | 61 +++++++++++++++++++++------ 1 file changed, 49 insertions(+), 12 deletions(-) diff --git a/cypress/e2e/composer/composer.spec.ts b/cypress/e2e/composer/composer.spec.ts index 513c4f47b71..7dc911ba158 100644 --- a/cypress/e2e/composer/composer.spec.ts +++ b/cypress/e2e/composer/composer.spec.ts @@ -117,21 +117,58 @@ describe("Composer", () => { cy.viewRoomByName("Composing Room"); }); - it.only("autocomplete works for slash commands", () => { - // Type a message - cy.get("div[contenteditable=true]").type("/spo"); + describe("commands", () => { + describe("plain text mode", () => { + it("autocomplete opens when / is entered and is populated", () => { + // Select plain text mode after composer is ready + cy.get("div[contenteditable=true]").should("exist"); + cy.findByRole("button", { name: "Hide formatting" }).click(); + + // Type a / + cy.get("div[contenteditable=true]").type("/"); + + // Check that the autocomplete options are visible and there are more than 0 + cy.findByTestId("autocomplete-wrapper").within(() => { + cy.findAllByRole("presentation").should("have.length.above", 0); + }); + }); - // Check that the autocomplete option is visible and click it - cy.findByTestId("autocomplete-wrapper").within(() => { - cy.findByText("/spoiler").click(); - }); + it("autocomplete can be used to enter a command", () => { + // Select plain text mode after composer is ready + cy.get("div[contenteditable=true]").should("exist"); + cy.findByRole("button", { name: "Hide formatting" }).click(); - // this should close the autocomplete and have completed with the - // expected text - cy.findByTestId("autocomplete-wrapper").should("not.be.visible"); - cy.findByRole("textbox").within(() => { - cy.findByText("/spoiler").should("exist"); + // Type a message + cy.get("div[contenteditable=true]").type("/spo"); + + // Check that the autocomplete /spoiler option is visible and click it + cy.findByTestId("autocomplete-wrapper").within(() => { + cy.findByText("/spoiler").click(); + }); + + // this should close the autocomplete and have inserted with the completion text + cy.findByTestId("autocomplete-wrapper").should("not.be.visible"); + cy.findByRole("textbox").within(() => { + cy.findByText("/spoiler").should("exist"); + }); + }); + + it("autocomplete is not displayed for a message starting with //", () => { + // Select plain text mode after composer is ready + cy.get("div[contenteditable=true]").should("exist"); + cy.findByRole("button", { name: "Hide formatting" }).click(); + + // Type a message + cy.get("div[contenteditable=true]").type("//spo"); + + // Check that the autocomplete options are not visible + cy.findByTestId("autocomplete-wrapper").within(() => { + cy.findAllByRole("presentation").should("have.length", 0); + }); + }); }); + + // TODO add tests for rich text mode }); it("sends a message when you click send or press Enter", () => { From 262c7c00c2ca74777733157614a09e12c2bf78d6 Mon Sep 17 00:00:00 2001 From: Alun Turner Date: Fri, 14 Apr 2023 14:02:40 +0100 Subject: [PATCH 24/61] add typing while autocomplete open test --- .../components/WysiwygComposer-test.tsx | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/test/components/views/rooms/wysiwyg_composer/components/WysiwygComposer-test.tsx b/test/components/views/rooms/wysiwyg_composer/components/WysiwygComposer-test.tsx index 0263ee22b85..1310ad2861d 100644 --- a/test/components/views/rooms/wysiwyg_composer/components/WysiwygComposer-test.tsx +++ b/test/components/views/rooms/wysiwyg_composer/components/WysiwygComposer-test.tsx @@ -290,6 +290,16 @@ describe("WysiwygComposer", () => { }); }); + it("typing with the autocomplete open still works as expected", async () => { + await insertMentionInput(); + + // add some more text, then check the autocomplete is open AND the text is in the composer + await userEvent.keyboard("extra"); + + expect(screen.queryByRole("presentation")).toBeInTheDocument(); + expect(screen.getByRole("textbox")).toHaveTextContent("@abcextra"); + }); + it("clicking on a mention in the composer dispatches the correct action", async () => { await insertMentionInput(); From 590650eb1ebd7a39f22d12de406178c67b2dfdf7 Mon Sep 17 00:00:00 2001 From: Alun Turner Date: Fri, 14 Apr 2023 15:57:30 +0100 Subject: [PATCH 25/61] refactor to single piece of state and update comments --- .../wysiwyg_composer/hooks/useSuggestion.ts | 149 ++++++++++-------- 1 file changed, 82 insertions(+), 67 deletions(-) diff --git a/src/components/views/rooms/wysiwyg_composer/hooks/useSuggestion.ts b/src/components/views/rooms/wysiwyg_composer/hooks/useSuggestion.ts index a93c4b2640c..d25aa46dd7b 100644 --- a/src/components/views/rooms/wysiwyg_composer/hooks/useSuggestion.ts +++ b/src/components/views/rooms/wysiwyg_composer/hooks/useSuggestion.ts @@ -14,104 +14,119 @@ See the License for the specific language governing permissions and limitations under the License. */ -import { Attributes, MappedSuggestion /* SuggestionChar */ } from "@matrix-org/matrix-wysiwyg"; +import { SuggestionType } from "@matrix-org/matrix-wysiwyg"; +import { Attributes, MappedSuggestion, SuggestionChar } from "@matrix-org/matrix-wysiwyg"; import { SyntheticEvent, useState } from "react"; -// WIP suggestion stuff, this needs tidying to be a single piece of state -// try to make this like the rust model for consistency, in Rust: -/** - * pub struct SuggestionPattern { - * pub key: PatternKey, // translate to keyChar so we're not using an enum - * pub text: String, - * pub start: usize, // use startOffset as we'll track the node - * pub end: usize, // use endOffset as we'll track the node - * } - */ -// but we also need to know which node we're looking at -// TODO shift over to using this type and the refactoring it will require, but get tests written first to -// give a safety net for the refactoring - -// type PlainTextSuggestionPattern = { -// keyChar: SuggestionChar; -// text: string; -// node: Node; -// startOffset: number; -// endOffset: number; -// } | null; -const emptySuggestion: MappedSuggestion = { keyChar: "", text: "", type: "unknown" }; -type SuggestionCandidate = { node: Node | null; startOffset: number; endOffset: number }; -const emptySuggestionCandidate: SuggestionCandidate = { node: null, startOffset: NaN, endOffset: NaN }; +// This type is a close approximation of what we use in the Rust model for the rich version of the +// editor, we use this to try and make this simple model as similar as possible to allow reuse of +// the autocomplete component +type PlainTextSuggestionPattern = { + keyChar: SuggestionChar; + type: SuggestionType; + text: string; + node: Node; + startOffset: number; + endOffset: number; +} | null; + +const mapSuggestion = (suggestion: PlainTextSuggestionPattern | null): MappedSuggestion | null => { + if (suggestion === null) { + return null; + } else { + const { node, startOffset, endOffset, ...mappedSuggestion } = suggestion; + return mappedSuggestion; + } +}; export function useSuggestion(editorRef: React.RefObject): { - suggestion: MappedSuggestion; handleMention: (link: string, text: string, attributes: Attributes) => void; handleCommand: (text: string) => void; onSelect: (event: SyntheticEvent) => void; + suggestion: MappedSuggestion | null; } { - // WIP - may become a custom hook in keeping with the rest of the design - const [suggestion, setSuggestion] = useState(emptySuggestion); - const [suggestionNodeInfo, setSuggestionNodeInfo] = useState(emptySuggestionCandidate); - const clearSuggestions = (): void => { - setSuggestion(emptySuggestion); - setSuggestionNodeInfo(emptySuggestionCandidate); - }; + const [suggestion, setSuggestion] = useState(null); + // TODO handle the mentions (@user, #room etc) const handleMention = (): void => {}; const handleCommand = (replacementText: string): void => { - // if this gets triggered, then we have a command on the page, so what we want to do is - // manually amend the html text content with the stored state - const { node /* startOffset, endOffset */ } = suggestionNodeInfo; - if (editorRef.current === null || node === null || node.textContent === null) return; + // if we do not have any of the values we need to do the work, do nothing + if ( + suggestion === null || + editorRef.current === null || + suggestion.node === null || + suggestion.node.textContent === null + ) { + return; + } - // for a command we know we're starting at the beginning, so it's a bit easier - const newContent = `${replacementText} `; // note the trailing space + const { node } = suggestion; + + // for a command we know we start at the beginning of the text node, so build the replacement + // string (note trailing space) and manually adjust the node's textcontent + const newContent = `${replacementText} `; node.textContent = newContent; - // then set the cursor to the end of the node, fire an inputEvent (updates - // the hook state), clear the suggestions from state + // then set the cursor to the end of the node, fire an inputEvent to update the hook state + // and then clear the suggestion from state document.getSelection()?.setBaseAndExtent(node, newContent.length, node, newContent.length); - const event = new Event("inputEvent"); - editorRef.current.dispatchEvent(event); - clearSuggestions(); + editorRef.current.dispatchEvent(new Event("inputEvent")); + setSuggestion(null); }; - // do for slash commands first, should probably use the event somehow + + // We create a `seletionchange` handler here because we need to know when the user has moved the cursor, + // we can not depend on input events only const onSelect = (): void => { - // whenever there's a change in selection, we're going to have to do some magic - const s = document.getSelection(); + if (editorRef.current === null) { + return; + } + const selection = document.getSelection(); + + // only carry out the checking if we have cursor inside a text node + if (selection && selection.isCollapsed && selection.anchorNode?.nodeName === "#text") { + // here we have established that anchorNode === focusNode, so rename to + const { anchorNode: currentNode } = selection; - // if we have a cursor inside a text node, then we're potentially interested in the text content - if (s && s.isCollapsed && s.anchorNode?.nodeName === "#text" && editorRef.current) { - // first check is that the text node is the first text node of the editor as we can also have - //

tags in the markup + // first check is that the text node is the first text node of the editor, as adding paragraphs can result + // in nested

tags inside the editor

const firstTextNode = document.createNodeIterator(editorRef.current, NodeFilter.SHOW_TEXT).nextNode(); - const isFirstTextNode = s.anchorNode === firstTextNode; - const textContent = s.anchorNode.textContent; // if we're not in the first text node or we have no text content, return - if (!isFirstTextNode || textContent === null) { + if (currentNode !== firstTextNode || currentNode.textContent === null) { return; } - // it's a command if: it is the first textnode, it starts with /, not //, then has letters all the way up to - // the end of the textcontent - nb think this last assumption may give us some behavioural inconsistency - // between the rust model and this, but it's a decent starting point - const commandRegex = /^\/{1}(\w*)$/; - const commandMatches = textContent.match(commandRegex); + // it's a command if: + // it is the first textnode AND + // it starts with /, not // AND + // then has letters all the way up to the end of the textcontent + const commandRegex = /^\/(\w*)$/; + const commandMatches = currentNode.textContent.match(commandRegex); - // if we don't have a command, clear the suggestion state and return + // if we don't have any matches, return, clearing the suggesiton state if it is non-null if (commandMatches === null) { - if (suggestionNodeInfo.node !== null) { - clearSuggestions(); + if (suggestion !== null) { + setSuggestion(null); } return; + } else { + setSuggestion({ + keyChar: "/", + type: "command", + text: commandMatches[1], + node: selection.anchorNode, + startOffset: 0, + endOffset: currentNode.textContent.length, + }); } - - // but if we do have some matches, use that to populate the suggestion state - setSuggestionNodeInfo({ node: s.anchorNode, startOffset: 0, endOffset: textContent.length }); - setSuggestion({ keyChar: "/", type: "command", text: commandMatches[1] }); } }; - return { suggestion, handleCommand, handleMention, onSelect }; + return { + suggestion: mapSuggestion(suggestion), + handleCommand, + handleMention, + onSelect, + }; } From 7ac231866851e0fdda527bf93d8c5399deda447b Mon Sep 17 00:00:00 2001 From: Alun Turner Date: Fri, 14 Apr 2023 16:02:31 +0100 Subject: [PATCH 26/61] update comment --- .../rooms/wysiwyg_composer/components/WysiwygComposer-test.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/components/views/rooms/wysiwyg_composer/components/WysiwygComposer-test.tsx b/test/components/views/rooms/wysiwyg_composer/components/WysiwygComposer-test.tsx index fa1570798db..b9720652bc3 100644 --- a/test/components/views/rooms/wysiwyg_composer/components/WysiwygComposer-test.tsx +++ b/test/components/views/rooms/wysiwyg_composer/components/WysiwygComposer-test.tsx @@ -301,7 +301,7 @@ describe("WysiwygComposer", () => { it("pressing escape closes the autocomplete", async () => { await insertMentionInput(); - // press enter + // press escape await userEvent.keyboard("{Escape}"); // check that it closes the autocomplete From 1d0312273081c46c03152d9af6b001d07433661c Mon Sep 17 00:00:00 2001 From: Alun Turner Date: Fri, 14 Apr 2023 17:04:17 +0100 Subject: [PATCH 27/61] extract functions for testing --- .../wysiwyg_composer/hooks/useSuggestion.ts | 194 ++++++++++-------- 1 file changed, 114 insertions(+), 80 deletions(-) diff --git a/src/components/views/rooms/wysiwyg_composer/hooks/useSuggestion.ts b/src/components/views/rooms/wysiwyg_composer/hooks/useSuggestion.ts index d25aa46dd7b..066be6cc13d 100644 --- a/src/components/views/rooms/wysiwyg_composer/hooks/useSuggestion.ts +++ b/src/components/views/rooms/wysiwyg_composer/hooks/useSuggestion.ts @@ -14,8 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -import { SuggestionType } from "@matrix-org/matrix-wysiwyg"; -import { Attributes, MappedSuggestion, SuggestionChar } from "@matrix-org/matrix-wysiwyg"; +import { Attributes, MappedSuggestion, SuggestionChar, SuggestionType } from "@matrix-org/matrix-wysiwyg"; import { SyntheticEvent, useState } from "react"; // This type is a close approximation of what we use in the Rust model for the rich version of the @@ -30,15 +29,13 @@ type PlainTextSuggestionPattern = { endOffset: number; } | null; -const mapSuggestion = (suggestion: PlainTextSuggestionPattern | null): MappedSuggestion | null => { - if (suggestion === null) { - return null; - } else { - const { node, startOffset, endOffset, ...mappedSuggestion } = suggestion; - return mappedSuggestion; - } -}; - +/** + * Given an input div element, return the current suggestion found in that div element if present + * as well as handlers for command or mention completions from the autocomplete, and a listener + * to be attached to the PlainTextComposer + * + * @param editorRef - a ref to the div that is the composer textbox + */ export function useSuggestion(editorRef: React.RefObject): { handleMention: (link: string, text: string, attributes: Attributes) => void; handleCommand: (text: string) => void; @@ -50,78 +47,12 @@ export function useSuggestion(editorRef: React.RefObject): { // TODO handle the mentions (@user, #room etc) const handleMention = (): void => {}; - const handleCommand = (replacementText: string): void => { - // if we do not have any of the values we need to do the work, do nothing - if ( - suggestion === null || - editorRef.current === null || - suggestion.node === null || - suggestion.node.textContent === null - ) { - return; - } - - const { node } = suggestion; - - // for a command we know we start at the beginning of the text node, so build the replacement - // string (note trailing space) and manually adjust the node's textcontent - const newContent = `${replacementText} `; - node.textContent = newContent; - - // then set the cursor to the end of the node, fire an inputEvent to update the hook state - // and then clear the suggestion from state - document.getSelection()?.setBaseAndExtent(node, newContent.length, node, newContent.length); - editorRef.current.dispatchEvent(new Event("inputEvent")); - setSuggestion(null); - }; - // We create a `seletionchange` handler here because we need to know when the user has moved the cursor, // we can not depend on input events only - const onSelect = (): void => { - if (editorRef.current === null) { - return; - } - const selection = document.getSelection(); + const onSelect = (): void => processSelectionChange(editorRef, suggestion, setSuggestion); - // only carry out the checking if we have cursor inside a text node - if (selection && selection.isCollapsed && selection.anchorNode?.nodeName === "#text") { - // here we have established that anchorNode === focusNode, so rename to - const { anchorNode: currentNode } = selection; - - // first check is that the text node is the first text node of the editor, as adding paragraphs can result - // in nested

tags inside the editor

- const firstTextNode = document.createNodeIterator(editorRef.current, NodeFilter.SHOW_TEXT).nextNode(); - - // if we're not in the first text node or we have no text content, return - if (currentNode !== firstTextNode || currentNode.textContent === null) { - return; - } - - // it's a command if: - // it is the first textnode AND - // it starts with /, not // AND - // then has letters all the way up to the end of the textcontent - const commandRegex = /^\/(\w*)$/; - const commandMatches = currentNode.textContent.match(commandRegex); - - // if we don't have any matches, return, clearing the suggesiton state if it is non-null - if (commandMatches === null) { - if (suggestion !== null) { - setSuggestion(null); - } - return; - } else { - setSuggestion({ - keyChar: "/", - type: "command", - text: commandMatches[1], - node: selection.anchorNode, - startOffset: 0, - endOffset: currentNode.textContent.length, - }); - } - } - }; + const handleCommand = (replacementText: string): void => + processCommand(replacementText, editorRef, suggestion, setSuggestion); return { suggestion: mapSuggestion(suggestion), @@ -130,3 +61,106 @@ export function useSuggestion(editorRef: React.RefObject): { onSelect, }; } + +/** + * Convert a PlainTextSuggestionPattern (or null) to a MappedSuggestion (or null) + * + * @param suggestion - the suggestion that is the JS equivalent of the rust model's representation + * @returns - null if the input is null, a MappedSuggestion if the input is non-null + * + */ +const mapSuggestion = (suggestion: PlainTextSuggestionPattern | null): MappedSuggestion | null => { + if (suggestion === null) { + return null; + } else { + const { node, startOffset, endOffset, ...mappedSuggestion } = suggestion; + return mappedSuggestion; + } +}; + +/** + * When a command is selected from the autocomplete, this function allows us to replace the relevant part + * of the editor text with the replacement text + * + * @param replacementText - the text that we will insert into the DOM + * @param suggestion - representation of the part of the DOM that will be replaced + */ +export const processCommand = ( + replacementText: string, + editorRef: React.RefObject, + suggestion: PlainTextSuggestionPattern | null, + setSuggestion: React.Dispatch>, +): void => { + // if we do not have any of the values we need to do the work, do nothing + if ( + suggestion === null || + editorRef.current === null || + suggestion.node === null || + suggestion.node.textContent === null + ) { + return; + } + + const { node } = suggestion; + + // for a command we know we start at the beginning of the text node, so build the replacement + // string (note trailing space) and manually adjust the node's textcontent + const newContent = `${replacementText} `; + node.textContent = newContent; + + // then set the cursor to the end of the node, fire an inputEvent to update the hook state + // and then clear the suggestion from state + document.getSelection()?.setBaseAndExtent(node, newContent.length, node, newContent.length); + editorRef.current.dispatchEvent(new Event("change")); + setSuggestion(null); +}; + +export const processSelectionChange = ( + editorRef: React.RefObject, + suggestion: PlainTextSuggestionPattern | null, + setSuggestion: React.Dispatch>, +): void => { + if (editorRef.current === null) { + return; + } + const selection = document.getSelection(); + + // only carry out the checking if we have cursor inside a text node + if (selection && selection.isCollapsed && selection.anchorNode?.nodeName === "#text") { + // here we have established that anchorNode === focusNode, so rename to + const { anchorNode: currentNode } = selection; + + // first check is that the text node is the first text node of the editor, as adding paragraphs can result + // in nested

tags inside the editor

+ const firstTextNode = document.createNodeIterator(editorRef.current, NodeFilter.SHOW_TEXT).nextNode(); + + // if we're not in the first text node or we have no text content, return + if (currentNode !== firstTextNode || currentNode.textContent === null) { + return; + } + + // it's a command if: + // it is the first textnode AND + // it starts with /, not // AND + // then has letters all the way up to the end of the textcontent + const commandRegex = /^\/(\w*)$/; + const commandMatches = currentNode.textContent.match(commandRegex); + + // if we don't have any matches, return, clearing the suggesiton state if it is non-null + if (commandMatches === null) { + if (suggestion !== null) { + setSuggestion(null); + } + return; + } else { + setSuggestion({ + keyChar: "/", + type: "command", + text: commandMatches[1], + node: selection.anchorNode, + startOffset: 0, + endOffset: currentNode.textContent.length, + }); + } + } +}; From cc357ae8ea5bd810c28b82961a66d7af7d68a73b Mon Sep 17 00:00:00 2001 From: Alun Turner Date: Fri, 14 Apr 2023 17:12:38 +0100 Subject: [PATCH 28/61] add first tests --- .../wysiwyg_composer/hooks/useSuggestion.ts | 4 +- .../hooks/useSuggestion-test.tsx | 49 +++++++++++++++++++ 2 files changed, 51 insertions(+), 2 deletions(-) create mode 100644 test/components/views/rooms/wysiwyg_composer/hooks/useSuggestion-test.tsx diff --git a/src/components/views/rooms/wysiwyg_composer/hooks/useSuggestion.ts b/src/components/views/rooms/wysiwyg_composer/hooks/useSuggestion.ts index 066be6cc13d..2431aa9542e 100644 --- a/src/components/views/rooms/wysiwyg_composer/hooks/useSuggestion.ts +++ b/src/components/views/rooms/wysiwyg_composer/hooks/useSuggestion.ts @@ -20,7 +20,7 @@ import { SyntheticEvent, useState } from "react"; // This type is a close approximation of what we use in the Rust model for the rich version of the // editor, we use this to try and make this simple model as similar as possible to allow reuse of // the autocomplete component -type PlainTextSuggestionPattern = { +export type PlainTextSuggestionPattern = { keyChar: SuggestionChar; type: SuggestionType; text: string; @@ -69,7 +69,7 @@ export function useSuggestion(editorRef: React.RefObject): { * @returns - null if the input is null, a MappedSuggestion if the input is non-null * */ -const mapSuggestion = (suggestion: PlainTextSuggestionPattern | null): MappedSuggestion | null => { +export const mapSuggestion = (suggestion: PlainTextSuggestionPattern | null): MappedSuggestion | null => { if (suggestion === null) { return null; } else { diff --git a/test/components/views/rooms/wysiwyg_composer/hooks/useSuggestion-test.tsx b/test/components/views/rooms/wysiwyg_composer/hooks/useSuggestion-test.tsx new file mode 100644 index 00000000000..ffbd5de1dc8 --- /dev/null +++ b/test/components/views/rooms/wysiwyg_composer/hooks/useSuggestion-test.tsx @@ -0,0 +1,49 @@ +/* +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 { MappedSuggestion } from "@matrix-org/matrix-wysiwyg"; + +import { + PlainTextSuggestionPattern, + mapSuggestion, +} from "../../../../../../src/components/views/rooms/wysiwyg_composer/hooks/useSuggestion"; + +describe("mapSuggestion", () => { + it("returns null if called with a null argument", () => { + expect(mapSuggestion(null)).toBeNull(); + }); + + it("returns a mapped suggestion when passed a suggestion", () => { + const input: PlainTextSuggestionPattern = { + keyChar: "/", + type: "command", + text: "some text", + node: document.createTextNode(""), + startOffset: 0, + endOffset: 0, + }; + + const expected: MappedSuggestion = { + keyChar: "/", + type: "command", + text: "some text", + }; + + const output = mapSuggestion(input); + + expect(output).toEqual(expected); + }); +}); From aac6630c1fe20e7adaaff762c9e6cb11623abbf8 Mon Sep 17 00:00:00 2001 From: Alun Turner Date: Tue, 18 Apr 2023 08:42:47 +0100 Subject: [PATCH 29/61] improve tests --- .../wysiwyg_composer/hooks/useSuggestion.ts | 12 ++- .../hooks/useSuggestion-test.tsx | 84 +++++++++++++++++-- 2 files changed, 83 insertions(+), 13 deletions(-) diff --git a/src/components/views/rooms/wysiwyg_composer/hooks/useSuggestion.ts b/src/components/views/rooms/wysiwyg_composer/hooks/useSuggestion.ts index 2431aa9542e..5fa16ac9eb6 100644 --- a/src/components/views/rooms/wysiwyg_composer/hooks/useSuggestion.ts +++ b/src/components/views/rooms/wysiwyg_composer/hooks/useSuggestion.ts @@ -94,9 +94,9 @@ export const processCommand = ( // if we do not have any of the values we need to do the work, do nothing if ( suggestion === null || - editorRef.current === null || - suggestion.node === null || - suggestion.node.textContent === null + editorRef.current === null + // suggestion.node === null || // don't think these can be hit? + // suggestion.node.textContent === null ) { return; } @@ -111,7 +111,11 @@ export const processCommand = ( // then set the cursor to the end of the node, fire an inputEvent to update the hook state // and then clear the suggestion from state document.getSelection()?.setBaseAndExtent(node, newContent.length, node, newContent.length); - editorRef.current.dispatchEvent(new Event("change")); + // this isn't quite right, we still need to figure out how to get the state updated + // in the useListeners hook + const inputEvent = new Event("input"); + editorRef.current.dispatchEvent(inputEvent); + console.log("<<< firing an event,", editorRef.current, inputEvent); setSuggestion(null); }; 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 ffbd5de1dc8..38921098d9b 100644 --- a/test/components/views/rooms/wysiwyg_composer/hooks/useSuggestion-test.tsx +++ b/test/components/views/rooms/wysiwyg_composer/hooks/useSuggestion-test.tsx @@ -13,28 +13,35 @@ 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 React from "react"; import { MappedSuggestion } from "@matrix-org/matrix-wysiwyg"; import { PlainTextSuggestionPattern, mapSuggestion, + processCommand, } from "../../../../../../src/components/views/rooms/wysiwyg_composer/hooks/useSuggestion"; +const createMockPlainTextSuggestionPattern = ( + props: Partial = {}, +): PlainTextSuggestionPattern => { + return { + keyChar: "/", + type: "command", + text: "some text", + node: document.createTextNode(""), + startOffset: 0, + endOffset: 0, + ...props, + }; +}; describe("mapSuggestion", () => { it("returns null if called with a null argument", () => { expect(mapSuggestion(null)).toBeNull(); }); it("returns a mapped suggestion when passed a suggestion", () => { - const input: PlainTextSuggestionPattern = { - keyChar: "/", - type: "command", - text: "some text", - node: document.createTextNode(""), - startOffset: 0, - endOffset: 0, - }; + const input = createMockPlainTextSuggestionPattern(); const expected: MappedSuggestion = { keyChar: "/", @@ -47,3 +54,62 @@ describe("mapSuggestion", () => { expect(output).toEqual(expected); }); }); + +describe("processCommand", () => { + it("does not change suggestion node text content if suggestion or editor ref is null", () => { + // create a div and append a text node to it with some initial text + const editorDiv = document.createElement("div"); + const initialText = "text"; + const textNode = document.createTextNode(initialText); + editorDiv.appendChild(textNode); + + // create a mockSuggestion using the text node above + const mockSuggestion = createMockPlainTextSuggestionPattern({ node: textNode }); + const mockSetSuggestion = jest.fn(); + + // call the function with a null editorRef.current value + processCommand( + "should not be seen", + { current: null } as React.RefObject, + mockSuggestion, + mockSetSuggestion, + ); + + // check that the text has not changed + expect(textNode.textContent).toBe(initialText); + + // call the function with a valid editorRef.current, but a null suggestion + processCommand( + "should not be seen", + { current: editorDiv } as React.RefObject, + null, + mockSetSuggestion, + ); + + // check that the text has not changed + expect(textNode.textContent).toBe(initialText); + }); + + it("can change the text content of the suggestion node", () => { + // create a div and append a text node to it with some initial text + const editorDiv = document.createElement("div"); + const initialText = "text"; + const textNode = document.createTextNode(initialText); + editorDiv.appendChild(textNode); + + // create a mockSuggestion using the text node above + const mockSuggestion = createMockPlainTextSuggestionPattern({ node: textNode }); + const mockSetSuggestion = jest.fn(); + const replacementText = "/replacement text"; + + processCommand( + replacementText, + { current: editorDiv } as React.RefObject, + mockSuggestion, + mockSetSuggestion, + ); + + // check that the text has changed and includes a trailing space + expect(textNode.textContent).toBe(`${replacementText} `); + }); +}); From 7e9a23068151502929ce07dddb82c0b7b3e9f609 Mon Sep 17 00:00:00 2001 From: Alun Turner Date: Tue, 18 Apr 2023 08:43:41 +0100 Subject: [PATCH 30/61] remove console log --- .../views/rooms/wysiwyg_composer/hooks/useSuggestion.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/components/views/rooms/wysiwyg_composer/hooks/useSuggestion.ts b/src/components/views/rooms/wysiwyg_composer/hooks/useSuggestion.ts index 5fa16ac9eb6..e77a3c40cc3 100644 --- a/src/components/views/rooms/wysiwyg_composer/hooks/useSuggestion.ts +++ b/src/components/views/rooms/wysiwyg_composer/hooks/useSuggestion.ts @@ -115,7 +115,6 @@ export const processCommand = ( // in the useListeners hook const inputEvent = new Event("input"); editorRef.current.dispatchEvent(inputEvent); - console.log("<<< firing an event,", editorRef.current, inputEvent); setSuggestion(null); }; From 2eb1203e3beb3b148a1d233e0eb78b5402611f6f Mon Sep 17 00:00:00 2001 From: Alun Turner Date: Tue, 18 Apr 2023 09:15:59 +0100 Subject: [PATCH 31/61] call useSuggestion hook from different location --- .../components/PlainTextComposer.tsx | 6 ++++-- .../hooks/usePlainTextListeners.ts | 15 +++++++++++++++ .../wysiwyg_composer/hooks/useSuggestion.ts | 19 ++++++++++--------- 3 files changed, 29 insertions(+), 11 deletions(-) diff --git a/src/components/views/rooms/wysiwyg_composer/components/PlainTextComposer.tsx b/src/components/views/rooms/wysiwyg_composer/components/PlainTextComposer.tsx index 17972a5fa54..db9a7733edb 100644 --- a/src/components/views/rooms/wysiwyg_composer/components/PlainTextComposer.tsx +++ b/src/components/views/rooms/wysiwyg_composer/components/PlainTextComposer.tsx @@ -60,10 +60,12 @@ export function PlainTextComposer({ onKeyDown, content, setContent, + suggestion, + onSelect, + handleCommand, + handleMention, } = usePlainTextListeners(autocompleteRef, initialContent, onChange, onSend); - const { suggestion, onSelect, handleCommand, handleMention } = useSuggestion(editorRef); - const composerFunctions = useComposerFunctions(editorRef, setContent); usePlainTextInitialization(initialContent, editorRef); useSetCursorPosition(disabled, editorRef); diff --git a/src/components/views/rooms/wysiwyg_composer/hooks/usePlainTextListeners.ts b/src/components/views/rooms/wysiwyg_composer/hooks/usePlainTextListeners.ts index a8a86ce5c2e..216c272662a 100644 --- a/src/components/views/rooms/wysiwyg_composer/hooks/usePlainTextListeners.ts +++ b/src/components/views/rooms/wysiwyg_composer/hooks/usePlainTextListeners.ts @@ -15,11 +15,13 @@ limitations under the License. */ import { KeyboardEvent, RefObject, SyntheticEvent, useCallback, useRef, useState } from "react"; +import { Attributes, MappedSuggestion } from "@matrix-org/matrix-wysiwyg"; import { useSettingValue } from "../../../../../hooks/useSettings"; import { IS_MAC, Key } from "../../../../../Keyboard"; import Autocomplete from "../../Autocomplete"; import { handleEventWithAutocomplete } from "./utils"; +import { useSuggestion } from "./useSuggestion"; function isDivElement(target: EventTarget): target is HTMLDivElement { return target instanceof HTMLDivElement; @@ -47,6 +49,10 @@ export function usePlainTextListeners( onPaste(event: SyntheticEvent): void; onKeyDown(event: KeyboardEvent): void; setContent(text: string): void; + handleMention: (link: string, text: string, attributes: Attributes) => void; + handleCommand: (text: string) => void; + onSelect: (event: SyntheticEvent) => void; + suggestion: MappedSuggestion | null; } { const ref = useRef(null); const [content, setContent] = useState(initialContent); @@ -66,6 +72,11 @@ export function usePlainTextListeners( [onChange], ); + // For separation of concerns, the suggestion handling is kept in a separate hook but is + // nested here because we do need to be able to update the `content` state in this hook + // when a user selects a suggestion from the autocomplete menu + const { suggestion, onSelect, handleCommand, handleMention } = useSuggestion(ref, setText); + const enterShouldSend = !useSettingValue("MessageComposerInput.ctrlEnterToSend"); const onInput = useCallback( (event: SyntheticEvent) => { @@ -115,5 +126,9 @@ export function usePlainTextListeners( onKeyDown, content, setContent: setText, + suggestion, + onSelect, + handleCommand, + handleMention, }; } diff --git a/src/components/views/rooms/wysiwyg_composer/hooks/useSuggestion.ts b/src/components/views/rooms/wysiwyg_composer/hooks/useSuggestion.ts index e77a3c40cc3..5f93bc89a52 100644 --- a/src/components/views/rooms/wysiwyg_composer/hooks/useSuggestion.ts +++ b/src/components/views/rooms/wysiwyg_composer/hooks/useSuggestion.ts @@ -36,7 +36,10 @@ export type PlainTextSuggestionPattern = { * * @param editorRef - a ref to the div that is the composer textbox */ -export function useSuggestion(editorRef: React.RefObject): { +export function useSuggestion( + editorRef: React.RefObject, + setText: (text: string) => void, +): { handleMention: (link: string, text: string, attributes: Attributes) => void; handleCommand: (text: string) => void; onSelect: (event: SyntheticEvent) => void; @@ -52,7 +55,7 @@ export function useSuggestion(editorRef: React.RefObject): { const onSelect = (): void => processSelectionChange(editorRef, suggestion, setSuggestion); const handleCommand = (replacementText: string): void => - processCommand(replacementText, editorRef, suggestion, setSuggestion); + processCommand(replacementText, editorRef, suggestion, setSuggestion, setText); return { suggestion: mapSuggestion(suggestion), @@ -90,6 +93,7 @@ export const processCommand = ( editorRef: React.RefObject, suggestion: PlainTextSuggestionPattern | null, setSuggestion: React.Dispatch>, + setText: (text: string) => void, ): void => { // if we do not have any of the values we need to do the work, do nothing if ( @@ -108,13 +112,10 @@ export const processCommand = ( const newContent = `${replacementText} `; node.textContent = newContent; - // then set the cursor to the end of the node, fire an inputEvent to update the hook state - // and then clear the suggestion from state + // then set the cursor to the end of the node, update the `content` state in the usePlainTextListeners + // hook and clear the suggestion from state document.getSelection()?.setBaseAndExtent(node, newContent.length, node, newContent.length); - // this isn't quite right, we still need to figure out how to get the state updated - // in the useListeners hook - const inputEvent = new Event("input"); - editorRef.current.dispatchEvent(inputEvent); + setText(newContent); setSuggestion(null); }; @@ -149,7 +150,7 @@ export const processSelectionChange = ( const commandRegex = /^\/(\w*)$/; const commandMatches = currentNode.textContent.match(commandRegex); - // if we don't have any matches, return, clearing the suggesiton state if it is non-null + // if we don't have any matches, return, clearing the suggeston state if it is non-null if (commandMatches === null) { if (suggestion !== null) { setSuggestion(null); From 3ad7c45a65875a86d0240be424c8938a0dfbb250 Mon Sep 17 00:00:00 2001 From: Alun Turner Date: Tue, 18 Apr 2023 09:16:07 +0100 Subject: [PATCH 32/61] update useSuggestion hook tests --- .../hooks/useSuggestion-test.tsx | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) 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 38921098d9b..9672859b4f5 100644 --- a/test/components/views/rooms/wysiwyg_composer/hooks/useSuggestion-test.tsx +++ b/test/components/views/rooms/wysiwyg_composer/hooks/useSuggestion-test.tsx @@ -56,7 +56,7 @@ describe("mapSuggestion", () => { }); describe("processCommand", () => { - it("does not change suggestion node text content if suggestion or editor ref is null", () => { + it("does not change parent hook state if suggestion or editor ref is null", () => { // create a div and append a text node to it with some initial text const editorDiv = document.createElement("div"); const initialText = "text"; @@ -66,6 +66,7 @@ describe("processCommand", () => { // create a mockSuggestion using the text node above const mockSuggestion = createMockPlainTextSuggestionPattern({ node: textNode }); const mockSetSuggestion = jest.fn(); + const mockSetText = jest.fn(); // call the function with a null editorRef.current value processCommand( @@ -73,10 +74,11 @@ describe("processCommand", () => { { current: null } as React.RefObject, mockSuggestion, mockSetSuggestion, + mockSetText, ); - // check that the text has not changed - expect(textNode.textContent).toBe(initialText); + // check that the parent state setter has not been called + expect(mockSetText).not.toHaveBeenCalled(); // call the function with a valid editorRef.current, but a null suggestion processCommand( @@ -84,13 +86,14 @@ describe("processCommand", () => { { current: editorDiv } as React.RefObject, null, mockSetSuggestion, + mockSetText, ); - // check that the text has not changed - expect(textNode.textContent).toBe(initialText); + // check that the parent state setter has not been called + expect(mockSetText).not.toHaveBeenCalled(); }); - it("can change the text content of the suggestion node", () => { + it("can change the parent hook state when required", () => { // create a div and append a text node to it with some initial text const editorDiv = document.createElement("div"); const initialText = "text"; @@ -100,6 +103,7 @@ describe("processCommand", () => { // create a mockSuggestion using the text node above const mockSuggestion = createMockPlainTextSuggestionPattern({ node: textNode }); const mockSetSuggestion = jest.fn(); + const mockSetText = jest.fn(); const replacementText = "/replacement text"; processCommand( @@ -107,9 +111,10 @@ describe("processCommand", () => { { current: editorDiv } as React.RefObject, mockSuggestion, mockSetSuggestion, + mockSetText, ); // check that the text has changed and includes a trailing space - expect(textNode.textContent).toBe(`${replacementText} `); + expect(mockSetText).toHaveBeenCalledWith(`${replacementText} `); }); }); From 9db2a45940815fe335071ab8f913521c4b3c6e5a Mon Sep 17 00:00:00 2001 From: Alun Turner Date: Tue, 18 Apr 2023 09:35:30 +0100 Subject: [PATCH 33/61] improve cypress tests --- cypress/e2e/composer/composer.spec.ts | 64 +++++++++++++++++++++++++-- 1 file changed, 61 insertions(+), 3 deletions(-) diff --git a/cypress/e2e/composer/composer.spec.ts b/cypress/e2e/composer/composer.spec.ts index 7dc911ba158..e7094484a94 100644 --- a/cypress/e2e/composer/composer.spec.ts +++ b/cypress/e2e/composer/composer.spec.ts @@ -118,8 +118,10 @@ describe("Composer", () => { }); describe("commands", () => { + // TODO add tests for rich text mode + describe("plain text mode", () => { - it("autocomplete opens when / is entered and is populated", () => { + it("autocomplete opens when / is pressed and contains autocomplete items", () => { // Select plain text mode after composer is ready cy.get("div[contenteditable=true]").should("exist"); cy.findByRole("button", { name: "Hide formatting" }).click(); @@ -153,6 +155,64 @@ describe("Composer", () => { }); }); + it("autocomplete can be used to write and send a command that takes arguments", () => { + // Select plain text mode after composer is ready + cy.get("div[contenteditable=true]").should("exist"); + cy.findByRole("button", { name: "Hide formatting" }).click(); + + // Type a message + cy.get("div[contenteditable=true]").type("/spo"); + + // Check that the autocomplete /spoiler option is visible and click it + cy.findByTestId("autocomplete-wrapper").within(() => { + cy.findByText("/spoiler").click(); + }); + + // This should close the autocomplete and have inserted with the completion text + cy.findByTestId("autocomplete-wrapper").should("not.be.visible"); + cy.findByRole("textbox").within(() => { + cy.findByText("/spoiler").should("exist"); + }); + + // Type some more text then send the message + const argumentText = "this is the spoiler text"; + cy.get("div[contenteditable=true]").type(argumentText); + cy.findByRole("button", { name: "Send message" }).click(); + + // Check that a spoiler item has appeared in the timeline and contains the spoiler command text + cy.get("mx_EventTile_Spoiler").within(() => { + cy.findByText("this is the spoiler text").should("exist"); + }); + }); + + it("autocomplete can be used to write and send a command that takes no arguments", () => { + // Select plain text mode after composer is ready + cy.get("div[contenteditable=true]").should("exist"); + cy.findByRole("button", { name: "Hide formatting" }).click(); + + // Type a message + cy.get("div[contenteditable=true]").type("/dev"); + + // Check that the autocomplete /spoiler option is visible and click it + cy.findByTestId("autocomplete-wrapper").within(() => { + cy.findByText("/devtools").click(); + }); + + // This should close the autocomplete and have inserted with the completion text + cy.findByTestId("autocomplete-wrapper").should("not.be.visible"); + cy.findByRole("textbox").within(() => { + cy.findByText("/devtools").should("exist"); + }); + + // Click the send message button + cy.findByRole("button", { name: "Send message" }).click(); + + // Check that the devtools dialog menut has appeared + cy.findByRole("dialog").within(() => { + cy.findByText("Developer Tools").should("exist"); + }); + }); + it("autocomplete is not displayed for a message starting with //", () => { // Select plain text mode after composer is ready cy.get("div[contenteditable=true]").should("exist"); @@ -167,8 +227,6 @@ describe("Composer", () => { }); }); }); - - // TODO add tests for rich text mode }); it("sends a message when you click send or press Enter", () => { From f83d0746007faef5470d4dd127d2921f0ab36729 Mon Sep 17 00:00:00 2001 From: Alun Turner Date: Tue, 18 Apr 2023 09:41:30 +0100 Subject: [PATCH 34/61] remove unused import --- .../rooms/wysiwyg_composer/components/PlainTextComposer.tsx | 1 - 1 file changed, 1 deletion(-) diff --git a/src/components/views/rooms/wysiwyg_composer/components/PlainTextComposer.tsx b/src/components/views/rooms/wysiwyg_composer/components/PlainTextComposer.tsx index db9a7733edb..6c5e3bd03d4 100644 --- a/src/components/views/rooms/wysiwyg_composer/components/PlainTextComposer.tsx +++ b/src/components/views/rooms/wysiwyg_composer/components/PlainTextComposer.tsx @@ -26,7 +26,6 @@ import { ComposerFunctions } from "../types"; import { Editor } from "./Editor"; import Autocomplete from "../../Autocomplete"; import { WysiwygAutocomplete } from "./WysiwygAutocomplete"; -import { useSuggestion } from "../hooks/useSuggestion"; interface PlainTextComposerProps { disabled?: boolean; From 368ce13bd541448d5617759d6bb98fac620978f4 Mon Sep 17 00:00:00 2001 From: Alun Turner Date: Tue, 18 Apr 2023 10:12:19 +0100 Subject: [PATCH 35/61] fix selector in cypress test --- cypress/e2e/composer/composer.spec.ts | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/cypress/e2e/composer/composer.spec.ts b/cypress/e2e/composer/composer.spec.ts index e7094484a94..0a0a5a0f644 100644 --- a/cypress/e2e/composer/composer.spec.ts +++ b/cypress/e2e/composer/composer.spec.ts @@ -120,7 +120,7 @@ describe("Composer", () => { describe("commands", () => { // TODO add tests for rich text mode - describe("plain text mode", () => { + describe.only("plain text mode", () => { it("autocomplete opens when / is pressed and contains autocomplete items", () => { // Select plain text mode after composer is ready cy.get("div[contenteditable=true]").should("exist"); @@ -180,9 +180,8 @@ describe("Composer", () => { cy.findByRole("button", { name: "Send message" }).click(); // Check that a spoiler item has appeared in the timeline and contains the spoiler command text - cy.get("mx_EventTile_Spoiler").within(() => { - cy.findByText("this is the spoiler text").should("exist"); - }); + cy.get("span.mx_EventTile_spoiler").should("exist"); + cy.findByText("this is the spoiler text").should("exist"); }); it("autocomplete can be used to write and send a command that takes no arguments", () => { From 449521a120d5fb2811a9bca36e0c54dd2ec55325 Mon Sep 17 00:00:00 2001 From: Alun Turner Date: Tue, 18 Apr 2023 10:43:57 +0100 Subject: [PATCH 36/61] add another set of util tests --- .../hooks/useSuggestion-test.tsx | 76 +++++++++++++++++++ 1 file changed, 76 insertions(+) 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 9672859b4f5..767e37c6bd7 100644 --- a/test/components/views/rooms/wysiwyg_composer/hooks/useSuggestion-test.tsx +++ b/test/components/views/rooms/wysiwyg_composer/hooks/useSuggestion-test.tsx @@ -20,7 +20,9 @@ import { PlainTextSuggestionPattern, mapSuggestion, processCommand, + processSelectionChange, } from "../../../../../../src/components/views/rooms/wysiwyg_composer/hooks/useSuggestion"; +import { text } from "stream/consumers"; const createMockPlainTextSuggestionPattern = ( props: Partial = {}, @@ -118,3 +120,77 @@ describe("processCommand", () => { expect(mockSetText).toHaveBeenCalledWith(`${replacementText} `); }); }); + +describe.only("processSelectionChange", () => { + it("returns early if current editorRef is null", () => { + const mockEditorRef = { current: null } as React.RefObject; + const getSelectionSpy = jest.spyOn(document, "getSelection"); + processSelectionChange(mockEditorRef, null, jest.fn()); + expect(getSelectionSpy).not.toHaveBeenCalled(); + }); + + it("does not call setSuggestion if selection is not a cursor", () => { + const mockEditor = document.createElement("div"); + const textContent = document.createTextNode("content"); + mockEditor.appendChild(textContent); + const mockSetSuggestion = jest.fn(); + const mockEditorRef = { current: mockEditor } as React.RefObject; + + document.getSelection()?.setBaseAndExtent(textContent, 0, textContent, 4); + + processSelectionChange(mockEditorRef, createMockPlainTextSuggestionPattern(), mockSetSuggestion); + + expect(mockSetSuggestion).not.toHaveBeenCalled(); + }); + + it("does not call setSuggestion if selection cursor is not inside a text node", () => { + const mockEditor = document.createElement("div"); + const textContent = document.createTextNode("content"); + mockEditor.appendChild(textContent); + const mockSetSuggestion = jest.fn(); + const mockEditorRef = { current: mockEditor } as React.RefObject; + + document.getSelection()?.setBaseAndExtent(mockEditor, 0, mockEditor, 0); + + processSelectionChange(mockEditorRef, createMockPlainTextSuggestionPattern(), mockSetSuggestion); + + expect(mockSetSuggestion).not.toHaveBeenCalled(); + }); + + it("calls setSuggestion with null if we have an existing suggestion but no command match", () => { + const mockEditor = document.createElement("div"); + const textNode = document.createTextNode("this will not match"); + mockEditor.appendChild(textNode); + document.body.appendChild(mockEditor); + const mockSetSuggestion = jest.fn(); + const mockEditorRef = { current: mockEditor } as React.RefObject; + + document.getSelection()?.setBaseAndExtent(textNode, 0, textNode, 0); + + processSelectionChange(mockEditorRef, createMockPlainTextSuggestionPattern(), mockSetSuggestion); + + expect(mockSetSuggestion).toHaveBeenCalledWith(null); + }); + + it("calls setSuggestion with the expected arguments when text node is valid command", () => { + const mockEditor = document.createElement("div"); + const textNode = document.createTextNode("/potentialCommand"); + mockEditor.appendChild(textNode); + document.body.appendChild(mockEditor); + const mockSetSuggestion = jest.fn(); + const mockEditorRef = { current: mockEditor } as React.RefObject; + + document.getSelection()?.setBaseAndExtent(textNode, 3, textNode, 3); + + processSelectionChange(mockEditorRef, null, mockSetSuggestion); + + expect(mockSetSuggestion).toHaveBeenCalledWith({ + keyChar: "/", + type: "command", + text: "potentialCommand", + node: textNode, + startOffset: 0, + endOffset: 17, + }); + }); +}); From c29d94545b3a35a51aca13fd243c8022d54bbdec Mon Sep 17 00:00:00 2001 From: Alun Turner Date: Tue, 18 Apr 2023 10:44:07 +0100 Subject: [PATCH 37/61] remove .only --- .../views/rooms/wysiwyg_composer/hooks/useSuggestion-test.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 767e37c6bd7..12bb4ea60f8 100644 --- a/test/components/views/rooms/wysiwyg_composer/hooks/useSuggestion-test.tsx +++ b/test/components/views/rooms/wysiwyg_composer/hooks/useSuggestion-test.tsx @@ -121,7 +121,7 @@ describe("processCommand", () => { }); }); -describe.only("processSelectionChange", () => { +describe("processSelectionChange", () => { it("returns early if current editorRef is null", () => { const mockEditorRef = { current: null } as React.RefObject; const getSelectionSpy = jest.spyOn(document, "getSelection"); From db4308420b74d00ed9fe10f5e89fd303de854efa Mon Sep 17 00:00:00 2001 From: Alun Turner Date: Tue, 18 Apr 2023 10:47:50 +0100 Subject: [PATCH 38/61] remove .only --- cypress/e2e/composer/composer.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cypress/e2e/composer/composer.spec.ts b/cypress/e2e/composer/composer.spec.ts index 0a0a5a0f644..44900c793a7 100644 --- a/cypress/e2e/composer/composer.spec.ts +++ b/cypress/e2e/composer/composer.spec.ts @@ -120,7 +120,7 @@ describe("Composer", () => { describe("commands", () => { // TODO add tests for rich text mode - describe.only("plain text mode", () => { + describe("plain text mode", () => { it("autocomplete opens when / is pressed and contains autocomplete items", () => { // Select plain text mode after composer is ready cy.get("div[contenteditable=true]").should("exist"); From c7d653ee2d631f0601f137c6a80109b6aae7c7a7 Mon Sep 17 00:00:00 2001 From: Alun Turner Date: Tue, 18 Apr 2023 10:55:23 +0100 Subject: [PATCH 39/61] remove import --- .../views/rooms/wysiwyg_composer/hooks/useSuggestion-test.tsx | 1 - 1 file changed, 1 deletion(-) 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 12bb4ea60f8..e8b97783b70 100644 --- a/test/components/views/rooms/wysiwyg_composer/hooks/useSuggestion-test.tsx +++ b/test/components/views/rooms/wysiwyg_composer/hooks/useSuggestion-test.tsx @@ -22,7 +22,6 @@ import { processCommand, processSelectionChange, } from "../../../../../../src/components/views/rooms/wysiwyg_composer/hooks/useSuggestion"; -import { text } from "stream/consumers"; const createMockPlainTextSuggestionPattern = ( props: Partial = {}, From b52eeb2729b1087f9f6c4cded30674e1f92dc418 Mon Sep 17 00:00:00 2001 From: Alun Turner Date: Tue, 18 Apr 2023 11:53:27 +0100 Subject: [PATCH 40/61] improve cypress tests --- cypress/e2e/composer/composer.spec.ts | 36 +++++++++++++++++++-------- 1 file changed, 25 insertions(+), 11 deletions(-) diff --git a/cypress/e2e/composer/composer.spec.ts b/cypress/e2e/composer/composer.spec.ts index 44900c793a7..c6c08f84a66 100644 --- a/cypress/e2e/composer/composer.spec.ts +++ b/cypress/e2e/composer/composer.spec.ts @@ -120,14 +120,14 @@ describe("Composer", () => { describe("commands", () => { // TODO add tests for rich text mode - describe("plain text mode", () => { + describe.only("plain text mode", () => { it("autocomplete opens when / is pressed and contains autocomplete items", () => { // Select plain text mode after composer is ready cy.get("div[contenteditable=true]").should("exist"); cy.findByRole("button", { name: "Hide formatting" }).click(); // Type a / - cy.get("div[contenteditable=true]").type("/"); + cy.findByRole("textbox").type("/"); // Check that the autocomplete options are visible and there are more than 0 cy.findByTestId("autocomplete-wrapper").within(() => { @@ -141,14 +141,14 @@ describe("Composer", () => { cy.findByRole("button", { name: "Hide formatting" }).click(); // Type a message - cy.get("div[contenteditable=true]").type("/spo"); + cy.findByRole("textbox").type("/spo"); // Check that the autocomplete /spoiler option is visible and click it cy.findByTestId("autocomplete-wrapper").within(() => { cy.findByText("/spoiler").click(); }); - // this should close the autocomplete and have inserted with the completion text + // Check the autocomplete is closed and the composer contains the completion cy.findByTestId("autocomplete-wrapper").should("not.be.visible"); cy.findByRole("textbox").within(() => { cy.findByText("/spoiler").should("exist"); @@ -161,14 +161,14 @@ describe("Composer", () => { cy.findByRole("button", { name: "Hide formatting" }).click(); // Type a message - cy.get("div[contenteditable=true]").type("/spo"); + cy.findByRole("textbox").type("/spo"); // Check that the autocomplete /spoiler option is visible and click it cy.findByTestId("autocomplete-wrapper").within(() => { cy.findByText("/spoiler").click(); }); - // This should close the autocomplete and have inserted with the completion text + // Check the autocomplete is closed and the composer contains the completion cy.findByTestId("autocomplete-wrapper").should("not.be.visible"); cy.findByRole("textbox").within(() => { cy.findByText("/spoiler").should("exist"); @@ -176,7 +176,7 @@ describe("Composer", () => { // Type some more text then send the message const argumentText = "this is the spoiler text"; - cy.get("div[contenteditable=true]").type(argumentText); + cy.findByRole("textbox").type(argumentText); cy.findByRole("button", { name: "Send message" }).click(); // Check that a spoiler item has appeared in the timeline and contains the spoiler command text @@ -190,14 +190,14 @@ describe("Composer", () => { cy.findByRole("button", { name: "Hide formatting" }).click(); // Type a message - cy.get("div[contenteditable=true]").type("/dev"); + cy.findByRole("textbox").type("/dev"); // Check that the autocomplete /spoiler option is visible and click it cy.findByTestId("autocomplete-wrapper").within(() => { cy.findByText("/devtools").click(); }); - // This should close the autocomplete and have inserted with the completion text + // Check the autocomplete is closed and the composer contains the completion cy.findByTestId("autocomplete-wrapper").should("not.be.visible"); cy.findByRole("textbox").within(() => { cy.findByText("/devtools").should("exist"); @@ -206,7 +206,7 @@ describe("Composer", () => { // Click the send message button cy.findByRole("button", { name: "Send message" }).click(); - // Check that the devtools dialog menut has appeared + // Check that the devtools dialog menu has appeared cy.findByRole("dialog").within(() => { cy.findByText("Developer Tools").should("exist"); }); @@ -218,7 +218,21 @@ describe("Composer", () => { cy.findByRole("button", { name: "Hide formatting" }).click(); // Type a message - cy.get("div[contenteditable=true]").type("//spo"); + cy.findByRole("textbox").type("//anyText"); + + // Check that the autocomplete options are not visible + cy.findByTestId("autocomplete-wrapper").within(() => { + cy.findAllByRole("presentation").should("have.length", 0); + }); + }); + + it("autocomplete is not displayed when user inserts whitespace after command", () => { + // Select plain text mode after composer is ready + cy.get("div[contenteditable=true]").should("exist"); + cy.findByRole("button", { name: "Hide formatting" }).click(); + + // Type a message + cy.findByRole("textbox").type("/spoiler followed by a space"); // Check that the autocomplete options are not visible cy.findByTestId("autocomplete-wrapper").within(() => { From 3198acbd1db5d66142deac1a585d6c0814eded56 Mon Sep 17 00:00:00 2001 From: Alun Turner Date: Tue, 18 Apr 2023 11:54:02 +0100 Subject: [PATCH 41/61] remove .only --- cypress/e2e/composer/composer.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cypress/e2e/composer/composer.spec.ts b/cypress/e2e/composer/composer.spec.ts index c6c08f84a66..dad12d24cd8 100644 --- a/cypress/e2e/composer/composer.spec.ts +++ b/cypress/e2e/composer/composer.spec.ts @@ -120,7 +120,7 @@ describe("Composer", () => { describe("commands", () => { // TODO add tests for rich text mode - describe.only("plain text mode", () => { + describe("plain text mode", () => { it("autocomplete opens when / is pressed and contains autocomplete items", () => { // Select plain text mode after composer is ready cy.get("div[contenteditable=true]").should("exist"); From 6647bb5ccf1b333355184eeb370afcf2cc2fe3ef Mon Sep 17 00:00:00 2001 From: Alun Turner Date: Tue, 18 Apr 2023 11:58:31 +0100 Subject: [PATCH 42/61] add comment --- .../views/rooms/wysiwyg_composer/hooks/usePlainTextListeners.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/components/views/rooms/wysiwyg_composer/hooks/usePlainTextListeners.ts b/src/components/views/rooms/wysiwyg_composer/hooks/usePlainTextListeners.ts index 216c272662a..c8d08265490 100644 --- a/src/components/views/rooms/wysiwyg_composer/hooks/usePlainTextListeners.ts +++ b/src/components/views/rooms/wysiwyg_composer/hooks/usePlainTextListeners.ts @@ -91,6 +91,7 @@ export function usePlainTextListeners( const onKeyDown = useCallback( (event: KeyboardEvent) => { + // we need autocomplete to take priority when it is open for using enter to select const isHandledByAutocomplete = handleEventWithAutocomplete(autocompleteRef, event); if (isHandledByAutocomplete) { return; From 750dea442ff4b2016d3b67eb60420ebd62d088ba Mon Sep 17 00:00:00 2001 From: Alun Turner Date: Tue, 18 Apr 2023 12:31:39 +0100 Subject: [PATCH 43/61] improve comments --- .../views/rooms/wysiwyg_composer/hooks/useSuggestion.ts | 7 +------ src/components/views/rooms/wysiwyg_composer/hooks/utils.ts | 2 +- 2 files changed, 2 insertions(+), 7 deletions(-) diff --git a/src/components/views/rooms/wysiwyg_composer/hooks/useSuggestion.ts b/src/components/views/rooms/wysiwyg_composer/hooks/useSuggestion.ts index 5f93bc89a52..436c66a10f8 100644 --- a/src/components/views/rooms/wysiwyg_composer/hooks/useSuggestion.ts +++ b/src/components/views/rooms/wysiwyg_composer/hooks/useSuggestion.ts @@ -96,12 +96,7 @@ export const processCommand = ( setText: (text: string) => void, ): void => { // if we do not have any of the values we need to do the work, do nothing - if ( - suggestion === null || - editorRef.current === null - // suggestion.node === null || // don't think these can be hit? - // suggestion.node.textContent === null - ) { + if (suggestion === null || editorRef.current === null) { return; } diff --git a/src/components/views/rooms/wysiwyg_composer/hooks/utils.ts b/src/components/views/rooms/wysiwyg_composer/hooks/utils.ts index 35febe6b778..b71b7de5211 100644 --- a/src/components/views/rooms/wysiwyg_composer/hooks/utils.ts +++ b/src/components/views/rooms/wysiwyg_composer/hooks/utils.ts @@ -66,7 +66,7 @@ export function setCursorPositionAtTheEnd(element: HTMLElement): void { */ export function handleEventWithAutocomplete( autocompleteRef: RefObject, - // we get a React Keyboard event from plain text, a Keyboard Event from the rich text + // we get a React Keyboard event from plain text composer, a Keyboard Event from the rich text composer event: KeyboardEvent | React.KeyboardEvent, ): boolean { const autocompleteIsOpen = autocompleteRef?.current && !autocompleteRef.current.state.hide; From a467a2bf345e34eb22aa117bdbaf7c2c4ff1730b Mon Sep 17 00:00:00 2001 From: Alun Turner Date: Tue, 18 Apr 2023 12:31:47 +0100 Subject: [PATCH 44/61] tidy up tests --- .../hooks/useSuggestion-test.tsx | 92 +++++++++++-------- 1 file changed, 54 insertions(+), 38 deletions(-) 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 e8b97783b70..8c712683d07 100644 --- a/test/components/views/rooms/wysiwyg_composer/hooks/useSuggestion-test.tsx +++ b/test/components/views/rooms/wysiwyg_composer/hooks/useSuggestion-test.tsx @@ -14,7 +14,6 @@ See the License for the specific language governing permissions and limitations under the License. */ import React from "react"; -import { MappedSuggestion } from "@matrix-org/matrix-wysiwyg"; import { PlainTextSuggestionPattern, @@ -42,22 +41,20 @@ describe("mapSuggestion", () => { }); it("returns a mapped suggestion when passed a suggestion", () => { - const input = createMockPlainTextSuggestionPattern(); - - const expected: MappedSuggestion = { - keyChar: "/", - type: "command", + const inputFields = { + keyChar: "/" as const, + type: "command" as const, text: "some text", }; - + const input = createMockPlainTextSuggestionPattern(inputFields); const output = mapSuggestion(input); - expect(output).toEqual(expected); + expect(output).toEqual(inputFields); }); }); describe("processCommand", () => { - it("does not change parent hook state if suggestion or editor ref is null", () => { + it("does not change parent hook state if editor ref or suggestion is null", () => { // create a div and append a text node to it with some initial text const editorDiv = document.createElement("div"); const initialText = "text"; @@ -121,75 +118,94 @@ describe("processCommand", () => { }); describe("processSelectionChange", () => { + function createMockEditorRef(element: HTMLDivElement | null = null): React.RefObject { + return { current: element } as React.RefObject; + } + + function appendEditorWithTextNodeContaining(initialText = ""): [HTMLDivElement, Node] { + // create the elements/nodes + const mockEditor = document.createElement("div"); + const textNode = document.createTextNode(initialText); + + // append text node to the editor, editor to the document body + mockEditor.appendChild(textNode); + document.body.appendChild(mockEditor); + + return [mockEditor, textNode]; + } + + const mockSetSuggestion = jest.fn(); + beforeEach(() => { + mockSetSuggestion.mockClear(); + }); + it("returns early if current editorRef is null", () => { - const mockEditorRef = { current: null } as React.RefObject; + const mockEditorRef = createMockEditorRef(null); + // we monitor for the call to document.getSelection to indicate an early return const getSelectionSpy = jest.spyOn(document, "getSelection"); + processSelectionChange(mockEditorRef, null, jest.fn()); expect(getSelectionSpy).not.toHaveBeenCalled(); + + // tidy up to avoid potential impacts on other tests + getSelectionSpy.mockRestore(); }); it("does not call setSuggestion if selection is not a cursor", () => { - const mockEditor = document.createElement("div"); - const textContent = document.createTextNode("content"); - mockEditor.appendChild(textContent); - const mockSetSuggestion = jest.fn(); - const mockEditorRef = { current: mockEditor } as React.RefObject; + const [mockEditor, textNode] = appendEditorWithTextNodeContaining("content"); + const mockEditorRef = createMockEditorRef(mockEditor); - document.getSelection()?.setBaseAndExtent(textContent, 0, textContent, 4); + // create a selection in the text node that has different start and end locations ie it + // is not a cursor + document.getSelection()?.setBaseAndExtent(textNode, 0, textNode, 4); + // process the selection and check that we do not attempt to set the suggestion processSelectionChange(mockEditorRef, createMockPlainTextSuggestionPattern(), mockSetSuggestion); - expect(mockSetSuggestion).not.toHaveBeenCalled(); }); it("does not call setSuggestion if selection cursor is not inside a text node", () => { - const mockEditor = document.createElement("div"); - const textContent = document.createTextNode("content"); - mockEditor.appendChild(textContent); - const mockSetSuggestion = jest.fn(); - const mockEditorRef = { current: mockEditor } as React.RefObject; + const [mockEditor] = appendEditorWithTextNodeContaining("content"); + const mockEditorRef = createMockEditorRef(mockEditor); + // create a selection that points at the editor element, not the text node it contains document.getSelection()?.setBaseAndExtent(mockEditor, 0, mockEditor, 0); + // process the selection and check that we do not attempt to set the suggestion processSelectionChange(mockEditorRef, createMockPlainTextSuggestionPattern(), mockSetSuggestion); - expect(mockSetSuggestion).not.toHaveBeenCalled(); }); it("calls setSuggestion with null if we have an existing suggestion but no command match", () => { - const mockEditor = document.createElement("div"); - const textNode = document.createTextNode("this will not match"); - mockEditor.appendChild(textNode); - document.body.appendChild(mockEditor); - const mockSetSuggestion = jest.fn(); - const mockEditorRef = { current: mockEditor } as React.RefObject; + const [mockEditor, textNode] = appendEditorWithTextNodeContaining("content"); + const mockEditorRef = createMockEditorRef(mockEditor); + // create a selection in the text node that has identical start and end locations, ie it is a cursor document.getSelection()?.setBaseAndExtent(textNode, 0, textNode, 0); + // the call to process the selection will have an existing suggestion in state due to the second + // argument being non-null, expect that we clear this suggestion now that the text is not a command processSelectionChange(mockEditorRef, createMockPlainTextSuggestionPattern(), mockSetSuggestion); - expect(mockSetSuggestion).toHaveBeenCalledWith(null); }); it("calls setSuggestion with the expected arguments when text node is valid command", () => { - const mockEditor = document.createElement("div"); - const textNode = document.createTextNode("/potentialCommand"); - mockEditor.appendChild(textNode); - document.body.appendChild(mockEditor); - const mockSetSuggestion = jest.fn(); - const mockEditorRef = { current: mockEditor } as React.RefObject; + const commandText = "/potentialCommand"; + const [mockEditor, textNode] = appendEditorWithTextNodeContaining(commandText); + const mockEditorRef = createMockEditorRef(mockEditor); + // create a selection in the text node that has identical start and end locations, ie it is a cursor document.getSelection()?.setBaseAndExtent(textNode, 3, textNode, 3); + // process the change and check the suggestion that is set looks as we expect it to processSelectionChange(mockEditorRef, null, mockSetSuggestion); - expect(mockSetSuggestion).toHaveBeenCalledWith({ keyChar: "/", type: "command", text: "potentialCommand", node: textNode, startOffset: 0, - endOffset: 17, + endOffset: commandText.length, }); }); }); From b3ca7305f2413aabd31c65b6263b41f7471e5e67 Mon Sep 17 00:00:00 2001 From: Alun Turner Date: Thu, 20 Apr 2023 12:22:03 +0100 Subject: [PATCH 45/61] consolidate all cypress tests to one --- cypress/e2e/composer/composer.spec.ts | 124 ++++++-------------------- 1 file changed, 29 insertions(+), 95 deletions(-) diff --git a/cypress/e2e/composer/composer.spec.ts b/cypress/e2e/composer/composer.spec.ts index dad12d24cd8..ff763b3b2eb 100644 --- a/cypress/e2e/composer/composer.spec.ts +++ b/cypress/e2e/composer/composer.spec.ts @@ -117,128 +117,62 @@ describe("Composer", () => { cy.viewRoomByName("Composing Room"); }); - describe("commands", () => { + describe("Commands", () => { // TODO add tests for rich text mode - describe("plain text mode", () => { - it("autocomplete opens when / is pressed and contains autocomplete items", () => { + describe("Plain text mode", () => { + it("autocomplete behaviour tests", () => { // Select plain text mode after composer is ready cy.get("div[contenteditable=true]").should("exist"); cy.findByRole("button", { name: "Hide formatting" }).click(); - // Type a / + // Typing a single / displays the autocomplete menu and contents cy.findByRole("textbox").type("/"); - // Check that the autocomplete options are visible and there are more than 0 - cy.findByTestId("autocomplete-wrapper").within(() => { - cy.findAllByRole("presentation").should("have.length.above", 0); - }); - }); - - it("autocomplete can be used to enter a command", () => { - // Select plain text mode after composer is ready - cy.get("div[contenteditable=true]").should("exist"); - cy.findByRole("button", { name: "Hide formatting" }).click(); + // Check that the autocomplete options are visible and there are more than 0 items + cy.findByTestId("autocomplete-wrapper").should("not.be.empty"); - // Type a message - cy.findByRole("textbox").type("/spo"); + // Entering `//` or `/ ` hides the autocomplete contents + cy.findByRole("textbox").type(" "); + cy.findByTestId("autocomplete-wrapper").should("be.empty"); + cy.findByRole("textbox").type("{Backspace}/"); + cy.findByTestId("autocomplete-wrapper").should("be.empty"); - // Check that the autocomplete /spoiler option is visible and click it + // Typing a command that takes no arguments (/devtools) and selecting by click works + cy.findByRole("textbox").type("{Backspace}dev"); cy.findByTestId("autocomplete-wrapper").within(() => { - cy.findByText("/spoiler").click(); + cy.findByText("/devtools").click(); }); - - // Check the autocomplete is closed and the composer contains the completion + // Check it has closed the autocomplete and put the text into the composer cy.findByTestId("autocomplete-wrapper").should("not.be.visible"); cy.findByRole("textbox").within(() => { - cy.findByText("/spoiler").should("exist"); + cy.findByText("/devtools").should("exist"); }); - }); - - it("autocomplete can be used to write and send a command that takes arguments", () => { - // Select plain text mode after composer is ready - cy.get("div[contenteditable=true]").should("exist"); - cy.findByRole("button", { name: "Hide formatting" }).click(); - - // Type a message - cy.findByRole("textbox").type("/spo"); + // Send the message and check the devtools dialog appeared, then close it + cy.findByRole("button", { name: "Send message" }).click(); + cy.findByRole("dialog").within(() => { + cy.findByText("Developer Tools").should("exist"); + }); + cy.findByRole("button", { name: "Close dialog" }).click(); - // Check that the autocomplete /spoiler option is visible and click it + // Typing a command that takes arguments (/spoiler) and selecting with enter works + cy.findByRole("textbox").type("/spoil"); cy.findByTestId("autocomplete-wrapper").within(() => { - cy.findByText("/spoiler").click(); + cy.findByText("/spoiler").should("exist"); }); - - // Check the autocomplete is closed and the composer contains the completion + cy.findByRole("textbox").type("{Enter}"); + // Check it has closed the autocomplete and put the text into the composer cy.findByTestId("autocomplete-wrapper").should("not.be.visible"); cy.findByRole("textbox").within(() => { cy.findByText("/spoiler").should("exist"); }); - - // Type some more text then send the message - const argumentText = "this is the spoiler text"; - cy.findByRole("textbox").type(argumentText); + // Enter some more text, then send the message + cy.findByRole("textbox").type("this is the spoiler text "); cy.findByRole("button", { name: "Send message" }).click(); - // Check that a spoiler item has appeared in the timeline and contains the spoiler command text cy.get("span.mx_EventTile_spoiler").should("exist"); cy.findByText("this is the spoiler text").should("exist"); }); - - it("autocomplete can be used to write and send a command that takes no arguments", () => { - // Select plain text mode after composer is ready - cy.get("div[contenteditable=true]").should("exist"); - cy.findByRole("button", { name: "Hide formatting" }).click(); - - // Type a message - cy.findByRole("textbox").type("/dev"); - - // Check that the autocomplete /spoiler option is visible and click it - cy.findByTestId("autocomplete-wrapper").within(() => { - cy.findByText("/devtools").click(); - }); - - // Check the autocomplete is closed and the composer contains the completion - cy.findByTestId("autocomplete-wrapper").should("not.be.visible"); - cy.findByRole("textbox").within(() => { - cy.findByText("/devtools").should("exist"); - }); - - // Click the send message button - cy.findByRole("button", { name: "Send message" }).click(); - - // Check that the devtools dialog menu has appeared - cy.findByRole("dialog").within(() => { - cy.findByText("Developer Tools").should("exist"); - }); - }); - - it("autocomplete is not displayed for a message starting with //", () => { - // Select plain text mode after composer is ready - cy.get("div[contenteditable=true]").should("exist"); - cy.findByRole("button", { name: "Hide formatting" }).click(); - - // Type a message - cy.findByRole("textbox").type("//anyText"); - - // Check that the autocomplete options are not visible - cy.findByTestId("autocomplete-wrapper").within(() => { - cy.findAllByRole("presentation").should("have.length", 0); - }); - }); - - it("autocomplete is not displayed when user inserts whitespace after command", () => { - // Select plain text mode after composer is ready - cy.get("div[contenteditable=true]").should("exist"); - cy.findByRole("button", { name: "Hide formatting" }).click(); - - // Type a message - cy.findByRole("textbox").type("/spoiler followed by a space"); - - // Check that the autocomplete options are not visible - cy.findByTestId("autocomplete-wrapper").within(() => { - cy.findAllByRole("presentation").should("have.length", 0); - }); - }); }); }); From cc17e0ebec3655488e648e1b93f88d1101feff15 Mon Sep 17 00:00:00 2001 From: Alun Turner Date: Thu, 20 Apr 2023 12:27:13 +0100 Subject: [PATCH 46/61] add early return --- .../rooms/wysiwyg_composer/hooks/utils.ts | 62 ++++++++++--------- 1 file changed, 33 insertions(+), 29 deletions(-) diff --git a/src/components/views/rooms/wysiwyg_composer/hooks/utils.ts b/src/components/views/rooms/wysiwyg_composer/hooks/utils.ts index b71b7de5211..636b5d2bf2a 100644 --- a/src/components/views/rooms/wysiwyg_composer/hooks/utils.ts +++ b/src/components/views/rooms/wysiwyg_composer/hooks/utils.ts @@ -70,39 +70,43 @@ export function handleEventWithAutocomplete( event: KeyboardEvent | React.KeyboardEvent, ): boolean { const autocompleteIsOpen = autocompleteRef?.current && !autocompleteRef.current.state.hide; + + if (!autocompleteIsOpen) { + return false; + } + let handled = false; + const autocompleteAction = getKeyBindingsManager().getAutocompleteAction(event); + const component = autocompleteRef.current; - if (autocompleteIsOpen) { - const autocompleteAction = getKeyBindingsManager().getAutocompleteAction(event); - const component = autocompleteRef.current; - if (component && component.countCompletions() > 0) { - switch (autocompleteAction) { - case KeyBindingAction.ForceCompleteAutocomplete: - case KeyBindingAction.CompleteAutocomplete: - autocompleteRef.current.onConfirmCompletion(); - handled = true; - break; - case KeyBindingAction.PrevSelectionInAutocomplete: - autocompleteRef.current.moveSelection(-1); - handled = true; - break; - case KeyBindingAction.NextSelectionInAutocomplete: - autocompleteRef.current.moveSelection(1); - handled = true; - break; - case KeyBindingAction.CancelAutocomplete: - autocompleteRef.current.onEscape(event as {} as React.KeyboardEvent); - handled = true; - break; - default: - break; // don't return anything, allow event to pass through - } + if (component && component.countCompletions() > 0) { + switch (autocompleteAction) { + case KeyBindingAction.ForceCompleteAutocomplete: + case KeyBindingAction.CompleteAutocomplete: + autocompleteRef.current.onConfirmCompletion(); + handled = true; + break; + case KeyBindingAction.PrevSelectionInAutocomplete: + autocompleteRef.current.moveSelection(-1); + handled = true; + break; + case KeyBindingAction.NextSelectionInAutocomplete: + autocompleteRef.current.moveSelection(1); + handled = true; + break; + case KeyBindingAction.CancelAutocomplete: + autocompleteRef.current.onEscape(event as {} as React.KeyboardEvent); + handled = true; + break; + default: + break; // don't return anything, allow event to pass through } + } - if (handled) { - event.preventDefault(); - event.stopPropagation(); - } + if (handled) { + event.preventDefault(); + event.stopPropagation(); } + return handled; } From d883727d58e1241da9f0a37b78ed01f4e4e64465 Mon Sep 17 00:00:00 2001 From: Alun Turner Date: Thu, 20 Apr 2023 12:39:04 +0100 Subject: [PATCH 47/61] fix typo, add documentation --- .../rooms/wysiwyg_composer/hooks/useSuggestion.ts | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/src/components/views/rooms/wysiwyg_composer/hooks/useSuggestion.ts b/src/components/views/rooms/wysiwyg_composer/hooks/useSuggestion.ts index 436c66a10f8..08ed533190d 100644 --- a/src/components/views/rooms/wysiwyg_composer/hooks/useSuggestion.ts +++ b/src/components/views/rooms/wysiwyg_composer/hooks/useSuggestion.ts @@ -18,7 +18,7 @@ import { Attributes, MappedSuggestion, SuggestionChar, SuggestionType } from "@m import { SyntheticEvent, useState } from "react"; // This type is a close approximation of what we use in the Rust model for the rich version of the -// editor, we use this to try and make this simple model as similar as possible to allow reuse of +// editor. We use this to try and make this simple model as similar as possible to allow reuse of // the autocomplete component export type PlainTextSuggestionPattern = { keyChar: SuggestionChar; @@ -102,7 +102,7 @@ export const processCommand = ( const { node } = suggestion; - // for a command we know we start at the beginning of the text node, so build the replacement + // for a command, we know we start at the beginning of the text node, so build the replacement // string (note trailing space) and manually adjust the node's textcontent const newContent = `${replacementText} `; node.textContent = newContent; @@ -114,6 +114,16 @@ export const processCommand = ( setSuggestion(null); }; +/** + * When the selection changes inside the current editor, check to see if the cursor is inside + * something that could require the autocomplete to be opened and update the suggestion state + * if so + * TODO expand this to handle mentions + * + * @param editorRef - ref to the composer + * @param suggestion - the current suggestion state + * @param setSuggestion - the setter for the suggestion state + */ export const processSelectionChange = ( editorRef: React.RefObject, suggestion: PlainTextSuggestionPattern | null, From 1a627ed02a38e2eb0750fe00dfa2df21ed536195 Mon Sep 17 00:00:00 2001 From: Alun Turner Date: Thu, 20 Apr 2023 12:45:03 +0100 Subject: [PATCH 48/61] add early return, tidy up comments --- .../wysiwyg_composer/hooks/useSuggestion.ts | 75 ++++++++++--------- 1 file changed, 40 insertions(+), 35 deletions(-) diff --git a/src/components/views/rooms/wysiwyg_composer/hooks/useSuggestion.ts b/src/components/views/rooms/wysiwyg_composer/hooks/useSuggestion.ts index 08ed533190d..96345b36fec 100644 --- a/src/components/views/rooms/wysiwyg_composer/hooks/useSuggestion.ts +++ b/src/components/views/rooms/wysiwyg_composer/hooks/useSuggestion.ts @@ -129,47 +129,52 @@ export const processSelectionChange = ( suggestion: PlainTextSuggestionPattern | null, setSuggestion: React.Dispatch>, ): void => { - if (editorRef.current === null) { + const selection = document.getSelection(); + + // return early if we do not have a current editor ref with a cursor selection inside a text node + if ( + editorRef.current === null || + selection === null || + !selection.isCollapsed || + selection.anchorNode?.nodeName !== "#text" + ) { return; } - const selection = document.getSelection(); - // only carry out the checking if we have cursor inside a text node - if (selection && selection.isCollapsed && selection.anchorNode?.nodeName === "#text") { - // here we have established that anchorNode === focusNode, so rename to - const { anchorNode: currentNode } = selection; + // here we have established that both anchor and focus nodes in the selection are + // the same node, so rename to `currentNode` for later use + const { anchorNode: currentNode } = selection; - // first check is that the text node is the first text node of the editor, as adding paragraphs can result - // in nested

tags inside the editor

- const firstTextNode = document.createNodeIterator(editorRef.current, NodeFilter.SHOW_TEXT).nextNode(); + // first check is that the text node is the first text node of the editor, as adding paragraphs can result + // in nested

tags inside the editor

+ const firstTextNode = document.createNodeIterator(editorRef.current, NodeFilter.SHOW_TEXT).nextNode(); - // if we're not in the first text node or we have no text content, return - if (currentNode !== firstTextNode || currentNode.textContent === null) { - return; - } + // if we're not in the first text node or we have no text content, return + if (currentNode !== firstTextNode || currentNode.textContent === null) { + return; + } - // it's a command if: - // it is the first textnode AND - // it starts with /, not // AND - // then has letters all the way up to the end of the textcontent - const commandRegex = /^\/(\w*)$/; - const commandMatches = currentNode.textContent.match(commandRegex); - - // if we don't have any matches, return, clearing the suggeston state if it is non-null - if (commandMatches === null) { - if (suggestion !== null) { - setSuggestion(null); - } - return; - } else { - setSuggestion({ - keyChar: "/", - type: "command", - text: commandMatches[1], - node: selection.anchorNode, - startOffset: 0, - endOffset: currentNode.textContent.length, - }); + // it's a command if: + // it is the first textnode AND + // it starts with /, not // AND + // then has letters all the way up to the end of the textcontent + const commandRegex = /^\/(\w*)$/; + const commandMatches = currentNode.textContent.match(commandRegex); + + // if we don't have any matches, return, clearing the suggeston state if it is non-null + if (commandMatches === null) { + if (suggestion !== null) { + setSuggestion(null); } + return; + } else { + setSuggestion({ + keyChar: "/", + type: "command", + text: commandMatches[1], + node: selection.anchorNode, + startOffset: 0, + endOffset: currentNode.textContent.length, + }); } }; From bec06f9ef74869059998879fe549e984256b1aed Mon Sep 17 00:00:00 2001 From: Alun Turner Date: Thu, 20 Apr 2023 12:47:48 +0100 Subject: [PATCH 49/61] change function expression to function declaration --- .../rooms/wysiwyg_composer/hooks/useSuggestion-test.tsx | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) 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 8c712683d07..d2e1c34e6f5 100644 --- a/test/components/views/rooms/wysiwyg_composer/hooks/useSuggestion-test.tsx +++ b/test/components/views/rooms/wysiwyg_composer/hooks/useSuggestion-test.tsx @@ -22,9 +22,9 @@ import { processSelectionChange, } from "../../../../../../src/components/views/rooms/wysiwyg_composer/hooks/useSuggestion"; -const createMockPlainTextSuggestionPattern = ( +function createMockPlainTextSuggestionPattern( props: Partial = {}, -): PlainTextSuggestionPattern => { +): PlainTextSuggestionPattern { return { keyChar: "/", type: "command", @@ -34,7 +34,8 @@ const createMockPlainTextSuggestionPattern = ( endOffset: 0, ...props, }; -}; +} + describe("mapSuggestion", () => { it("returns null if called with a null argument", () => { expect(mapSuggestion(null)).toBeNull(); From 6fdee583fab2e2a9091ac0e0fc136f4c1e88a680 Mon Sep 17 00:00:00 2001 From: Alun Turner Date: Thu, 20 Apr 2023 13:02:03 +0100 Subject: [PATCH 50/61] add documentation --- .../hooks/usePlainTextListeners.ts | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/src/components/views/rooms/wysiwyg_composer/hooks/usePlainTextListeners.ts b/src/components/views/rooms/wysiwyg_composer/hooks/usePlainTextListeners.ts index c8d08265490..9303a2fc6b0 100644 --- a/src/components/views/rooms/wysiwyg_composer/hooks/usePlainTextListeners.ts +++ b/src/components/views/rooms/wysiwyg_composer/hooks/usePlainTextListeners.ts @@ -37,6 +37,21 @@ function amendInnerHtml(text: string): string { .replace(/<\/div>/g, ""); } +/** + * Generates all of the listeners and the ref to be attached to the editor. Also returns + * pieces of state and utility functions that are required for use in other hooks and by + * the autocomplete component. + * + * @param autocompleteRef - a ref to the autocomplete used for commands and mentions + * @param initialContent - can set the content of the editor on mount + * @param onChange - called whenever there is change in the editor content + * @param onSend - called whenever the user sends the message + * @returns + * - a ref to be attached the editor + * - the editor's current content and a setter + * - handlers for input, paste and keyDown events + * - the output from the {@link useSuggestion} hook + */ export function usePlainTextListeners( autocompleteRef: React.RefObject, initialContent?: string, From 7948fa715b1b52c1e87e2a6ae39a1282204b3245 Mon Sep 17 00:00:00 2001 From: Alun Turner Date: Thu, 20 Apr 2023 13:27:40 +0100 Subject: [PATCH 51/61] fix broken test --- .../rooms/wysiwyg_composer/hooks/useSuggestion-test.tsx | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) 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 d2e1c34e6f5..96ed58ffe23 100644 --- a/test/components/views/rooms/wysiwyg_composer/hooks/useSuggestion-test.tsx +++ b/test/components/views/rooms/wysiwyg_composer/hooks/useSuggestion-test.tsx @@ -142,14 +142,14 @@ describe("processSelectionChange", () => { it("returns early if current editorRef is null", () => { const mockEditorRef = createMockEditorRef(null); - // we monitor for the call to document.getSelection to indicate an early return - const getSelectionSpy = jest.spyOn(document, "getSelection"); + // we monitor for the call to document.createNodeIterator to indicate an early return + const nodeIteratorSpy = jest.spyOn(document, "createNodeIterator"); processSelectionChange(mockEditorRef, null, jest.fn()); - expect(getSelectionSpy).not.toHaveBeenCalled(); + expect(nodeIteratorSpy).not.toHaveBeenCalled(); // tidy up to avoid potential impacts on other tests - getSelectionSpy.mockRestore(); + nodeIteratorSpy.mockRestore(); }); it("does not call setSuggestion if selection is not a cursor", () => { From 799f9a25a390dd0708eb915d8eb118ced8f2730e Mon Sep 17 00:00:00 2001 From: Alun Turner Date: Fri, 21 Apr 2023 15:51:15 +0100 Subject: [PATCH 52/61] add check to cypress tests --- cypress/e2e/composer/composer.spec.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/cypress/e2e/composer/composer.spec.ts b/cypress/e2e/composer/composer.spec.ts index ff763b3b2eb..69c468ac0a5 100644 --- a/cypress/e2e/composer/composer.spec.ts +++ b/cypress/e2e/composer/composer.spec.ts @@ -135,6 +135,8 @@ describe("Composer", () => { // Entering `//` or `/ ` hides the autocomplete contents cy.findByRole("textbox").type(" "); cy.findByTestId("autocomplete-wrapper").should("be.empty"); + cy.findByRole("textbox").type("{Backspace}"); + cy.findByTestId("autocomplete-wrapper").should("not.be.empty"); cy.findByRole("textbox").type("{Backspace}/"); cy.findByTestId("autocomplete-wrapper").should("be.empty"); From eeb3c6c32765dd3f7bc9becaee8ab3e302792900 Mon Sep 17 00:00:00 2001 From: Alun Turner Date: Fri, 21 Apr 2023 15:51:49 +0100 Subject: [PATCH 53/61] update types --- .../rooms/wysiwyg_composer/hooks/useSuggestion.ts | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/src/components/views/rooms/wysiwyg_composer/hooks/useSuggestion.ts b/src/components/views/rooms/wysiwyg_composer/hooks/useSuggestion.ts index 96345b36fec..bffc41babd0 100644 --- a/src/components/views/rooms/wysiwyg_composer/hooks/useSuggestion.ts +++ b/src/components/views/rooms/wysiwyg_composer/hooks/useSuggestion.ts @@ -27,7 +27,8 @@ export type PlainTextSuggestionPattern = { node: Node; startOffset: number; endOffset: number; -} | null; +}; +type Suggestion = PlainTextSuggestionPattern | null; /** * Given an input div element, return the current suggestion found in that div element if present @@ -45,7 +46,7 @@ export function useSuggestion( onSelect: (event: SyntheticEvent) => void; suggestion: MappedSuggestion | null; } { - const [suggestion, setSuggestion] = useState(null); + const [suggestion, setSuggestion] = useState(null); // TODO handle the mentions (@user, #room etc) const handleMention = (): void => {}; @@ -72,7 +73,7 @@ export function useSuggestion( * @returns - null if the input is null, a MappedSuggestion if the input is non-null * */ -export const mapSuggestion = (suggestion: PlainTextSuggestionPattern | null): MappedSuggestion | null => { +export const mapSuggestion = (suggestion: Suggestion): MappedSuggestion | null => { if (suggestion === null) { return null; } else { @@ -91,8 +92,8 @@ export const mapSuggestion = (suggestion: PlainTextSuggestionPattern | null): Ma export const processCommand = ( replacementText: string, editorRef: React.RefObject, - suggestion: PlainTextSuggestionPattern | null, - setSuggestion: React.Dispatch>, + suggestion: Suggestion, + setSuggestion: React.Dispatch>, setText: (text: string) => void, ): void => { // if we do not have any of the values we need to do the work, do nothing @@ -126,8 +127,8 @@ export const processCommand = ( */ export const processSelectionChange = ( editorRef: React.RefObject, - suggestion: PlainTextSuggestionPattern | null, - setSuggestion: React.Dispatch>, + suggestion: Suggestion, + setSuggestion: React.Dispatch>, ): void => { const selection = document.getSelection(); From ab5abb4144c3fabcce02d6d208a98be877ec2989 Mon Sep 17 00:00:00 2001 From: Alun Turner Date: Fri, 21 Apr 2023 15:51:54 +0100 Subject: [PATCH 54/61] update comment --- .../rooms/wysiwyg_composer/hooks/usePlainTextListeners.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/components/views/rooms/wysiwyg_composer/hooks/usePlainTextListeners.ts b/src/components/views/rooms/wysiwyg_composer/hooks/usePlainTextListeners.ts index 9303a2fc6b0..a5cff7ec2be 100644 --- a/src/components/views/rooms/wysiwyg_composer/hooks/usePlainTextListeners.ts +++ b/src/components/views/rooms/wysiwyg_composer/hooks/usePlainTextListeners.ts @@ -47,9 +47,10 @@ function amendInnerHtml(text: string): string { * @param onChange - called whenever there is change in the editor content * @param onSend - called whenever the user sends the message * @returns - * - a ref to be attached the editor - * - the editor's current content and a setter - * - handlers for input, paste and keyDown events + * - `ref`: a ref object which the caller must attach to the HTML `div` node for the editor + * - `content`: state representing the editor's current text content + * - `setContent`: the setter for the content state + * - `onInput`, `onPaste`, `onKeyDown`: handlers for input, paste and keyDown events * - the output from the {@link useSuggestion} hook */ export function usePlainTextListeners( From 9b8ee00f08058116a44c07382748a5c38b2f30f1 Mon Sep 17 00:00:00 2001 From: Alun Turner Date: Fri, 21 Apr 2023 16:35:40 +0100 Subject: [PATCH 55/61] update comments --- .../wysiwyg_composer/hooks/useSuggestion.ts | 31 ++++++++++++++----- 1 file changed, 23 insertions(+), 8 deletions(-) diff --git a/src/components/views/rooms/wysiwyg_composer/hooks/useSuggestion.ts b/src/components/views/rooms/wysiwyg_composer/hooks/useSuggestion.ts index bffc41babd0..8ca4dd12928 100644 --- a/src/components/views/rooms/wysiwyg_composer/hooks/useSuggestion.ts +++ b/src/components/views/rooms/wysiwyg_composer/hooks/useSuggestion.ts @@ -17,9 +17,10 @@ limitations under the License. import { Attributes, MappedSuggestion, SuggestionChar, SuggestionType } from "@matrix-org/matrix-wysiwyg"; import { SyntheticEvent, useState } from "react"; -// This type is a close approximation of what we use in the Rust model for the rich version of the -// editor. We use this to try and make this simple model as similar as possible to allow reuse of -// the autocomplete component +/* To allow reuse of the autocomplete component, we need to record similar information as used in +the Rust model (keyChar, type and text) as well as information required to allow us to do the +DOM manipulation when replacing the suggestion with a mention or command (node, start and +end offsets). */ export type PlainTextSuggestionPattern = { keyChar: SuggestionChar; type: SuggestionType; @@ -31,12 +32,22 @@ export type PlainTextSuggestionPattern = { type Suggestion = PlainTextSuggestionPattern | null; /** - * Given an input div element, return the current suggestion found in that div element if present - * as well as handlers for command or mention completions from the autocomplete, and a listener - * to be attached to the PlainTextComposer + * React hook to allow tracking and replacing of mentions and commands in a div element * * @param editorRef - a ref to the div that is the composer textbox + * @param setText - setter function to allow the hook to update the content state when replacing + * text with text selected from the autocomplete + * @returns + * - `handleMention`: TODO a function that will allow @ or # mentions to be selected from + * the autocomplete and inserted into the composer + * - `handleCommand`: a function that allows slash commands selected from the autocomplete to be inserted + * into the composer + * - `onSelect`: a selection change listener to be attached to the plain text composer + * - `suggestion`: if the cursor is inside something that could be interpreted as a command or a mention, + * this will be an object representing that command or mention, otherwise it is null + * */ + export function useSuggestion( editorRef: React.RefObject, setText: (text: string) => void, @@ -83,11 +94,15 @@ export const mapSuggestion = (suggestion: Suggestion): MappedSuggestion | null = }; /** - * When a command is selected from the autocomplete, this function allows us to replace the relevant part - * of the editor text with the replacement text + * Replaces the relevant part of the editor text with the replacement text after a command is selected + * from the autocomplete. * * @param replacementText - the text that we will insert into the DOM + * @param editorRef - a ref to the editor * @param suggestion - representation of the part of the DOM that will be replaced + * @param setSuggestion - a setter to allow the suggestion state to be updated + * @param setText - a setter to update the state of the composer content after replacing a mention + * or a command */ export const processCommand = ( replacementText: string, From 88840bf52b92dd5df3a4a1a90d9f435f9eae50f8 Mon Sep 17 00:00:00 2001 From: Alun Turner Date: Fri, 21 Apr 2023 16:38:51 +0100 Subject: [PATCH 56/61] shift ref declaration inside the hook --- .../rooms/wysiwyg_composer/components/PlainTextComposer.tsx | 6 ++---- .../rooms/wysiwyg_composer/hooks/usePlainTextListeners.ts | 6 ++++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/components/views/rooms/wysiwyg_composer/components/PlainTextComposer.tsx b/src/components/views/rooms/wysiwyg_composer/components/PlainTextComposer.tsx index 6c5e3bd03d4..428dab00498 100644 --- a/src/components/views/rooms/wysiwyg_composer/components/PlainTextComposer.tsx +++ b/src/components/views/rooms/wysiwyg_composer/components/PlainTextComposer.tsx @@ -24,7 +24,6 @@ import { usePlainTextListeners } from "../hooks/usePlainTextListeners"; import { useSetCursorPosition } from "../hooks/useSetCursorPosition"; import { ComposerFunctions } from "../types"; import { Editor } from "./Editor"; -import Autocomplete from "../../Autocomplete"; import { WysiwygAutocomplete } from "./WysiwygAutocomplete"; interface PlainTextComposerProps { @@ -50,10 +49,9 @@ export function PlainTextComposer({ leftComponent, rightComponent, }: PlainTextComposerProps): JSX.Element { - const autocompleteRef = useRef(null); - const { ref: editorRef, + autocompleteRef, onInput, onPaste, onKeyDown, @@ -63,7 +61,7 @@ export function PlainTextComposer({ onSelect, handleCommand, handleMention, - } = usePlainTextListeners(autocompleteRef, initialContent, onChange, onSend); + } = usePlainTextListeners(initialContent, onChange, onSend); const composerFunctions = useComposerFunctions(editorRef, setContent); usePlainTextInitialization(initialContent, editorRef); diff --git a/src/components/views/rooms/wysiwyg_composer/hooks/usePlainTextListeners.ts b/src/components/views/rooms/wysiwyg_composer/hooks/usePlainTextListeners.ts index a5cff7ec2be..b25c4eaa09d 100644 --- a/src/components/views/rooms/wysiwyg_composer/hooks/usePlainTextListeners.ts +++ b/src/components/views/rooms/wysiwyg_composer/hooks/usePlainTextListeners.ts @@ -42,24 +42,24 @@ function amendInnerHtml(text: string): string { * pieces of state and utility functions that are required for use in other hooks and by * the autocomplete component. * - * @param autocompleteRef - a ref to the autocomplete used for commands and mentions * @param initialContent - can set the content of the editor on mount * @param onChange - called whenever there is change in the editor content * @param onSend - called whenever the user sends the message * @returns * - `ref`: a ref object which the caller must attach to the HTML `div` node for the editor + * * `autocompleteRef`: a ref object which the caller must attach to the autocomplete component * - `content`: state representing the editor's current text content * - `setContent`: the setter for the content state * - `onInput`, `onPaste`, `onKeyDown`: handlers for input, paste and keyDown events * - the output from the {@link useSuggestion} hook */ export function usePlainTextListeners( - autocompleteRef: React.RefObject, initialContent?: string, onChange?: (content: string) => void, onSend?: () => void, ): { ref: RefObject; + autocompleteRef: React.RefObject; content?: string; onInput(event: SyntheticEvent): void; onPaste(event: SyntheticEvent): void; @@ -71,6 +71,7 @@ export function usePlainTextListeners( suggestion: MappedSuggestion | null; } { const ref = useRef(null); + const autocompleteRef = useRef(null); const [content, setContent] = useState(initialContent); const send = useCallback(() => { @@ -138,6 +139,7 @@ export function usePlainTextListeners( return { ref, + autocompleteRef, onInput, onPaste: onInput, onKeyDown, From ed1b78c13dfa8fac22ba8a93295596f5785ce504 Mon Sep 17 00:00:00 2001 From: Alun Turner Date: Fri, 21 Apr 2023 16:40:36 +0100 Subject: [PATCH 57/61] remove unused import --- .../rooms/wysiwyg_composer/components/PlainTextComposer.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/views/rooms/wysiwyg_composer/components/PlainTextComposer.tsx b/src/components/views/rooms/wysiwyg_composer/components/PlainTextComposer.tsx index 428dab00498..c6abc1230b4 100644 --- a/src/components/views/rooms/wysiwyg_composer/components/PlainTextComposer.tsx +++ b/src/components/views/rooms/wysiwyg_composer/components/PlainTextComposer.tsx @@ -15,7 +15,7 @@ limitations under the License. */ import classNames from "classnames"; -import React, { MutableRefObject, ReactNode, useRef } from "react"; +import React, { MutableRefObject, ReactNode } from "react"; import { useComposerFunctions } from "../hooks/useComposerFunctions"; import { useIsFocused } from "../hooks/useIsFocused"; From fe01f99009282a4760446a8b3f69d5e9e327f0f9 Mon Sep 17 00:00:00 2001 From: Alun Turner Date: Tue, 25 Apr 2023 09:58:56 +0100 Subject: [PATCH 58/61] update cypress test and add comments --- cypress/e2e/composer/composer.spec.ts | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/cypress/e2e/composer/composer.spec.ts b/cypress/e2e/composer/composer.spec.ts index 69c468ac0a5..85d1477116c 100644 --- a/cypress/e2e/composer/composer.spec.ts +++ b/cypress/e2e/composer/composer.spec.ts @@ -133,11 +133,14 @@ describe("Composer", () => { cy.findByTestId("autocomplete-wrapper").should("not.be.empty"); // Entering `//` or `/ ` hides the autocomplete contents - cy.findByRole("textbox").type(" "); + // Add an extra slash for `//` + cy.findByRole("textbox").type("/"); cy.findByTestId("autocomplete-wrapper").should("be.empty"); + // Remove the extra slash to go back to `/` cy.findByRole("textbox").type("{Backspace}"); cy.findByTestId("autocomplete-wrapper").should("not.be.empty"); - cy.findByRole("textbox").type("{Backspace}/"); + // Add a trailing space for `/ ` + cy.findByRole("textbox").type(" "); cy.findByTestId("autocomplete-wrapper").should("be.empty"); // Typing a command that takes no arguments (/devtools) and selecting by click works From 0d1852cb7cfcff50e8e9d5c6cc4e9c436b0cda70 Mon Sep 17 00:00:00 2001 From: Alun Turner Date: Tue, 25 Apr 2023 10:02:43 +0100 Subject: [PATCH 59/61] update usePlainTextListener comments --- .../wysiwyg_composer/hooks/usePlainTextListeners.ts | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/components/views/rooms/wysiwyg_composer/hooks/usePlainTextListeners.ts b/src/components/views/rooms/wysiwyg_composer/hooks/usePlainTextListeners.ts index b25c4eaa09d..8369d2684fb 100644 --- a/src/components/views/rooms/wysiwyg_composer/hooks/usePlainTextListeners.ts +++ b/src/components/views/rooms/wysiwyg_composer/hooks/usePlainTextListeners.ts @@ -38,18 +38,19 @@ function amendInnerHtml(text: string): string { } /** - * Generates all of the listeners and the ref to be attached to the editor. Also returns - * pieces of state and utility functions that are required for use in other hooks and by - * the autocomplete component. + * React hook which generates all of the listeners and the ref to be attached to the editor. * - * @param initialContent - can set the content of the editor on mount + * Also returns pieces of state and utility functions that are required for use in other hooks + * and by the autocomplete component. + * + * @param initialContent - the content of the editor when it is first mounted * @param onChange - called whenever there is change in the editor content * @param onSend - called whenever the user sends the message * @returns * - `ref`: a ref object which the caller must attach to the HTML `div` node for the editor * * `autocompleteRef`: a ref object which the caller must attach to the autocomplete component * - `content`: state representing the editor's current text content - * - `setContent`: the setter for the content state + * - `setContent`: the setter function for `content` * - `onInput`, `onPaste`, `onKeyDown`: handlers for input, paste and keyDown events * - the output from the {@link useSuggestion} hook */ From cbcb170686d6d5429135324f5242006dd05f6126 Mon Sep 17 00:00:00 2001 From: Alun Turner Date: Tue, 25 Apr 2023 10:41:27 +0100 Subject: [PATCH 60/61] apply suggested changes to useSuggestion --- .../wysiwyg_composer/hooks/useSuggestion.ts | 60 +++++++++---------- 1 file changed, 28 insertions(+), 32 deletions(-) diff --git a/src/components/views/rooms/wysiwyg_composer/hooks/useSuggestion.ts b/src/components/views/rooms/wysiwyg_composer/hooks/useSuggestion.ts index 8ca4dd12928..e1db110847f 100644 --- a/src/components/views/rooms/wysiwyg_composer/hooks/useSuggestion.ts +++ b/src/components/views/rooms/wysiwyg_composer/hooks/useSuggestion.ts @@ -14,38 +14,38 @@ See the License for the specific language governing permissions and limitations under the License. */ -import { Attributes, MappedSuggestion, SuggestionChar, SuggestionType } from "@matrix-org/matrix-wysiwyg"; +import { Attributes, MappedSuggestion } from "@matrix-org/matrix-wysiwyg"; import { SyntheticEvent, useState } from "react"; -/* To allow reuse of the autocomplete component, we need to record similar information as used in -the Rust model (keyChar, type and text) as well as information required to allow us to do the -DOM manipulation when replacing the suggestion with a mention or command (node, start and -end offsets). */ -export type PlainTextSuggestionPattern = { - keyChar: SuggestionChar; - type: SuggestionType; - text: string; +/** + * Information about the current state of the `useSuggestion` hook. + */ +export type Suggestion = MappedSuggestion & { + /** + * The information in a `MappedSuggestion` is sufficient to generate a query for the autocomplete + * component but more information is required to allow manipulation of the correct part of the DOM + * when selecting an option from the autocomplete. These three pieces of information allow us to + * do that. + */ node: Node; startOffset: number; endOffset: number; }; -type Suggestion = PlainTextSuggestionPattern | null; +type SuggestionState = Suggestion | null; /** * React hook to allow tracking and replacing of mentions and commands in a div element * * @param editorRef - a ref to the div that is the composer textbox - * @param setText - setter function to allow the hook to update the content state when replacing - * text with text selected from the autocomplete + * @param setText - setter function to set the content of the composer * @returns - * - `handleMention`: TODO a function that will allow @ or # mentions to be selected from - * the autocomplete and inserted into the composer - * - `handleCommand`: a function that allows slash commands selected from the autocomplete to be inserted - * into the composer + * - `handleMention`: TODO a function that will insert @ or # mentions which are selected from + * the autocomplete into the composer + * - `handleCommand`: a function that will replace the content of the composer with the given replacement text. + * Can be used to process autocomplete of slash commands * - `onSelect`: a selection change listener to be attached to the plain text composer * - `suggestion`: if the cursor is inside something that could be interpreted as a command or a mention, * this will be an object representing that command or mention, otherwise it is null - * */ export function useSuggestion( @@ -57,7 +57,7 @@ export function useSuggestion( onSelect: (event: SyntheticEvent) => void; suggestion: MappedSuggestion | null; } { - const [suggestion, setSuggestion] = useState(null); + const [suggestion, setSuggestion] = useState(null); // TODO handle the mentions (@user, #room etc) const handleMention = (): void => {}; @@ -67,7 +67,7 @@ export function useSuggestion( const onSelect = (): void => processSelectionChange(editorRef, suggestion, setSuggestion); const handleCommand = (replacementText: string): void => - processCommand(replacementText, editorRef, suggestion, setSuggestion, setText); + processCommand(replacementText, suggestion, setSuggestion, setText); return { suggestion: mapSuggestion(suggestion), @@ -82,9 +82,8 @@ export function useSuggestion( * * @param suggestion - the suggestion that is the JS equivalent of the rust model's representation * @returns - null if the input is null, a MappedSuggestion if the input is non-null - * */ -export const mapSuggestion = (suggestion: Suggestion): MappedSuggestion | null => { +export const mapSuggestion = (suggestion: SuggestionState): MappedSuggestion | null => { if (suggestion === null) { return null; } else { @@ -98,21 +97,18 @@ export const mapSuggestion = (suggestion: Suggestion): MappedSuggestion | null = * from the autocomplete. * * @param replacementText - the text that we will insert into the DOM - * @param editorRef - a ref to the editor * @param suggestion - representation of the part of the DOM that will be replaced - * @param setSuggestion - a setter to allow the suggestion state to be updated - * @param setText - a setter to update the state of the composer content after replacing a mention - * or a command + * @param setSuggestion - setter function to set the suggestion state + * @param setText - setter function to set the content of the composer */ export const processCommand = ( replacementText: string, - editorRef: React.RefObject, - suggestion: Suggestion, - setSuggestion: React.Dispatch>, + suggestion: SuggestionState, + setSuggestion: React.Dispatch>, setText: (text: string) => void, ): void => { - // if we do not have any of the values we need to do the work, do nothing - if (suggestion === null || editorRef.current === null) { + // if we do not have a suggestion, return early + if (suggestion === null) { return; } @@ -142,8 +138,8 @@ export const processCommand = ( */ export const processSelectionChange = ( editorRef: React.RefObject, - suggestion: Suggestion, - setSuggestion: React.Dispatch>, + suggestion: SuggestionState, + setSuggestion: React.Dispatch>, ): void => { const selection = document.getSelection(); From 6f3b7f059445a3a8af0a72df57725bcfcc731000 Mon Sep 17 00:00:00 2001 From: Alun Turner Date: Tue, 25 Apr 2023 10:41:54 +0100 Subject: [PATCH 61/61] update tests --- .../hooks/useSuggestion-test.tsx | 45 +++---------------- 1 file changed, 6 insertions(+), 39 deletions(-) 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 96ed58ffe23..c25a869a09f 100644 --- a/test/components/views/rooms/wysiwyg_composer/hooks/useSuggestion-test.tsx +++ b/test/components/views/rooms/wysiwyg_composer/hooks/useSuggestion-test.tsx @@ -16,15 +16,13 @@ limitations under the License. import React from "react"; import { - PlainTextSuggestionPattern, + Suggestion, mapSuggestion, processCommand, processSelectionChange, } from "../../../../../../src/components/views/rooms/wysiwyg_composer/hooks/useSuggestion"; -function createMockPlainTextSuggestionPattern( - props: Partial = {}, -): PlainTextSuggestionPattern { +function createMockPlainTextSuggestionPattern(props: Partial = {}): Suggestion { return { keyChar: "/", type: "command", @@ -55,38 +53,13 @@ describe("mapSuggestion", () => { }); describe("processCommand", () => { - it("does not change parent hook state if editor ref or suggestion is null", () => { - // create a div and append a text node to it with some initial text - const editorDiv = document.createElement("div"); - const initialText = "text"; - const textNode = document.createTextNode(initialText); - editorDiv.appendChild(textNode); - + it("does not change parent hook state if suggestion is null", () => { // create a mockSuggestion using the text node above - const mockSuggestion = createMockPlainTextSuggestionPattern({ node: textNode }); const mockSetSuggestion = jest.fn(); const mockSetText = jest.fn(); - // call the function with a null editorRef.current value - processCommand( - "should not be seen", - { current: null } as React.RefObject, - mockSuggestion, - mockSetSuggestion, - mockSetText, - ); - - // check that the parent state setter has not been called - expect(mockSetText).not.toHaveBeenCalled(); - - // call the function with a valid editorRef.current, but a null suggestion - processCommand( - "should not be seen", - { current: editorDiv } as React.RefObject, - null, - mockSetSuggestion, - mockSetText, - ); + // call the function with a null suggestion + processCommand("should not be seen", null, mockSetSuggestion, mockSetText); // check that the parent state setter has not been called expect(mockSetText).not.toHaveBeenCalled(); @@ -105,13 +78,7 @@ describe("processCommand", () => { const mockSetText = jest.fn(); const replacementText = "/replacement text"; - processCommand( - replacementText, - { current: editorDiv } as React.RefObject, - mockSuggestion, - mockSetSuggestion, - mockSetText, - ); + processCommand(replacementText, mockSuggestion, mockSetSuggestion, mockSetText); // check that the text has changed and includes a trailing space expect(mockSetText).toHaveBeenCalledWith(`${replacementText} `);