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

Commit

Permalink
Safer focusedEditorChanged event:
Browse files Browse the repository at this point in the history
* Revert main-editor-swapping codepath back to how it used to work, mostly
disentangled from focus events.
* Don't fire focusedEditorChanged on the many times focus returns to the
same Editor that had it last (due to window reactivation, closing a dialog,
closing a search bar, etc.). All the current use cases for this event don't
care about those cases. Add more detailed docs on event cases.
* Simplify how Editor signals focus: trigger only from "onFocus" (a superset
of the other case), and remove _internalFocus flag that was guarding against
the overlap.
* Don't call resizeEditor() on every editor swap just because statusbar
*might* have been shown/hidden: only call when statusbar actually did
change (going to/from no editor). Fixes scrolling issue in #1864.

#1257 still isn't 100% fixed: there are at least two cases where getFocusedEditor()
lags behind the focusedEditorChanged event: opening an inline editor; and
moving focus from open inline editor to its host editor. These same cases -
and a number of others - were broken before this change, however.
  • Loading branch information
peterflynn committed Oct 18, 2012
1 parent 6a034b2 commit 4a34b88
Show file tree
Hide file tree
Showing 3 changed files with 55 additions and 51 deletions.
13 changes: 1 addition & 12 deletions src/editor/Editor.js
Original file line number Diff line number Diff line change
Expand Up @@ -626,10 +626,7 @@ define(function (require, exports, module) {
// Convert CodeMirror onFocus events to EditorManager focusedEditorChanged
this._codeMirror.setOption("onFocus", function () {
self._focused = true;

if (!self._internalFocus) {
EditorManager._doFocusedEditorChanged(self);
}
EditorManager._notifyFocusedEditorChanged(self);
});

this._codeMirror.setOption("onBlur", function () {
Expand Down Expand Up @@ -981,15 +978,7 @@ define(function (require, exports, module) {

/** Gives focus to the editor control */
Editor.prototype.focus = function () {
// Capture the currently focused editor before making CodeMirror changes
var previous = EditorManager.getFocusedEditor();

// Prevent duplicate focusedEditorChanged events with this _internalFocus flag
this._internalFocus = true;
this._codeMirror.focus();
this._internalFocus = false;

EditorManager._doFocusedEditorChanged(this, previous);
};

/** Returns true if the editor has focus */
Expand Down
80 changes: 44 additions & 36 deletions src/editor/EditorManager.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,16 @@
* must have some knowledge about Document's internal state (we access its _editor property).
*
* This module dispatches the following events:
* - focusedEditorChange -- Fires after the focused editor (full or inline)
* changes and size/visibility are complete.
* - focusedEditorChange -- Fires after the focused editor (full or inline) changes and size/visibility
* are complete. Doesn't fire when editor temporarily loses focus to a non-editor
* control (e.g. search toolbar or modal dialog, or window deactivation). Does
* fire when focus moves between inline editor and its full-size container.
* Roughly, this event tracks getFocusedEditor() changes, while DocumentManager's
* currentDocumentChange tracks getCurrentFullEditor() changes.
* The 2nd arg to the listener is which Editor gained focus; the 3rd arg is
* which Editor lost focus as a result. Either one may be null.
* TODO (#1257): getFocusedEditor() sometimes lags behind this event. Listeners
* should use the arguments to reliably see which Editor just gained focus.
*/
define(function (require, exports, module) {
"use strict";
Expand All @@ -60,11 +68,20 @@ define(function (require, exports, module) {
/** @type {jQueryObject} DOM node that contains all editors (visible and hidden alike) */
var _editorHolder = null;

/** @type {Editor} */
/**
* Currently visible full-size Editor, or null if no editors open
* @type {?Editor}
*/
var _currentEditor = null;
/** @type {Document} */
/** @type {?Document} */
var _currentEditorsDocument = null;

/**
* Currently focused Editor (full-size, inline, or otherwise)
* @type {?Editor}
*/
var _lastFocusedEditor = null;

/**
* Registered inline-editor widget providers. See {@link #registerInlineEditProvider()}.
* @type {Array.<function(...)>}
Expand Down Expand Up @@ -360,34 +377,19 @@ define(function (require, exports, module) {
resizeEditor(true);
}


/**
* @private
*/
function _doFocusedEditorChanged(current, previous) {
// Skip if the new editor is already the focused editor.
// This may happen if the window loses then regains focus.
if (previous === current) {
function _notifyFocusedEditorChanged(current) {
// Skip if the Editor that gained focus was already the most recently focused editor.
// This may happen e.g. if the window loses then regains focus.
if (_lastFocusedEditor === current) {
return;
}

// if switching to no-editor, hide the last full editor
if (_currentEditor && !current) {
_currentEditor.setVisible(false);
}

// _currentEditor must be a full editor or null (show no editor).
// _currentEditor must not be an inline editor
if (!current || (current && !current._visibleRange)) {
_currentEditor = current;
}

// Window may have been resized since last time editor was visible, so kick it now
if (_currentEditor) {
_currentEditor.setVisible(true);
resizeEditor();
}

$(exports).triggerHandler("focusedEditorChange", [current, previous]);
_lastFocusedEditor = current;

$(exports).triggerHandler("focusedEditorChange", [current, _lastFocusedEditor]);
}

/**
Expand All @@ -396,7 +398,12 @@ define(function (require, exports, module) {
function _doShow(document) {
// Show new editor
_currentEditorsDocument = document;
_doFocusedEditorChanged(document._masterEditor, _currentEditor);
_currentEditor = document._masterEditor;

_currentEditor.setVisible(true);

// Window may have been resized since last time editor was visible, so kick it now
resizeEditor();
}

/**
Expand Down Expand Up @@ -426,12 +433,16 @@ define(function (require, exports, module) {
/** Hide the currently visible editor and show a placeholder UI in its place */
function _showNoEditor() {
if (_currentEditor) {
var origCurrentEditor = _currentEditor;
_currentEditor.setVisible(false);
_destroyEditorIfUnneeded(_currentEditorsDocument);
_doFocusedEditorChanged(null, origCurrentEditor);

_currentEditorsDocument = null;
_currentEditor = null;

$("#not-editor").css("display", "");

// No other Editor is gaining focus, so in this one special case we must trigger event manually
_notifyFocusedEditorChanged(null);
}
}

Expand Down Expand Up @@ -674,12 +685,9 @@ define(function (require, exports, module) {
}

if (!current) {
StatusBar.hide();
resizeEditor();
StatusBar.hide(); // calls resizeEditor() if needed
} else {
// Check if the statusbar is not visible to show it
StatusBar.show();
resizeEditor();
StatusBar.show(); // calls resizeEditor() if needed

$(current).on("cursorActivity.statusbar", _updateCursorInfo);
$(current).on("change.statusbar", function () {
Expand Down Expand Up @@ -754,7 +762,7 @@ define(function (require, exports, module) {
// For unit tests and internal use only
exports._init = _init;
exports._openInlineWidget = _openInlineWidget;
exports._doFocusedEditorChanged = _doFocusedEditorChanged;
exports._notifyFocusedEditorChanged = _notifyFocusedEditorChanged;
exports._createFullEditorForDocument = _createFullEditorForDocument;
exports._destroyEditorIfUnneeded = _destroyEditorIfUnneeded;

Expand Down
13 changes: 10 additions & 3 deletions src/widgets/StatusBar.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ define(function (require, exports, module) {

var AppInit = require("utils/AppInit"),
StatusBarHTML = require("text!widgets/StatusBar.html"),
EditorManager = require("editor/EditorManager"),
Strings = require("strings");

var _init = false;
Expand Down Expand Up @@ -139,8 +140,11 @@ define(function (require, exports, module) {
if (!_init) {
return;
}

$statusBar.hide();

if ($statusBar.is(":visible")) {
$statusBar.hide();
EditorManager.resizeEditor(); // changes available ht for editor area
}
}

/**
Expand All @@ -151,7 +155,10 @@ define(function (require, exports, module) {
return;
}

$statusBar.show();
if (!$statusBar.is(":visible")) {
$statusBar.show();
EditorManager.resizeEditor(); // changes available ht for editor area
}
}

function init($parent) {
Expand Down

0 comments on commit 4a34b88

Please sign in to comment.