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

Commit

Permalink
Code review cleanup: changed onClosed() to close(), other minor fixes.
Browse files Browse the repository at this point in the history
  • Loading branch information
njx committed Apr 4, 2012
1 parent 25fd303 commit 1f74f8a
Show file tree
Hide file tree
Showing 5 changed files with 13 additions and 17 deletions.
6 changes: 3 additions & 3 deletions src/editor/CSSInlineEditor.js
Original file line number Diff line number Diff line change
Expand Up @@ -182,10 +182,10 @@ define(function (require, exports, module) {
};

/**
* Called any time inline was closed, whether manually (via closeThisInline()) or automatically
* Called any time inline is closed, whether manually (via closeThisInline()) or automatically
*/
CSSInlineEditor.prototype.onClosed = function () {
this.parentClass.onClosed.call(this); // super.onClosed()
CSSInlineEditor.prototype.close = function () {
this.parentClass.close.call(this); // super.close()

// remove resize handlers for relatedContainer
$(this.hostEditor).off("change", this._updateRelatedContainer);
Expand Down
6 changes: 2 additions & 4 deletions src/editor/Editor.js
Original file line number Diff line number Diff line change
Expand Up @@ -347,7 +347,7 @@ define(function (require, exports, module) {
// Destroying us destroys any inline widgets we're hosting. Make sure their closeCallbacks
// run, at least, since they may also need to release Document refs
this._inlineWidgets.forEach(function (inlineWidget) {
inlineWidget.onClosed();
inlineWidget.close();
});
};

Expand Down Expand Up @@ -699,12 +699,10 @@ define(function (require, exports, module) {
var self = this;
inlineWidget.id = this._codeMirror.addInlineWidget(pos, inlineWidget.htmlContent, inlineWidget.height, function (id) {
self._removeInlineWidgetInternal(id);
inlineWidget.onClosed();
inlineWidget.close();
});
this._inlineWidgets.push(inlineWidget);
inlineWidget.onAdded();

return inlineWidget.id;
};

/**
Expand Down
10 changes: 4 additions & 6 deletions src/editor/EditorManager.js
Original file line number Diff line number Diff line change
Expand Up @@ -143,10 +143,10 @@ define(function (require, exports, module) {
}

/**
* Removes the given widget UI from the given hostEdtior (agnostic of what the widget's content
* is). The widget's onClosed() callback will be run as a result.
* @param {!Editor} hostEditor
* @param {!number} inlineId
* Removes the given widget UI from the given hostEditor (agnostic of what the widget's content
* is). The widget's close() method will be run as a result.
* @param {!Editor} hostEditor The editor containing the widget.
* @param {!InlineWidget} inlineWidget The inline widget to close.
* @param {!boolean} moveFocus If true, focuses hostEditor and ensures the cursor position lies
* near the inline's location.
*/
Expand All @@ -164,9 +164,7 @@ define(function (require, exports, module) {
}

hostEditor.removeInlineWidget(inlineWidget);

}


/**
* Registers a new inline provider. When _openInlineWidget() is called each registered inline
Expand Down
4 changes: 2 additions & 2 deletions src/editor/InlineTextEditor.js
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ define(function (require, exports, module) {
/**
* Called any time inline was closed, whether manually (via close()) or automatically
*/
InlineTextEditor.prototype.onClosed = function () {
InlineTextEditor.prototype.close = function () {
_syncGutterWidths(this.hostEditor);

this.editors.forEach(function (editor) {
Expand Down Expand Up @@ -258,7 +258,7 @@ define(function (require, exports, module) {
InlineTextEditor.prototype.close = function () {
var shouldMoveFocus = this._editorHasFocus();

This comment has been minimized.

Copy link
@jasonsanjose

jasonsanjose Apr 4, 2012

Member

Oops. InlineTextEditor has 2 close() methods now. This one is the inline gesture handler passed to EditorManager

EditorManager.closeInlineWidget(this.hostEditor, this, shouldMoveFocus);
// closeInlineWidget() causes our onClosed() to get run
// closeInlineWidget() causes our close() to be called
};


Expand Down
4 changes: 2 additions & 2 deletions src/editor/InlineWidget.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,9 @@ define(function (require, exports, module) {
InlineWidget.prototype.hostEditor = null;

/**
* Called any time inline was closed, whether manually (via close()) or automatically
* Called any time inline is closed, whether manually or automatically
*/
InlineWidget.prototype.onClosed = function () {
InlineWidget.prototype.close = function () {
// do nothing - base implementation
};

Expand Down

0 comments on commit 1f74f8a

Please sign in to comment.