Issue #11261: keep the search bar open (#14141)

* It fixes issue #11261 by preventing the search bar from closing as a result of clicking in the main editor. This is done by commenting out the code in line 256. The search bar will also close when the ESC key is pressed while the main editor has focus * Commented out unnecessary code around line 256 * [issue#11261] * Added in a menu option to allow find bar to stay open/automatically close * Add a close icon to the search bar (for completeness) * Fix escape key for in-editor searches * Remember last search phrase if no selection for in-editor searches * [issue#11261] Code clean up of initial mistakes of the previous commit * [issue#11261] Strange issue with spacing in src/nls/fr/strings.js... * [issue#11261] Strange issue with spacing in src/nls/fr/strings.js... * [issue#11261] Further merge conflict resolution and clean up * [issue#11261] Fix to branch and to testing * [issue#11261] Fixes for incorrect regexp processing * [issue#11261] Why does JSLint keep changing its own file * [issue#11261] Not sure about the french one, let's exclude it for now until I can confirm with a Quebecois at work * [issue#11261] Test left test file with change... * [issue #11261] Minor fixes as per conversation of pull request * [Issue #11261] Fixes mainly to the regular expression side of things * [Issue #11261] Revert accidental change in JSLint * [issue #11261] Remove loosy TODO and wording consistency change * [Issue #11261] Remove Find Next on 'Find' option (we already have Find Next) * [Issue #11261] (Retry the Travis CI build) * Totally forgot to actually save the merge conflict resolution. Here it is. * Merged master incorrectly - fix bad merge * Fix test Signed-off-by: petetnt <[email protected]>
adobe · Mar 2, 2018 · 32300dc · 32300dc
1 parent 51a5908
commit 32300dc
Show file tree

Hide file tree

Showing 11 changed files with 100 additions and 34 deletions.
diff --git a/src/command/Commands.js b/src/command/Commands.js
@@ -110,6 +110,7 @@ define(function (require, exports, module) {
     exports.TOGGLE_LINE_NUMBERS         = "view.toggleLineNumbers";     // EditorOptionHandlers.js      _getToggler()
     exports.TOGGLE_ACTIVE_LINE          = "view.toggleActiveLine";      // EditorOptionHandlers.js      _getToggler()
     exports.TOGGLE_WORD_WRAP            = "view.toggleWordWrap";        // EditorOptionHandlers.js      _getToggler()
+    exports.TOGGLE_SEARCH_AUTOHIDE      = "view.toggleSearchAutoHide";  // EditorOptionHandlers.js      _getToggler() 
 
     exports.CMD_OPEN                        = "cmd.open";
     exports.CMD_ADD_TO_WORKINGSET_AND_OPEN  = "cmd.addToWorkingSetAndOpen";          // DocumentCommandHandlers.js   handleOpenDocumentInNewPane()

diff --git a/src/command/DefaultMenus.js b/src/command/DefaultMenus.js
@@ -151,6 +151,7 @@ define(function (require, exports, module) {
         menu.addMenuItem(Commands.CMD_SPLITVIEW_HORIZONTAL);
         menu.addMenuDivider();
         menu.addMenuItem(Commands.VIEW_HIDE_SIDEBAR);
+        menu.addMenuItem(Commands.TOGGLE_SEARCH_AUTOHIDE);
         menu.addMenuDivider();
         menu.addMenuItem(Commands.VIEW_INCREASE_FONT_SIZE);
         menu.addMenuItem(Commands.VIEW_DECREASE_FONT_SIZE);

diff --git a/src/editor/Editor.js b/src/editor/Editor.js
@@ -98,6 +98,8 @@ define(function (require, exports, module) {
         UPPERCASE_COLORS    = "uppercaseColors",
         USE_TAB_CHAR        = "useTabChar",
         WORD_WRAP           = "wordWrap",
+        AUTO_HIDE_SEARCH    = "autoHideSearch",
+        INDENT_LINE_COMMENT   = "indentLineComment",
         INDENT_LINE_COMMENT = "indentLineComment",
         INPUT_STYLE         = "inputStyle";
 
@@ -228,6 +230,11 @@ define(function (require, exports, module) {
     PreferencesManager.definePreference(WORD_WRAP,          "boolean", true, {
         description: Strings.DESCRIPTION_WORD_WRAP
     });
+
+    PreferencesManager.definePreference(AUTO_HIDE_SEARCH,   "boolean", true, {
+        description: Strings.DESCRIPTION_SEARCH_AUTOHIDE
+    });
+
     PreferencesManager.definePreference(INDENT_LINE_COMMENT,  "boolean", false, {
         description: Strings.DESCRIPTION_INDENT_LINE_COMMENT
     });

diff --git a/src/editor/EditorOptionHandlers.js b/src/editor/EditorOptionHandlers.js
@@ -36,7 +36,9 @@ define(function (require, exports, module) {
     var SHOW_LINE_NUMBERS = "showLineNumbers",
         STYLE_ACTIVE_LINE = "styleActiveLine",
         WORD_WRAP         = "wordWrap",
-        CLOSE_BRACKETS    = "closeBrackets";
+        CLOSE_BRACKETS    = "closeBrackets",
+        AUTO_HIDE_SEARCH  = "autoHideSearch";
+
 
     /**
      * @private
@@ -48,6 +50,9 @@ define(function (require, exports, module) {
     _optionMapping[STYLE_ACTIVE_LINE] = Commands.TOGGLE_ACTIVE_LINE;
     _optionMapping[WORD_WRAP] = Commands.TOGGLE_WORD_WRAP;
     _optionMapping[CLOSE_BRACKETS] = Commands.TOGGLE_CLOSE_BRACKETS;
+    _optionMapping[AUTO_HIDE_SEARCH] = Commands.TOGGLE_SEARCH_AUTOHIDE;
+
+
 
     /**
      * @private
@@ -97,6 +102,7 @@ define(function (require, exports, module) {
     CommandManager.register(Strings.CMD_TOGGLE_ACTIVE_LINE, Commands.TOGGLE_ACTIVE_LINE, _getToggler(STYLE_ACTIVE_LINE));
     CommandManager.register(Strings.CMD_TOGGLE_WORD_WRAP, Commands.TOGGLE_WORD_WRAP, _getToggler(WORD_WRAP));
     CommandManager.register(Strings.CMD_TOGGLE_CLOSE_BRACKETS, Commands.TOGGLE_CLOSE_BRACKETS, _getToggler(CLOSE_BRACKETS));
+    CommandManager.register(Strings.CMD_TOGGLE_SEARCH_AUTOHIDE, Commands.TOGGLE_SEARCH_AUTOHIDE, _getToggler(AUTO_HIDE_SEARCH));
 
     AppInit.htmlReady(_init);
 });
diff --git a/src/htmlContent/findreplace-bar.html b/src/htmlContent/findreplace-bar.html
@@ -38,3 +38,5 @@
     <div class="indexing-message">{{Strings.FIND_IN_FILES_INDEXING}}</div>
 </div>
 {{/multifile}}
+
+<a href="#" class="close">&times;</a>
diff --git a/src/nls/root/strings.js b/src/nls/root/strings.js
@@ -409,6 +409,7 @@ define({
     "CMD_WORKINGSET_SORT_BY_TYPE"         : "Sort by Type",
     "CMD_WORKING_SORT_TOGGLE_AUTO"        : "Automatic Sort",
     "CMD_THEMES"                          : "Themes\u2026",
+    "CMD_TOGGLE_SEARCH_AUTOHIDE"          : "Automatically close search",
 
     // Navigate menu commands
     "NAVIGATE_MENU"                       : "Navigate",
@@ -813,6 +814,7 @@ define({
     "DESCRIPTION_USE_TAB_CHAR"                       : "true to use tabs instead of spaces",
     "DESCRIPTION_UPPERCASE_COLORS"                   : "true to generate uppercase hex colors in Inline Color Editor",
     "DESCRIPTION_WORD_WRAP"                          : "Wrap lines that exceed the viewport width",
+    "DESCRIPTION_SEARCH_AUTOHIDE"                    : "Close the search as soon as the editor is focused",
     "DESCRIPTION_DETECTED_EXCLUSIONS"                : "A list of files that have been detected to cause Tern to run out of control",
     "DESCRIPTION_INFERENCE_TIMEOUT"                  : "The amount of time after which Tern will time out when trying to understand files",
     "DESCRIPTION_SHOW_ERRORS_IN_STATUS_BAR"          : "true to show errors in status bar",

diff --git a/src/search/FindBar.js b/src/search/FindBar.js
@@ -54,6 +54,7 @@ define(function (require, exports, module) {
         intervalId = 0,
         lastQueriedText = "",
         lastTypedText = "",
+        lastTypedTextWasRegexp = false,
         lastKeyCode;
 
     /**
@@ -95,6 +96,7 @@ define(function (require, exports, module) {
         this._enabled = true;
         this.lastQueriedText = "";
         this.lastTypedText = "";
+        this.lastTypedTextWasRegexp = false;
     }
     EventDispatcher.makeEventDispatcher(FindBar.prototype);
 
@@ -209,8 +211,10 @@ define(function (require, exports, module) {
      * Save the prefs state based on the state of the toggles.
      */
     FindBar.prototype._updatePrefsFromSearchBar = function () {
+        var isRegexp = this.$("#find-regexp").is(".active");
         PreferencesManager.setViewState("caseSensitive", this.$("#find-case-sensitive").is(".active"));
-        PreferencesManager.setViewState("regexp",        this.$("#find-regexp").is(".active"));
+        PreferencesManager.setViewState("regexp", isRegexp);
+        lastTypedTextWasRegexp = isRegexp;
     };
 
     /**
@@ -273,10 +277,27 @@ define(function (require, exports, module) {
 
         self._addElementToSearchHistory(this._options.initialQuery);
 
-        this._modalBar = new ModalBar(Mustache.render(_searchBarTemplate, templateVars), true);  // 2nd arg = auto-close on Esc/blur
-
+        this._modalBar = new ModalBar(
+            Mustache.render(_searchBarTemplate, templateVars),
+            !!PreferencesManager.get('autoHideSearch')		// 2nd arg = auto-close on Esc/blur
+        );
+
+        // Done this way because ModalBar.js seems to react unreliably when
+        // modifying it to handle the escape key - the findbar wasn't getting
+        // closed as it should, instead persisting in the background
+        function _handleKeydown(e) {
+            if (e.keyCode === KeyEvent.DOM_VK_ESCAPE) {
+                e.stopPropagation();
+                e.preventDefault();
+                self.close();
+            }
+        }
+        window.document.body.addEventListener("keydown", _handleKeydown, true);
+
         // When the ModalBar closes, clean ourselves up.
         this._modalBar.on("close", function (event) {
+            window.document.body.removeEventListener("keydown", _handleKeydown, true);
+
             // Hide error popup, since it hangs down low enough to make the slide-out look awkward
             self.showError(null);
             self._modalBar = null;
@@ -299,7 +320,9 @@ define(function (require, exports, module) {
         $root
             .on("input", "#find-what", function () {
                 self.trigger("queryChange");
-                lastTypedText = self.getQueryInfo().query;
+                var queryInfo = self.getQueryInfo();
+                lastTypedText = queryInfo.query;
+                lastTypedTextWasRegexp = queryInfo.isRegexp;
             })
             .on("click", "#find-case-sensitive, #find-regexp", function (e) {
                 $(e.currentTarget).toggleClass("active");
@@ -334,8 +357,11 @@ define(function (require, exports, module) {
                         return;
                     }
                     currentTime = new Date().getTime();
-                    if (lastTypedTime && (currentTime - lastTypedTime >= 100) && self.getQueryInfo().query !==  lastQueriedText &&
+
+                    if (lastTypedTime && (currentTime - lastTypedTime >= 100) &&
+                            self.getQueryInfo().query !== lastQueriedText &&
                             !FindUtils.isNodeSearchInProgress()) {
+
                         // init Search
                         if (self._options.multifile) {
                             if ($(e.target).is("#find-what")) {
@@ -384,6 +410,9 @@ define(function (require, exports, module) {
                         quickSearchContainer.show();
                     }
                 }
+            })
+            .on("click", ".close", function () {
+                self.close();
             });
 
         if (!this._options.multifile) {
@@ -652,6 +681,21 @@ define(function (require, exports, module) {
         this.trigger("doFind");
     };
 
+    /*
+     * Returns the string used to prepopulate the find bar
+     * @param {!Editor} editor
+     * @return {string} first line of primary selection to populate the find bar
+     */
+    FindBar._getInitialQueryFromSelection = function(editor) {
+        var selectionText = editor.getSelectedText();
+        if (selectionText) {
+            return selectionText
+                .replace(/^\n*/, "") // Trim possible newlines at the very beginning of the selection
+                .split("\n")[0];
+        }
+        return "";
+    };
+
     /**
      * Gets you the right query and replace text to prepopulate the Find Bar.
      * @static
@@ -660,39 +704,28 @@ define(function (require, exports, module) {
      * @return {query: string, replaceText: string} Query and Replace text to prepopulate the Find Bar with
      */
     FindBar.getInitialQuery = function (currentFindBar, editor) {
-        var query = lastTypedText,
+        var query,
+            selection = FindBar._getInitialQueryFromSelection(editor),
             replaceText = "";
 
-        /*
-         * Returns the string used to prepopulate the find bar
-         * @param {!Editor} editor
-         * @return {string} first line of primary selection to populate the find bar
-         */
-        function getInitialQueryFromSelection(editor) {
-            var selectionText = editor.getSelectedText();
-            if (selectionText) {
-                return selectionText
-                    .replace(/^\n*/, "") // Trim possible newlines at the very beginning of the selection
-                    .split("\n")[0];
-            }
-            return "";
-        }
-
         if (currentFindBar && !currentFindBar.isClosed()) {
             // The modalBar was already up. When creating the new modalBar, copy the
             // current query instead of using the passed-in selected text.
-            query = currentFindBar.getQueryInfo().query;
+            var queryInfo = currentFindBar.getQueryInfo();
+            query = (!queryInfo.isRegexp && selection) || queryInfo.query;
             replaceText = currentFindBar.getReplaceText();
         } else {
-            var openedFindBar = FindBar._bars && _.find(FindBar._bars, function (bar) {
+            var openedFindBar = FindBar._bars && _.find(FindBar._bars,
+                function (bar) {
                     return !bar.isClosed();
-                });
+                }
+            );
 
             if (openedFindBar) {
                 query = openedFindBar.getQueryInfo().query;
                 replaceText = openedFindBar.getReplaceText();
             } else if (editor) {
-                query = getInitialQueryFromSelection(editor) || lastQueriedText || lastTypedText;
+                query = (!lastTypedTextWasRegexp && selection) || lastQueriedText || lastTypedText;
             }
         }
 

diff --git a/src/search/FindInFilesUI.js b/src/search/FindInFilesUI.js
@@ -97,7 +97,8 @@ define(function (require, exports, module) {
                         var showMessage = false;
                         _findBar.enable(true);
                         if (zeroFilesToken === FindInFiles.ZERO_FILES_TO_SEARCH) {
-                            _findBar.showError(StringUtils.format(Strings.FIND_IN_FILES_ZERO_FILES, FindUtils.labelForScope(FindInFiles.searchModel.scope)), true);
+                            _findBar.showError(StringUtils.format(Strings.FIND_IN_FILES_ZERO_FILES,
+                                FindUtils.labelForScope(FindInFiles.searchModel.scope)), true);
                         } else {
                             showMessage = true;
                         }

diff --git a/src/search/FindReplace.js b/src/search/FindReplace.js
@@ -226,6 +226,7 @@ define(function (require, exports, module) {
         if (!primary) {
             primary = _.last(selections);
         }
+
         editor._codeMirror.scrollIntoView({from: primary.start, to: primary.end});
         editor.setSelections(selections, center, centerOptions);
     }
@@ -583,6 +584,8 @@ define(function (require, exports, module) {
             // Blank or invalid query: just jump back to initial pos
             editor._codeMirror.setCursor(state.searchStartPos);
         }
+
+        editor.lastParsedQuery = state.parsedQuery;
     }
 
 
@@ -603,6 +606,9 @@ define(function (require, exports, module) {
 
         // Prepopulate the search field
         var initialQuery = FindBar.getInitialQuery(findBar, editor);
+        if (initialQuery.query === "" && editor.lastParsedQuery !== "") {
+            initialQuery.query = editor.lastParsedQuery;
+        }
 
         // Close our previous find bar, if any. (The open() of the new findBar will
         // take care of closing any other find bar instances.)
@@ -628,6 +634,7 @@ define(function (require, exports, module) {
                 findNext(editor, searchBackwards);
             })
             .on("close.FindReplace", function (e) {
+                editor.lastParsedQuery = state.parsedQuery;
                 // Clear highlights but leave search state in place so Find Next/Previous work after closing
                 clearHighlights(cm, state);
 
@@ -647,12 +654,12 @@ define(function (require, exports, module) {
      */
     function doSearch(editor, searchBackwards) {
         var state = getSearchState(editor._codeMirror);
+
         if (state.parsedQuery) {
             findNext(editor, searchBackwards);
-            return;
+        } else {
+            openSearchBar(editor, false);
         }
-
-        openSearchBar(editor, false);
     }
 
 
@@ -720,6 +727,7 @@ define(function (require, exports, module) {
 
     function _launchFind() {
         var editor = EditorManager.getActiveEditor();
+
         if (editor) {
             // Create a new instance of the search bar UI
             clearSearch(editor._codeMirror);

diff --git a/src/styles/brackets.less b/src/styles/brackets.less
@@ -680,8 +680,6 @@ a, img {
         .close {
             position: absolute;
             right: 10px;
-
-            // vertically centers close button
             top: 50%;
             margin-top: -10px;
         }
@@ -1714,6 +1712,13 @@ a, img {
         margin-left: 0;
         border-radius: 0;
     }
+
+    .close {
+        position: absolute;
+        right: 10px;
+        top: 50%;
+        margin-top: -8px;
+    }
 }
 
 // File exclusion filter (used only in Find in Files search bar, for now)

diff --git a/test/spec/SpecRunnerUtils.js b/test/spec/SpecRunnerUtils.js
@@ -1349,7 +1349,7 @@ define(function (require, exports, module) {
             /**
              * Expects the given editor's selection to be a cursor at the given position (no range selected)
              */
-            toHaveCursorPosition: function (line, ch) {
+            toHaveCursorPosition: function (line, ch, ignoreSelection) {
                 var editor = this.actual;
                 var selection = editor.getSelection();
                 var notString = this.isNot ? "not " : "";
@@ -1371,7 +1371,7 @@ define(function (require, exports, module) {
 
                 // when adding the not operator, it's confusing to check both the size of the
                 // selection and the position. We just check the position in that case.
-                if (this.isNot) {
+                if (this.isNot || ignoreSelection) {
                     return positionsMatch;
                 } else {
                     return !selectionMoreThanOneCharacter && positionsMatch;