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

Commit

Permalink
Factor out width-resizing logic for inline widgets into the InlineWid…
Browse files Browse the repository at this point in the history 
…get base class.

This addresses #2221 and #2218.
As part of this, cleaned up some issues around how we're doing inheritance:
* For #872, switched to using `Object.create()` to hook up prototypes
  (this avoids subtle bugs when binding event handlers in base classes).
* Made it so all child classes call base class implementation for overridden functions
  (even if the base class impl is empty right now). The one exception is `close()`, which
  is a bit weird right now--will file a separate issue on this.
  • Loading branch information
@njx
njx committed Dec 6, 2012
1 parent d5074de commit 9644ba9
Show file tree
Hide file tree
Showing 7 changed files with 66 additions and 62 deletions.
2 changes: 1 addition & 1 deletion src/command/Menus.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -748,7 +748,7 @@ define(function (require, exports, module) {
},
false);
}
ContextMenu.prototype = new Menu();
ContextMenu.prototype = Object.create(Menu.prototype);
ContextMenu.prototype.constructor = ContextMenu;
ContextMenu.prototype.parentClass = Menu.prototype;

Expand Down
11 changes: 9 additions & 2 deletions src/editor/InlineTextEditor.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ define(function (require, exports, module) {
/* @type {Array.<{Editor}>}*/
this.editors = [];
}
InlineTextEditor.prototype = new InlineWidget();
InlineTextEditor.prototype = Object.create(InlineWidget.prototype);
InlineTextEditor.prototype.constructor = InlineTextEditor;
InlineTextEditor.prototype.parentClass = InlineWidget.prototype;

Expand Down Expand Up @@ -125,11 +125,15 @@ define(function (require, exports, module) {
* Called any time inline was closed, whether manually (via close()) or automatically
*/
InlineTextEditor.prototype.onClosed = function () {
InlineTextEditor.prototype.parentClass.onClosed.call(this);

_syncGutterWidths(this.hostEditor);

this.editors.forEach(function (editor) {
editor.destroy(); //release ref on Document
});

InlineTextEditor.prototype.parentClass.onClosed.call(this);
};

/**
Expand Down Expand Up @@ -170,6 +174,8 @@ define(function (require, exports, module) {
* editor is constructed and added to the DOM
*/
InlineTextEditor.prototype.onAdded = function () {
InlineTextEditor.prototype.parentClass.onAdded.call(this);

this.editors.forEach(function (editor) {
editor.refresh();
});
Expand Down Expand Up @@ -260,7 +266,7 @@ define(function (require, exports, module) {
* @param {Editor} hostEditor
*/
InlineTextEditor.prototype.load = function (hostEditor) {
this.hostEditor = hostEditor;
InlineTextEditor.prototype.parentClass.load.call(this, hostEditor);

// TODO: incomplete impelementation. It's not clear yet if InlineTextEditor
// will fuction as an abstract class or as generic inline editor implementation
Expand All @@ -271,6 +277,7 @@ define(function (require, exports, module) {
* Called when the editor containing the inline is made visible.
*/
InlineTextEditor.prototype.onParentShown = function () {
InlineTextEditor.prototype.parentClass.onParentShown.call(this);
// We need to call this explicitly whenever the host editor is reshown, since
// we don't actually resize the inline editor while its host is invisible (see
// isFullyVisible() check in sizeInlineWidgetToContents()).
Expand Down
38 changes: 32 additions & 6 deletions src/editor/InlineWidget.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,8 @@ define(function (require, exports, module) {
e.stopImmediatePropagation();
}
});

this.updateWidth = this.updateWidth.bind(this);
}
InlineWidget.prototype.htmlContent = null;
InlineWidget.prototype.$htmlContent = null;
Expand All @@ -63,6 +65,29 @@ define(function (require, exports, module) {
*/
InlineWidget.prototype.height = 0;

/**
* Automatically updates the width of the inline editor when the parent editor's width changes due to
* edits or window resizes.
*/
InlineWidget.prototype.updateWidth = function () {
// Set the minimum width of the widget (which doesn't include the padding) to the width
// of CodeMirror's linespace, so that the total width will be at least as large as the
// width of the host editor's code plus any internal padding required by the widget.
// We can't just set the min-width to 100% because that would be 100% of the clientWidth of
// the host editor, rather than the overall scroll width.
// We also can't just use the host editor's scrollWidth, because if the host editor's own
// content becomes less wide, our own width will continue to prop open the host editor's
// scrollWidth.
// So instead, we peg our width to the right edge of CodeMirror's lineSpace (which is its
// width plus its offset from the left edge of the $htmlContent container).
// If the lineSpace is less than the scroller's clientWidth, we want to use the clientWidth instead.
// This is a bit of a hack since it relies on knowing some detail about the innards of CodeMirror.
var lineSpace = this.hostEditor._getLineSpaceElement(),
scroller = this.hostEditor.getScrollerElement(),
minWidth = Math.max(scroller.clientWidth, $(lineSpace).offset().left - this.$htmlContent.offset().left + lineSpace.scrollWidth);
this.$htmlContent.css("min-width", minWidth + "px");
};

/**
* Closes this inline widget and all its contained Editors
*/
Expand All @@ -76,26 +101,27 @@ define(function (require, exports, module) {
* Called any time inline is closed, whether manually or automatically
*/
InlineWidget.prototype.onClosed = function () {
// do nothing - base implementation
$(this.hostEditor).off("change", this.updateWidth);
$(window).off("resize", this.updateWidth);
};

/**
* Called once content is parented in the host editor's DOM. Useful for performing tasks like setting
* focus or measuring content, which require htmlContent to be in the DOM tree.
*/
InlineWidget.prototype.onAdded = function () {
// do nothing - base implementation
var self = this;
// Autosize the inline widget to the scrollable width of the main editor.
$(window).on("resize", this.updateWidth);
$(this.hostEditor).on("change", this.updateWidth);
window.setTimeout(this.updateWidth, 0);
};

/**
* @param {Editor} hostEditor
*/
InlineWidget.prototype.load = function (hostEditor) {
this.hostEditor = hostEditor;

// TODO: incomplete impelementation. It's not clear yet if InlineTextEditor
// will fuction as an abstract class or as generic inline editor implementation
// that just shows a range of text. See CSSInlineEditor.css for an implementation of load()
};

/**
Expand Down
23 changes: 6 additions & 17 deletions src/editor/MultiRangeInlineEditor.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ define(function (require, exports, module) {

this._selectedRangeIndex = -1;
}
MultiRangeInlineEditor.prototype = new InlineTextEditor();
MultiRangeInlineEditor.prototype = Object.create(InlineTextEditor.prototype);
MultiRangeInlineEditor.prototype.constructor = MultiRangeInlineEditor;
MultiRangeInlineEditor.prototype.parentClass = InlineTextEditor.prototype;

Expand All @@ -113,7 +113,7 @@ define(function (require, exports, module) {
*
*/
MultiRangeInlineEditor.prototype.load = function (hostEditor) {
this.parentClass.load.call(this, hostEditor);
MultiRangeInlineEditor.prototype.parentClass.load.call(this, hostEditor);

// Container to hold all editors
var self = this;
Expand Down Expand Up @@ -320,7 +320,7 @@ define(function (require, exports, module) {
*/
MultiRangeInlineEditor.prototype.onClosed = function () {
// Superclass onClosed() destroys editor
this.parentClass.onClosed.call(this);
MultiRangeInlineEditor.prototype.parentClass.onClosed.call(this);

// remove resize handlers for relatedContainer
$(this.hostEditor).off("change", this._updateRelatedContainer);
Expand Down Expand Up @@ -416,17 +416,6 @@ define(function (require, exports, module) {

// Add extra padding to the right edge of the widget to account for the range list.
this.$htmlContent.css("padding-right", this.$relatedContainer.outerWidth() + "px");

// Set the minimum width of the widget (which doesn't include the padding) to the width
// of CodeMirror's linespace, so that the total width will be at least as large as the
// width of the host editor's code plus the padding for the range list. We need to do this
// rather than just setting min-width to 100% because adding padding for the range list
// actually pushes out the width of the container, so we would end up continuously
// growing the overall width.
// This is a bit of a hack since it relies on knowing some detail about the innards of CodeMirror.
var lineSpace = this.hostEditor._getLineSpaceElement(),
minWidth = $(lineSpace).offset().left - this.$htmlContent.offset().left + lineSpace.scrollWidth;
this.$htmlContent.css("min-width", minWidth + "px");
};

/**
Expand Down Expand Up @@ -476,7 +465,7 @@ define(function (require, exports, module) {
// Ignore when the editor's content got lost due to a deleted file
if (cause && cause.type === "deleted") { return; }
// Else yield to the parent's implementation
return this.parentClass._onLostContent.apply(this, arguments);
return MultiRangeInlineEditor.prototype.parentClass._onLostContent.apply(this, arguments);
};

/**
Expand Down Expand Up @@ -515,7 +504,7 @@ define(function (require, exports, module) {
*/
MultiRangeInlineEditor.prototype.sizeInlineWidgetToContents = function (force, ensureVisibility) {
// Size the code mirror editors height to the editor content
this.parentClass.sizeInlineWidgetToContents.call(this, force);
MultiRangeInlineEditor.prototype.parentClass.sizeInlineWidgetToContents.call(this, force);
// Size the widget height to the max between the editor content and the related ranges list
var widgetHeight = Math.max(this.$relatedContainer.find(".related").height(), this.$editorsDiv.height());
this.hostEditor.setInlineWidgetHeight(this, widgetHeight, ensureVisibility);
Expand All @@ -530,6 +519,7 @@ define(function (require, exports, module) {
* @override
*/
MultiRangeInlineEditor.prototype.refresh = function () {
MultiRangeInlineEditor.prototype.parentClass.refresh.call(this);
this.sizeInlineWidgetToContents(true);
this._updateRelatedContainer();
this.editors.forEach(function (editor, j, arr) {
Expand All @@ -542,7 +532,6 @@ define(function (require, exports, module) {
* @returns {MultiRangeInlineEditor}
*/
function _getFocusedMultiRangeInlineEditor() {

var focusedMultiRangeInlineEditor = null,
result = EditorManager.getFocusedInlineWidget();

Expand Down
44 changes: 13 additions & 31 deletions src/extensions/default/InlineColorEditor/InlineColorEditor.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,11 +50,7 @@ define(function (require, exports, module) {

this._handleColorChange = this._handleColorChange.bind(this);
this._handleHostDocumentChange = this._handleHostDocumentChange.bind(this);
this._handleHostResize = this._handleHostResize.bind(this);

$(window).on("resize", this._handleHostResize);
window.setTimeout(this._handleHostResize, 0);

InlineWidget.call(this);
}

Expand All @@ -65,7 +61,7 @@ define(function (require, exports, module) {
*/
InlineColorEditor.COLOR_REGEX = /#[a-f0-9]{6}|#[a-f0-9]{3}|rgb\( ?\b([0-9]{1,2}|1[0-9]{2}|2[0-4][0-9]|25[0-5])\b ?, ?\b([0-9]{1,2}|1[0-9]{2}|2[0-4][0-9]|25[0-5])\b ?, ?\b([0-9]{1,2}|1[0-9]{2}|2[0-4][0-9]|25[0-5])\b ?\)|rgba\( ?\b([0-9]{1,2}|1[0-9]{2}|2[0-4][0-9]|25[0-5])\b ?, ?\b([0-9]{1,2}|1[0-9]{2}|2[0-4][0-9]|25[0-5])\b ?, ?\b([0-9]{1,2}|1[0-9]{2}|2[0-4][0-9]|25[0-5])\b ?, ?\b(1|0|0\.[0-9]{1,3}) ?\)|hsl\( ?\b([0-9]{1,2}|[12][0-9]{2}|3[0-5][0-9]|360)\b ?, ?\b([0-9]{1,2}|100)\b% ?, ?\b([0-9]{1,2}|100)\b% ?\)|hsla\( ?\b([0-9]{1,2}|[12][0-9]{2}|3[0-5][0-9]|360)\b ?, ?\b([0-9]{1,2}|100)\b% ?, ?\b([0-9]{1,2}|100)\b% ?, ?\b(1|0|0\.[0-9]{1,3}) ?\)/gi;

InlineColorEditor.prototype = new InlineWidget();
InlineColorEditor.prototype = Object.create(InlineWidget.prototype);
InlineColorEditor.prototype.constructor = InlineColorEditor;
InlineColorEditor.prototype.parentClass = InlineWidget.prototype;

Expand Down Expand Up @@ -121,15 +117,15 @@ define(function (require, exports, module) {
// instead of two bookmarks to track the range. (In our current old version of
// CodeMirror v2, markText() isn't robust enough for this case.)

var line = this.editor.document.getLine(start.line),
var line = this.hostEditor.document.getLine(start.line),
matches = line.substr(start.ch).match(InlineColorEditor.COLOR_REGEX);

// Note that end.ch is exclusive, so we don't need to add 1 before comparing to
// the matched length here.
if (matches && (end.ch === undefined || end.ch - start.ch < matches[0].length)) {
end.ch = start.ch + matches[0].length;
this._endBookmark.clear();
this._endBookmark = this.editor._codeMirror.setBookmark(end);
this._endBookmark = this.hostEditor._codeMirror.setBookmark(end);
}

if (end.ch === undefined) {
Expand All @@ -155,9 +151,9 @@ define(function (require, exports, module) {
if (!this._isHostChange) {
// Replace old color in code with the picker's color, and select it
this._isOwnChange = true;
this.editor.document.replaceRange(colorString, range.start, range.end);
this.hostEditor.document.replaceRange(colorString, range.start, range.end);
this._isOwnChange = false;
this.editor.setSelection(range.start, {
this.hostEditor.setSelection(range.start, {
line: range.start.line,
ch: range.start.ch + colorString.length
});
Expand All @@ -167,23 +163,15 @@ define(function (require, exports, module) {
}
};

/**
* Update the width of the inline editor based on the host editor's width.
*/
InlineColorEditor.prototype._handleHostResize = function () {
this.$htmlContent.css("min-width", this.hostEditor.getScrollerElement().scrollWidth + "px");
};

/**
* @override
* @param {!Editor} hostEditor
*/
InlineColorEditor.prototype.load = function (hostEditor) {
this.editor = hostEditor;
this.parentClass.load.call(this, hostEditor);
InlineColorEditor.prototype.parentClass.load.call(this, hostEditor);

// Create color picker control
var allColorsInDoc = this.editor.document.getText().match(InlineColorEditor.COLOR_REGEX);
var allColorsInDoc = this.hostEditor.document.getText().match(InlineColorEditor.COLOR_REGEX);
var swatchInfo = this._collateColors(allColorsInDoc, MAX_USED_COLORS);
this.colorEditor = new ColorEditor(this.$htmlContent, this._color, this._handleColorChange, swatchInfo);
};
Expand All @@ -198,9 +186,9 @@ define(function (require, exports, module) {
* Perform sizing & focus once we've been added to Editor's DOM
*/
InlineColorEditor.prototype.onAdded = function () {
this.parentClass.onAdded.call(this);
InlineColorEditor.prototype.parentClass.onAdded.call(this);

var doc = this.editor.document;
var doc = this.hostEditor.document;
doc.addRef();
$(doc).on("change", this._handleHostDocumentChange);

Expand All @@ -213,20 +201,18 @@ define(function (require, exports, module) {
* Called whenever the inline widget is closed, whether automatically or explicitly
*/
InlineColorEditor.prototype.onClosed = function () {
InlineColorEditor.prototype.parentClass.onClosed.call(this);

if (this._startBookmark) {
this._startBookmark.clear();
}
if (this._endBookmark) {
this._endBookmark.clear();
}

var doc = this.editor.document;
var doc = this.hostEditor.document;
$(doc).off("change", this._handleHostDocumentChange);
doc.releaseRef();

$(window).off("resize", this._handleHostResize);

this.parentClass.onClosed.call(this);
};

InlineColorEditor.prototype._sizeEditorToContent = function () {
Expand Down Expand Up @@ -282,18 +268,14 @@ define(function (require, exports, module) {
* When text in the code editor changes, update color picker to reflect it
*/
InlineColorEditor.prototype._handleHostDocumentChange = function () {
// Any host document change might change the scroll width, so we need to
// recalculate our own width.
this._handleHostResize();

// Don't push the change into the color editor if it came from the color editor.
if (this._isOwnChange) {
return;
}

var range = this.getCurrentRange();
if (range) {
var newColor = this.editor.document.getRange(range.start, range.end);
var newColor = this.hostEditor.document.getRange(range.start, range.end);
if (newColor !== this._color) {
this._isHostChange = true;
this.colorEditor.setColorFromString(newColor);
Expand Down
4 changes: 2 additions & 2 deletions src/extensions/samples/InlineImageViewer/InlineImageViewer.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ define(function (require, exports, module) {
this.fullPath = fullPath;
InlineWidget.call(this);
}
InlineImageViewer.prototype = new InlineWidget();
InlineImageViewer.prototype = Object.create(InlineWidget.prototype);
InlineImageViewer.prototype.constructor = InlineImageViewer;
InlineImageViewer.prototype.parentClass = InlineWidget.prototype;

Expand All @@ -49,7 +49,7 @@ define(function (require, exports, module) {
InlineImageViewer.prototype.$image = null;

InlineImageViewer.prototype.load = function (hostEditor) {
this.parentClass.load.call(this, hostEditor);
InlineImageViewer.prototype.parentClass.load.call(this, hostEditor);

this.$wrapperDiv = $(inlineEditorTemplate);

Expand Down
6 changes: 3 additions & 3 deletions src/file/NativeFileSystem.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -301,7 +301,7 @@ define(function (require, exports, module) {
NativeFileSystem.FileEntry = function (name) {
NativeFileSystem.Entry.call(this, name, false);
};
NativeFileSystem.FileEntry.prototype = new NativeFileSystem.Entry();
NativeFileSystem.FileEntry.prototype = Object.create(NativeFileSystem.Entry.prototype);

NativeFileSystem.FileEntry.prototype.toString = function () {
return "[FileEntry " + this.fullPath + "]";
Expand Down Expand Up @@ -535,7 +535,7 @@ define(function (require, exports, module) {

// TODO (issue #241): void removeRecursively (VoidCallback successCallback, optional ErrorCallback errorCallback);
};
NativeFileSystem.DirectoryEntry.prototype = new NativeFileSystem.Entry();
NativeFileSystem.DirectoryEntry.prototype = Object.create(NativeFileSystem.Entry.prototype);

NativeFileSystem.DirectoryEntry.prototype.toString = function () {
return "[DirectoryEntry " + this.fullPath + "]";
Expand Down Expand Up @@ -849,7 +849,7 @@ define(function (require, exports, module) {
this.onloadend = null;
};
// TODO (issue #241): extend EventTarget (draft status, not implememnted in webkit)
// NativeFileSystem.FileReader.prototype = new NativeFileSystem.EventTarget()
// NativeFileSystem.FileReader.prototype = Object.create(NativeFileSystem.EventTarget.prototype);

NativeFileSystem.FileReader.prototype.readAsArrayBuffer = function (blob) {
// TODO (issue #241): implement
Expand Down

0 comments on commit 9644ba9

Please sign in to comment.