From 956893594260c27385c507388ef1a74cdb36256a Mon Sep 17 00:00:00 2001 From: Sagar Date: Sun, 23 Sep 2012 17:04:30 -0400 Subject: [PATCH 1/5] Committing fix for Issue 1705 Added a switch case to set the dialogLabel correctly depending on which Navigation option is chosen. --- src/search/QuickOpen.js | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/src/search/QuickOpen.js b/src/search/QuickOpen.js index ea600f3009c..ca91ae3f69b 100644 --- a/src/search/QuickOpen.js +++ b/src/search/QuickOpen.js @@ -651,8 +651,21 @@ define(function (require, exports, module) { origSelection = null; } + // Set dialogLabel depending on which Navigation option is chosen + var dialogLabel = ""; + switch (prefix) { + case "": + dialogLabel = Strings.CMD_QUICK_OPEN; + break; + case ":": + dialogLabel = Strings.CMD_GOTO_LINE; + break; + case "@": + dialogLabel = Strings.CMD_GOTO_DEFINITION; + break; + } // Show the search bar ("dialog") - var dialogHTML = Strings.CMD_QUICK_OPEN + ": "; + var dialogHTML = dialogLabel + ": "; that._createDialogDiv(dialogHTML); that.$searchField = $("input#quickOpenSearch"); From 2a62d64a088368adf9ced83b4aeb98099d3340e6 Mon Sep 17 00:00:00 2001 From: Sagar Date: Sun, 23 Sep 2012 17:04:30 -0400 Subject: [PATCH 2/5] Committing fix for Issue 1705 Added a switch case to set the dialogLabel correctly depending on which Navigation option is chosen. --- src/search/QuickOpen.js | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/src/search/QuickOpen.js b/src/search/QuickOpen.js index ea600f3009c..ca91ae3f69b 100644 --- a/src/search/QuickOpen.js +++ b/src/search/QuickOpen.js @@ -651,8 +651,21 @@ define(function (require, exports, module) { origSelection = null; } + // Set dialogLabel depending on which Navigation option is chosen + var dialogLabel = ""; + switch (prefix) { + case "": + dialogLabel = Strings.CMD_QUICK_OPEN; + break; + case ":": + dialogLabel = Strings.CMD_GOTO_LINE; + break; + case "@": + dialogLabel = Strings.CMD_GOTO_DEFINITION; + break; + } // Show the search bar ("dialog") - var dialogHTML = Strings.CMD_QUICK_OPEN + ": "; + var dialogHTML = dialogLabel + ": "; that._createDialogDiv(dialogHTML); that.$searchField = $("input#quickOpenSearch"); From 84b210ec8c2fb262e7d1bb3459aaf510ede503e2 Mon Sep 17 00:00:00 2001 From: Narciso Jaramillo Date: Thu, 6 Dec 2012 14:49:37 -0800 Subject: [PATCH 3/5] Modified Quick Open dialog label fix to handle mode changes if the user changes the first character. As part of this, changed some module functions so they're methods of QuickNavigateDialog instead, and added a module var to track the currently open dialog. --- src/search/QuickOpen.js | 104 +++++++++++++++++++++++++--------------- 1 file changed, 66 insertions(+), 38 deletions(-) diff --git a/src/search/QuickOpen.js b/src/search/QuickOpen.js index 79290848d4f..c8ed8217d81 100644 --- a/src/search/QuickOpen.js +++ b/src/search/QuickOpen.js @@ -79,6 +79,11 @@ define(function (require, exports, module) { /** @type {boolean} */ var dialogOpen = false; + + /** + * The currently open quick open dialog. + */ + var _curDialog; /** Object representing a search result with associated metadata (added as extra ad hoc fields) */ function SearchResult(label) { @@ -151,6 +156,8 @@ define(function (require, exports, module) { */ function QuickNavigateDialog() { this.$searchField = undefined; // defined when showDialog() is called + this._handleFilter = this._handleFilter.bind(this); + this._handleResultsFormatter = this._handleResultsFormatter.bind(this); } /** @@ -699,7 +706,13 @@ define(function (require, exports, module) { return filteredList; } - function _handleFilter(query) { + /** + * Handles changes to the current query in the search field. + * @param {string} query The new query. + */ + QuickNavigateDialog.prototype._handleFilter = function (query) { + this._updateDialogLabel(query); + var curDoc = DocumentManager.getCurrentDocument(); if (curDoc) { var filename = _filenameFromPath(curDoc.file.fullPath, true); @@ -719,8 +732,7 @@ define(function (require, exports, module) { // No plugin: use default file search mode currentPlugin = null; return searchFileList(query); - } - + }; /** * Formats item's label as properly escaped HTML text, highlighting sections that match 'query'. @@ -786,8 +798,12 @@ define(function (require, exports, module) { return "
  • " + displayName + "
    " + displayPath + "
    • "; } - function _handleResultsFormatter(item) { - var query = $("input#quickOpenSearch").val(); + /** + * Formats the entry for the given item to be displayed in the dropdown. + * @param {Object} item The item to be displayed. + */ + QuickNavigateDialog.prototype._handleResultsFormatter = function (item) { + var query = this.$searchField.val(); var formatter; @@ -799,23 +815,49 @@ define(function (require, exports, module) { formatter = _filenameResultsFormatter; } return formatter(item, query); - } + }; - function setSearchFieldValue(prefix, initialString) { + /** + * Sets the value in the search field, updating the current mode and label based on the + * given prefix. + * @param {string} prefix The prefix that determines which mode we're in: must be empty (for file search), + * "@" for go to definition, or ":" for go to line. + * @param {string} initialString The query string to search for (without the prefix). + */ + QuickNavigateDialog.prototype.setSearchFieldValue = function (prefix, initialString) { prefix = prefix || ""; initialString = initialString || ""; initialString = prefix + initialString; - var $field = $("input#quickOpenSearch"); - if ($field) { - $field.val(initialString); - $field.get(0).setSelectionRange(prefix.length, initialString.length); - - // Kick smart-autocomplete to update (it only listens for keyboard events) - // (due to #1855, this will only pop up results list; it won't auto-"focus" the first result) - $field.trigger("keyIn", [initialString]); + var $field = this.$searchField; + $field.val(initialString); + $field.get(0).setSelectionRange(prefix.length, initialString.length); + + // Kick smart-autocomplete to update (it only listens for keyboard events) + // (due to #1855, this will only pop up results list; it won't auto-"focus" the first result) + $field.trigger("keyIn", [initialString]); + + this._updateDialogLabel(initialString); + }; + + QuickNavigateDialog.prototype._updateDialogLabel = function (query) { + var prefix = (query.length > 0 ? query.charAt(0) : ""); + + // Update the dialog label based on the current prefix. + var dialogLabel = ""; + switch (prefix) { + case ":": + dialogLabel = Strings.CMD_GOTO_LINE; + break; + case "@": + dialogLabel = Strings.CMD_GOTO_DEFINITION; + break; + default: + dialogLabel = Strings.CMD_QUICK_OPEN; + break; } - } + $(".find-dialog-label", this.dialog).text(dialogLabel); + }; /** * Close the dialog when the user clicks outside of it. Smart-autocomplete listens for this and automatically closes its popup, @@ -863,26 +905,12 @@ define(function (require, exports, module) { } else { origSelection = null; } - - // Set dialogLabel depending on which Navigation option is chosen - var dialogLabel = ""; - switch (prefix) { - case "": - dialogLabel = Strings.CMD_QUICK_OPEN; - break; - case ":": - dialogLabel = Strings.CMD_GOTO_LINE; - break; - case "@": - dialogLabel = Strings.CMD_GOTO_DEFINITION; - break; - } + // Show the search bar ("dialog") - var dialogHTML = dialogLabel + ": "; + var dialogHTML = ": "; that._createDialogDiv(dialogHTML); that.$searchField = $("input#quickOpenSearch"); - that.$searchField.smartAutoComplete({ source: [], maxResults: 20, @@ -891,8 +919,8 @@ define(function (require, exports, module) { forceSelect: false, typeAhead: false, // won't work right now because smart auto complete // using internal raw results instead of filtered results for matching - filter: _handleFilter, - resultFormatter: _handleResultsFormatter + filter: this._handleFilter, + resultFormatter: this._handleResultsFormatter }); that.$searchField.bind({ @@ -906,7 +934,7 @@ define(function (require, exports, module) { // triggers lostFocus }); - setSearchFieldValue(prefix, initialString); + this.setSearchFieldValue(prefix, initialString); // Start fetching the file list, which will be needed the first time the user enters an un-prefixed query. If FileIndexManager's // caches are out of date, this list might take some time to asynchronously build. See searchFileList() for how this is handled. @@ -931,10 +959,10 @@ define(function (require, exports, module) { */ function beginSearch(prefix, initialString) { if (dialogOpen) { - setSearchFieldValue(prefix, initialString); + _curDialog.setSearchFieldValue(prefix, initialString); } else { - var dialog = new QuickNavigateDialog(); - dialog.showDialog(prefix, initialString); + _curDialog = new QuickNavigateDialog(); + _curDialog.showDialog(prefix, initialString); } } From 8206f6bd0f37455ea6fc2226ed9801aa89bfec94 Mon Sep 17 00:00:00 2001 From: Narciso Jaramillo Date: Tue, 11 Dec 2012 10:22:46 -0800 Subject: [PATCH 4/5] Code review cleanups --- src/search/QuickOpen.js | 79 +++++++++++++++++++++++++---------------- 1 file changed, 49 insertions(+), 30 deletions(-) diff --git a/src/search/QuickOpen.js b/src/search/QuickOpen.js index c8ed8217d81..c35160d46f8 100644 --- a/src/search/QuickOpen.js +++ b/src/search/QuickOpen.js @@ -93,8 +93,7 @@ define(function (require, exports, module) { /** * Defines API for new QuickOpen plug-ins */ - function QuickOpenPlugin(name, fileTypes, done, search, match, itemFocus, itemSelect, resultsFormatter) { - + function QuickOpenPlugin(name, fileTypes, done, search, match, itemFocus, itemSelect, resultsFormatter) { this.name = name; this.fileTypes = fileTypes; this.done = done; @@ -156,8 +155,19 @@ define(function (require, exports, module) { */ function QuickNavigateDialog() { this.$searchField = undefined; // defined when showDialog() is called - this._handleFilter = this._handleFilter.bind(this); - this._handleResultsFormatter = this._handleResultsFormatter.bind(this); + + // Bind event handlers + this._handleItemSelect = this._handleItemSelect.bind(this); + this._handleItemFocus = this._handleItemFocus.bind(this); + this._handleKeyUp = this._handleKeyUp.bind(this); + this._handleKeyDown = this._handleKeyDown.bind(this); + this._handleResultsReady = this._handleResultsReady.bind(this); + this._handleBlur = this._handleBlur.bind(this); + this._handleDocumentMouseDown = this._handleDocumentMouseDown.bind(this); + + // Bind callbacks from smart-autocomplete + this._filterCallback = this._filterCallback.bind(this); + this._resultsFormatterCallback = this._resultsFormatterCallback.bind(this); } /** @@ -218,10 +228,10 @@ define(function (require, exports, module) { } // Smart Autocomplete uses this assumption internally: index of DOM node in results list container - // exactly matches index of search result in list returned by _handleFilter() + // exactly matches index of search result in list returned by _filterCallback() var index = $(domItem).index(); - // This is just the last return value of _handleFilter(), which smart autocomplete helpfully caches + // This is just the last return value of _filterCallback(), which smart autocomplete helpfully caches var lastFilterResult = $("input#quickOpenSearch").data("smart-autocomplete").rawResults; return lastFilterResult[index]; } @@ -234,7 +244,7 @@ define(function (require, exports, module) { * that may have not matched anything in in the list, but may have information * for carrying out an action (e.g. go to line). */ - QuickNavigateDialog.prototype._handleItemSelect = function (selectedDOMItem) { + QuickNavigateDialog.prototype._handleItemSelect = function (e, selectedDOMItem) { // This is a work-around to select first item when a selection event occurs // (usually from pressing the enter key) and no item is selected in the list. @@ -281,7 +291,7 @@ define(function (require, exports, module) { * Opens the file specified by selected item if there is no current plug-in, otherwise defers handling * to the currentPlugin */ - QuickNavigateDialog.prototype._handleItemFocus = function (selectedDOMItem) { + QuickNavigateDialog.prototype._handleItemFocus = function (e, selectedDOMItem) { var selectedItem = domItemToSearchResult(selectedDOMItem); if (currentPlugin) { @@ -362,7 +372,7 @@ define(function (require, exports, module) { /** * Give visual clue when there are no results */ - QuickNavigateDialog.prototype._handleResultsReady = function (results) { + QuickNavigateDialog.prototype._handleResultsReady = function (e, results) { var isNoResults = (results.length === 0 && isNaN(extractLineNumber(this.$searchField.val()))); this.$searchField.toggleClass("no-results", isNoResults); }; @@ -372,7 +382,6 @@ define(function (require, exports, module) { * searching is done. */ QuickNavigateDialog.prototype._close = function () { - if (!dialogOpen) { return; } @@ -406,7 +415,7 @@ define(function (require, exports, module) { $(".smart_autocomplete_container").remove(); - $(window.document).off("mousedown", this.handleDocumentMouseDown); + $(window.document).off("mousedown", this._handleDocumentMouseDown); }; /** @@ -709,8 +718,9 @@ define(function (require, exports, module) { /** * Handles changes to the current query in the search field. * @param {string} query The new query. + * @return {Array} The filtered list of results. */ - QuickNavigateDialog.prototype._handleFilter = function (query) { + QuickNavigateDialog.prototype._filterCallback = function (query) { this._updateDialogLabel(query); var curDoc = DocumentManager.getCurrentDocument(); @@ -801,8 +811,9 @@ define(function (require, exports, module) { /** * Formats the entry for the given item to be displayed in the dropdown. * @param {Object} item The item to be displayed. + * @return {string} The HTML to be displayed. */ - QuickNavigateDialog.prototype._handleResultsFormatter = function (item) { + QuickNavigateDialog.prototype._resultsFormatterCallback = function (item) { var query = this.$searchField.val(); var formatter; @@ -840,6 +851,10 @@ define(function (require, exports, module) { this._updateDialogLabel(initialString); }; + /** + * Sets the dialog label based on the type of the given query. + * @param {string} query The user's current query. + */ QuickNavigateDialog.prototype._updateDialogLabel = function (query) { var prefix = (query.length > 0 ? query.charAt(0) : ""); @@ -864,7 +879,7 @@ define(function (require, exports, module) { * but we want to close the whole search "dialog." (And we can't just piggyback on the popup closing event, since there are cases * where the popup closes that we want the dialog to remain open (e.g. deleting search term via backspace). */ - QuickNavigateDialog.prototype.handleDocumentMouseDown = function (e) { + QuickNavigateDialog.prototype._handleDocumentMouseDown = function (e) { if ($(this.dialog).find(e.target).length === 0 && $(".smart_autocomplete_container").find(e.target).length === 0) { this._close(); } else { @@ -876,21 +891,25 @@ define(function (require, exports, module) { } } }; + + /** + * Close the dialog when it loses focus. + */ + QuickNavigateDialog.prototype._handleBlur = function (e) { + this._close(); + }; /** * Shows the search dialog and initializes the auto suggestion list with filenames from the current project */ QuickNavigateDialog.prototype.showDialog = function (prefix, initialString) { - var that = this; - if (dialogOpen) { return; } dialogOpen = true; // Global listener to hide search bar & popup - this.handleDocumentMouseDown = this.handleDocumentMouseDown.bind(this); - $(window.document).on("mousedown", this.handleDocumentMouseDown); + $(window.document).on("mousedown", this._handleDocumentMouseDown); // Ty TODO: disabled for now while file switching is disabled in _handleItemFocus @@ -908,10 +927,10 @@ define(function (require, exports, module) { // Show the search bar ("dialog") var dialogHTML = ": "; - that._createDialogDiv(dialogHTML); - that.$searchField = $("input#quickOpenSearch"); + this._createDialogDiv(dialogHTML); + this.$searchField = $("input#quickOpenSearch"); - that.$searchField.smartAutoComplete({ + this.$searchField.smartAutoComplete({ source: [], maxResults: 20, minCharLimit: 0, @@ -919,17 +938,17 @@ define(function (require, exports, module) { forceSelect: false, typeAhead: false, // won't work right now because smart auto complete // using internal raw results instead of filtered results for matching - filter: this._handleFilter, - resultFormatter: this._handleResultsFormatter + filter: this._filterCallback, + resultFormatter: this._resultsFormatterCallback }); - that.$searchField.bind({ - resultsReady: function (e, results) { that._handleResultsReady(results); }, - itemSelect: function (e, selectedItem) { that._handleItemSelect(selectedItem); }, - itemFocus: function (e, selectedItem) { that._handleItemFocus(selectedItem); }, - keydown: function (e) { that._handleKeyDown(e); }, - keyup: function (e, query) { that._handleKeyUp(e); }, - blur: function (e) { that._close(); } + this.$searchField.bind({ + resultsReady: this._handleResultsReady, + itemSelect: this._handleItemSelect, + itemFocus: this._handleItemFocus, + keydown: this._handleKeyDown, + keyup: this._handleKeyUp, + blur: this._handleBlur // Note: lostFocus event DOESN'T work because auto smart complete catches the key up from shift-command-o and immediately // triggers lostFocus }); From 3bd7dda5b20aad545e91fb22324842ad7d214010 Mon Sep 17 00:00:00 2001 From: Narciso Jaramillo Date: Tue, 11 Dec 2012 10:25:05 -0800 Subject: [PATCH 5/5] Align equals signs --- src/search/QuickOpen.js | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/search/QuickOpen.js b/src/search/QuickOpen.js index c35160d46f8..593c37b7de3 100644 --- a/src/search/QuickOpen.js +++ b/src/search/QuickOpen.js @@ -157,16 +157,16 @@ define(function (require, exports, module) { this.$searchField = undefined; // defined when showDialog() is called // Bind event handlers - this._handleItemSelect = this._handleItemSelect.bind(this); - this._handleItemFocus = this._handleItemFocus.bind(this); - this._handleKeyUp = this._handleKeyUp.bind(this); - this._handleKeyDown = this._handleKeyDown.bind(this); - this._handleResultsReady = this._handleResultsReady.bind(this); - this._handleBlur = this._handleBlur.bind(this); - this._handleDocumentMouseDown = this._handleDocumentMouseDown.bind(this); + this._handleItemSelect = this._handleItemSelect.bind(this); + this._handleItemFocus = this._handleItemFocus.bind(this); + this._handleKeyUp = this._handleKeyUp.bind(this); + this._handleKeyDown = this._handleKeyDown.bind(this); + this._handleResultsReady = this._handleResultsReady.bind(this); + this._handleBlur = this._handleBlur.bind(this); + this._handleDocumentMouseDown = this._handleDocumentMouseDown.bind(this); // Bind callbacks from smart-autocomplete - this._filterCallback = this._filterCallback.bind(this); + this._filterCallback = this._filterCallback.bind(this); this._resultsFormatterCallback = this._resultsFormatterCallback.bind(this); }