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.
This commit is contained in:
Mark VanLandingham 2023-12-22 09:48:00 -06:00 committed by GitHub
parent 1010bbf2ce
commit 64438fff25
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 43 additions and 14 deletions

View File

@ -1,6 +1,6 @@
<MenuPanel @animationClass={{this.animationClass}}>
<SearchMenu
@closeSearchMenu={{@closeSearchMenu}}
@onClose={{@closeSearchMenu}}
@inlineResults={{true}}
@autofocusInput={{true}}
/>

View File

@ -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;
}

View File

@ -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"
);
});
});