Enable rule switching and resize logic between rule list and displayed CSS rule #515
Conversation
tvoliter
commented
Mar 30, 2012
tvoliter
commented
- renamed InlineEditor to InlineWidget
- CSSInlineEditor.setSelectedRule() now removes the current rule editor and replaces it with a new one
- added CSSInlineEditor.sizeInlineWidgetToContents to resize widget contents based on rule list and the displayed rule
…obe/brackets into tvoliter/rule-switching * 'tvoliter/rule-switching' of ssh://ssh.github.com:443/adobe/brackets:
|
reviewing |
|
|
||
| // Add new editor | ||
| var rule = this.getSelectedRule(); | ||
| this.createInlineEditorFromText(rule.document, rule.lineStart, rule.lineEnd, this.$editorsDiv.get(0), extraKeys); |
There was a problem hiding this comment.
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.
| var height = editor.totalHeight(true); | ||
| if (force || height !== this.editorHeight) { | ||
| $(editor.getScrollerElement()).height(height); | ||
| this.editorHeight = height; |
There was a problem hiding this comment.
editorHeight is only accounting for CodeMirror now. The previous height calc used scrollHeight to account for editorsDiv. Is that the intent?
|
Initial review complete |
| $(this.editors[0]).on("change.CSSInlineEditor", this.updateRelatedContainerProxy); | ||
| this.sizeInlineWidgetToContents(true); | ||
| this._updateRelatedContainer(); | ||
|
|
There was a problem hiding this comment.
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?
…e list and the inline code mirror editor height
|
Jason, I've addressed all your comments, but haven't gotten to Peter's regarding the shortcuts |
| */ | ||
| CSSInlineEditor.prototype.sizeInlineWidgetToContents = function (force) { | ||
| // Size the code mirror editors height to the editor content | ||
| this.parentClass.sizeInlineWidgetToContents.call(this, force); |
|
Thanks for the quick fixes. One last issue with the rule list scrolling in CSSInlineEditor lines 93 and 97. |
Enable rule switching and resize logic between rule list and displayed CSS rule
