From 2e6b76bc4a3e364a6199e422c34f375bc4a6f8b8 Mon Sep 17 00:00:00 2001 From: Alun Turner Date: Mon, 10 Apr 2023 14:30:50 +0100 Subject: [PATCH 1/8] handle at-room --- .../components/WysiwygAutocomplete.tsx | 20 +++++++++++++++---- .../wysiwyg_composer/utils/autocomplete.ts | 6 +++++- 2 files changed, 21 insertions(+), 5 deletions(-) diff --git a/src/components/views/rooms/wysiwyg_composer/components/WysiwygAutocomplete.tsx b/src/components/views/rooms/wysiwyg_composer/components/WysiwygAutocomplete.tsx index 709de5fbbc1..98bdb253d61 100644 --- a/src/components/views/rooms/wysiwyg_composer/components/WysiwygAutocomplete.tsx +++ b/src/components/views/rooms/wysiwyg_composer/components/WysiwygAutocomplete.tsx @@ -59,16 +59,28 @@ const WysiwygAutocomplete = forwardRef( 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 (client === undefined || room === undefined) { + return; + } 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")) { + if (completion.type === "at-room") { + // TODO improve handling of at-room to either become a span or use a placeholder href + // We have an issue in that we can't use a placeholder because the rust model is always + // applying a prefix to the href, so an href of "#" becomes https://# and also we can not + // represent a plain span in rust + handleMention( + window.location.href, + getMentionDisplayText(completion, client), + getMentionAttributes(completion, client, room), + ); + } + + if (completion.href && (completion.type === "room" || completion.type === "user")) { handleMention( completion.href, getMentionDisplayText(completion, client), diff --git a/src/components/views/rooms/wysiwyg_composer/utils/autocomplete.ts b/src/components/views/rooms/wysiwyg_composer/utils/autocomplete.ts index 86dd2791a71..f5a249ac6e0 100644 --- a/src/components/views/rooms/wysiwyg_composer/utils/autocomplete.ts +++ b/src/components/views/rooms/wysiwyg_composer/utils/autocomplete.ts @@ -74,12 +74,15 @@ export function getRoomFromCompletion(completion: ICompletion, client: MatrixCli * @returns the text to display in the mention */ export function getMentionDisplayText(completion: ICompletion, client: MatrixClient): string { + console.log(completion, "<<< completion"); if (completion.type === "user") { return completion.completion; } else if (completion.type === "room") { // try and get the room and use it's name, if not available, fall back to // completion.completion return getRoomFromCompletion(completion, client)?.name || completion.completion; + } else if (completion.type === "at-room") { + return completion.completion; } return ""; } @@ -130,7 +133,8 @@ export function getMentionAttributes(completion: ICompletion, client: MatrixClie "data-mention-type": completion.type, "style": `--avatar-background: url(${avatarUrl}); --avatar-letter: '${initialLetter}'`, }; + } else if (completion.type === "at-room") { + return { "data-mention-type": completion.type }; } - return {}; } From 96bb037e3fc0c94b596709295f6596b8a6bb5c8c Mon Sep 17 00:00:00 2001 From: Alun Turner Date: Mon, 10 Apr 2023 13:59:27 +0100 Subject: [PATCH 2/8] remove console log --- .../views/rooms/wysiwyg_composer/utils/autocomplete.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/components/views/rooms/wysiwyg_composer/utils/autocomplete.ts b/src/components/views/rooms/wysiwyg_composer/utils/autocomplete.ts index f5a249ac6e0..e09899be80a 100644 --- a/src/components/views/rooms/wysiwyg_composer/utils/autocomplete.ts +++ b/src/components/views/rooms/wysiwyg_composer/utils/autocomplete.ts @@ -74,7 +74,6 @@ export function getRoomFromCompletion(completion: ICompletion, client: MatrixCli * @returns the text to display in the mention */ export function getMentionDisplayText(completion: ICompletion, client: MatrixClient): string { - console.log(completion, "<<< completion"); if (completion.type === "user") { return completion.completion; } else if (completion.type === "room") { From 4765215aabbf9ef3f05b4bb16a5fb69a067db1b3 Mon Sep 17 00:00:00 2001 From: Alun Turner Date: Mon, 10 Apr 2023 13:59:36 +0100 Subject: [PATCH 3/8] update and add tests --- .../utils/autocomplete-test.ts | 26 +++++++++++++++---- 1 file changed, 21 insertions(+), 5 deletions(-) diff --git a/test/components/views/rooms/wysiwyg_composer/utils/autocomplete-test.ts b/test/components/views/rooms/wysiwyg_composer/utils/autocomplete-test.ts index dc89caba65e..37027b632e2 100644 --- a/test/components/views/rooms/wysiwyg_composer/utils/autocomplete-test.ts +++ b/test/components/views/rooms/wysiwyg_composer/utils/autocomplete-test.ts @@ -100,8 +100,8 @@ describe("getRoomFromCompletion", () => { }); describe("getMentionDisplayText", () => { - it("returns an empty string if we are not handling a user or a room type", () => { - const nonHandledCompletionTypes = ["at-room", "community", "command"] as const; + it("returns an empty string if we are not handling a user, room or at-room type", () => { + const nonHandledCompletionTypes = ["community", "command"] as const; const nonHandledCompletions = nonHandledCompletionTypes.map((type) => createMockCompletion({ type })); nonHandledCompletions.forEach((completion) => { @@ -131,12 +131,18 @@ describe("getMentionDisplayText", () => { // as this uses the mockClient, the name will be the mock room name returned from there expect(getMentionDisplayText(userCompletion, mockClient)).toBe(testCompletion); }); + + it("returns the completion if we are handling an at-room completion", () => { + const testCompletion = "display this"; + const atRoomCompletion = createMockCompletion({ type: "at-room", completion: testCompletion }); + + expect(getMentionDisplayText(atRoomCompletion, mockClient)).toBe(testCompletion); + }); }); describe("getMentionAttributes", () => { - // TODO handle all completion types - it("returns an empty object for completion types other than room or user", () => { - const nonHandledCompletionTypes = ["at-room", "community", "command"] as const; + it("returns an empty object for completion types other than room, user or at-room", () => { + const nonHandledCompletionTypes = ["community", "command"] as const; const nonHandledCompletions = nonHandledCompletionTypes.map((type) => createMockCompletion({ type })); nonHandledCompletions.forEach((completion) => { @@ -218,4 +224,14 @@ describe("getMentionAttributes", () => { }); }); }); + + describe("at-room mentions", () => { + it("returns expected attributes", () => { + const atRoomCompletion = createMockCompletion({ type: "at-room" }); + + const result = getMentionAttributes(atRoomCompletion, mockClient, mockRoom); + + expect(result).toEqual({ "data-mention-type": "at-room" }); + }); + }); }); From 88e590f98829d7b32379a7d98b3dedbc3e274518 Mon Sep 17 00:00:00 2001 From: Alun Turner Date: Mon, 10 Apr 2023 14:34:35 +0100 Subject: [PATCH 4/8] tidy up --- .../components/WysiwygAutocomplete.tsx | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/src/components/views/rooms/wysiwyg_composer/components/WysiwygAutocomplete.tsx b/src/components/views/rooms/wysiwyg_composer/components/WysiwygAutocomplete.tsx index 98bdb253d61..4465264ee59 100644 --- a/src/components/views/rooms/wysiwyg_composer/components/WysiwygAutocomplete.tsx +++ b/src/components/views/rooms/wysiwyg_composer/components/WysiwygAutocomplete.tsx @@ -62,17 +62,19 @@ const WysiwygAutocomplete = forwardRef( if (client === undefined || room === undefined) { return; } + + // 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 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()); } + + // TODO improve handling of at-room to either become a span or use a placeholder href + // We have an issue in that we can't use a placeholder because the rust model is always + // applying a prefix to the href, so an href of "#" becomes https://# and also we can not + // represent a plain span in rust if (completion.type === "at-room") { - // TODO improve handling of at-room to either become a span or use a placeholder href - // We have an issue in that we can't use a placeholder because the rust model is always - // applying a prefix to the href, so an href of "#" becomes https://# and also we can not - // represent a plain span in rust handleMention( window.location.href, getMentionDisplayText(completion, client), From 75111b58b67470326efcf086ce489862a2bf7ad8 Mon Sep 17 00:00:00 2001 From: Alun Turner Date: Mon, 10 Apr 2023 14:38:33 +0100 Subject: [PATCH 5/8] refactor to switch statement --- .../components/WysiwygAutocomplete.tsx | 59 +++++++++++-------- 1 file changed, 34 insertions(+), 25 deletions(-) diff --git a/src/components/views/rooms/wysiwyg_composer/components/WysiwygAutocomplete.tsx b/src/components/views/rooms/wysiwyg_composer/components/WysiwygAutocomplete.tsx index 4465264ee59..0d32ee5740f 100644 --- a/src/components/views/rooms/wysiwyg_composer/components/WysiwygAutocomplete.tsx +++ b/src/components/views/rooms/wysiwyg_composer/components/WysiwygAutocomplete.tsx @@ -63,31 +63,40 @@ const WysiwygAutocomplete = forwardRef( return; } - // 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 - if (completion.type === "command") { - handleCommand(completion.completion.trim()); - } - - // TODO improve handling of at-room to either become a span or use a placeholder href - // We have an issue in that we can't use a placeholder because the rust model is always - // applying a prefix to the href, so an href of "#" becomes https://# and also we can not - // represent a plain span in rust - if (completion.type === "at-room") { - handleMention( - window.location.href, - getMentionDisplayText(completion, client), - getMentionAttributes(completion, client, room), - ); - } - - if (completion.href && (completion.type === "room" || completion.type === "user")) { - handleMention( - completion.href, - getMentionDisplayText(completion, client), - getMentionAttributes(completion, client, room), - ); + switch (completion.type) { + case "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()); + return; + } + case "at-room": { + // TODO improve handling of at-room to either become a span or use a placeholder href + // We have an issue in that we can't use a placeholder because the rust model is always + // applying a prefix to the href, so an href of "#" becomes https://# and also we can not + // represent a plain span in rust + handleMention( + window.location.href, + getMentionDisplayText(completion, client), + getMentionAttributes(completion, client, room), + ); + return; + } + case "room": + case "user": { + if (Boolean(completion.href)) { + handleMention( + completion.href, + getMentionDisplayText(completion, client), + getMentionAttributes(completion, client, room), + ); + } + return; + } + // TODO - handle "community" type + default: + return; } } From ad1f4d6a98eab0e515681abaadf500d83f152ab0 Mon Sep 17 00:00:00 2001 From: Alun Turner Date: Mon, 10 Apr 2023 15:33:11 +0100 Subject: [PATCH 6/8] fix TS error --- .../rooms/wysiwyg_composer/components/WysiwygAutocomplete.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/views/rooms/wysiwyg_composer/components/WysiwygAutocomplete.tsx b/src/components/views/rooms/wysiwyg_composer/components/WysiwygAutocomplete.tsx index 0d32ee5740f..2b4e0f96820 100644 --- a/src/components/views/rooms/wysiwyg_composer/components/WysiwygAutocomplete.tsx +++ b/src/components/views/rooms/wysiwyg_composer/components/WysiwygAutocomplete.tsx @@ -85,7 +85,7 @@ const WysiwygAutocomplete = forwardRef( } case "room": case "user": { - if (Boolean(completion.href)) { + if (typeof completion.href === "string") { handleMention( completion.href, getMentionDisplayText(completion, client), From eade4f768b25a83252456efe4be32e56bda2110e Mon Sep 17 00:00:00 2001 From: Alun Turner Date: Mon, 10 Apr 2023 16:18:19 +0100 Subject: [PATCH 7/8] expand tests --- .../components/WysiwygComposer-test.tsx | 54 ++++++++++++++++++- 1 file changed, 52 insertions(+), 2 deletions(-) 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..25f3c8815e8 100644 --- a/test/components/views/rooms/wysiwyg_composer/components/WysiwygComposer-test.tsx +++ b/test/components/views/rooms/wysiwyg_composer/components/WysiwygComposer-test.tsx @@ -158,7 +158,7 @@ describe("WysiwygComposer", () => { }); }); - describe("Mentions", () => { + describe("Mentions and commands", () => { const dispatchSpy = jest.spyOn(defaultDispatcher, "dispatch"); const mockCompletions: ICompletion[] = [ @@ -181,6 +181,7 @@ describe("WysiwygComposer", () => { { // no href user type: "user", + href: undefined, completion: "user_without_href", completionId: "@user_3:host.local", range: { start: 1, end: 1 }, @@ -201,6 +202,24 @@ describe("WysiwygComposer", () => { range: { start: 1, end: 1 }, component:
room_without_completion_id
, }, + { + type: "command", + completion: "/spoiler", + range: { start: 1, end: 1 }, + component:
/spoiler
, + }, + { + type: "at-room", + completion: "@room", + range: { start: 1, end: 1 }, + component:
@room
, + }, + { + type: "community", + completion: "community-completion", + range: { start: 1, end: 1 }, + component:
community
, + }, ]; const constructMockProvider = (data: ICompletion[]) => @@ -211,9 +230,10 @@ describe("WysiwygComposer", () => { } as unknown as AutocompleteProvider); // for each test we will insert input simulating a user mention + const initialInput = "@abc"; const insertMentionInput = async () => { fireEvent.input(screen.getByRole("textbox"), { - data: "@abc", + data: initialInput, inputType: "insertText", }); @@ -349,6 +369,36 @@ describe("WysiwygComposer", () => { // check that it has inserted a link and falls back to the completion text expect(screen.getByRole("link", { name: "#room_without_completion_id" })).toBeInTheDocument(); }); + + it("selecting a command inserts the command", async () => { + await insertMentionInput(); + + // select the room suggestion + await userEvent.click(screen.getByText("/spoiler")); + + // check that it has inserted the plain text + expect(screen.getByText("/spoiler")).toBeInTheDocument(); + }); + + it("selecting an at-room completion inserts @room", async () => { + await insertMentionInput(); + + // select the room suggestion + await userEvent.click(screen.getByText("@room")); + + // check that it has inserted the @room link + expect(screen.getByRole("link", { name: "@room" })).toBeInTheDocument(); + }); + + it("allows a community completion to pass through", async () => { + await insertMentionInput(); + + // select the room suggestion + await userEvent.click(screen.getByText("community")); + + // check that it we still have the initial text + expect(screen.getByText(initialInput)).toBeInTheDocument(); + }); }); describe("When settings require Ctrl+Enter to send", () => { From be23a1defa45efd6392583f9e335f63a8854da8d Mon Sep 17 00:00:00 2001 From: Alun Turner Date: Thu, 13 Apr 2023 09:45:02 +0100 Subject: [PATCH 8/8] consolidate similar if/else if blocks --- .../views/rooms/wysiwyg_composer/utils/autocomplete.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/components/views/rooms/wysiwyg_composer/utils/autocomplete.ts b/src/components/views/rooms/wysiwyg_composer/utils/autocomplete.ts index e30249ca44c..7f48d8afea6 100644 --- a/src/components/views/rooms/wysiwyg_composer/utils/autocomplete.ts +++ b/src/components/views/rooms/wysiwyg_composer/utils/autocomplete.ts @@ -74,14 +74,12 @@ export function getRoomFromCompletion(completion: ICompletion, client: MatrixCli * @returns the text to display in the mention */ export function getMentionDisplayText(completion: ICompletion, client: MatrixClient): string { - if (completion.type === "user") { + if (completion.type === "user" || completion.type === "at-room") { return completion.completion; } else if (completion.type === "room") { // try and get the room and use it's name, if not available, fall back to // completion.completion return getRoomFromCompletion(completion, client)?.name || completion.completion; - } else if (completion.type === "at-room") { - return completion.completion; } return ""; }