From 0b210e0fc325153547ec8c0dce3102407572baf1 Mon Sep 17 00:00:00 2001
From: Joffrey JAFFEUX <j.jaffeux@gmail.com>
Date: Thu, 21 Nov 2024 07:39:47 +0100
Subject: [PATCH] FIX: correctly closes search menu on escape (#29848)

Prior to this fix the menu would not close if a child was in focus, and the search suggestions had a special implementation to handle this. The fix now relies on trapping the keydown escape event on the top dip of the search menu.
---
 .../discourse/app/components/search-menu.hbs         |  7 ++++++-
 .../discourse/app/components/search-menu.js          |  7 +++++++
 .../components/search-menu/results/assistant-item.js | 12 +++---------
 .../app/components/search-menu/results/assistant.hbs |  7 -------
 .../discourse/tests/acceptance/search-test.js        | 12 +++++++++++-
 5 files changed, 27 insertions(+), 18 deletions(-)

diff --git a/app/assets/javascripts/discourse/app/components/search-menu.hbs b/app/assets/javascripts/discourse/app/components/search-menu.hbs
index 9bc89954b24..ba9d259db5a 100644
--- a/app/assets/javascripts/discourse/app/components/search-menu.hbs
+++ b/app/assets/javascripts/discourse/app/components/search-menu.hbs
@@ -1,4 +1,9 @@
-<div class={{this.classNames}} {{did-insert this.setupEventListeners}}>
+<div
+  class={{this.classNames}}
+  {{did-insert this.setupEventListeners}}
+  {{! template-lint-disable no-invalid-interactive }}
+  {{on "keydown" this.onKeydown}}
+>
   <div class="search-input">
     {{#if this.search.inTopicContext}}
       <DButton
diff --git a/app/assets/javascripts/discourse/app/components/search-menu.js b/app/assets/javascripts/discourse/app/components/search-menu.js
index a15aec68804..351d405cf85 100644
--- a/app/assets/javascripts/discourse/app/components/search-menu.js
+++ b/app/assets/javascripts/discourse/app/components/search-menu.js
@@ -105,6 +105,13 @@ export default class SearchMenu extends Component {
     return false;
   }
 
+  @action
+  onKeydown(event) {
+    if (event.key === "Escape") {
+      this.close();
+    }
+  }
+
   @action
   close() {
     if (this.args?.onClose) {
diff --git a/app/assets/javascripts/discourse/app/components/search-menu/results/assistant-item.js b/app/assets/javascripts/discourse/app/components/search-menu/results/assistant-item.js
index d6202fc0cf5..a1c4965aec9 100644
--- a/app/assets/javascripts/discourse/app/components/search-menu/results/assistant-item.js
+++ b/app/assets/javascripts/discourse/app/components/search-menu/results/assistant-item.js
@@ -70,19 +70,13 @@ export default class AssistantItem extends Component {
       return;
     }
 
-    if (e.key === "Escape") {
-      this.args.closeSearchMenu();
-      e.preventDefault();
-      return false;
-    }
-
     if (e.key === "Enter") {
       this.itemSelected();
     }
 
-    this.search.handleArrowUpOrDown(e);
-    e.stopPropagation();
-    e.preventDefault();
+    if (e.key === "ArrowUp" || e.key === "ArrowDown") {
+      this.search.handleArrowUpOrDown(e);
+    }
   }
 
   @action
diff --git a/app/assets/javascripts/discourse/app/components/search-menu/results/assistant.hbs b/app/assets/javascripts/discourse/app/components/search-menu/results/assistant.hbs
index 96edd7cd035..29de87a014e 100644
--- a/app/assets/javascripts/discourse/app/components/search-menu/results/assistant.hbs
+++ b/app/assets/javascripts/discourse/app/components/search-menu/results/assistant.hbs
@@ -11,7 +11,6 @@
         @withInLabel={{@withInLabel}}
         @isIntersection={{true}}
         @searchAllTopics={{true}}
-        @closeSearchMenu={{@closeSearchMenu}}
         @searchTermChanged={{@searchTermChanged}}
         @suggestionKeyword={{@suggestionKeyword}}
         @typeClass="tag-intersection"
@@ -27,7 +26,6 @@
           @slug={{get this.fullSlugForCategoryMap result.model.id}}
           @withInLabel={{@withInLabel}}
           @searchAllTopics={{true}}
-          @closeSearchMenu={{@closeSearchMenu}}
           @searchTermChanged={{@searchTermChanged}}
           @suggestionKeyword={{@suggestionKeyword}}
           @typeClass="category"
@@ -39,7 +37,6 @@
           @slug={{concat this.prefix "#" result.name}}
           @withInLabel={{@withInLabel}}
           @searchAllTopics={{true}}
-          @closeSearchMenu={{@closeSearchMenu}}
           @searchTermChanged={{@searchTermChanged}}
           @suggestionKeyword={{@suggestionKeyword}}
           @typeClass="tag"
@@ -55,7 +52,6 @@
         @slug={{concat this.prefix "@" this.user.username}}
         @suffix={{i18n "search.in_topics_posts"}}
         @searchAllTopics={{true}}
-        @closeSearchMenu={{@closeSearchMenu}}
         @searchTermChanged={{@searchTermChanged}}
         @suggestionKeyword={{@suggestionKeyword}}
         @typeClass="user"
@@ -65,7 +61,6 @@
         @user={{this.user}}
         @slug={{concat this.prefix "@" this.user.username}}
         @suffix={{i18n "search.in_this_topic"}}
-        @closeSearchMenu={{@closeSearchMenu}}
         @searchTermChanged={{@searchTermChanged}}
         @suggestionKeyword={{@suggestionKeyword}}
         @typeClass="user"
@@ -76,7 +71,6 @@
           @user={{result}}
           @slug={{concat this.prefix "@" result.username}}
           @searchAllTopics={{true}}
-          @closeSearchMenu={{@closeSearchMenu}}
           @searchTermChanged={{@searchTermChanged}}
           @suggestionKeyword={{@suggestionKeyword}}
           @typeClass="user"
@@ -93,7 +87,6 @@
         @slug={{concat this.prefix item}}
         @label={{item}}
         @searchAllTopics={{true}}
-        @closeSearchMenu={{@closeSearchMenu}}
         @searchTermChanged={{@searchTermChanged}}
         @suggestionKeyword={{@suggestionKeyword}}
         @typeClass="shortcut"
diff --git a/app/assets/javascripts/discourse/tests/acceptance/search-test.js b/app/assets/javascripts/discourse/tests/acceptance/search-test.js
index 966849dfd20..6ea46b08fe6 100644
--- a/app/assets/javascripts/discourse/tests/acceptance/search-test.js
+++ b/app/assets/javascripts/discourse/tests/acceptance/search-test.js
@@ -2,6 +2,7 @@ import {
   click,
   currentURL,
   fillIn,
+  focus,
   triggerEvent,
   triggerKeyEvent,
   visit,
@@ -415,6 +416,15 @@ acceptance("Search - Anonymous", function (needs) {
       .dom(".search-menu .search-result-topic .item .topic-title img[alt='+1']")
       .exists(":+1: in the topic title is properly converted to an emoji");
   });
+
+  test("pressing escape correctly closes the menu", async function (assert) {
+    await visit("/");
+    await click("#search-button");
+    await focus(".show-advanced-search");
+    await triggerKeyEvent("#search-term", "keydown", "Escape");
+
+    assert.dom(".search-menu-panel").doesNotExist();
+  });
 });
 
 acceptance("Search - Authenticated", function (needs) {
@@ -1253,7 +1263,7 @@ acceptance("Search - assistant", function (needs) {
     );
   });
 
-  test("topic results - soft loads the topic results after closing then  search menu", async function (assert) {
+  test("topic results - soft loads the topic results after closing the search menu", async function (assert) {
     await visit("/");
     await click("#search-button");
     await fillIn("#search-term", "Development mode");