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

Handle command completions in RTE #10521

Merged
merged 38 commits into from
Apr 10, 2023
Merged
Show file tree
Hide file tree
Changes from 33 commits
Commits
Show all changes
38 commits
Select commit Hold shift + click to select a range
02939fe
pass handleCommand prop down and use it in WysiwygAutocomplete
Apr 3, 2023
d4cadad
allow a command to generate a query from buildQuery
Apr 3, 2023
4fe0077
port command functionality into the sendMessage util
Apr 3, 2023
6b623d0
tidy up comments
Apr 3, 2023
e056d5d
remove use of shouldSend and update comments
Apr 3, 2023
c0668f4
remove console log
Apr 3, 2023
212a998
make logic more explicit and amend comment
Apr 5, 2023
c65a1b7
uncomment replyToEvent block
Apr 5, 2023
f4c272d
update util test
Apr 5, 2023
6f46bf8
remove commented out test
Apr 5, 2023
0db94cb
use local text over import from current composer
Apr 5, 2023
f8bddc0
expand tests
Apr 5, 2023
735e8a8
expand tests
Apr 6, 2023
b0c62a3
handle the FocusAComposer action for the wysiwyg composer
Apr 6, 2023
86347a4
remove TODO comment
Apr 6, 2023
0282c14
remove TODO
Apr 6, 2023
377e191
test for action dispatch
Apr 6, 2023
66f8380
fix failing tests
Apr 6, 2023
8cddb25
tidy up tests
Apr 6, 2023
000d744
fix TS error and improve typing
Apr 6, 2023
a1a5917
Merge remote-tracking branch 'origin/develop' into alunturner/handle-…
Apr 6, 2023
b1ca999
fix TS error
Apr 6, 2023
4abd905
amend return types for sendMessage, editMessage
Apr 6, 2023
fa13702
fix null content TS error
Apr 6, 2023
01fa6a7
fix another null content TS error
Apr 6, 2023
cd4ce1b
use as to correct final TS error
Apr 6, 2023
63579d0
Merge branch 'develop' into alunturner/handle-command-completions
Apr 6, 2023
45cc670
remove undefined argument
Apr 6, 2023
b77025b
try to fix TS errors for editMessage function usage
Apr 6, 2023
91ba491
Merge remote-tracking branch 'origin/develop' into alunturner/handle-…
Apr 6, 2023
2863b3d
tidy up
Apr 6, 2023
61946a1
add TODO
Apr 6, 2023
a72b837
Merge remote-tracking branch 'origin/develop' into alunturner/handle-…
Apr 6, 2023
276f4f7
improve comments
Apr 6, 2023
425d946
Merge branch 'develop' into alunturner/handle-command-completions
artcodespace Apr 6, 2023
eb4e1a8
update comment
Apr 10, 2023
c08290f
Merge remote-tracking branch 'origin/develop' into alunturner/handle-…
Apr 10, 2023
b523ef7
Merge branch 'develop' into alunturner/handle-command-completions
Apr 10, 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
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ export const dynamicImportSendMessage = async (
message: string,
isHTML: boolean,
params: SendMessageParams,
): Promise<ISendEventResponse> => {
): Promise<ISendEventResponse | undefined> => {
const { sendMessage } = await import("./utils/message");

return sendMessage(message, isHTML, params);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,12 @@ interface WysiwygAutocompleteProps {
* a mention in the autocomplete list or pressing enter on a selected item
*/
handleMention: FormattingFunctions["mention"];

/**
* This handler will be called with the display text for a command on clicking
* a command in the autocomplete list or pressing enter on a selected item
*/
handleCommand: FormattingFunctions["command"];
}

/**
Expand All @@ -45,13 +51,23 @@ interface WysiwygAutocompleteProps {
* @param props.ref - the ref will be attached to the rendered `<Autocomplete />` component
*/
const WysiwygAutocomplete = forwardRef(
({ suggestion, handleMention }: WysiwygAutocompleteProps, ref: ForwardedRef<Autocomplete>): JSX.Element | null => {
(
{ suggestion, handleMention, handleCommand }: WysiwygAutocompleteProps,
ref: ForwardedRef<Autocomplete>,
): JSX.Element | null => {
const { room } = useRoomContext();
const client = useMatrixClientContext();

function handleConfirm(completion: ICompletion): void {
// TODO handle all of the completion types
// Using this to pick out the ones we can handle during implementation
if (completion.type === "command") {
// TODO determine if utils in SlashCommands.tsx are required

// trim the completion as some include trailing spaces, but we always insert a
// trailing space in the rust model anyway
handleCommand(completion.completion.trim());
}
if (client && room && completion.href && (completion.type === "room" || completion.type === "user")) {
handleMention(
completion.href,
Expand All @@ -61,6 +77,8 @@ const WysiwygAutocomplete = forwardRef(
}
}

// TODO - determine if we show all of the /command suggestions, there are some options in the
// list which don't seem to make sense in this context, specifically /html and /plain
return room ? (
<div className="mx_WysiwygComposer_AutoCompleteWrapper" data-testid="autocomplete-wrapper">
<Autocomplete
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ export const WysiwygComposer = memo(function WysiwygComposer({
}
}

const mentions = ref.current?.querySelectorAll("a[data-mention-type]");
const mentions: NodeList | undefined = ref.current?.querySelectorAll("a[data-mention-type]");
if (mentions) {
mentions.forEach((mention) => mention.addEventListener("click", handleClick));
}
Expand All @@ -108,7 +108,12 @@ export const WysiwygComposer = memo(function WysiwygComposer({
onFocus={onFocus}
onBlur={onFocus}
>
<WysiwygAutocomplete ref={autocompleteRef} suggestion={suggestion} handleMention={wysiwyg.mention} />
<WysiwygAutocomplete
ref={autocompleteRef}
suggestion={suggestion}
handleMention={wysiwyg.mention}
handleCommand={wysiwyg.command}
/>
<FormattingButtons composer={wysiwyg} actionStates={actionStates} />
<Editor
ref={ref}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ export function useEditing(
): {
isSaveDisabled: boolean;
onChange(content: string): void;
editMessage(): Promise<ISendEventResponse>;
editMessage(): Promise<ISendEventResponse | undefined>;
endEditing(): void;
} {
const roomContext = useRoomContext();
Expand All @@ -45,11 +45,12 @@ export function useEditing(
[initialContent],
);

const editMessageMemoized = useCallback(
() =>
!!mxClient && content !== undefined && editMessage(content, { roomContext, mxClient, editorStateTransfer }),
[content, roomContext, mxClient, editorStateTransfer],
);
const editMessageMemoized = useCallback(async () => {
if (mxClient === undefined || content === undefined) {
return;
}
return editMessage(content, { roomContext, mxClient, editorStateTransfer });
}, [content, roomContext, mxClient, editorStateTransfer]);
Comment on lines +48 to +53
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was previously returning false | Promise<...> which I don't think is what we were aiming for here.


const endEditingMemoized = useCallback(() => endEditing(roomContext), [roomContext]);
return { onChange, editMessage: editMessageMemoized, endEditing: endEditingMemoized, isSaveDisabled };
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ export function useWysiwygSendActionHandler(

switch (payload.action) {
case "reply_to_event":
case Action.FocusAComposer:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the action that is fired when a user enters a bad command and then makes a decision from the dialog they're presented with - it ensures that we put the cursor back in the composer when the dialog closes.

Copy link
Member

Choose a reason for hiding this comment

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

Is that documented in the actions file? Would be good to if not

case Action.FocusSendMessageComposer:
focusComposer(composerElement, context, roomContext, timeoutId);
break;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,8 @@ import * as Avatar from "../../../../../Avatar";
* with @ for a user query, # for a room or space query
*/
export function buildQuery(suggestion: MappedSuggestion | null): string {
if (!suggestion || !suggestion.keyChar || suggestion.type === "command") {
if (!suggestion || !suggestion.keyChar) {
// if we have an empty key character, we do not build a query
// TODO implement the command functionality
return "";
}

Expand Down
63 changes: 55 additions & 8 deletions src/components/views/rooms/wysiwyg_composer/utils/message.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ limitations under the License.
*/

import { Composer as ComposerEvent } from "@matrix-org/analytics-events/types/typescript/Composer";
import { IEventRelation, MatrixEvent } from "matrix-js-sdk/src/models/event";
import { IContent, IEventRelation, MatrixEvent } from "matrix-js-sdk/src/models/event";
import { ISendEventResponse, MatrixClient } from "matrix-js-sdk/src/matrix";
import { THREAD_RELATION_TYPE } from "matrix-js-sdk/src/models/thread";

Expand All @@ -33,6 +33,11 @@ import { endEditing, cancelPreviousPendingEdit } from "./editing";
import EditorStateTransfer from "../../../../../utils/EditorStateTransfer";
import { createMessageContent } from "./createMessageContent";
import { isContentModified } from "./isContentModified";
import { CommandCategories, getCommand } from "../../../../../SlashCommands";
import { runSlashCommand, shouldSendAnyway } from "../../../../../editor/commands";
import { Action } from "../../../../../dispatcher/actions";
import { addReplyToMessageContent } from "../../../../../utils/Reply";
import { attachRelation } from "../../SendMessageComposer";

export interface SendMessageParams {
mxClient: MatrixClient;
Expand All @@ -47,8 +52,8 @@ export async function sendMessage(
message: string,
isHTML: boolean,
{ roomContext, mxClient, ...params }: SendMessageParams,
): Promise<ISendEventResponse> {
const { relation, replyToEvent } = params;
): Promise<ISendEventResponse | undefined> {
const { relation, replyToEvent, permalinkCreator } = params;
const { room } = roomContext;
const roomId = room?.roomId;

Expand All @@ -71,9 +76,51 @@ export async function sendMessage(
}*/
PosthogAnalytics.instance.trackEvent<ComposerEvent>(posthogEvent);

const content = await createMessageContent(message, isHTML, params);
let content: IContent | null = null;

// TODO slash comment
// Functionality here approximates what can be found in SendMessageComposer.sendMessage()
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The logic in this block is not really new, it has been taken from the current composer's code that is run when a message is sent (can be seen in SendMessageComposer.tsx), with changes made where appropriate to pass correct variables to function calls etc

if (message.startsWith("/") && !message.startsWith("//")) {
const { cmd, args } = getCommand(message);
if (cmd) {
// we will need to handle /me separately, see SlashCommands.tsx:1387
Copy link
Member

Choose a reason for hiding this comment

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

It seems a little brittle to refer to a line number like this, is it possible to just describe the location?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep the action is already documented as it's what is used by the current composer. Fair point on the comment, I couldn't figure out a way to make it a useful link so I've just described the location more generally.

const threadId = relation?.rel_type === THREAD_RELATION_TYPE.name ? relation?.event_id : null;
let commandSuccessful: boolean;
[content, commandSuccessful] = await runSlashCommand(cmd, args, roomId, threadId ?? null);

if (!commandSuccessful) {
return; // errored
}

if (
content &&
(cmd.category === CommandCategories.messages || cmd.category === CommandCategories.effects)
) {
attachRelation(content, relation);
if (replyToEvent) {
addReplyToMessageContent(content, replyToEvent, {
permalinkCreator,
// Exclude the legacy fallback for custom event types such as those used by /fireworks
includeLegacyFallback: content.msgtype?.startsWith("m.") ?? true,
});
}
} else {
// instead of setting shouldSend to false as in SendMessageComposer, just return
return;
}
} else {
const sendAnyway = await shouldSendAnyway(message);
// re-focus the composer after QuestionDialog is closed
dis.dispatch({
action: Action.FocusAComposer,
context: roomContext.timelineRenderingType,
});
// if !sendAnyway bail to let the user edit the composer and try again
if (!sendAnyway) return;
}
}

// if content is null, we haven't done any slash command processing, so generate some content
content ??= await createMessageContent(message, isHTML, params);

// TODO replace emotion end of message ?

Expand All @@ -92,7 +139,7 @@ export async function sendMessage(

const prom = doMaybeLocalRoomAction(
roomId,
(actualRoomId: string) => mxClient.sendMessage(actualRoomId, threadId, content),
(actualRoomId: string) => mxClient.sendMessage(actualRoomId, threadId, content as IContent),
mxClient,
);

Expand All @@ -108,7 +155,7 @@ export async function sendMessage(

dis.dispatch({ action: "message_sent" });
CHAT_EFFECTS.forEach((effect) => {
if (containsEmoji(content, effect.emojis)) {
if (content && containsEmoji(content, effect.emojis)) {
// For initial threads launch, chat effects are disabled
// see #19731
const isNotThread = relation?.rel_type !== THREAD_RELATION_TYPE.name;
Expand Down Expand Up @@ -146,7 +193,7 @@ interface EditMessageParams {
export async function editMessage(
html: string,
{ roomContext, mxClient, editorStateTransfer }: EditMessageParams,
): Promise<ISendEventResponse> {
): Promise<ISendEventResponse | undefined> {
const editedEvent = editorStateTransfer.getEvent();

PosthogAnalytics.instance.trackEvent<ComposerEvent>({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,8 +69,9 @@ describe("WysiwygAutocomplete", () => {
},
]);
const mockHandleMention = jest.fn();
const mockHandleCommand = jest.fn();

const renderComponent = (props = {}) => {
const renderComponent = (props: Partial<React.ComponentProps<typeof WysiwygAutocomplete>> = {}) => {
const mockClient = stubClient();
const mockRoom = mkStubRoom("test_room", "test_room", mockClient);
const mockRoomContext = getRoomContext(mockRoom, {});
Expand All @@ -82,6 +83,7 @@ describe("WysiwygAutocomplete", () => {
ref={autocompleteRef}
suggestion={null}
handleMention={mockHandleMention}
handleCommand={mockHandleCommand}
{...props}
/>
</RoomContext.Provider>
Expand All @@ -90,7 +92,14 @@ describe("WysiwygAutocomplete", () => {
};

it("does not show the autocomplete when room is undefined", () => {
render(<WysiwygAutocomplete ref={autocompleteRef} suggestion={null} handleMention={mockHandleMention} />);
render(
<WysiwygAutocomplete
ref={autocompleteRef}
suggestion={null}
handleMention={mockHandleMention}
handleCommand={mockHandleCommand}
/>,
);
expect(screen.queryByTestId("autocomplete-wrapper")).not.toBeInTheDocument();
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,15 +53,12 @@ describe("buildQuery", () => {
expect(buildQuery(noKeyCharSuggestion)).toBe("");
});

it("returns an empty string when suggestion is a command", () => {
// TODO alter this test when commands are implemented
const commandSuggestion = { keyChar: "/" as const, text: "slash", type: "command" as const };
expect(buildQuery(commandSuggestion)).toBe("");
});

it("combines the keyChar and text of the suggestion in the query", () => {
const handledSuggestion = { keyChar: "@" as const, text: "alice", type: "mention" as const };
expect(buildQuery(handledSuggestion)).toBe("@alice");

const handledCommand = { keyChar: "/" as const, text: "spoiler", type: "mention" as const };
expect(buildQuery(handledCommand)).toBe("/spoiler");
});
});

Expand Down
Loading