From b46c8d1dc30b7f07664a21f15946ed9ef0bdb8b2 Mon Sep 17 00:00:00 2001 From: Steve Howell Date: Wed, 6 May 2020 19:44:50 +0000 Subject: [PATCH] typeahead: Avoid tracebacks due to navbar changes. In our recent navbar changes, we made it so that the Esc key auto-closed the navbar. Unfortunately, that code would break other typeaheads with a traceback. One user-facing symptom was that if you drafted a PM and started a typeahead on a recipient, then hitting the Esc key wouldn't close the typeahead. Now we use an `on_escape` mechanism that is specific to the navbar typeahead, so that it's both generic and harder to break for widgets that don't opt in to it. See bbdc66a2143a13270d7b0df068184938085c248d for more details on the commit that introduced this regression. Note that I only call `tab_bar.exit_search` now. I don't check the class name of the input element, since I know that the Esc key is happening in the context of search. And I don't blur the input, since it's going to be hidden. --- static/js/search.js | 4 ++++ static/third/bootstrap-typeahead/typeahead.js | 21 +++++++------------ 2 files changed, 12 insertions(+), 13 deletions(-) diff --git a/static/js/search.js b/static/js/search.js index d2f94885a1..53ac93ab8d 100644 --- a/static/js/search.js +++ b/static/js/search.js @@ -112,6 +112,10 @@ exports.initialize = function () { }, stopAdvance: page_params.search_pills_enabled, advanceKeyCodes: [8], + + // Use our custom typeahead `on_escape` hook to exit + // the search bar as soon as the user hits Esc. + on_escape: tab_bar.exit_search, }); searchbox_form.on('compositionend', function () { diff --git a/static/third/bootstrap-typeahead/typeahead.js b/static/third/bootstrap-typeahead/typeahead.js index 4e7534b490..5a8e6a2e55 100644 --- a/static/third/bootstrap-typeahead/typeahead.js +++ b/static/third/bootstrap-typeahead/typeahead.js @@ -50,13 +50,11 @@ * in compose.scss and splitting $container out of $menu so we can insert * additional HTML before $menu. * - * 4. Navbar changes: + * 4. Escape hooks: * - * Typically, typeahead hotkey actions are independent of other application - * actions, however, the navbar is one case where we want to try and do more - * than a simple typeahead action with a single hotkey press, and so we've - * been forced to make a custom modification, see inline comment at `Esc` - * keyup for more details. + * You can set an on_escape hook to take extra actions when the user hits + * the `Esc` key. We use this in our navbar code to close the navbar when + * a user hits escape while in the typeahead. * ============================================================ */ !function($){ @@ -83,6 +81,7 @@ this.fixed = this.options.fixed || false; this.automated = this.options.automated || this.automated; this.trigger_selection = this.options.trigger_selection || this.trigger_selection; + this.on_escape = this.options.on_escape; this.header = this.options.header || this.header; if (this.fixed) { @@ -356,14 +355,10 @@ case 27: // escape if (!this.shown) return - // Custom Zulip code to achieve the following goals: - // when the searchbox is open in the navbar, and the typeahead is open, a single `Esc` should - // be able to close both the searchbox and the typeahead. - if ($("input:focus,textarea:focus")[0].className === "search-query input-block-level") { - tab_bar.exit_search(); - $("input:focus,textarea:focus").blur(); - } this.hide() + if (this.on_escape) { + this.on_escape(); + } break default: