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

Mentions as links rte #10422

Merged
merged 55 commits into from
Mar 24, 2023
Merged
Show file tree
Hide file tree
Changes from 49 commits
Commits
Show all changes
55 commits
Select commit Hold shift + click to select a range
177d861
create autocomplete component
Mar 20, 2023
a52aa38
use autocomplete component and add click handlers
Mar 20, 2023
1a17948
handle focus when autocomplete open
Mar 20, 2023
05cc8a3
add placeholder for pill styling
Mar 20, 2023
be01b92
make width of autocomplete sensible
Mar 20, 2023
7ba881d
fix TS issue
Mar 20, 2023
8abcb62
Merge remote-tracking branch 'origin/develop' into alunturner/mention…
Mar 21, 2023
03c6509
bump package json to new version of wysiwyg
Mar 21, 2023
cbc3723
yarn
Mar 21, 2023
6b34a80
get composer tests passing
Mar 21, 2023
a4e1cdf
rough setup of new autocomplete tests
Mar 21, 2023
1c34cdc
WIP cypress tests
Mar 21, 2023
ff12d92
add test id
Mar 22, 2023
8e45663
tidy up test setup
Mar 22, 2023
2ad0eab
progress on getting something rendering
Mar 22, 2023
191f58f
get the list item components rendering
Mar 22, 2023
37b2043
make tests work as expected
Mar 22, 2023
76eb06f
Merge remote-tracking branch 'origin/develop' into alunturner/mention…
Mar 22, 2023
74c3a23
fix TS error
Mar 22, 2023
013c22e
remove unused cypress test
Mar 22, 2023
ab0e84b
remove commented import
Mar 22, 2023
99a98f1
wrap wysiwyg composer tests in contexts
Mar 22, 2023
631ec37
Merge branch 'develop' into alunturner/mentions-as-links-rte
Mar 22, 2023
14cd947
add required autocomplete mocks for composer tests
Mar 22, 2023
8fe6390
fix type issue
Mar 22, 2023
a97d57c
fix TS issue
Mar 22, 2023
9d4b330
fix console error in test run
Mar 22, 2023
cf9456c
fix errors in send wysiwyg composer
Mar 22, 2023
9c2e71a
fix edit wysiwyg composer test
Mar 22, 2023
d3bf2f5
fix send wysiwyg composer test
Mar 22, 2023
84576bc
improve autocomplete tests
Mar 23, 2023
26dfec7
expand composer tests
Mar 23, 2023
036a04b
comment out todo code for coverage
Mar 23, 2023
75ecc15
improve autocomplete test code
Mar 23, 2023
a48f87b
add some room autocompletion tests for composer
Mar 23, 2023
587ea1a
tidy up tests
Mar 23, 2023
6c4d893
add clicking on user link test
Mar 23, 2023
5616f99
Merge remote-tracking branch 'origin/develop' into alunturner/mention…
Mar 23, 2023
d47234b
use stubClient, not createTestClient
Mar 23, 2023
78100d9
consolidate mentions test and setup in single describe
Mar 23, 2023
392bc86
Merge branch 'develop' into alunturner/mentions-as-links-rte
alunturner Mar 23, 2023
7457b4e
revert unneeded changes
Mar 23, 2023
a834963
improve consistency
Mar 23, 2023
0b1e510
remove unneccesary hook
Mar 23, 2023
cde55ab
remove comment
Mar 23, 2023
57d4db5
improve assertion
Mar 23, 2023
21cc283
improve test variables for legibility
Mar 23, 2023
c3d3da8
Merge remote-tracking branch 'origin/develop' into alunturner/mention…
Mar 23, 2023
ec61bef
improve comment
Mar 23, 2023
9b964d1
remove code from autocomplete
Mar 24, 2023
c277cbe
Translate comments to TSDoc
Mar 24, 2023
108b3d8
split if logic up and add comments
Mar 24, 2023
e2522d5
add more TSDoc
Mar 24, 2023
b5a5af9
update comment
Mar 24, 2023
6501013
Merge branch 'develop' into alunturner/mentions-as-links-rte
alunturner Mar 24, 2023
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@
"dependencies": {
"@babel/runtime": "^7.12.5",
"@matrix-org/analytics-events": "^0.5.0",
"@matrix-org/matrix-wysiwyg": "^1.1.1",
"@matrix-org/matrix-wysiwyg": "^1.4.0",
"@matrix-org/react-sdk-module-api": "^0.0.4",
"@sentry/browser": "^7.0.0",
"@sentry/tracing": "^7.0.0",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,3 +84,10 @@ limitations under the License.
border-color: $quaternary-content;
}
}

.mx_SendWysiwygComposer_AutoCompleteWrapper {
position: relative;
> .mx_Autocomplete {
min-width: 100%;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,11 @@ limitations under the License.
padding: unset;
}
}

/* this selector represents what will become a pill */
a[data-mention-type] {
cursor: text;
}
}

.mx_WysiwygComposer_Editor_content_placeholder::before {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,112 @@
/*
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 React, { ForwardedRef, forwardRef, useRef } from "react";
import { MatrixClient, Room } from "matrix-js-sdk/src/matrix";
import { FormattingFunctions, MappedSuggestion } from "@matrix-org/matrix-wysiwyg";

import { useRoomContext } from "../../../../../contexts/RoomContext";
import Autocomplete from "../../Autocomplete";
import { ICompletion } from "../../../../../autocomplete/Autocompleter";
import { useMatrixClientContext } from "../../../../../contexts/MatrixClientContext";

interface WysiwygAutocompleteProps {
suggestion: MappedSuggestion | null;
handleMention: FormattingFunctions["mention"];
}

// Helper function that takes the rust suggestion and builds the query for the
// Autocomplete. This will change as we implement / commands.
// Returns an empty string if we don't want to show the suggestion menu.
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm fairly sure we write our comments in tsdoc style - I have asked for confirmation in our internal room. Assuming I'm right, please could you use that style here?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yep, tsdoc please.

function buildQuery(suggestion: MappedSuggestion | null): string {
if (!suggestion || !suggestion.keyChar || suggestion.type === "command") {
// if we have an empty key character, we do not build a query
// TODO implement the command functionality
return "";
}

return `${suggestion.keyChar}${suggestion.text}`;
}

// Helper function to get the mention text for a room as this is less straightforward
// than it is for determining the text we display for a user.
// TODO determine if it's worth bringing the switch case into this function to make it
// into a more general `getMentionText` component
function getRoomMentionText(completion: ICompletion, client: MatrixClient): string {
const alias = completion.completion;
const roomId = completion.completionId;

let roomForAutocomplete: Room | undefined;
if (roomId || alias[0] !== "#") {
roomForAutocomplete = client.getRoom(roomId || alias) ?? undefined;
Copy link
Contributor

Choose a reason for hiding this comment

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

I found this logic slightly confusing, and I think it also does something surprising (but probably harmless) when there is no roomId and alias does not start with #.

It might be easier to understand if we separated the if(roomId) part from the if(alias[0] === '#') part. What do you think?

Copy link
Contributor Author

@alunturner alunturner Mar 24, 2023

Choose a reason for hiding this comment

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

I'm reticent to change the behaviour as this comes from roomPill behaviour in the PartCreator in parts.ts, but definitely agree splitting it up makes it easier to reason about (and will make changing behaviour easier in the future if required).

Let me know what you think about the change.

Copy link
Contributor

Choose a reason for hiding this comment

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

The change looks good. Could you put a comment in? Maybe something like Not sure it makes much sense to look up using an invalid-looking alias, but kept for consistency with PathCreator. Then maybe if we revisit later, we can know that there is not some clever reason why it has to be this way...

} else {
roomForAutocomplete = client.getRooms().find((r) => {
return r.getCanonicalAlias() === alias || r.getAltAliases().includes(alias);
});
}

return roomForAutocomplete?.name || alias;
}

const WysiwygAutocomplete = forwardRef(
andybalaam marked this conversation as resolved.
Show resolved Hide resolved
({ suggestion, handleMention }: WysiwygAutocompleteProps, ref: ForwardedRef<Autocomplete>): JSX.Element | null => {
const { room } = useRoomContext();
const client = useMatrixClientContext();

const autocompleteIndexRef = useRef<number>(0);
andybalaam marked this conversation as resolved.
Show resolved Hide resolved

function handleConfirm(completion: ICompletion): void {
if (!completion.href) return;

switch (completion.type) {
case "user":
handleMention(completion.href, completion.completion);
break;
case "room": {
handleMention(completion.href, getRoomMentionText(completion, client));
break;
}
// TODO implement the command functionality
// case "command":
// console.log("/command functionality not yet in place");
// break;
default:
break;
}
}

function handleSelectionChange(completionIndex: number): void {
autocompleteIndexRef.current = completionIndex;
}

return room ? (
<div className="mx_SendWysiwygComposer_AutoCompleteWrapper" data-testid="autocomplete-wrapper">
<Autocomplete
ref={ref}
query={buildQuery(suggestion)}
onConfirm={handleConfirm}
onSelectionChange={handleSelectionChange}
selection={{ start: 0, end: 0 }}
room={room}
/>
</div>
) : null;
},
);

WysiwygAutocomplete.displayName = "WysiwygAutocomplete";

export { WysiwygAutocomplete };
Original file line number Diff line number Diff line change
Expand Up @@ -14,15 +14,21 @@ See the License for the specific language governing permissions and
limitations under the License.
*/

import React, { memo, MutableRefObject, ReactNode, useEffect } from "react";
import React, { memo, MutableRefObject, ReactNode, useEffect, useRef } from "react";
import { useWysiwyg, FormattingFunctions } from "@matrix-org/matrix-wysiwyg";
import classNames from "classnames";

import Autocomplete from "../../Autocomplete";
import { WysiwygAutocomplete } from "./WysiwygAutocomplete";
import { FormattingButtons } from "./FormattingButtons";
import { Editor } from "./Editor";
import { useInputEventProcessor } from "../hooks/useInputEventProcessor";
import { useSetCursorPosition } from "../hooks/useSetCursorPosition";
import { useIsFocused } from "../hooks/useIsFocused";
import { useRoomContext } from "../../../../../contexts/RoomContext";
import defaultDispatcher from "../../../../../dispatcher/dispatcher";
import { Action } from "../../../../../dispatcher/actions";
import { parsePermalink } from "../../../../../utils/permalinks/Permalinks";

interface WysiwygComposerProps {
disabled?: boolean;
Expand All @@ -47,21 +53,53 @@ export const WysiwygComposer = memo(function WysiwygComposer({
rightComponent,
children,
}: WysiwygComposerProps) {
const inputEventProcessor = useInputEventProcessor(onSend, initialContent);
const { room } = useRoomContext();
const autocompleteRef = useRef<Autocomplete | null>(null);

const { ref, isWysiwygReady, content, actionStates, wysiwyg } = useWysiwyg({ initialContent, inputEventProcessor });
const inputEventProcessor = useInputEventProcessor(onSend, autocompleteRef, initialContent);
const { ref, isWysiwygReady, content, actionStates, wysiwyg, suggestion } = useWysiwyg({
initialContent,
inputEventProcessor,
});
const { isFocused, onFocus } = useIsFocused();

const isReady = isWysiwygReady && !disabled;
const computedPlaceholder = (!content && placeholder) || undefined;

useSetCursorPosition(!isReady, ref);

useEffect(() => {
if (!disabled && content !== null) {
onChange?.(content);
}
}, [onChange, content, disabled]);

const isReady = isWysiwygReady && !disabled;
useSetCursorPosition(!isReady, ref);
useEffect(() => {
function handleClick(e: Event): void {
e.preventDefault();
if (
e.target &&
e.target instanceof HTMLAnchorElement &&
e.target.getAttribute("data-mention-type") === "user"
) {
const parsedLink = parsePermalink(e.target.href);
if (room && parsedLink?.userId)
defaultDispatcher.dispatch({
action: Action.ViewUser,
member: room.getMember(parsedLink.userId),
});
}
}

const { isFocused, onFocus } = useIsFocused();
const computedPlaceholder = (!content && placeholder) || undefined;
const mentions = ref.current?.querySelectorAll("a[data-mention-type]");
if (mentions) {
mentions.forEach((mention) => mention.addEventListener("click", handleClick));
}

return () => {
if (mentions) mentions.forEach((mention) => mention.removeEventListener("click", handleClick));
};
}, [ref, room, content]);
Comment on lines +77 to +102
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We get link tags (representing mentions) from the rust model in a string of HTML that gets set as inner HTML on any update to the rust model.

We will be able to style the links using attributes they have, but for click handling we need to add/remove the handlers on any change of the rust model output.


return (
<div
Expand All @@ -70,6 +108,7 @@ export const WysiwygComposer = memo(function WysiwygComposer({
onFocus={onFocus}
onBlur={onFocus}
>
<WysiwygAutocomplete ref={autocompleteRef} suggestion={suggestion} handleMention={wysiwyg.mention} />
<FormattingButtons composer={wysiwyg} actionStates={actionStates} />
<Editor
ref={ref}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,11 @@ import { useMatrixClientContext } from "../../../../../contexts/MatrixClientCont
import { isCaretAtEnd, isCaretAtStart } from "../utils/selection";
import { getEventsFromEditorStateTransfer, getEventsFromRoom } from "../utils/event";
import { endEditing } from "../utils/editing";
import Autocomplete from "../../Autocomplete";

export function useInputEventProcessor(
onSend: () => void,
autocompleteRef: React.RefObject<Autocomplete>,
initialContent?: string,
): (event: WysiwygEvent, composer: Wysiwyg, editor: HTMLElement) => WysiwygEvent | null {
const roomContext = useRoomContext();
Expand All @@ -51,6 +53,10 @@ export function useInputEventProcessor(
const send = (): void => {
event.stopPropagation?.();
event.preventDefault?.();
// do not send the message if we have the autocomplete open, regardless of settings
if (autocompleteRef?.current && !autocompleteRef.current.state.hide) {
return;
}
onSend();
};

Expand All @@ -65,12 +71,13 @@ export function useInputEventProcessor(
roomContext,
composerContext,
mxClient,
autocompleteRef,
);
} else {
return handleInputEvent(event, send, isCtrlEnterToSend);
}
},
[isCtrlEnterToSend, onSend, initialContent, roomContext, composerContext, mxClient],
[isCtrlEnterToSend, onSend, initialContent, roomContext, composerContext, mxClient, autocompleteRef],
);
}

Expand All @@ -85,12 +92,51 @@ function handleKeyboardEvent(
roomContext: IRoomState,
composerContext: ComposerContextState,
mxClient: MatrixClient,
autocompleteRef: React.RefObject<Autocomplete>,
): KeyboardEvent | null {
const { editorStateTransfer } = composerContext;
const isEditing = Boolean(editorStateTransfer);
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;
}
}

switch (action) {
case KeyBindingAction.SendMessage:
send();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -93,15 +93,15 @@ describe("SendWysiwygComposer", () => {
customRender(jest.fn(), jest.fn(), false, true);

// Then
await waitFor(() => expect(screen.getByTestId("WysiwygComposer")).toBeTruthy());
expect(await screen.findByTestId("WysiwygComposer")).toBeInTheDocument();
});

it("Should render PlainTextComposer when isRichTextEnabled is at false", () => {
it("Should render PlainTextComposer when isRichTextEnabled is at false", async () => {
// When
customRender(jest.fn(), jest.fn(), false, false);

// Then
expect(screen.getByTestId("PlainTextComposer")).toBeTruthy();
expect(await screen.findByTestId("PlainTextComposer")).toBeInTheDocument();
});

describe.each([{ isRichTextEnabled: true }, { isRichTextEnabled: false }])(
Expand Down
Loading