From c5c3362a21dc4e2c8ec1fa750043b98a75ce8e7c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexandre=20K=C3=BChn?= Date: Fri, 5 May 2023 11:29:16 +0000 Subject: [PATCH] [FIX] mail: arrow up/down should change selection inside composer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Before this commit, when composer had some text content in multi-line, arrow up/down was not jumping to previous/next line. This happens because the navigable list is active even when not shown, and was preventing all keydown events. This commit fixes the issue by limiting handling of keydown events of navigable list to when it is shown. Also fixes a bug where the quick edit of last message was triggered when composer text was non-empty with ArrowUp. This should only be triggered when the composer text input is empty. closes odoo/odoo#120657 Signed-off-by: Sébastien Theys (seb) --- addons/mail/static/src/composer/composer.js | 5 +---- addons/mail/static/src/composer/navigable_list.js | 15 +++------------ .../mail/static/tests/composer/composer_tests.js | 13 +++++++++++++ 3 files changed, 17 insertions(+), 16 deletions(-) diff --git a/addons/mail/static/src/composer/composer.js b/addons/mail/static/src/composer/composer.js index 7b0728ad0d0..7716a4cba98 100644 --- a/addons/mail/static/src/composer/composer.js +++ b/addons/mail/static/src/composer/composer.js @@ -325,10 +325,7 @@ export class Composer extends Component { onKeydown(ev) { switch (ev.key) { case "ArrowUp": - if (this.hasSuggestions) { - return; - } - if (this.props.messageEdition) { + if (this.props.messageEdition && this.props.composer.textInputContent === "") { const messageToEdit = this.props.composer.thread.lastEditableMessageOfSelf; if (messageToEdit) { this.props.messageEdition.editingMessage = messageToEdit; diff --git a/addons/mail/static/src/composer/navigable_list.js b/addons/mail/static/src/composer/navigable_list.js index 120cfdc4491..988e99524e8 100644 --- a/addons/mail/static/src/composer/navigable_list.js +++ b/addons/mail/static/src/composer/navigable_list.js @@ -145,6 +145,9 @@ export class NavigableList extends Component { } onKeydown(ev) { + if (!this.show) { + return; + } const hotkey = getActiveHotkey(ev); switch (hotkey) { case "enter": @@ -155,29 +158,17 @@ export class NavigableList extends Component { this.selectOption(ev, this.state.activeOption); break; case "escape": - if (!this.show) { - return; - } markEventHandled(ev, "NavigableList.close"); this.close(); break; case "tab": this.navigate("next"); - if (!this.show) { - this.open(); - } break; case "arrowup": this.navigate("previous"); - if (!this.show) { - this.open(); - } break; case "arrowdown": this.navigate("next"); - if (!this.show) { - this.open(); - } break; default: return; diff --git a/addons/mail/static/tests/composer/composer_tests.js b/addons/mail/static/tests/composer/composer_tests.js index 0406dca57bd..87334d6f8e3 100644 --- a/addons/mail/static/tests/composer/composer_tests.js +++ b/addons/mail/static/tests/composer/composer_tests.js @@ -581,6 +581,19 @@ QUnit.test("quick edit last self-message from UP arrow", async (assert) => { await afterNextRender(() => triggerHotkey("ArrowUp")); assert.containsOnce($, ".o-mail-Message .o-mail-Composer"); + + await afterNextRender(() => triggerHotkey("Escape")); + assert.containsNone($, ".o-mail-Message .o-mail-Composer"); + assert.strictEqual(document.activeElement, $(".o-mail-Composer-input")[0]); + + // non-empty composer should not trigger quick edit + await insertText(".o-mail-Composer-input", "Shrek"); + await triggerHotkey("ArrowUp"); + // Navigable List relies on useEffect, which behaves with 2 animation frames + // Wait 2 animation frames to make sure it doesn't show quick edit + await nextTick(); + await nextTick(); + assert.containsNone($, ".o-mail-Message .o-mail-Composer"); }); QUnit.test("Select composer suggestion via Enter does not send the message", async (assert) => {