Skip to content
This repository was archived by the owner on Sep 6, 2021. It is now read-only.

Commit d73470f

Browse files
author
Narciso Jaramillo
committed
For #5086, wait for old Quick Open bar to close before creating a new one. Also move some global state into the QuickNavigateDialog object.
1 parent f862294 commit d73470f

2 files changed

Lines changed: 94 additions & 57 deletions

File tree

src/search/QuickOpen.js

Lines changed: 86 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -67,30 +67,6 @@ define(function (require, exports, module) {
6767
/** @type $.Promise */
6868
var fileListPromise;
6969

70-
/**
71-
* Remembers the current document that was displayed when showDialog() was called.
72-
* TODO: in the future, if focusing an item can switch documents, need to restore this on Escape.
73-
* @type {?string} full path
74-
*/
75-
var origDocPath;
76-
77-
/**
78-
* Remembers the selection in origDocPath that was present when showDialog() was called. Focusing on an
79-
* item can change the selection; we restore this original selection if the user presses Escape. Null if
80-
* no document was open when Quick Open was invoked.
81-
* @type {?{start:{line:number, ch:number}, end:{line:number, ch:number}}}
82-
*/
83-
var origSelection;
84-
85-
/**
86-
* Remembers the scroll position in origDocPath when showDialog() was called (see origSelection above).
87-
* @type {?{x:number, y:number}}
88-
*/
89-
var origScrollPos;
90-
91-
/** @type {boolean} */
92-
var dialogOpen = false;
93-
9470
/**
9571
* The currently open quick open dialog.
9672
*/
@@ -197,6 +173,13 @@ define(function (require, exports, module) {
197173
}
198174

199175
/**
176+
* True if the dialog is currently open.
177+
* @type {boolean}
178+
*/
179+
QuickNavigateDialog.prototype.isOpen = false;
180+
181+
/**
182+
* @private
200183
* Handles caching of filename search information for the lifetime of a
201184
* QuickNavigateDialog (a single search until the dialog is dismissed)
202185
*
@@ -205,13 +188,47 @@ define(function (require, exports, module) {
205188
QuickNavigateDialog.prototype._filenameMatcher = null;
206189

207190
/**
191+
* @private
208192
* StringMatcher caches for each QuickOpen plugin that keep track of search
209193
* information for the lifetime of a QuickNavigateDialog (a single search
210194
* until the dialog is dismissed)
211195
*
212196
* @type {Object.<string, StringMatch.StringMatcher>}
213197
*/
214198
QuickNavigateDialog.prototype._matchers = null;
199+
200+
/**
201+
* @private
202+
* If the dialog is closing, this will contain a deferred that is resolved
203+
* when it's done closing.
204+
* @type {$.Deferred}
205+
*/
206+
QuickNavigateDialog.prototype._closeDeferred = null;
207+
208+
209+
/**
210+
* @private
211+
* Remembers the current document that was displayed when showDialog() was called.
212+
* TODO: in the future, if focusing an item can switch documents, need to restore this on Escape.
213+
* @type {?string} full path
214+
*/
215+
QuickNavigateDialog.prototype._origDocPath = null;
216+
217+
/**
218+
* @private
219+
* Remembers the selection in origDocPath that was present when showDialog() was called. Focusing on an
220+
* item can change the selection; we restore this original selection if the user presses Escape. Null if
221+
* no document was open when Quick Open was invoked.
222+
* @type {?{start:{line:number, ch:number}, end:{line:number, ch:number}}}
223+
*/
224+
QuickNavigateDialog.prototype._origSelection = null;
225+
226+
/**
227+
* @private
228+
* Remembers the scroll position in origDocPath when showDialog() was called (see origSelection above).
229+
* @type {?{x:number, y:number}}
230+
*/
231+
QuickNavigateDialog.prototype._origScrollPos = null;
215232

216233
function _filenameFromPath(path, includeExtension) {
217234
var end;
@@ -318,7 +335,7 @@ define(function (require, exports, module) {
318335
}
319336

320337

321-
this._close();
338+
this.close();
322339
EditorManager.focusEditor();
323340
};
324341

@@ -367,17 +384,22 @@ define(function (require, exports, module) {
367384
var self = this;
368385
setTimeout(function () {
369386
if (e.keyCode === KeyEvent.DOM_VK_ESCAPE) {
370-
self._close()
371-
.done(function () {
372-
// Restore original selection / scroll pos
373-
if (origSelection) {
374-
EditorManager.getCurrentFullEditor().setSelection(origSelection.start, origSelection.end);
375-
EditorManager.getCurrentFullEditor().setScrollPos(origScrollPos.x, origScrollPos.y);
376-
}
377-
});
387+
self.close();
378388

389+
// This is tricky. We want to restore the original selection, and we don't actually want to wait
390+
// until the modal bar has fully animated closed. However, we do need to wait until the next
391+
// event loop after ModalBar.close() has (synchronously) finished, because ModalBar.close() itself
392+
// tries to rescroll the editor (in order to restore the *apparent* scroll position that gets
393+
// changed when the ModalBar pushes the editor down). So we do this on yet another timeout.
394+
setTimeout(function () {
395+
// Restore original selection / scroll pos
396+
if (self._origSelection) {
397+
EditorManager.getCurrentFullEditor().setSelection(self._origSelection.start, self._origSelection.end);
398+
EditorManager.getCurrentFullEditor().setScrollPos(self._origScrollPos.x, self._origScrollPos.y);
399+
}
400+
}, 0);
379401
} else if (e.keyCode === KeyEvent.DOM_VK_RETURN) {
380-
self._handleItemSelect(null, $(".smart_autocomplete_highlight").get(0)); // calls _close() too
402+
self._handleItemSelect(null, $(".smart_autocomplete_highlight").get(0)); // calls close() too
381403
}
382404
}, 0);
383405

@@ -431,11 +453,12 @@ define(function (require, exports, module) {
431453
* searching is done.
432454
* @return {$.Promise} Resolved when the search bar is entirely closed.
433455
*/
434-
QuickNavigateDialog.prototype._close = function () {
435-
if (!dialogOpen) {
436-
return new $.Deferred().reject();
456+
QuickNavigateDialog.prototype.close = function () {
457+
if (!this.isOpen) {
458+
return this._closeDeferred.promise();
437459
}
438-
dialogOpen = false;
460+
this.isOpen = false;
461+
this._closeDeferred = new $.Deferred();
439462

440463
var i;
441464
for (i = 0; i < plugins.length; i++) {
@@ -448,21 +471,21 @@ define(function (require, exports, module) {
448471
this.$searchField.trigger("lostFocus");
449472

450473
// Closing the dialog is a little tricky (see #1384): some Smart Autocomplete code may run later (e.g.
451-
// (because it's a later handler of the event that just triggered _close()), and that code expects to
474+
// (because it's a later handler of the event that just triggered close()), and that code expects to
452475
// find metadata that it stuffed onto the DOM node earlier. But $.remove() strips that metadata.
453476
// So we wait until after this call chain is complete before actually closing the dialog.
454-
var result = new $.Deferred();
455477
var self = this;
456478
setTimeout(function () {
457-
self.modalBar.close();
458-
result.resolve();
479+
self.modalBar.close().done(function () {
480+
self._closeDeferred.resolve();
481+
});
459482
}, 0);
460483

461484
$(".smart_autocomplete_container").remove();
462485

463486
$(window.document).off("mousedown", this._handleDocumentMouseDown);
464487

465-
return result.promise();
488+
return this._closeDeferred.promise();
466489
};
467490

468491
/**
@@ -719,7 +742,7 @@ define(function (require, exports, module) {
719742
*/
720743
QuickNavigateDialog.prototype._handleDocumentMouseDown = function (e) {
721744
if (this.modalBar.getRoot().find(e.target).length === 0 && $(".smart_autocomplete_container").find(e.target).length === 0) {
722-
this._close();
745+
this.close();
723746
} else {
724747
// Allow clicks in the search field to propagate. Clicks in the menu should be
725748
// blocked to prevent focus from leaving the search field.
@@ -734,31 +757,31 @@ define(function (require, exports, module) {
734757
* Close the dialog when it loses focus.
735758
*/
736759
QuickNavigateDialog.prototype._handleBlur = function (e) {
737-
this._close();
760+
this.close();
738761
};
739762

740763
/**
741764
* Shows the search dialog and initializes the auto suggestion list with filenames from the current project
742765
*/
743766
QuickNavigateDialog.prototype.showDialog = function (prefix, initialString) {
744-
if (dialogOpen) {
767+
if (this.isOpen) {
745768
return;
746769
}
747-
dialogOpen = true;
770+
this.isOpen = true;
748771

749772
// Global listener to hide search bar & popup
750773
$(window.document).on("mousedown", this._handleDocumentMouseDown);
751774

752775
// Record current document & cursor pos so we can restore it if search is canceled
753776
// We record scroll pos *before* modal bar is opened since we're going to restore it *after* it's closed
754777
var curDoc = DocumentManager.getCurrentDocument();
755-
origDocPath = curDoc ? curDoc.file.fullPath : null;
778+
this._origDocPath = curDoc ? curDoc.file.fullPath : null;
756779
if (curDoc) {
757-
origSelection = EditorManager.getCurrentFullEditor().getSelection();
758-
origScrollPos = EditorManager.getCurrentFullEditor().getScrollPos();
780+
this._origSelection = EditorManager.getCurrentFullEditor().getSelection();
781+
this._origScrollPos = EditorManager.getCurrentFullEditor().getScrollPos();
759782
} else {
760-
origSelection = null;
761-
origScrollPos = null;
783+
this._origSelection = null;
784+
this._origScrollPos = null;
762785
}
763786

764787
// Show the search bar ("dialog")
@@ -818,12 +841,20 @@ define(function (require, exports, module) {
818841
* @param {?string} initialString
819842
*/
820843
function beginSearch(prefix, initialString) {
821-
if (dialogOpen) {
822-
_curDialog.setSearchFieldValue(prefix, initialString);
823-
} else {
844+
function createDialog() {
824845
_curDialog = new QuickNavigateDialog();
825846
_curDialog.showDialog(prefix, initialString);
826847
}
848+
849+
if (_curDialog && _curDialog.isOpen) {
850+
_curDialog.setSearchFieldValue(prefix, initialString);
851+
} else {
852+
if (_curDialog) {
853+
_curDialog.close().done(createDialog);
854+
} else {
855+
createDialog();
856+
}
857+
}
827858
}
828859

829860
function doFileSearch() {

src/widgets/ModalBar.js

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -118,11 +118,14 @@ define(function (require, exports, module) {
118118
};
119119

120120
/**
121-
* Closes the modal bar and returns focus to the active editor.
121+
* Closes the modal bar and returns focus to the active editor. Returns a promise that is
122+
* resolved when the bar is fully closed and the container is removed from the DOM.
123+
* @return {$.Promise} promise resolved when close is finished
122124
*/
123125
ModalBar.prototype.close = function () {
124126
// Store our height before closing, while we can still measure it
125-
var barHeight = this.height();
127+
var result = new $.Deferred(),
128+
barHeight = this.height();
126129

127130
if (this._autoClose) {
128131
window.document.body.removeEventListener("focusin", this._handleFocusChange, true);
@@ -131,6 +134,7 @@ define(function (require, exports, module) {
131134
var self = this;
132135
this._$root.addClass("modal-bar-hide").one("webkitTransitionEnd", function () {
133136
self._$root.remove();
137+
result.resolve();
134138
});
135139

136140
// Preserve scroll position of the current full editor across the editor refresh, adjusting for the
@@ -145,6 +149,8 @@ define(function (require, exports, module) {
145149
fullEditor._codeMirror.scrollTo(scrollPos.x, scrollPos.y - barHeight);
146150
}
147151
EditorManager.focusEditor();
152+
153+
return result.promise();
148154
};
149155

150156
/**

0 commit comments

Comments
 (0)