From 64438fff253ad3444570ae7c89ed6845beba7c59 Mon Sep 17 00:00:00 2001
From: Mark VanLandingham <markvanlan@gmail.com>
Date: Fri, 22 Dec 2023 09:48:00 -0600
Subject: [PATCH] FIX: Bind events properly in search-menu.js & fix focus issue
 (#25006)

The problem:
Removing the options to addEventListener results in events that are properly cleaned up when the search menu is removed. Previously every time you opened the search menu, the listeners would be attached again, and clicking outside even after it was closed would fire the function again and again (N times as you opened the search menu!)

This was made far far worse in this commit c91d053, where I called close() to remove focus from the search input in the event that the search menu is rendered outside the header.

The problem with this was 2-fold. The close function tried to focus the search header button in core here. When the events aren't cleanup up and that happens... you can't do anything in the app.

The solution:
We don't need the event listeners to close the search menu when it's rendered from the header. The widget header handles clicks outside of the header. Sooo

1. Only register them for standalone search menus
2. Remove the passive options to the listeners so that they are properly removed on close
3. Call close() to unfocus input rather than just closing panel
4. Rename passed in are closeSearchMenu -> onClose because it's more accurate. It's really a callback.
---
 .../app/components/search-menu-panel.hbs      |  2 +-
 .../discourse/app/components/search-menu.js   | 32 +++++++++++--------
 .../components/search-menu-test.gjs           | 23 +++++++++++++
 3 files changed, 43 insertions(+), 14 deletions(-)

diff --git a/app/assets/javascripts/discourse/app/components/search-menu-panel.hbs b/app/assets/javascripts/discourse/app/components/search-menu-panel.hbs
index 51d562c165e..e5009a35c21 100644
--- a/app/assets/javascripts/discourse/app/components/search-menu-panel.hbs
+++ b/app/assets/javascripts/discourse/app/components/search-menu-panel.hbs
@@ -1,6 +1,6 @@
 <MenuPanel @animationClass={{this.animationClass}}>
   <SearchMenu
-    @closeSearchMenu={{@closeSearchMenu}}
+    @onClose={{@closeSearchMenu}}
     @inlineResults={{true}}
     @autofocusInput={{true}}
   />
diff --git a/app/assets/javascripts/discourse/app/components/search-menu.js b/app/assets/javascripts/discourse/app/components/search-menu.js
index 4987b2980a3..91df1cbb19c 100644
--- a/app/assets/javascripts/discourse/app/components/search-menu.js
+++ b/app/assets/javascripts/discourse/app/components/search-menu.js
@@ -51,24 +51,30 @@ export default class SearchMenu extends Component {
 
   @bind
   setupEventListeners() {
-    document.addEventListener("mousedown", this.onDocumentPress, true);
-    document.addEventListener("touchend", this.onDocumentPress, {
-      capture: true,
-      passive: true,
-    });
+    // We only need to register click events when the search menu is rendered outside of the header.
+    // The header handles clicking outside.
+    if (!this.args.inlineResults) {
+      document.addEventListener("mousedown", this.onDocumentPress);
+      document.addEventListener("touchend", this.onDocumentPress);
+    }
   }
 
   willDestroy() {
-    document.removeEventListener("mousedown", this.onDocumentPress);
-    document.removeEventListener("touchend", this.onDocumentPress);
-
+    if (!this.args.inlineResults) {
+      document.removeEventListener("mousedown", this.onDocumentPress);
+      document.removeEventListener("touchend", this.onDocumentPress);
+    }
     super.willDestroy(...arguments);
   }
 
   @bind
   onDocumentPress(event) {
+    if (!this.menuPanelOpen) {
+      return;
+    }
+
     if (!event.target.closest(".search-menu-container.menu-panel-results")) {
-      this.menuPanelOpen = false;
+      this.close();
     }
   }
 
@@ -100,13 +106,13 @@ export default class SearchMenu extends Component {
 
   @action
   close() {
-    if (this.args?.closeSearchMenu) {
-      return this.args.closeSearchMenu();
+    if (this.args?.onClose) {
+      return this.args.onClose();
     }
 
-    // We want to blur the active element (search input) when in stand-alone mode
+    // We want to blur the search input when in stand-alone mode
     // so that when we focus on the search input again, the menu panel pops up
-    document.activeElement.blur();
+    document.getElementById(SEARCH_INPUT_ID)?.blur();
     this.menuPanelOpen = false;
   }
 
diff --git a/app/assets/javascripts/discourse/tests/integration/components/search-menu-test.gjs b/app/assets/javascripts/discourse/tests/integration/components/search-menu-test.gjs
index 1a953b3d05b..e477ddc946f 100644
--- a/app/assets/javascripts/discourse/tests/integration/components/search-menu-test.gjs
+++ b/app/assets/javascripts/discourse/tests/integration/components/search-menu-test.gjs
@@ -55,4 +55,27 @@ module("Integration | Component | search-menu", function (hooks) {
       "Clicking the term brought back search results"
     );
   });
+
+  test("clicking outside results hides and blurs input", async function (assert) {
+    await render(<template><div id="click-me"><SearchMenu /></div></template>);
+    await click("#search-term");
+
+    assert.strictEqual(
+      document.activeElement,
+      query("#search-term"),
+      "Clicking the search term input focuses it"
+    );
+
+    await click("#click-me");
+
+    assert.strictEqual(
+      document.activeElement,
+      document.body,
+      "Clicking outside blurs focus and closes panel"
+    );
+    assert.notOk(
+      exists(".menu-panel .search-menu-initial-options"),
+      "Menu panel is hidden"
+    );
+  });
 });