From 8f15329c5ceee91bd1c6aa6ead915b30f6b41a34 Mon Sep 17 00:00:00 2001 From: Jason San Jose Date: Wed, 4 Apr 2012 14:01:37 -0700 Subject: [PATCH 1/9] Fix for #532 and #537 Shrink rule list when host editor width shrinks. Update rule list position when scroll bars appear in the host editor. --- src/editor/CSSInlineEditor.js | 46 +++++++++++++++++++++++++++++++---- 1 file changed, 41 insertions(+), 5 deletions(-) diff --git a/src/editor/CSSInlineEditor.js b/src/editor/CSSInlineEditor.js index 9074bfa7def..25fed22ca8a 100644 --- a/src/editor/CSSInlineEditor.js +++ b/src/editor/CSSInlineEditor.js @@ -50,7 +50,6 @@ define(function (require, exports, module) { // Container to hold all editors var self = this, - hostScroller = hostEditor._codeMirror.getScrollerElement(), $ruleItem, $location; @@ -62,9 +61,9 @@ define(function (require, exports, module) { // Outer container for border-left and scrolling this.$relatedContainer = $(document.createElement("div")).addClass("relatedContainer"); - // Position immediately to the left of the main editor's scrollbar. - var rightOffset = $(document.body).outerWidth() - ($(hostScroller).offset().left + $(hostScroller).get(0).clientWidth); - this.$relatedContainer.css("right", rightOffset + "px"); + this._relatedContainerInserted = false; + this._relatedContainerInsertedHandler = this._relatedContainerInsertedHandler.bind(this); + this.$relatedContainer.on("DOMNodeInserted", this._relatedContainerInsertedHandler); // List "selection" highlight this.$selectedMarker = $(document.createElement("div")).appendTo(this.$relatedContainer).addClass("selection"); @@ -108,6 +107,10 @@ define(function (require, exports, module) { // Listen to the editor's scroll event to reposition the relatedContainer. $(this.hostEditor).on("scroll", this._updateRelatedContainer); + + // Listen to the window resize event to reposition the relatedContainer + // when the hostEditor's scrollbars visibility changes + $(window).on("resize", this._updateRelatedContainer); return (new $.Deferred()).resolve(); }; @@ -191,6 +194,16 @@ define(function (require, exports, module) { $(this.hostEditor).off("change", this._updateRelatedContainer); $(this.editors[0]).off("change", this._updateRelatedContainer); $(this.hostEditor).off("scroll", this._updateRelatedContainer); + $(window).off("resize", this._updateRelatedContainer); + }; + + /** + * @private + * Set _relatedContainerInserted flag once the $relatedContainer is inserted in the DOM. + */ + CSSInlineEditor.prototype._relatedContainerInsertedHandler = function () { + this.$relatedContainer.off("DOMNodeInserted", this._relatedContainerInsertedHandler); + this._relatedContainerInserted = true; }; /** @@ -209,13 +222,36 @@ define(function (require, exports, module) { rcHeight = this.$relatedContainer.outerHeight(), rcBottom = rcTop + rcHeight, scrollerTop = $(hostScroller).offset().top, - scrollerBottom = scrollerTop + hostScroller.clientHeight; + scrollerBottom = scrollerTop + hostScroller.clientHeight, + scrollerLeft = $(hostScroller).offset().left, + rightOffset = $(document.body).outerWidth() - (scrollerLeft + hostScroller.clientWidth); if (rcTop < scrollerTop || rcBottom > scrollerBottom) { this.$relatedContainer.css("clip", "rect(" + Math.max(scrollerTop - rcTop, 0) + "px, auto, " + (rcHeight - Math.max(rcBottom - scrollerBottom, 0)) + "px, auto)"); } else { this.$relatedContainer.css("clip", ""); } + + // Constrain relatedContainer width to half of the scroller width + var relatedContainerWidth = this.$relatedContainer.width(); + if (this._relatedContainerInserted) { + if (this._relatedContainerDefaultWidth === undefined) { + this._relatedContainerDefaultWidth = relatedContainerWidth; + } + + var halfWidth = hostScroller.clientWidth / 2; + + if (this._relatedContainerDefaultWidth >= halfWidth) { + relatedContainerWidth = halfWidth; + } else { + relatedContainerWidth = this._relatedContainerDefaultWidth; + } + + this.$relatedContainer.width(Math.floor(relatedContainerWidth)); + } + + // Position immediately to the left of the main editor's scrollbar. + this.$relatedContainer.css("right", rightOffset + "px"); }; /** From 649bab560e6958d7e5c80680e8aa37626203e060 Mon Sep 17 00:00:00 2001 From: Peter Flynn Date: Wed, 4 Apr 2012 15:23:16 -0700 Subject: [PATCH 2/9] - Support CSS files being deleted in live development (now updates browser accordingly) - Fix exception in Document when unsaved changes are discarded but live development still has a ref to the doc (bug #546). More comprehensive fix probably to follow, though - Add warning in DocumentManager when deleting a doc does not clear all refs --- src/LiveDevelopment/Agents/CSSAgent.js | 10 ++++++++++ src/LiveDevelopment/Documents/CSSDocument.js | 15 ++++++++++++++- src/LiveDevelopment/LiveDevelopment.js | 13 +++++++++++++ src/document/DocumentManager.js | 7 ++++++- 4 files changed, 43 insertions(+), 2 deletions(-) diff --git a/src/LiveDevelopment/Agents/CSSAgent.js b/src/LiveDevelopment/Agents/CSSAgent.js index 1c620241ebc..d2384e68271 100644 --- a/src/LiveDevelopment/Agents/CSSAgent.js +++ b/src/LiveDevelopment/Agents/CSSAgent.js @@ -59,6 +59,15 @@ define(function CSSAgent(require, exports, module) { console.assert(style, "Style Sheet for document not loaded: " + doc.url); Inspector.CSS.setStyleSheetText(style.styleSheetId, doc.getText()); } + + /** Empties a CSS style sheet given a document that has been deleted + * @param {Document} document + */ + function reloadDeletedDocument(doc) { + var style = styleForURL(doc.url); + console.assert(style, "Style Sheet for document not loaded: " + doc.url); + Inspector.CSS.setStyleSheetText(style.styleSheetId, ""); + } /** Initialize the agent */ function load() { @@ -76,6 +85,7 @@ define(function CSSAgent(require, exports, module) { exports.styleForURL = styleForURL; exports.getStylesheetURLs = getStylesheetURLs; exports.reloadDocument = reloadDocument; + exports.reloadDeletedDocument = reloadDeletedDocument; exports.load = load; exports.unload = unload; }); \ No newline at end of file diff --git a/src/LiveDevelopment/Documents/CSSDocument.js b/src/LiveDevelopment/Documents/CSSDocument.js index 725d11dcefc..0b58e9b300b 100644 --- a/src/LiveDevelopment/Documents/CSSDocument.js +++ b/src/LiveDevelopment/Documents/CSSDocument.js @@ -26,6 +26,7 @@ define(function CSSDocumentModule(require, exports, module) { var Inspector = require("LiveDevelopment/Inspector/Inspector"); var CSSAgent = require("LiveDevelopment/Agents/CSSAgent"); var HighlightAgent = require("LiveDevelopment/Agents/HighlightAgent"); + var LiveDevelopment = require("LiveDevelopment/LiveDevelopment"); /** Constructor * @@ -47,7 +48,9 @@ define(function CSSDocumentModule(require, exports, module) { // Add a ref to the doc since we're listening for change events this.doc.addRef(); this.onChange = this.onChange.bind(this); + this.onDeleted = this.onDeleted.bind(this); $(this.doc).on("change", this.onChange); + $(this.doc).on("deleted", this.onDeleted); /* $(this.editor).on("cursorActivity", this.onCursorActivity); @@ -67,6 +70,7 @@ define(function CSSDocumentModule(require, exports, module) { /** Close the document */ CSSDocument.prototype.close = function close() { $(this.doc).off("change", this.onChange); + $(this.doc).off("deleted", this.onDeleted); this.doc.releaseRef(); /* Inspector.off("HighlightAgent.highlight", this.onHighlight); @@ -104,11 +108,20 @@ define(function CSSDocumentModule(require, exports, module) { } }; - /** Triggered on change of the editor */ + /** Triggered whenever the Document is edited */ CSSDocument.prototype.onChange = function onChange(event, editor, change) { // brute force: update the CSS CSSAgent.reloadDocument(this.doc); }; + /** Triggered if the Document's file is deleted */ + CSSDocument.prototype.onDeleted = function onChange(event, editor, change) { + // clear the CSS + CSSAgent.reloadDeletedDocument(this.doc); + + // shut down, since our Document is now dead + this.close(); + LiveDevelopment.removeRelatedDocument(this); + }; /** Triggered by the HighlightAgent to highlight a node in the editor */ CSSDocument.prototype.onHighlight = function onHighlight(node) { diff --git a/src/LiveDevelopment/LiveDevelopment.js b/src/LiveDevelopment/LiveDevelopment.js index 132512caa43..8601a416f3d 100644 --- a/src/LiveDevelopment/LiveDevelopment.js +++ b/src/LiveDevelopment/LiveDevelopment.js @@ -138,6 +138,18 @@ define(function LiveDevelopment(require, exports, module) { } } + /** + * Removes the given CSS/JSDocument from _relatedDocuments. Signals that the + * given file is no longer associated with the HTML document that is live (e.g. + * if the related file has been deleted on disk). + */ + function removeRelatedDocument(liveDoc) { + var index = _relatedDocuments.indexOf(liveDoc); + if (index !== -1) { + _relatedDocuments.splice(index, 1); + } + } + /** Create a live version of a Brackets document */ function _createDocument(doc, editor) { var DocClass = _classForDocument(doc); @@ -373,6 +385,7 @@ define(function LiveDevelopment(require, exports, module) { exports.agents = agents; exports.open = open; exports.close = close; + exports.removeRelatedDocument = removeRelatedDocument; exports.hideHighlight = hideHighlight; exports.init = init; }); \ No newline at end of file diff --git a/src/document/DocumentManager.js b/src/document/DocumentManager.js index 775d410eac5..c7b32555964 100644 --- a/src/document/DocumentManager.js +++ b/src/document/DocumentManager.js @@ -447,7 +447,7 @@ define(function (require, exports, module) { // to a dummy value to trigger that closing. Ultimately, the nicer "revert" behavior // should probably live in DocumentCommandHandlers.handleFileClose() (see note there). if (this.isDirty) { - this.refreshText(""); + this.refreshText("", this.diskTimestamp); } } }; @@ -644,6 +644,11 @@ define(function (require, exports, module) { if (doc) { $(doc).triggerHandler("deleted"); } + + // At this point, all those other views SHOULD have released the Doc + if (doc._refCount > 0) { + console.log("WARNING: deleted Document still has " + doc._refCount + " references. Did someone addRef() without listening for 'deleted'?"); + } } From 550d8d19293dd325dbd0bef83c3f75ec401ddfa4 Mon Sep 17 00:00:00 2001 From: Peter Flynn Date: Wed, 4 Apr 2012 15:56:13 -0700 Subject: [PATCH 3/9] Change CSSDocument so it doesn't know about LiveDevelopment, per discussion with Glenn. When a CSSDocument gets a Document "deleted" event, it now dispatches another "deleted" event of its own rather than pinging LiveDevelopment directly to unregister itself; LiveDevelopment is now responsible for listening for this CSSDocument event and unregistering the CSSDocument automatically. --- src/LiveDevelopment/Documents/CSSDocument.js | 9 +++++++-- src/LiveDevelopment/LiveDevelopment.js | 4 ++-- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/src/LiveDevelopment/Documents/CSSDocument.js b/src/LiveDevelopment/Documents/CSSDocument.js index 0b58e9b300b..a331923c598 100644 --- a/src/LiveDevelopment/Documents/CSSDocument.js +++ b/src/LiveDevelopment/Documents/CSSDocument.js @@ -19,6 +19,12 @@ * CSSDocument supports highlighting nodes from the HighlightAgent and * highlighting all DOMNode corresponding to the rule at the cursor position * in the editor. + * + * # EVENTS + * + * CSSDocument dispatches these events: + * deleted - When the file for the underlying Document has been deleted. The + * 2nd argument to the listener will be this CSSDocument. */ define(function CSSDocumentModule(require, exports, module) { 'use strict'; @@ -26,7 +32,6 @@ define(function CSSDocumentModule(require, exports, module) { var Inspector = require("LiveDevelopment/Inspector/Inspector"); var CSSAgent = require("LiveDevelopment/Agents/CSSAgent"); var HighlightAgent = require("LiveDevelopment/Agents/HighlightAgent"); - var LiveDevelopment = require("LiveDevelopment/LiveDevelopment"); /** Constructor * @@ -120,7 +125,7 @@ define(function CSSDocumentModule(require, exports, module) { // shut down, since our Document is now dead this.close(); - LiveDevelopment.removeRelatedDocument(this); + $(this).triggerHandler("deleted", [this]); }; /** Triggered by the HighlightAgent to highlight a node in the editor */ diff --git a/src/LiveDevelopment/LiveDevelopment.js b/src/LiveDevelopment/LiveDevelopment.js index 8601a416f3d..7edd3418f96 100644 --- a/src/LiveDevelopment/LiveDevelopment.js +++ b/src/LiveDevelopment/LiveDevelopment.js @@ -143,7 +143,7 @@ define(function LiveDevelopment(require, exports, module) { * given file is no longer associated with the HTML document that is live (e.g. * if the related file has been deleted on disk). */ - function removeRelatedDocument(liveDoc) { + function _handleRelatedDocumentDeleted(event, liveDoc) { var index = _relatedDocuments.indexOf(liveDoc); if (index !== -1) { _relatedDocuments.splice(index, 1); @@ -191,6 +191,7 @@ define(function LiveDevelopment(require, exports, module) { var liveDoc = _createDocument(doc); if (liveDoc) { _relatedDocuments.push(liveDoc); + $(liveDoc).on("deleted", _handleRelatedDocumentDeleted); } }); }); @@ -385,7 +386,6 @@ define(function LiveDevelopment(require, exports, module) { exports.agents = agents; exports.open = open; exports.close = close; - exports.removeRelatedDocument = removeRelatedDocument; exports.hideHighlight = hideHighlight; exports.init = init; }); \ No newline at end of file From 6f9610bfd78dd6642e6dadfd686b4fca190fa1b2 Mon Sep 17 00:00:00 2001 From: Peter Flynn Date: Wed, 4 Apr 2012 17:50:36 -0700 Subject: [PATCH 4/9] When main editor is closed without saving changes, revert the Document to whatever's on disk - because other parts of the UI might still be showing the contents of the Document, and we don't want them to keep reflecting the unsaved (now discarded) changes. --- src/document/DocumentCommandHandlers.js | 44 +++++++++++++++++++++---- src/document/DocumentManager.js | 18 ++++++---- 2 files changed, 48 insertions(+), 14 deletions(-) diff --git a/src/document/DocumentCommandHandlers.js b/src/document/DocumentCommandHandlers.js index 069aea6caed..92dcabf95f1 100644 --- a/src/document/DocumentCommandHandlers.js +++ b/src/document/DocumentCommandHandlers.js @@ -338,6 +338,31 @@ define(function (require, exports, module) { ); } + /** + * Reverts the Document to the current contents of its file on disk. Discards any unsaved changes + * in the Document. + * @param {Document} doc + * @return {$.Promise} a Promise that's resolved when done, or rejected with a FileError if the + * file cannot be read (after showing an error dialog to the user). + */ + function doRevert(doc) { + var result = new $.Deferred(); + + FileUtils.readAsText(doc.file) + .done(function (text, readTimestamp) { + doc.refreshText(text, readTimestamp); + result.resolve(); + }) + .fail(function (error) { + FileUtils.showFileOpenError(error.code, doc.file.fullPath) + .always(function () { + result.reject(error); + }); + }); + + return result.promise(); + } + /** * Closes the specified file: removes it from the working set, and closes the main editor if one @@ -395,6 +420,7 @@ define(function (require, exports, module) { if (id === Dialogs.DIALOG_BTN_CANCEL) { result.reject(); } else if (id === Dialogs.DIALOG_BTN_OK) { + // "Save" case: wait until we confirm save has succeeded before closing doSave(doc) .done(function () { doClose(file); @@ -404,13 +430,17 @@ define(function (require, exports, module) { result.reject(); }); } else { - // This is the "Don't Save" case--we can just go ahead and close the file. - // FUTURE: If other views of the Document will remain open, we need to revert the - // Document to the clean content on disk. Currently secondary Editors all lose - // sync & close if you refresh the whole doc's text, so we just fake this rather - // than pointlessly load text from disk. See hack in Document._makeNonEditable(). - doClose(file); - result.resolve(); + // "Don't Save" case: even though we're closing the main editor, other views of + // the Document may remain in the UI. So we need to revert the Document to a clean + // copy of whatever's on disk. + doRevert(doc) + .done(function () { + doClose(file); + result.resolve(); + }) + .fail(function () { + result.reject(); + }); } }); result.always(function () { diff --git a/src/document/DocumentManager.js b/src/document/DocumentManager.js index c7b32555964..3a0191c5bdb 100644 --- a/src/document/DocumentManager.js +++ b/src/document/DocumentManager.js @@ -441,13 +441,10 @@ define(function (require, exports, module) { this._text = this.getText(); this._masterEditor = null; - // FUTURE: If main editor was closed without saving changes, we should revert _text to - // what's on disk. But since we currently close all secondary editors when anyone else - // touches the Document content, there's no point in doing that yet. Just change the text - // to a dummy value to trigger that closing. Ultimately, the nicer "revert" behavior - // should probably live in DocumentCommandHandlers.handleFileClose() (see note there). + // Anyone calling closeFullEditor() should have dealt with unsaved changes already, + // either by saving the Document or by reverting its content. if (this.isDirty) { - this.refreshText("", this.diskTimestamp); + throw new Error("Attempt to make Document with unsaved changes non-editable: " + this); } } }; @@ -636,11 +633,18 @@ define(function (require, exports, module) { * sort of "project file model," making this just a private event handler. */ function notifyFileDeleted(file) { + // Since the Document is going away for all clients, it's actually ok in this case to + // _makeNonEditable() it despite it being out of sync with what's on disk. So manually mark + // it clean to avoid hitting the assertion in _makeNonEditable(). + var doc = getOpenDocumentForPath(file.fullPath); + if (doc) { + doc.isDirty = false; + } + // First ensure it's not currentDocument, and remove from working set closeFullEditor(file); // Notify all other editors to close as well - var doc = getOpenDocumentForPath(file.fullPath); if (doc) { $(doc).triggerHandler("deleted"); } From 85dcd0a3c3161d5bb6c499a684f4fb51b20d9d4b Mon Sep 17 00:00:00 2001 From: Peter Flynn Date: Wed, 4 Apr 2012 18:28:05 -0700 Subject: [PATCH 5/9] - Remove the isDirty assertion check in Document._makeNonEditable(). This was breaking many unit tests, which frequently throw away unsaved Documents. It also required somewhat hacky code in notifyFileDeleted() to avoid hitting the assertion in that case. The assertion was sort of out of place anyway: there's nothing intrinsic about destroying _masterEditor and moving the text back into a string that makes unsaved changes not ok; the idea that unsaved changes should be dealt with before closing the main editor is really just a UI feature/rule. - Fix bug where notifyFileDeleted() crashed if a file was deleted that was only open in the main editor, or in the working set and not open at all. In those cases 'doc' is null and the refCount check at the end would crash. - Update unit test comment --- src/document/DocumentManager.js | 17 ++--------------- test/spec/InlineEditorProviders-test.js | 2 +- 2 files changed, 3 insertions(+), 16 deletions(-) diff --git a/src/document/DocumentManager.js b/src/document/DocumentManager.js index 3a0191c5bdb..03a863a2b40 100644 --- a/src/document/DocumentManager.js +++ b/src/document/DocumentManager.js @@ -440,12 +440,6 @@ define(function (require, exports, module) { } else { this._text = this.getText(); this._masterEditor = null; - - // Anyone calling closeFullEditor() should have dealt with unsaved changes already, - // either by saving the Document or by reverting its content. - if (this.isDirty) { - throw new Error("Attempt to make Document with unsaved changes non-editable: " + this); - } } }; @@ -633,24 +627,17 @@ define(function (require, exports, module) { * sort of "project file model," making this just a private event handler. */ function notifyFileDeleted(file) { - // Since the Document is going away for all clients, it's actually ok in this case to - // _makeNonEditable() it despite it being out of sync with what's on disk. So manually mark - // it clean to avoid hitting the assertion in _makeNonEditable(). - var doc = getOpenDocumentForPath(file.fullPath); - if (doc) { - doc.isDirty = false; - } - // First ensure it's not currentDocument, and remove from working set closeFullEditor(file); // Notify all other editors to close as well + var doc = getOpenDocumentForPath(file.fullPath); if (doc) { $(doc).triggerHandler("deleted"); } // At this point, all those other views SHOULD have released the Doc - if (doc._refCount > 0) { + if (doc && doc._refCount > 0) { console.log("WARNING: deleted Document still has " + doc._refCount + " references. Did someone addRef() without listening for 'deleted'?"); } } diff --git a/test/spec/InlineEditorProviders-test.js b/test/spec/InlineEditorProviders-test.js index 402b152e5bc..730f295aa41 100644 --- a/test/spec/InlineEditorProviders-test.js +++ b/test/spec/InlineEditorProviders-test.js @@ -528,7 +528,7 @@ define(function (require, exports, module) { }); // FUTURE: Eventually we'll instead want it to stay open and revert to the content on disk. - // See notes in Document._makeNonEditable() & DocumentCommandHandlers.handleFileClose(). + // Editor's syncing code can't yet handle blowing away the whole Document like that, though. it("should close inline when file is closed without saving changes", function () { initInlineTest("test1.html", 1); From 8b88262349f81d2f1ce382223d06fa3df5613166 Mon Sep 17 00:00:00 2001 From: Peter Flynn Date: Wed, 4 Apr 2012 23:13:26 -0700 Subject: [PATCH 6/9] Respond to code review: - unregister LiveDevelopment's listeners on CSSDocument when LiveDevelopment stops caring about a particular document - fix misnamed function --- src/LiveDevelopment/Documents/CSSDocument.js | 2 +- src/LiveDevelopment/LiveDevelopment.js | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/src/LiveDevelopment/Documents/CSSDocument.js b/src/LiveDevelopment/Documents/CSSDocument.js index a331923c598..f3ce27475c0 100644 --- a/src/LiveDevelopment/Documents/CSSDocument.js +++ b/src/LiveDevelopment/Documents/CSSDocument.js @@ -119,7 +119,7 @@ define(function CSSDocumentModule(require, exports, module) { CSSAgent.reloadDocument(this.doc); }; /** Triggered if the Document's file is deleted */ - CSSDocument.prototype.onDeleted = function onChange(event, editor, change) { + CSSDocument.prototype.onDeleted = function onDeleted(event, editor, change) { // clear the CSS CSSAgent.reloadDeletedDocument(this.doc); diff --git a/src/LiveDevelopment/LiveDevelopment.js b/src/LiveDevelopment/LiveDevelopment.js index 7edd3418f96..77cbafabbe2 100644 --- a/src/LiveDevelopment/LiveDevelopment.js +++ b/src/LiveDevelopment/LiveDevelopment.js @@ -133,6 +133,7 @@ define(function LiveDevelopment(require, exports, module) { if (_relatedDocuments) { _relatedDocuments.forEach(function (liveDoc) { liveDoc.close(); + $(liveDoc).off("deleted", _handleRelatedDocumentDeleted); }); _relatedDocuments = undefined; } @@ -146,6 +147,7 @@ define(function LiveDevelopment(require, exports, module) { function _handleRelatedDocumentDeleted(event, liveDoc) { var index = _relatedDocuments.indexOf(liveDoc); if (index !== -1) { + $(liveDoc).on("deleted", _handleRelatedDocumentDeleted); _relatedDocuments.splice(index, 1); } } From 5cd730e3ace42bd92be63cd69dec32a86e073642 Mon Sep 17 00:00:00 2001 From: Peter Flynn Date: Thu, 5 Apr 2012 12:47:25 -0700 Subject: [PATCH 7/9] More code review changes: - Rename CSSAgent methods for clarity - When discarding unsaved changes, revert Document contents after closing main editor instead of before. As a bonus, this makes it easy to skip the revert if the main editor was the only view attached to the Document. --- src/LiveDevelopment/Agents/CSSAgent.js | 8 ++++---- src/LiveDevelopment/Documents/CSSDocument.js | 4 ++-- src/document/DocumentCommandHandlers.js | 17 +++++++++-------- 3 files changed, 15 insertions(+), 14 deletions(-) diff --git a/src/LiveDevelopment/Agents/CSSAgent.js b/src/LiveDevelopment/Agents/CSSAgent.js index d2384e68271..63aff2a0dae 100644 --- a/src/LiveDevelopment/Agents/CSSAgent.js +++ b/src/LiveDevelopment/Agents/CSSAgent.js @@ -54,7 +54,7 @@ define(function CSSAgent(require, exports, module) { /** Reload a CSS style sheet from a document * @param {Document} document */ - function reloadDocument(doc) { + function reloadCSSForDocument(doc) { var style = styleForURL(doc.url); console.assert(style, "Style Sheet for document not loaded: " + doc.url); Inspector.CSS.setStyleSheetText(style.styleSheetId, doc.getText()); @@ -63,7 +63,7 @@ define(function CSSAgent(require, exports, module) { /** Empties a CSS style sheet given a document that has been deleted * @param {Document} document */ - function reloadDeletedDocument(doc) { + function clearCSSForDocument(doc) { var style = styleForURL(doc.url); console.assert(style, "Style Sheet for document not loaded: " + doc.url); Inspector.CSS.setStyleSheetText(style.styleSheetId, ""); @@ -84,8 +84,8 @@ define(function CSSAgent(require, exports, module) { // Export public functions exports.styleForURL = styleForURL; exports.getStylesheetURLs = getStylesheetURLs; - exports.reloadDocument = reloadDocument; - exports.reloadDeletedDocument = reloadDeletedDocument; + exports.reloadCSSForDocument = reloadCSSForDocument; + exports.clearCSSForDocument = clearCSSForDocument; exports.load = load; exports.unload = unload; }); \ No newline at end of file diff --git a/src/LiveDevelopment/Documents/CSSDocument.js b/src/LiveDevelopment/Documents/CSSDocument.js index f3ce27475c0..5648ac2295e 100644 --- a/src/LiveDevelopment/Documents/CSSDocument.js +++ b/src/LiveDevelopment/Documents/CSSDocument.js @@ -116,12 +116,12 @@ define(function CSSDocumentModule(require, exports, module) { /** Triggered whenever the Document is edited */ CSSDocument.prototype.onChange = function onChange(event, editor, change) { // brute force: update the CSS - CSSAgent.reloadDocument(this.doc); + CSSAgent.reloadCSSForDocument(this.doc); }; /** Triggered if the Document's file is deleted */ CSSDocument.prototype.onDeleted = function onDeleted(event, editor, change) { // clear the CSS - CSSAgent.reloadDeletedDocument(this.doc); + CSSAgent.clearCSSForDocument(this.doc); // shut down, since our Document is now dead this.close(); diff --git a/src/document/DocumentCommandHandlers.js b/src/document/DocumentCommandHandlers.js index 92dcabf95f1..824e16f87dd 100644 --- a/src/document/DocumentCommandHandlers.js +++ b/src/document/DocumentCommandHandlers.js @@ -433,14 +433,15 @@ define(function (require, exports, module) { // "Don't Save" case: even though we're closing the main editor, other views of // the Document may remain in the UI. So we need to revert the Document to a clean // copy of whatever's on disk. - doRevert(doc) - .done(function () { - doClose(file); - result.resolve(); - }) - .fail(function () { - result.reject(); - }); + doClose(file); + + // Only reload from disk if other views still exist + if (DocumentManager.getOpenDocumentForPath(file.fullPath)) { + doRevert(doc) + .pipe(result.resolve, result.reject); + } else { + result.resolve(); + } } }); result.always(function () { From f6a2a6c764a14455afd34d4e97d16ef050fff3cf Mon Sep 17 00:00:00 2001 From: Peter Flynn Date: Thu, 5 Apr 2012 12:55:22 -0700 Subject: [PATCH 8/9] Mix merge midair collision from API rename --- src/LiveDevelopment/Documents/CSSDocument.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/LiveDevelopment/Documents/CSSDocument.js b/src/LiveDevelopment/Documents/CSSDocument.js index 22fd0f425d5..8d628b03a51 100644 --- a/src/LiveDevelopment/Documents/CSSDocument.js +++ b/src/LiveDevelopment/Documents/CSSDocument.js @@ -73,7 +73,7 @@ define(function CSSDocumentModule(require, exports, module) { // If the CSS document is dirty, push the changes into the browser now if (doc.isDirty) { - CSSAgent.reloadDocument(this.doc); + CSSAgent.reloadCSSForDocument(this.doc); } }; From f5eb2ac1467658a58e53da1d088e5ed2ccc3d242 Mon Sep 17 00:00:00 2001 From: Jason San Jose Date: Thu, 5 Apr 2012 14:19:32 -0700 Subject: [PATCH 9/9] Code review comments --- src/editor/CSSInlineEditor.js | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/src/editor/CSSInlineEditor.js b/src/editor/CSSInlineEditor.js index 201f96ba0f8..6f8417bf9a1 100644 --- a/src/editor/CSSInlineEditor.js +++ b/src/editor/CSSInlineEditor.js @@ -243,9 +243,10 @@ define(function (require, exports, module) { rcTop = this.$relatedContainer.offset().top, rcHeight = this.$relatedContainer.outerHeight(), rcBottom = rcTop + rcHeight, - scrollerTop = $(hostScroller).offset().top, + scrollerOffset = $(hostScroller).offset(), + scrollerTop = scrollerOffset.top, scrollerBottom = scrollerTop + hostScroller.clientHeight, - scrollerLeft = $(hostScroller).offset().left, + scrollerLeft = scrollerOffset.left, rightOffset = $(document.body).outerWidth() - (scrollerLeft + hostScroller.clientWidth); if (rcTop < scrollerTop || rcBottom > scrollerBottom) { this.$relatedContainer.css("clip", "rect(" + Math.max(scrollerTop - rcTop, 0) + "px, auto, " + @@ -261,15 +262,9 @@ define(function (require, exports, module) { this._relatedContainerDefaultWidth = relatedContainerWidth; } - var halfWidth = hostScroller.clientWidth / 2; - - if (this._relatedContainerDefaultWidth >= halfWidth) { - relatedContainerWidth = halfWidth; - } else { - relatedContainerWidth = this._relatedContainerDefaultWidth; - } - - this.$relatedContainer.width(Math.floor(relatedContainerWidth)); + var halfWidth = Math.floor(hostScroller.clientWidth / 2); + relatedContainerWidth = Math.min(this._relatedContainerDefaultWidth, halfWidth); + this.$relatedContainer.width(relatedContainerWidth); } // Position immediately to the left of the main editor's scrollbar.