Clean up addWidget flow

src/editor/CSSInlineEditor.js

@@ -186,7 +186,7 @@ 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()
@@ -279,7 +279,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, widgetHeight, true);
 
         // The related rules container size itself based on htmlContent which is set by setInlineWidgetHeight above.
         this._updateRelatedContainer();

src/editor/Editor.js

@@ -346,8 +346,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();
         });
     };
 
@@ -711,35 +711,25 @@ define(function (require, exports, module) {
      * 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 });
-
-        return inlineId;
+        this._inlineWidgets.push(inlineWidget);
+        inlineWidget.onAdded();
     };
 
     /**
      * 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);
     };
 
     /**
@@ -766,13 +756,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);
     };
 
 
@@ -803,10 +793,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();
             });
         }
     };

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('<div class="shadow top"/>')
-            .append('<div class="shadow bottom"/>');
-
-        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();
@@ -167,18 +143,18 @@ define(function (require, exports, module) {
     }
 
     /**
-     * Removes the given widget UI from the given hostEdtior (agnostic of what the widget's content
+     * Removes the given widget UI from the given hostEditor (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
+     * @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.
      */
-    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,10 +163,8 @@ define(function (require, exports, module) {
             hostEditor.focus();
         }
 
-        hostEditor.removeInlineWidget(inlineId);
-
+        hostEditor.removeInlineWidget(inlineWidget);
     }
-
 
     /**
      * Registers a new inline provider. When _openInlineWidget() is called each registered inline
@@ -219,8 +193,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 +423,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 +454,6 @@ define(function (require, exports, module) {
 
     // For unit tests
     exports._openInlineWidget = _openInlineWidget;
-    exports._addInlineWidget = _addInlineWidget;
 
     // Define public API
     exports.setEditorHolder = setEditorHolder;

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,8 +257,8 @@ 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);
-        // closeInlineWidget() causes our onClosed() to get run
+        EditorManager.closeInlineWidget(this.hostEditor, this, shouldMoveFocus);
+        // closeInlineWidget() causes our onClosed() to be called
     };
 
 

src/editor/InlineWidget.js

@@ -19,26 +19,26 @@ define(function (require, exports, module) {
         // create the outer wrapper div
         this.htmlContent = document.createElement("div");
         this.$htmlContent = $(this.htmlContent).addClass("InlineWidget");
+        this.$htmlContent.append('<div class="shadow top"/>')
+            .append('<div class="shadow bottom"/>');
     }
     InlineWidget.prototype.htmlContent = null;
     InlineWidget.prototype.height = 0;
-    InlineWidget.prototype.inlineId = null;
+    InlineWidget.prototype.id = null;
     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 () {
         // do nothing - base implementation
     };
 
     /**
      * 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
     };
 
     /**

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);