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

Enable rule switching and resize logic between rule list and displayed CSS rule #515

Merged
merged 13 commits into from
Apr 2, 2012
Merged

Enable rule switching and resize logic between rule list and displayed CSS rule #515

Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
92cd898
working on resize
tvoliter Mar 30, 2012
1d30a1d
Make css editor resize to rule list and editor size
tvoliter Mar 30, 2012
842bdc2
Make css editor resize to rule list and editor size
tvoliter Mar 30, 2012
1ddd301
Merge branch 'tvoliter/rule-switching' of ssh://ssh.github.com:443/ad…
tvoliter Mar 30, 2012
4c14327
merging origin master
tvoliter Mar 30, 2012
66e58e5
merging origin/master
tvoliter Mar 30, 2012
b326bef
code review fixes
tvoliter Mar 31, 2012
044ee96
code review fixes
tvoliter Mar 31, 2012
b97e8c2
making the cssinline editor set its height to the max between the rul…
tvoliter Apr 2, 2012
1f44754
merging with Jason's changes
tvoliter Apr 2, 2012
21d3407
Focusing inline editor after rule change
tvoliter Apr 2, 2012
07fc5cd
remove unused var and add semicolon
tvoliter Apr 2, 2012
784d7c1
Fix typo
tvoliter Apr 2, 2012
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
142 changes: 99 additions & 43 deletions src/editor/CSSInlineEditor.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ define(function (require, exports, module) {

/**
* @constructor
* @extends {InlineEditor}
* @extends {InlineWidget}
*/
function CSSInlineEditor(rules) {
InlineTextEditor.call(this);
Expand All @@ -28,6 +28,9 @@ define(function (require, exports, module) {
CSSInlineEditor.prototype = new InlineTextEditor();
CSSInlineEditor.prototype.constructor = CSSInlineEditor;
CSSInlineEditor.prototype.parentClass = InlineTextEditor.prototype;
CSSInlineEditor.prototype.$editorsDiv = null;
CSSInlineEditor.prototype.$relatedContainer = null;
CSSInlineEditor.prototype.updateRelatedContainerProxy = null;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make private "_"


/**
* @override
Expand All @@ -43,7 +46,7 @@ define(function (require, exports, module) {
$location;

// Create DOM to hold editors and related list
var $editorsDiv = $(document.createElement('div')).addClass("inlineEditorHolder");
this.$editorsDiv = $(document.createElement('div')).addClass("inlineEditorHolder");

// Outer container for border-left and scrolling
this.$relatedContainer = $(document.createElement("div")).addClass("relatedContainer");
Expand All @@ -57,16 +60,6 @@ define(function (require, exports, module) {
// Rule list
var $ruleList = $(document.createElement("ul")).appendTo($related);

// Keyboard shortcuts
var extraKeys = {
"Alt-Up" : $.proxy(this.previousRule, this),
"Alt-Down" : $.proxy(this.nextRule, this)
};

// load first rule
var rule = this._rules[0];
this.createInlineEditorFromText(rule.document, rule.lineStart, rule.lineEnd, $editorsDiv.get(0), extraKeys);

// create rule list
this._ruleItems = [];
this._rules.forEach(function (rule, i) {
Expand All @@ -87,29 +80,76 @@ define(function (require, exports, module) {
self.setSelectedRule(0);

// attach to main container
this.$htmlContent.append($editorsDiv).append(this.$relatedContainer);
this.$htmlContent.append(this.$editorsDiv).append(this.$relatedContainer);

// initialize position based on the main #editorHolder
setTimeout(function () {
self._updateRelatedContainer();
}, 0);

var updateRelatedContainerProxy = $.proxy(this._updateRelatedContainer, this);
this._updateRelatedContainerProxy = $.proxy(this._updateRelatedContainer, this);

// Changes to the host editor should update the relatedContainer
$(this.hostEditor).on("change.CSSInlineEditor", updateRelatedContainerProxy);

// TODO (jasonsj): install on active inline editor
// Changes in size to the inline editor should update the relatedContainer
$(this.editors[0]).on("change.CSSInlineEditor", updateRelatedContainerProxy);
$(this.hostEditor).on("change.CSSInlineEditor", this.updateRelatedContainerProxy);

// Since overflow-y is hidden on the CM scrollerElement, the scroll event is never fired.
// Instead, we add a hook to CM's onScroll to reposition the relatedContainer.
this.hostEditor._codeMirror.setOption("onScroll", updateRelatedContainerProxy);
this.hostEditor._codeMirror.setOption("onScroll", this.updateRelatedContainerProxy);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here and on line 93 I think you meant this. _updateRelatedContainerProxy. This is why the rules list isn't scrolling with the inline widget.


return (new $.Deferred()).resolve();
};

/**
*
*
*/
CSSInlineEditor.prototype.setSelectedRule = function (index) {
var newIndex = Math.min(Math.max(0, index), this._rules.length - 1);

this._selectedRuleIndex = newIndex;
var $ruleItem = this._ruleItems[this._selectedRuleIndex];

this._ruleItems[this._selectedRuleIndex].toggleClass("selected", true);

// Remove previous editors
this.editors.forEach(function (editor) {
editor.destroy(); //release ref on Document
});
this.editors = [];
this.$editorsDiv.children().remove();
$(this.editors[0]).off("change.CSSInlineEditor");

// Keyboard shortcuts
var extraKeys = {
"Alt-Up" : $.proxy(this.previousRule, this),
"Alt-Down" : $.proxy(this.nextRule, this)
};


// Add new editor
var rule = this.getSelectedRule();
this.createInlineEditorFromText(rule.document, rule.lineStart, rule.lineEnd, this.$editorsDiv.get(0), extraKeys);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need to re-add change listeners "change.CSSInlineEditor". See load(), my previous impl was always installing this to the first editor. Also, I noticed that focus doesn't go to the new editor after pressing Alt-Up/Down.

this.editors[0].focus()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing semi-colon

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed


// Changes in size to the inline editor should update the relatedContainer
$(this.editors[0]).on("change.CSSInlineEditor", this.updateRelatedContainerProxy);

this.sizeInlineWidgetToContents(true);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should call this._updateRelatedContainer() immediately after this line so that relatedContainer can take on the new height.

this._updateRelatedContainer();

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Drive-by comment: out of curiosity, how come the keyboard shortcuts and editor-switching code live in InlineTextEditor? I see that the rule list lives in CSSInlineProvider (seems reasonable for now), but since the notion of having multiple editors lives in the base class InlineTextEditor, shouldn't the API to let that UI code switch amongst those multiple editors live in the base class too?

// scroll the selection to the ruleItem, use setTimeout to wait for DOM updates
var self = this;
setTimeout(function () {
var itemTop = $ruleItem.position().top;
self.$selectedMarker.css("top", itemTop);
self.$selectedMarker.height($ruleItem.height());

// FUTURE (jasonsj): figure out if rule list should scroll
itemTop -= $ruleItem.parent().css("paddingTop").replace("px", "");
self.$relatedContainer.scrollTop(itemTop);
}, 0);
};

/**
* Called any time inline was closed, whether manually (via closeThisInline()) or automatically
*/
Expand All @@ -122,47 +162,63 @@ define(function (require, exports, module) {
this.hostEditor._codeMirror.setOption("onScroll", null);
};

/**
*
*
*/
CSSInlineEditor.prototype._updateRelatedContainer = function () {
var borderThickness = (this.$htmlContent.outerHeight() - this.$htmlContent.innerHeight()) / 2;
this.$relatedContainer.css("top", this.$htmlContent.offset().top + borderThickness);
this.$relatedContainer.height(this.$htmlContent.height());
};

// TY TODO: part of sprint 6
/**
*
*
*/
CSSInlineEditor.prototype.getRules = function () {
return this._rules;
};


/**
*
*
*/
CSSInlineEditor.prototype.getSelectedRule = function () {
return this._rules[this._selectedRuleIndex];
};


CSSInlineEditor.prototype.setSelectedRule = function (index) {
var newIndex = Math.min(Math.max(0, index), this._rules.length - 1);

this._selectedRuleIndex = newIndex;

var $ruleItem = this._ruleItems[this._selectedRuleIndex];

// scroll the selection to the ruleItem, use setTimeout to wait for DOM updates
var self = this;
setTimeout(function () {
var itemTop = $ruleItem.position().top;
self.$selectedMarker.css("top", itemTop);
self.$selectedMarker.height($ruleItem.height());

// FUTURE (jasonsj): figure out if rule list should scroll
itemTop -= $ruleItem.parent().css("paddingTop").replace("px", "");
self.$relatedContainer.scrollTop(itemTop);
}, 0);
};

/**
* Display the next css rule in the rule list
*/
CSSInlineEditor.prototype.nextRule = function () {
this.setSelectedRule(this._selectedRuleIndex + 1);
};

/**
* Display the previous css rule in the rule list
*/
CSSInlineEditor.prototype.previousRule = function () {
this.setSelectedRule(this._selectedRuleIndex - 1);
};

/**
* Sizes the inline widget height to be the maximum between the rule list height and the editor height
* @overide
*/
CSSInlineEditor.prototype.sizeInlineWidgetToContents = function (force) {
// Size the code mirror editors height to the editor content
this.parentClass.sizeInlineWidgetToContents.call(this, force);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we need this override anymore. As long as _updateRelatedContainer() is installed correctly to each editor and we call in load() (see comment above) we should be ok. If we need to enforce a min/max height, it may be better to do in _updateRelatedContainer().

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like $htmlContent.scrollHeight does include the relatedContainer even though it uses position:fixed. This is causing the +1px height change even when the editor content gets smaller. We should change widget height to use the .inlineEditorHolder div instead.

widgetHeight = this.$htmlContent.find(".inlineEditorHolder").get(0).scrollHeight;

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good find. Looking into that now...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I modified the .related class and removed the 100% height because I need one div to measure the size that the related list wants to be and another div to set it. If I use the same div the list never shrinks. So now I measure related and _updateRelatedContainer sets relatedContainer (related's parent)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unused var?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

var relatedSize = this.$relatedContainer.find("related").height;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unused var

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

// The related rules container size itself based on htmlContent which is set by setInlineWidgetHeight above.
this._updateRelatedContainer();
};


/**
* Given a position in an HTML editor, returns the relevant selector for the attribute/tag
Expand Down Expand Up @@ -215,7 +271,7 @@ define(function (require, exports, module) {
*
* @param {!Editor} editor
* @param {!{line:Number, ch:Number}} pos
* @return {$.Promise} a promise that will be resolved with an InlineEditor
* @return {$.Promise} a promise that will be resolved with an InlineWidget
* or null if we're not going to provide anything.
*/
function htmlToCSSProvider(hostEditor, pos) {
Expand Down
20 changes: 10 additions & 10 deletions src/editor/EditorManager.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ define(function (require, exports, module) {
* @private
* Bound to Ctrl+E on outermost editors.
* @param {!Editor} editor the candidate host editor
* @return {$.Promise} a promise that will be resolved when an InlineEditor
* @return {$.Promise} a promise that will be resolved when an InlineWidget
* is created or rejected when no inline editors are available.
*/
function _openInlineWidget(editor) {
Expand All @@ -129,21 +129,21 @@ define(function (require, exports, module) {

// If one of them will provide a widget, show it inline once ready
if (inlinePromise) {
inlinePromise.done(function (inlineEditor) {
$(inlineEditor.htmlContent).append('<div class="shadow top"/>')
inlinePromise.done(function (inlineWidget) {
$(inlineWidget.htmlContent).append('<div class="shadow top"/>')
.append('<div class="shadow bottom"/>');

var closeCallback = function () {
inlineEditor.onClosed();
inlineWidget.onClosed();
};
var parentShowCallback = function () {
inlineEditor.onParentShown();
inlineWidget.onParentShown();
};

var inlineId = editor.addInlineWidget(pos, inlineEditor.htmlContent, inlineEditor.height,
parentShowCallback, closeCallback, inlineEditor);
var inlineId = editor.addInlineWidget(pos, inlineWidget.htmlContent, inlineWidget.height,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In InlineWidget you replaced assigning height with editorHeight. I believe inlineWidget.height here will always be zero.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

editorHeight is now removed

parentShowCallback, closeCallback, inlineWidget);

inlineEditor.onAdded(inlineId);
inlineWidget.onAdded(inlineId);
result.resolve();
}).fail(function () {
result.reject();
Expand Down Expand Up @@ -190,7 +190,7 @@ define(function (require, exports, module) {
* {!Editor} editor, {!{line:Number, ch:Number}} pos
*
* Returns:
* {$.Promise} a promise that will be resolved with an InlineEditor
* {$.Promise} a promise that will be resolved with an inlineWidget
* or null to indicate the provider doesn't create an editor in this case
*/
function registerInlineEditProvider(provider) {
Expand Down Expand Up @@ -246,7 +246,7 @@ define(function (require, exports, module) {
* @param {?{startLine:Number, endLine:Number}} range If specified, all lines outside the given
* range are hidden from the editor. Range is inclusive. Line numbers start at 0.
* @param {HTMLDivContainer} inlineContent
* @param {function(InlineEditor)} closeThisInline
* @param {function(inlineWidget)} closeThisInline
*
* @return {{content:DOMElement, editor:Editor}}
*/
Expand Down
50 changes: 27 additions & 23 deletions src/editor/InlineTextEditor.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ define(function (require, exports, module) {
// Load dependent modules
var DocumentManager = require("document/DocumentManager"),
EditorManager = require("editor/EditorManager"),
InlineEditor = require("editor/InlineEditor").InlineEditor;
InlineWidget = require("editor/InlineWidget").InlineWidget;

/**
* Returns editor holder width (not CodeMirror's width).
Expand Down Expand Up @@ -54,13 +54,14 @@ define(function (require, exports, module) {
*
*/
function InlineTextEditor() {
InlineEditor.call(this);
this._docRangeToEditorMap = {};
InlineWidget.call(this);

/* @type {Array.<{Editor}>}*/
this.editors = [];
}
InlineTextEditor.prototype = new InlineEditor();
InlineTextEditor.prototype = new InlineWidget();
InlineTextEditor.prototype.constructor = InlineTextEditor;
InlineTextEditor.prototype.parentClass = InlineEditor.prototype;
InlineTextEditor.prototype.parentClass = InlineWidget.prototype;
InlineTextEditor.prototype.editors = null;

/**
Expand Down Expand Up @@ -118,29 +119,32 @@ define(function (require, exports, module) {

/**
* Update the inline editor's height when the number of lines change
* @param {boolean} force the editor to resize
*/
InlineTextEditor.prototype.sizeInlineEditorToContents = function (force) {
InlineTextEditor.prototype.sizeInlineWidgetToContents = function (force) {
var i,
len = this.editors.length,
editor;

// TODO: only handles 1 editor right now. Add multiple editor support when
// the design is finalized

// Reize the editors to the content
for (i = 0; i < len; i++) {
// Only supports 1 editor right now
if (i === 1) {
break;
}

editor = this.editors[i];

if (editor.isFullyVisible()) {
// set inner editor height
var editorHeight = editor.totalHeight(true);
if (force || editorHeight !== this._editorHeight) {
this._editorHeight = editorHeight;
$(editor.getScrollerElement()).height(editorHeight);
var height = editor.totalHeight(true);
if (force || height !== this.height) {
$(editor.getScrollerElement()).height(height);
this.height = height;
editor.refresh();
}

// use outermost wrapper (htmlContent) scrollHeight to prop open the host editor
this.height = this.htmlContent.scrollHeight;
this.hostEditor.setInlineWidgetHeight(this.inlineId, this.height, true);

break; // there should only be 1 visible editor
}
}
};
Expand All @@ -160,7 +164,7 @@ define(function (require, exports, module) {
_syncGutterWidths(this.hostEditor);

// Set initial size
this.sizeInlineEditorToContents();
this.sizeInlineWidgetToContents();

this.editors[0].focus();
};
Expand Down Expand Up @@ -208,7 +212,7 @@ define(function (require, exports, module) {

// Size editor to content whenever it changes (via edits here or any other view of the doc)
$(inlineInfo.editor).on("change", function () {
self.sizeInlineEditorToContents();
self.sizeInlineWidgetToContents();
});

// If Document's file is deleted, or Editor loses sync with Document, just close
Expand Down Expand Up @@ -242,18 +246,18 @@ define(function (require, exports, module) {
InlineTextEditor.prototype.onParentShown = function () {
// 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 sizeInlineEditorToContents()).
this.sizeInlineEditorToContents(true);
// isFullyVisible() check in sizeInlineWidgetToContents()).
this.sizeInlineWidgetToContents(true);
};

InlineEditor.prototype._editorHasFocus = function () {
InlineTextEditor.prototype._editorHasFocus = function () {
return this.editors.some(function (editor) {
return editor.hasFocus();
});
};

/** Closes this inline widget and all its contained Editors */
InlineEditor.prototype.close = function () {
InlineTextEditor.prototype.close = function () {
var shouldMoveFocus = this._editorHasFocus();
EditorManager.closeInlineWidget(this.hostEditor, this.inlineId, shouldMoveFocus);
// closeInlineWidget() causes our onClosed() to get run
Expand Down
Loading