Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
5 changes: 5 additions & 0 deletions .changeset/chatty-socks-hear.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@rocket.chat/meteor': patch
---

Fixes the composer popups (mentions, slash commands and emojis) staying open after programmatic changes to the composer text, such as canceling the edition of a message that contains a mention.
Comment thread
ricardogarim marked this conversation as resolved.
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ type ComposerBoxPopupResult<T extends { _id: string; sort?: number }> =
suspended: boolean;
filter: unknown;
clear: () => void;
update: () => void;
}
| {
option: undefined;
Expand All @@ -40,7 +39,6 @@ type ComposerBoxPopupResult<T extends { _id: string; sort?: number }> =
suspended: undefined;
filter: unknown;
clear: () => void;
update: () => void;
};

const keys = {
Expand Down Expand Up @@ -81,7 +79,7 @@ export const useComposerBoxPopup = <T extends { _id: string; sort?: number }>(
setFocused((focused) => {
const sortedItems = items
.filter((item) => item.isSuccess)
.flatMap((item) => item.data as T[])
.flatMap((item) => item.data)
.sort((a, b) => (('sort' in a && a.sort) || 0) - (('sort' in b && b.sort) || 0));
return sortedItems.find((item) => item._id === focused?._id) ?? sortedItems[0];
});
Expand Down Expand Up @@ -157,21 +155,22 @@ export const useComposerBoxPopup = <T extends { _id: string; sort?: number }>(
setOptionByInput();
});

const handleKeyUp = useStableCallback((event: KeyboardEvent) => {
if (!setOptionByInput()) {
return;
}
const handleInput = useStableCallback(() => {
setOptionByInput();
});

if (!option) {
const handleKeyUp = useStableCallback((event: KeyboardEvent) => {
if (event.which === keys.ESC) {
if (option?.closeOnEsc === true) {
setOptionIndex(-1);
setFocused(undefined);
event.preventDefault();
event.stopImmediatePropagation();
}
return;
}

if (option.closeOnEsc === true && event.which === keys.ESC) {
setOptionIndex(-1);
setFocused(undefined);
event.preventDefault();
event.stopImmediatePropagation();
}
setOptionByInput();
});

const handleKeyDown = useStableCallback((event: KeyboardEvent) => {
Expand All @@ -194,7 +193,7 @@ export const useComposerBoxPopup = <T extends { _id: string; sort?: number }>(
setFocused((focused) => {
const list = items
.filter((item) => item.isSuccess)
.flatMap((item) => item.data as T[])
.flatMap((item) => item.data)
.sort((a, b) => (('sort' in a && a.sort) || 0) - (('sort' in b && b.sort) || 0));

if (!list) {
Expand All @@ -203,7 +202,7 @@ export const useComposerBoxPopup = <T extends { _id: string; sort?: number }>(

const focusedIndex = list.findIndex((item) => item === focused);

return (focusedIndex > 0 ? list[focusedIndex - 1] : list[list.length - 1]) as T;
return focusedIndex > 0 ? list[focusedIndex - 1] : list[list.length - 1];
});
event.preventDefault();
event.stopImmediatePropagation();
Expand All @@ -213,7 +212,7 @@ export const useComposerBoxPopup = <T extends { _id: string; sort?: number }>(
setFocused((focused) => {
const list = items
.filter((item) => item.isSuccess)
.flatMap((item) => item.data as T[])
.flatMap((item) => item.data)
.sort((a, b) => (('sort' in a && a.sort) || 0) - (('sort' in b && b.sort) || 0));

if (!list) {
Expand All @@ -222,7 +221,7 @@ export const useComposerBoxPopup = <T extends { _id: string; sort?: number }>(

const focusedIndex = list.findIndex((item) => item === focused);

return (focusedIndex < list.length - 1 ? list[focusedIndex + 1] : list[0]) as T;
return focusedIndex < list.length - 1 ? list[focusedIndex + 1] : list[0];
});
event.preventDefault();
event.stopImmediatePropagation();
Expand All @@ -231,10 +230,6 @@ export const useComposerBoxPopup = <T extends { _id: string; sort?: number }>(
});

const clear = useStableCallback(() => {
if (!option) {
return;
}

setOptionIndex(-1);
setFocused(undefined);
setFilter('');
Expand All @@ -244,6 +239,7 @@ export const useComposerBoxPopup = <T extends { _id: string; sort?: number }>(
const callbackRef = useCallback(
(node: HTMLElement | null) => {
if (ref.current) {
ref.current.removeEventListener('input', handleInput);
ref.current.removeEventListener('keyup', handleKeyUp);
ref.current.removeEventListener('keydown', handleKeyDown);
ref.current.removeEventListener('focus', handleFocus);
Expand All @@ -252,12 +248,13 @@ export const useComposerBoxPopup = <T extends { _id: string; sort?: number }>(

if (node) {
ref.current = node;
node.addEventListener('input', handleInput);
node.addEventListener('keyup', handleKeyUp);
node.addEventListener('keydown', handleKeyDown);
node.addEventListener('focus', handleFocus);
}
},
[handleKeyUp, handleKeyDown, handleFocus],
[handleInput, handleKeyUp, handleKeyDown, handleFocus],
);

if (!option) {
Expand All @@ -271,7 +268,6 @@ export const useComposerBoxPopup = <T extends { _id: string; sort?: number }>(
suspended: undefined,
filter: undefined,
clear,
update: setOptionByInput,
};
}

Expand All @@ -285,6 +281,5 @@ export const useComposerBoxPopup = <T extends { _id: string; sort?: number }>(
suspended,
filter,
clear,
update: setOptionByInput,
};
};
22 changes: 10 additions & 12 deletions apps/meteor/client/views/room/composer/messageBox/MessageBox.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ import { useMessageBoxAutoFocus } from './hooks/useMessageBoxAutoFocus';
import { useMessageBoxPlaceholder } from './hooks/useMessageBoxPlaceholder';

const reducer = (_: unknown, event: ChangeEvent<HTMLInputElement>): boolean => {
const target = event.target as HTMLInputElement;
const { target } = event;

return Boolean(target.value.trim());
};
Expand Down Expand Up @@ -195,23 +195,21 @@ const MessageBox = ({
});
});

const closeEditing = (event: KeyboardEvent | MouseEvent<HTMLElement>) => {
const closeEditing = async (event: KeyboardEvent | MouseEvent<HTMLElement>) => {
const mid = chat.currentEditingMessage.getMID();
if (mid) {
event.preventDefault();
event.stopPropagation();

chat.currentEditingMessage.reset().then((reset) => {
// NOTE: if the message was reset (i.e. content changed), we just update the popup (to re-apply/remove the preview)
if (reset) {
popup.update();
return;
}
// NOTE: if the message was reset (i.e. content changed), we keep the editing mode on
const reset = await chat.currentEditingMessage.reset();

if (!reset) {
await chat.currentEditingMessage.cancel();
await chat.currentEditingMessage.stop();
}

chat.currentEditingMessage.cancel();
chat.currentEditingMessage.stop();
popup.clear();
});
popup.clear();
}
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -372,7 +372,7 @@ test.describe('E2EE Encrypted Channels', () => {
await poHomeChannel.content.btnOptionEditMessage.click();
await poHomeChannel.composer.inputMessage.fill(editedMessage);

await page.keyboard.press('Enter');
await poHomeChannel.composer.btnSend.click();

await expect(poHomeChannel.content.lastUserMessageBody).toHaveText(displayedMessage);
await expect(poHomeChannel.content.lastUserMessage.locator('.rcx-icon--name-key')).toBeVisible();
Expand Down
38 changes: 31 additions & 7 deletions apps/meteor/tests/e2e/message-composer.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ test.describe.serial('message-composer', () => {
let targetChannel: string;

test.beforeAll(async ({ api }) => {
targetChannel = await createTargetChannel(api);
targetChannel = await createTargetChannel(api, { members: ['rocket.cat'] });
});

test.beforeEach(async ({ page }) => {
Expand Down Expand Up @@ -110,13 +110,13 @@ test.describe.serial('message-composer', () => {
});

test('should close mention popup when canceling a message edit via "Cancel" button', async ({ page }) => {
await poHomeChannel.content.sendMessage('hello composer');
await poHomeChannel.content.sendMessage('hello composer @rocket.cat');

await test.step('expect to edit last message', async () => {
await expect(poHomeChannel.composer.inputMessage).toHaveValue('');
await poHomeChannel.content.openLastMessageMenu();
await poHomeChannel.content.btnOptionEditMessage.click();
await expect(poHomeChannel.composer.inputMessage).toHaveValue('hello composer');
await expect(poHomeChannel.composer.inputMessage).toHaveValue('hello composer @rocket.cat');
});

await test.step('expect to open popup on mention', async () => {
Expand All @@ -126,7 +126,7 @@ test.describe.serial('message-composer', () => {

await test.step('expect popup to close after the first edit is cancelled', async () => {
await poHomeChannel.composer.btnCancel.click();
await expect(poHomeChannel.composer.inputMessage).toHaveValue('hello composer');
await expect(poHomeChannel.composer.inputMessage).toHaveValue('hello composer @rocket.cat');
await expect(poHomeChannel.composer.boxPopup).not.toBeVisible();
});

Expand All @@ -137,13 +137,13 @@ test.describe.serial('message-composer', () => {
});

test('should close mention popup when canceling a message edit via keyboard', async ({ page }) => {
await poHomeChannel.content.sendMessage('hello composer');
await poHomeChannel.content.sendMessage('hello composer @rocket.cat');

await test.step('expect to edit last message', async () => {
await expect(poHomeChannel.composer.inputMessage).toHaveValue('');
await poHomeChannel.content.openLastMessageMenu();
await poHomeChannel.content.btnOptionEditMessage.click();
await expect(poHomeChannel.composer.inputMessage).toHaveValue('hello composer');
await expect(poHomeChannel.composer.inputMessage).toHaveValue('hello composer @rocket.cat');
});

await test.step('expect to open popup on mention', async () => {
Expand All @@ -153,7 +153,7 @@ test.describe.serial('message-composer', () => {

await test.step('expect popup to close after the first edit is cancelled', async () => {
await page.keyboard.press('Escape');
await expect(poHomeChannel.composer.inputMessage).toHaveValue('hello composer');
await expect(poHomeChannel.composer.inputMessage).toHaveValue('hello composer @rocket.cat');
await expect(poHomeChannel.composer.boxPopup).not.toBeVisible();
});

Expand All @@ -163,6 +163,30 @@ test.describe.serial('message-composer', () => {
});
});

test('should close mention popup after sending a message ending with a mention', async ({ page }) => {
await poHomeChannel.composer.inputMessage.click();

await test.step('expect to open popup on mention', async () => {
await page.keyboard.type('hello composer @rocket.cat');
await expect(poHomeChannel.composer.boxPopup).toBeVisible();
});

await test.step('expect popup to close after sending the message', async () => {
await poHomeChannel.composer.btnSend.click();
await expect(poHomeChannel.composer.inputMessage).toHaveValue('');
await expect(poHomeChannel.composer.boxPopup).not.toBeVisible();
});
});

test('should open mention popup on text inserted without keyboard events', async ({ page }) => {
await poHomeChannel.composer.inputMessage.click();

await page.keyboard.insertText('hello composer @rocket.cat');
await expect(poHomeChannel.composer.boxPopup).toBeVisible();

await poHomeChannel.composer.inputMessage.fill('');
});

test.describe('audio recorder', () => {
test('should open audio recorder', async () => {
await test.step('should be able to record an audio with text content in composer ', async () => {
Expand Down
Loading