From 3b319c93a48e52fb2dcefd8271753e9cb7459bf0 Mon Sep 17 00:00:00 2001 From: Narciso Jaramillo Date: Tue, 3 Apr 2012 17:56:22 -0700 Subject: [PATCH 1/4] Get rid of EditorManager._addInlineWidget, an dmake it so Editor.addInlineWidget just takes an actual inlineWidget and stores the widget itself in the _inlineWidgets list. --- src/editor/CSSInlineEditor.js | 2 +- src/editor/Editor.js | 41 ++++++++-------------- src/editor/EditorManager.js | 41 +++++----------------- src/editor/InlineTextEditor.js | 6 ++-- src/editor/InlineWidget.js | 10 +++--- test/spec/CSSInlineEditor-test.js | 4 +-- test/spec/InlineEditorProviders-test.js | 46 +++++++++++++------------ 7 files changed, 57 insertions(+), 93 deletions(-) diff --git a/src/editor/CSSInlineEditor.js b/src/editor/CSSInlineEditor.js index 9074bfa7def..5a72f792bfd 100644 --- a/src/editor/CSSInlineEditor.js +++ b/src/editor/CSSInlineEditor.js @@ -257,7 +257,7 @@ define(function (require, exports, module) { this.parentClass.sizeInlineWidgetToContents.call(this, force); // Size the widget height to the max between the editor content and the related rules list var widgetHeight = Math.max(this.$relatedContainer.find(".related").height(), this.$editorsDiv.height()); - this.hostEditor.setInlineWidgetHeight(this.inlineId, widgetHeight, true); + this.hostEditor.setInlineWidgetHeight(this.id, widgetHeight, true); // The related rules container size itself based on htmlContent which is set by setInlineWidgetHeight above. this._updateRelatedContainer(); diff --git a/src/editor/Editor.js b/src/editor/Editor.js index 9a12dd539d4..476297818b2 100644 --- a/src/editor/Editor.js +++ b/src/editor/Editor.js @@ -341,8 +341,8 @@ 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 (inlineInfo) { - inlineInfo.closeCallback(); + this._inlineWidgets.forEach(function (inlineWidget) { + inlineWidget.onClosed(); }); }; @@ -666,40 +666,31 @@ define(function (require, exports, module) { return this._codeMirror.getScrollerElement(); }; - /** * Adds an inline widget below the given line. If any inline widget was already open for that * line, it is closed without warning. * @param {!{line:number, ch:number}} pos Position in text to anchor the inline. - * @param {!DOMElement} domContent DOM node of widget UI to insert. - * @param {number} initialHeight Initial height to accomodate. - * @param {function()} parentShowCallback Function called when the host editor is shown - * (via Editor.setVisible()). - * @param {function()} closeCallback Function called when inline is closed, either automatically - * by CodeMirror, or by this host Editor closing, or manually via removeInlineWidget(). - * @param {Object} data Extra data to track along with the widget. Accessible later via - * {@link #getInlineWidgets()}. - * @return {number} id for this inline widget instance; unique to this Editor - */ - Editor.prototype.addInlineWidget = function (pos, domContent, initialHeight, parentShowCallback, closeCallback, data) { - // Now add the new widget + * @param {!InlineWidget} inlineWidget The widget to add. + */ + Editor.prototype.addInlineWidget = function (pos, inlineWidget) { var self = this; - var inlineId = this._codeMirror.addInlineWidget(pos, domContent, initialHeight, function (id) { + inlineWidget.id = this._codeMirror.addInlineWidget(pos, inlineWidget.htmlContent, inlineWidget.height, function (id) { self._removeInlineWidgetInternal(id); - closeCallback(); + inlineWidget.onClosed(); }); - this._inlineWidgets.push({ id: inlineId, data: data, parentShowCallback: parentShowCallback, closeCallback: closeCallback }); + this._inlineWidgets.push(inlineWidget); + inlineWidget.onAdded(); - return inlineId; + return inlineWidget.id; }; /** * Removes the given inline widget. - * @param {number} inlineId id returned by addInlineWidget(). + * @param {number} inlineWidget The widget to remove. */ - Editor.prototype.removeInlineWidget = function (inlineId) { + Editor.prototype.removeInlineWidget = function (inlineWidget) { // _removeInlineWidgetInternal will get called from the destroy callback in CodeMirror. - this._codeMirror.removeInlineWidget(inlineId); + this._codeMirror.removeInlineWidget(inlineWidget.id); }; /** @@ -763,10 +754,8 @@ define(function (require, exports, module) { $(this._codeMirror.getWrapperElement()).css("display", (show ? "" : "none")); this._codeMirror.refresh(); if (show) { - this._inlineWidgets.forEach(function (widget) { - if (widget.parentShowCallback) { - widget.parentShowCallback(); - } + this._inlineWidgets.forEach(function (inlineWidget) { + inlineWidget.onParentShown(); }); } }; diff --git a/src/editor/EditorManager.js b/src/editor/EditorManager.js index 77e4c5a0e57..c868449d00c 100644 --- a/src/editor/EditorManager.js +++ b/src/editor/EditorManager.js @@ -108,30 +108,6 @@ define(function (require, exports, module) { return new Editor(doc, makeMasterEditor, mode, container, extraKeys, range); } - /** - * @private - * Adds a new widget to the host Editor. - * @param {!Editor} editor the candidate host editor - * @param !{line:number, ch:number} pos - * @param {!InlineWidget} inlineWidget - */ - function _addInlineWidget(editor, pos, inlineWidget) { - $(inlineWidget.htmlContent).append('
') - .append('
'); - - var closeCallback = function () { - inlineWidget.onClosed(); - }; - var parentShowCallback = function () { - inlineWidget.onParentShown(); - }; - - var inlineId = editor.addInlineWidget(pos, inlineWidget.htmlContent, inlineWidget.height, - parentShowCallback, closeCallback, inlineWidget); - - inlineWidget.onAdded(inlineId); - } - /** * @private * Bound to Ctrl+E on outermost editors. @@ -154,7 +130,7 @@ define(function (require, exports, module) { // If one of them will provide a widget, show it inline once ready if (inlinePromise) { inlinePromise.done(function (inlineWidget) { - _addInlineWidget(editor, pos, inlineWidget); + editor.addInlineWidget(pos, inlineWidget); result.resolve(); }).fail(function () { result.reject(); @@ -174,11 +150,11 @@ define(function (require, exports, module) { * @param {!boolean} moveFocus If true, focuses hostEditor and ensures the cursor position lies * near the inline's location. */ - function closeInlineWidget(hostEditor, inlineId, moveFocus) { + function closeInlineWidget(hostEditor, inlineWidget, moveFocus) { if (moveFocus) { // Place cursor back on the line just above the inline (the line from which it was opened) // If cursor's already on that line, leave it be to preserve column position - var widgetLine = hostEditor._codeMirror.getInlineWidgetInfo(inlineId).line; + var widgetLine = hostEditor._codeMirror.getInlineWidgetInfo(inlineWidget.id).line; var cursorLine = hostEditor.getCursorPos().line; if (cursorLine !== widgetLine) { hostEditor.setCursorPos({ line: widgetLine, pos: 0 }); @@ -187,7 +163,7 @@ define(function (require, exports, module) { hostEditor.focus(); } - hostEditor.removeInlineWidget(inlineId); + hostEditor.removeInlineWidget(inlineWidget); } @@ -219,8 +195,8 @@ define(function (require, exports, module) { function getInlineEditors(hostEditor) { var inlineEditors = []; hostEditor.getInlineWidgets().forEach(function (widget) { - if (widget.data instanceof InlineTextEditor) { - inlineEditors.concat(widget.data.editors); + if (widget instanceof InlineTextEditor) { + inlineEditors.concat(widget.editors); } }); return inlineEditors; @@ -449,8 +425,8 @@ define(function (require, exports, module) { // See if any inlines have focus _currentEditor.getInlineWidgets().forEach(function (widget) { - if (widget.data instanceof InlineTextEditor) { - widget.data.editors.forEach(function (editor) { + if (widget instanceof InlineTextEditor) { + widget.editors.forEach(function (editor) { if (editor.hasFocus()) { focusedInline = editor; } @@ -480,7 +456,6 @@ define(function (require, exports, module) { // For unit tests exports._openInlineWidget = _openInlineWidget; - exports._addInlineWidget = _addInlineWidget; // Define public API exports.setEditorHolder = setEditorHolder; diff --git a/src/editor/InlineTextEditor.js b/src/editor/InlineTextEditor.js index 8c890a6dedc..0984c0f5e2d 100644 --- a/src/editor/InlineTextEditor.js +++ b/src/editor/InlineTextEditor.js @@ -154,9 +154,7 @@ define(function (require, exports, module) { * @param {string} the inline ID that is generated by CodeMirror after the widget that holds the inline * editor is constructed and added to the DOM */ - InlineTextEditor.prototype.onAdded = function (inlineId) { - this.inlineId = inlineId; - + InlineTextEditor.prototype.onAdded = function () { this.editors.forEach(function (editor) { editor.refresh(); }); @@ -259,7 +257,7 @@ define(function (require, exports, module) { /** Closes this inline widget and all its contained Editors */ InlineTextEditor.prototype.close = function () { var shouldMoveFocus = this._editorHasFocus(); - EditorManager.closeInlineWidget(this.hostEditor, this.inlineId, shouldMoveFocus); + EditorManager.closeInlineWidget(this.hostEditor, this, shouldMoveFocus); // closeInlineWidget() causes our onClosed() to get run }; diff --git a/src/editor/InlineWidget.js b/src/editor/InlineWidget.js index c847b27eedb..cf79762b99b 100644 --- a/src/editor/InlineWidget.js +++ b/src/editor/InlineWidget.js @@ -19,10 +19,12 @@ define(function (require, exports, module) { // create the outer wrapper div this.htmlContent = document.createElement("div"); this.$htmlContent = $(this.htmlContent).addClass("InlineWidget"); + this.$htmlContent.append('
') + .append('
'); } InlineWidget.prototype.htmlContent = null; InlineWidget.prototype.height = 0; - InlineWidget.prototype.inlineId = null; + InlineWidget.prototype.id = null; InlineWidget.prototype.hostEditor = null; /** @@ -34,11 +36,9 @@ define(function (require, exports, module) { /** * Some tasks have to wait until we've been parented into the outer editor - * @param {string} the inline ID that is generated by CodeMirror after the widget that holds the inline - * editor is constructed and added to the DOM */ - InlineWidget.prototype.onAdded = function (inlineId) { - this.inlineId = inlineId; + InlineWidget.prototype.onAdded = function () { + // do nothing - base implementation }; /** diff --git a/test/spec/CSSInlineEditor-test.js b/test/spec/CSSInlineEditor-test.js index c21f4b9a9e7..1c3fbb6eada 100644 --- a/test/spec/CSSInlineEditor-test.js +++ b/test/spec/CSSInlineEditor-test.js @@ -44,7 +44,7 @@ define(function (require, exports, module) { expect(cssInlineEditor.editors.length).toBe(0); expect(cssInlineEditor.htmlContent instanceof HTMLElement).toBe(true); expect(cssInlineEditor.height).toBe(0); - expect(cssInlineEditor.inlineId).toBeNull(); + expect(cssInlineEditor.id).toBeNull(); expect(cssInlineEditor.hostEditor).toBeNull(); }); @@ -218,7 +218,7 @@ define(function (require, exports, module) { cssInlineEditor.load(hostEditor); // add widget directly, bypass _openInlineWidget - EditorManager._addInlineWidget(hostEditor, {line: 0, ch: 0}, cssInlineEditor); + hostEditor.addInlineWidget({line: 0, ch: 0}, cssInlineEditor); // verify it was added expect(hostEditor.hasFocus()).toBe(false); diff --git a/test/spec/InlineEditorProviders-test.js b/test/spec/InlineEditorProviders-test.js index 402b152e5bc..dbcfa975616 100644 --- a/test/spec/InlineEditorProviders-test.js +++ b/test/spec/InlineEditorProviders-test.js @@ -3,7 +3,7 @@ */ /*jslint vars: true, plusplus: true, devel: true, browser: true, nomen: true, indent: 4, maxerr: 50 */ -/*global define: false, describe: false, it: false, xit: false, expect: false, beforeEach: false, afterEach: false, waitsFor: false, runs: false, $: false */ +/*global define: false, describe: false, it: false, xit: false, expect: false, beforeEach: false, afterEach: false, waitsFor: false, runs: false, $: false, brackets: false */ define(function (require, exports, module) { 'use strict'; @@ -224,8 +224,8 @@ define(function (require, exports, module) { initInlineTest("test1.html", 0); runs(function () { - var inlineData = EditorManager.getCurrentFullEditor().getInlineWidgets()[0].data; - var inlinePos = inlineData.editors[0].getCursorPos(); + var inlineWidget = EditorManager.getCurrentFullEditor().getInlineWidgets()[0]; + var inlinePos = inlineWidget.editors[0].getCursorPos(); // verify cursor position in inline editor expect(inlinePos).toEqual(this.infos["test1.css"].offsets[0]); @@ -236,8 +236,8 @@ define(function (require, exports, module) { initInlineTest("test1.html", 1); runs(function () { - var inlineData = EditorManager.getCurrentFullEditor().getInlineWidgets()[0].data; - var inlinePos = inlineData.editors[0].getCursorPos(); + var inlineWidget = EditorManager.getCurrentFullEditor().getInlineWidgets()[0]; + var inlinePos = inlineWidget.editors[0].getCursorPos(); // verify cursor position in inline editor expect(inlinePos).toEqual(this.infos["test1.css"].offsets[1]); @@ -248,8 +248,8 @@ define(function (require, exports, module) { initInlineTest("test1.html", 2); runs(function () { - var inlineData = EditorManager.getCurrentFullEditor().getInlineWidgets()[0].data; - var inlinePos = inlineData.editors[0].getCursorPos(); + var inlineWidget = EditorManager.getCurrentFullEditor().getInlineWidgets()[0]; + var inlinePos = inlineWidget.editors[0].getCursorPos(); // verify cursor position in inline editor expect(inlinePos).toEqual(this.infos["test1.css"].offsets[2]); @@ -265,13 +265,13 @@ define(function (require, exports, module) { hostEditor = EditorManager.getCurrentFullEditor(); savedPos = hostEditor.getCursorPos(); inlineWidget = hostEditor.getInlineWidgets()[0]; - inlinePos = inlineWidget.data.editors[0].getCursorPos(); + inlinePos = inlineWidget.editors[0].getCursorPos(); // verify cursor position in inline editor expect(inlinePos).toEqual(this.infos["test1.css"].offsets[0]); // close the editor - EditorManager.closeInlineWidget(hostEditor, inlineWidget.id, true); + EditorManager.closeInlineWidget(hostEditor, inlineWidget, true); // verify no inline widgets expect(hostEditor.getInlineWidgets().length).toBe(0); @@ -305,7 +305,7 @@ define(function (require, exports, module) { var inlineEditor, widgetHeight; runs(function () { - inlineEditor = EditorManager.getCurrentFullEditor().getInlineWidgets()[0].data.editors[0]; + inlineEditor = EditorManager.getCurrentFullEditor().getInlineWidgets()[0].editors[0]; widgetHeight = inlineEditor.totalHeight(true); // verify original line count @@ -333,7 +333,7 @@ define(function (require, exports, module) { var inlineEditor, widgetHeight; runs(function () { - inlineEditor = EditorManager.getCurrentFullEditor().getInlineWidgets()[0].data.editors[0]; + inlineEditor = EditorManager.getCurrentFullEditor().getInlineWidgets()[0].editors[0]; widgetHeight = inlineEditor.totalHeight(true); // verify original line count @@ -365,7 +365,7 @@ define(function (require, exports, module) { runs(function () { hostEditor = EditorManager.getCurrentFullEditor(); - inlineEditor = hostEditor.getInlineWidgets()[0].data.editors[0]; + inlineEditor = hostEditor.getInlineWidgets()[0].editors[0]; // insert text at the inline editor's cursor position // can't mutate document directly at this point @@ -420,7 +420,7 @@ define(function (require, exports, module) { runs(function () { hostEditor = EditorManager.getCurrentFullEditor(); - inlineEditor = hostEditor.getInlineWidgets()[0].data.editors[0]; + inlineEditor = hostEditor.getInlineWidgets()[0].editors[0]; // insert text at the host editor's cursor position hostEditor._codeMirror.replaceRange(newHostText, hostEditor.getCursorPos()); @@ -538,7 +538,7 @@ define(function (require, exports, module) { runs(function () { hostEditor = EditorManager.getCurrentFullEditor(); - inlineEditor = hostEditor.getInlineWidgets()[0].data.editors[0]; + inlineEditor = hostEditor.getInlineWidgets()[0].editors[0]; // verify inline is open expect(hostEditor.getInlineWidgets().length).toBe(1); @@ -581,7 +581,7 @@ define(function (require, exports, module) { var inlineEditor, widgetHeight; runs(function () { - inlineEditor = EditorManager.getCurrentFullEditor().getInlineWidgets()[0].data.editors[0]; + inlineEditor = EditorManager.getCurrentFullEditor().getInlineWidgets()[0].editors[0]; widgetHeight = inlineEditor.totalHeight(true); // change inline editor content @@ -610,7 +610,7 @@ define(function (require, exports, module) { var cssDoc = DocumentManager.getOpenDocumentForPath(cssPath); // edit the inline editor - inlineEditor = EditorManager.getCurrentFullEditor().getInlineWidgets()[0].data.editors[0]; + inlineEditor = EditorManager.getCurrentFullEditor().getInlineWidgets()[0].editors[0]; inlineEditor._codeMirror.replaceRange( newInlineText, inlineEditor.getCursorPos() @@ -663,7 +663,7 @@ define(function (require, exports, module) { var cssPath = this.infos["test1.css"].fileEntry.fullPath; var cssDoc = DocumentManager.getOpenDocumentForPath(cssPath); hostEditor = EditorManager.getCurrentFullEditor(); - inlineEditor = hostEditor.getInlineWidgets()[0].data.editors[0]; + inlineEditor = hostEditor.getInlineWidgets()[0].editors[0]; // activate the full editor DocumentManager.setCurrentDocument(cssDoc); @@ -729,7 +729,8 @@ define(function (require, exports, module) { // replace text at start of inline range with text without newlines fullEditor._codeMirror.replaceRange( ".bar", - {line: start.line, ch: 0}, {line: start.line, ch: 4} + {line: start.line, ch: 0}, + {line: start.line, ch: 4} ); expect(inlineEditor).toHaveInlineEditorRange(toRange(start.line, end.line)); fullEditor._codeMirror.undo(); @@ -775,7 +776,8 @@ define(function (require, exports, module) { // replace all text on last line (without replacing trailing newline) fullEditor._codeMirror.replaceRange( "ABCD", - end, endOfEnd + end, + endOfEnd ); expect(inlineEditor).toHaveInlineEditorRange(toRange(start.line, end.line)); fullEditor._codeMirror.undo(); @@ -971,7 +973,7 @@ define(function (require, exports, module) { var cssPath = this.infos["testOneRuleFile.css"].fileEntry.fullPath; var cssDoc = DocumentManager.getOpenDocumentForPath(cssPath); hostEditor = EditorManager.getCurrentFullEditor(); - inlineEditor = hostEditor.getInlineWidgets()[0].data.editors[0]; + inlineEditor = hostEditor.getInlineWidgets()[0].editors[0]; // activate the full editor DocumentManager.setCurrentDocument(cssDoc); @@ -983,7 +985,7 @@ define(function (require, exports, module) { expect(inlineEditor).toHaveInlineEditorRange(toRange(0, 2)); // delete all of last line in range - fullEditor._codeMirror.replaceRange("", {line:1, ch:16}, {line:2, ch:1}); + fullEditor._codeMirror.replaceRange("", {line: 1, ch: 16}, {line: 2, ch: 1}); expect(hostEditor.getInlineWidgets().length).toBe(1); expect(inlineEditor).toHaveInlineEditorRange(toRange(0, 1)); fullEditor._codeMirror.undo(); @@ -994,7 +996,7 @@ define(function (require, exports, module) { expect(inlineEditor).toHaveInlineEditorRange(toRange(0, 2)); // delete all of last line in range - fullEditor._codeMirror.replaceRange("\n/* New last line */", {line:2, ch:1}); + fullEditor._codeMirror.replaceRange("\n/* New last line */", {line: 2, ch: 1}); expect(hostEditor.getInlineWidgets().length).toBe(1); expect(inlineEditor).toHaveInlineEditorRange(toRange(0, 3)); fullEditor._codeMirror.undo(); From 25fd303258acc56b5cff31025bddf1dc24d0de98 Mon Sep 17 00:00:00 2001 From: Narciso Jaramillo Date: Wed, 4 Apr 2012 09:37:59 -0700 Subject: [PATCH 2/4] Made Editor.setInlineWidgetHeight() also take a widget instead of an ID --- src/editor/CSSInlineEditor.js | 2 +- src/editor/Editor.js | 12 ++++++------ 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/editor/CSSInlineEditor.js b/src/editor/CSSInlineEditor.js index 5a72f792bfd..7ba387429cc 100644 --- a/src/editor/CSSInlineEditor.js +++ b/src/editor/CSSInlineEditor.js @@ -257,7 +257,7 @@ define(function (require, exports, module) { this.parentClass.sizeInlineWidgetToContents.call(this, force); // Size the widget height to the max between the editor content and the related rules list var widgetHeight = Math.max(this.$relatedContainer.find(".related").height(), this.$editorsDiv.height()); - this.hostEditor.setInlineWidgetHeight(this.id, widgetHeight, true); + this.hostEditor.setInlineWidgetHeight(this, widgetHeight, true); // The related rules container size itself based on htmlContent which is set by setInlineWidgetHeight above. this._updateRelatedContainer(); diff --git a/src/editor/Editor.js b/src/editor/Editor.js index 4117fa7e7c7..68bdeaa57e7 100644 --- a/src/editor/Editor.js +++ b/src/editor/Editor.js @@ -740,13 +740,13 @@ define(function (require, exports, module) { }; /** - * Sets the height of the inline widget for this editor. The inline editor is identified by id. - * @param {!number} id - * @param {!height} height - * @param {boolean} ensureVisible + * Sets the height of an inline widget in this editor. + * @param {!InlineWidget} inlineWidget The widget whose height should be set. + * @param {!height} height The height of the widget. + * @param {boolean} ensureVisible Whether to scroll the entire widget into view. */ - Editor.prototype.setInlineWidgetHeight = function (id, height, ensureVisible) { - this._codeMirror.setInlineWidgetHeight(id, height, ensureVisible); + Editor.prototype.setInlineWidgetHeight = function (inlineWidget, height, ensureVisible) { + this._codeMirror.setInlineWidgetHeight(inlineWidget.id, height, ensureVisible); }; From 1f74f8a2c2f6c5131b9123d7bedd30f153dfe616 Mon Sep 17 00:00:00 2001 From: Narciso Jaramillo Date: Wed, 4 Apr 2012 12:11:34 -0700 Subject: [PATCH 3/4] Code review cleanup: changed onClosed() to close(), other minor fixes. --- src/editor/CSSInlineEditor.js | 6 +++--- src/editor/Editor.js | 6 ++---- src/editor/EditorManager.js | 10 ++++------ src/editor/InlineTextEditor.js | 4 ++-- src/editor/InlineWidget.js | 4 ++-- 5 files changed, 13 insertions(+), 17 deletions(-) diff --git a/src/editor/CSSInlineEditor.js b/src/editor/CSSInlineEditor.js index 7ba387429cc..2673e2cc745 100644 --- a/src/editor/CSSInlineEditor.js +++ b/src/editor/CSSInlineEditor.js @@ -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); diff --git a/src/editor/Editor.js b/src/editor/Editor.js index 68bdeaa57e7..26a6ad3bf6c 100644 --- a/src/editor/Editor.js +++ b/src/editor/Editor.js @@ -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(); }); }; @@ -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; }; /** diff --git a/src/editor/EditorManager.js b/src/editor/EditorManager.js index c868449d00c..e9b8f42594d 100644 --- a/src/editor/EditorManager.js +++ b/src/editor/EditorManager.js @@ -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. */ @@ -164,9 +164,7 @@ define(function (require, exports, module) { } hostEditor.removeInlineWidget(inlineWidget); - } - /** * Registers a new inline provider. When _openInlineWidget() is called each registered inline diff --git a/src/editor/InlineTextEditor.js b/src/editor/InlineTextEditor.js index 0984c0f5e2d..67ac0d81181 100644 --- a/src/editor/InlineTextEditor.js +++ b/src/editor/InlineTextEditor.js @@ -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) { @@ -258,7 +258,7 @@ define(function (require, exports, module) { InlineTextEditor.prototype.close = function () { var shouldMoveFocus = this._editorHasFocus(); EditorManager.closeInlineWidget(this.hostEditor, this, shouldMoveFocus); - // closeInlineWidget() causes our onClosed() to get run + // closeInlineWidget() causes our close() to be called }; diff --git a/src/editor/InlineWidget.js b/src/editor/InlineWidget.js index cf79762b99b..6cd14a40077 100644 --- a/src/editor/InlineWidget.js +++ b/src/editor/InlineWidget.js @@ -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 }; From 3d23f64ba821d62923e9a82a87896184abbc722c Mon Sep 17 00:00:00 2001 From: Narciso Jaramillo Date: Wed, 4 Apr 2012 13:58:06 -0700 Subject: [PATCH 4/4] Reverted close() back to onClosed() since close() already means something --- src/editor/CSSInlineEditor.js | 4 ++-- src/editor/Editor.js | 4 ++-- src/editor/EditorManager.js | 2 +- src/editor/InlineTextEditor.js | 4 ++-- src/editor/InlineWidget.js | 2 +- 5 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/editor/CSSInlineEditor.js b/src/editor/CSSInlineEditor.js index 2673e2cc745..33a9ee345cd 100644 --- a/src/editor/CSSInlineEditor.js +++ b/src/editor/CSSInlineEditor.js @@ -184,8 +184,8 @@ define(function (require, exports, module) { /** * Called any time inline is closed, whether manually (via closeThisInline()) or automatically */ - CSSInlineEditor.prototype.close = function () { - this.parentClass.close.call(this); // super.close() + CSSInlineEditor.prototype.onClosed = function () { + this.parentClass.onClosed.call(this); // super.onClosed() // remove resize handlers for relatedContainer $(this.hostEditor).off("change", this._updateRelatedContainer); diff --git a/src/editor/Editor.js b/src/editor/Editor.js index 26a6ad3bf6c..d835c037726 100644 --- a/src/editor/Editor.js +++ b/src/editor/Editor.js @@ -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.close(); + inlineWidget.onClosed(); }); }; @@ -699,7 +699,7 @@ define(function (require, exports, module) { var self = this; inlineWidget.id = this._codeMirror.addInlineWidget(pos, inlineWidget.htmlContent, inlineWidget.height, function (id) { self._removeInlineWidgetInternal(id); - inlineWidget.close(); + inlineWidget.onClosed(); }); this._inlineWidgets.push(inlineWidget); inlineWidget.onAdded(); diff --git a/src/editor/EditorManager.js b/src/editor/EditorManager.js index e9b8f42594d..eb52c4ed1b5 100644 --- a/src/editor/EditorManager.js +++ b/src/editor/EditorManager.js @@ -144,7 +144,7 @@ define(function (require, exports, module) { /** * 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. + * is). The widget's onClosed() callback 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 diff --git a/src/editor/InlineTextEditor.js b/src/editor/InlineTextEditor.js index 67ac0d81181..0c98f07a90b 100644 --- a/src/editor/InlineTextEditor.js +++ b/src/editor/InlineTextEditor.js @@ -109,7 +109,7 @@ define(function (require, exports, module) { /** * Called any time inline was closed, whether manually (via close()) or automatically */ - InlineTextEditor.prototype.close = function () { + InlineTextEditor.prototype.onClosed = function () { _syncGutterWidths(this.hostEditor); this.editors.forEach(function (editor) { @@ -258,7 +258,7 @@ define(function (require, exports, module) { InlineTextEditor.prototype.close = function () { var shouldMoveFocus = this._editorHasFocus(); EditorManager.closeInlineWidget(this.hostEditor, this, shouldMoveFocus); - // closeInlineWidget() causes our close() to be called + // closeInlineWidget() causes our onClosed() to be called }; diff --git a/src/editor/InlineWidget.js b/src/editor/InlineWidget.js index 6cd14a40077..5b476ce4c77 100644 --- a/src/editor/InlineWidget.js +++ b/src/editor/InlineWidget.js @@ -30,7 +30,7 @@ define(function (require, exports, module) { /** * Called any time inline is closed, whether manually or automatically */ - InlineWidget.prototype.close = function () { + InlineWidget.prototype.onClosed = function () { // do nothing - base implementation };