From efd050ad92fb6087ab07a7ec9394decc67c267dc Mon Sep 17 00:00:00 2001 From: Dennis Kehrig Date: Thu, 4 Oct 2012 15:36:48 +0200 Subject: [PATCH 1/4] Fixes #1256: Deleting a file should remove entry from inline editor --- src/document/TextRange.js | 10 +++- src/editor/Editor.js | 5 +- src/editor/InlineTextEditor.js | 14 ++++-- src/editor/MultiRangeInlineEditor.js | 69 +++++++++++++++++++++++----- 4 files changed, 80 insertions(+), 18 deletions(-) diff --git a/src/document/TextRange.js b/src/document/TextRange.js index fe34de5e0fe..cdb73563912 100644 --- a/src/document/TextRange.js +++ b/src/document/TextRange.js @@ -45,6 +45,7 @@ define(function (require, exports, module) { * - lostSync -- When the backing Document changes in such a way that the range can no longer * accurately be maintained. Generally, occurs whenever an edit spans a range boundary. * After this, startLine & endLine will be unusable (set to null). + * Also occurs when the document is deleted, thought startLine & endLine won't be modified * These events only ever occur in response to Document changes, so if you are already listening * to the Document, you could ignore the TextRange events and just read its updated value in your * own Document change handler. @@ -59,9 +60,11 @@ define(function (require, exports, module) { this.document = document; document.addRef(); - // store this-bound version of listener so we can remove them later + // store this-bound versions of listeners so we can remove them later this._handleDocumentChange = this._handleDocumentChange.bind(this); + this._handleDocumentDeleted = this._handleDocumentDeleted.bind(this); $(document).on("change", this._handleDocumentChange); + $(document).on("deleted", this._handleDocumentDeleted); } /** Detaches from the Document. The TextRange will no longer update or send change events */ @@ -69,6 +72,7 @@ define(function (require, exports, module) { // Disconnect from Document this.document.releaseRef(); $(this.document).off("change", this._handleDocumentChange); + $(this.document).off("deleted", this._handleDocumentDeleted); }; @@ -167,6 +171,10 @@ define(function (require, exports, module) { this._applyChangesToRange(changeList); }; + TextRange.prototype._handleDocumentDeleted = function (event) { + $(this).triggerHandler("lostSync"); + }; + /* (pretty toString(), to aid debugging) */ TextRange.prototype.toString = function () { diff --git a/src/editor/Editor.js b/src/editor/Editor.js index 7dbcad5a374..8846120a9c6 100644 --- a/src/editor/Editor.js +++ b/src/editor/Editor.js @@ -561,8 +561,9 @@ define(function (require, exports, module) { * Responds to the Document's underlying file being deleted. The Document is now basically dead, * so we must close. */ - Editor.prototype._handleDocumentDeleted = function () { - $(this).triggerHandler("lostContent"); + Editor.prototype._handleDocumentDeleted = function (event) { + // Pass the delete event along as the cause (needed in MultiRangeInlineEditor) + $(this).triggerHandler("lostContent", [event]); }; diff --git a/src/editor/InlineTextEditor.js b/src/editor/InlineTextEditor.js index 8f1bb7cf889..10e37de296e 100644 --- a/src/editor/InlineTextEditor.js +++ b/src/editor/InlineTextEditor.js @@ -247,11 +247,9 @@ define(function (require, exports, module) { $lineNumber.text(inlineInfo.editor.getFirstVisibleLine() + 1); }); - // If Document's file is deleted, or Editor loses sync with Document, just close + // If Document's file is deleted, or Editor loses sync with Document $(inlineInfo.editor).on("lostContent", function () { - // Note: this closes the entire inline widget if any one Editor loses sync. This seems - // better than leaving it open but suddenly removing one rule from the result list. - self.close(); + self._onLostContent.apply(self, arguments); }); // set dirty indicator state @@ -285,6 +283,14 @@ define(function (require, exports, module) { }); }; + /** + * If Document's file is deleted, or Editor loses sync with Document, just close + */ + InlineTextEditor.prototype._onLostContent = function () { + // Note: this closes the entire inline widget if any one Editor loses sync. This seems + // better than leaving it open but suddenly removing one rule from the result list. + this.close(); + }; // consolidate all dirty document updates $(DocumentManager).on("dirtyFlagChange", _dirtyFlagChangeHandler); diff --git a/src/editor/MultiRangeInlineEditor.js b/src/editor/MultiRangeInlineEditor.js index af992d10d4f..949eed7333b 100644 --- a/src/editor/MultiRangeInlineEditor.js +++ b/src/editor/MultiRangeInlineEditor.js @@ -146,25 +146,24 @@ define(function (require, exports, module) { // create range list & add listeners for range textrange changes var rangeItemText; - this._ranges.forEach(function (range, i) { + this._ranges.forEach(function (range) { // Create list item UI var $rangeItem = $(window.document.createElement("li")).appendTo($rangeList); _updateRangeLabel($rangeItem, range); $rangeItem.mousedown(function () { - self.setSelectedIndex(i); + self.setSelectedIndex(self._ranges.indexOf(range)); }); - self._ranges[i].$listItem = $rangeItem; + range.$listItem = $rangeItem; // Update list item as TextRange changes - $(self._ranges[i].textRange).on("change", function () { + $(range.textRange).on("change", function () { _updateRangeLabel($rangeItem, range); }); - // If TextRange lost sync, react just as we do for an inline Editor's lostContent event: - // close the whole inline widget - $(self._ranges[i].textRange).on("lostSync", function () { - self.close(); + // If TextRange lost sync, remove it from the list (and close the widget if no other ranges are left) + $(range.textRange).on("lostSync", function () { + self._removeRange(range); }); }); @@ -207,10 +206,10 @@ define(function (require, exports, module) { } // Remove selected class(es) - var previousItem = (this._selectedRangeIndex >= 0) ? this._ranges[this._selectedRangeIndex].$listItem : null; + var $previousItem = (this._selectedRangeIndex >= 0) ? this._ranges[this._selectedRangeIndex].$listItem : null; - if (previousItem) { - previousItem.toggleClass("selected", false); + if ($previousItem) { + $previousItem.toggleClass("selected", false); } this._selectedRangeIndex = newIndex; @@ -249,6 +248,44 @@ define(function (require, exports, module) { this.sizeInlineWidgetToContents(true, false); this._updateRelatedContainer(); + this._updateSelectedMarker(); + }; + + MultiRangeInlineEditor.prototype._removeRange = function (range) { + // If this is the last range, just close the whole widget + if (this._ranges.length <= 1) { + this.close(); + return; + } + + // Now we know there is at least one other range -> found out which one this is + var index = this._ranges.indexOf(range); + + // If the range to be removed is the selected one, first switch to another one + if (index === this._selectedRangeIndex) { + // If possible, select the one below, else select the one above + if (index + 1 < this._ranges.length) { + this.setSelectedIndex(index + 1); + } else { + this.setSelectedIndex(index - 1); + } + } + + // Now we can remove this range + range.$listItem.remove(); + range.textRange.dispose(); + this._ranges.splice(index, 1); + + // If the selected range is below, we need to update the index + if (index < this._selectedRangeIndex) { + this._selectedRangeIndex--; + this._updateSelectedMarker(); + } + }; + + MultiRangeInlineEditor.prototype._updateSelectedMarker = function () { + var $rangeItem = this._ranges[this._selectedRangeIndex].$listItem; + // scroll the selection to the rangeItem, use setTimeout to wait for DOM updates var self = this; window.setTimeout(function () { @@ -429,6 +466,16 @@ define(function (require, exports, module) { this._ensureCursorVisible(); }; + /** + * If Document's file is deleted, or Editor loses sync with Document + */ + MultiRangeInlineEditor.prototype._onLostContent = function (event, cause) { + // Ignore when the editor's content got lost due to a deleted file, this is handled via TextRange's lostSync + if (cause && cause.type === 'deleted') { return; } + // Else yield to the parent's implementation + return this.parentClass._onLostContent.apply(this, arguments); + }; + /** * @return {Array.} */ From ff4200fd286c4c717bd78cc185102220aee88045 Mon Sep 17 00:00:00 2001 From: Dennis Kehrig Date: Tue, 9 Oct 2012 00:52:29 +0200 Subject: [PATCH 2/4] Fixed a typo: thought => though --- src/document/TextRange.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/document/TextRange.js b/src/document/TextRange.js index cdb73563912..5201cdd5b8e 100644 --- a/src/document/TextRange.js +++ b/src/document/TextRange.js @@ -45,7 +45,7 @@ define(function (require, exports, module) { * - lostSync -- When the backing Document changes in such a way that the range can no longer * accurately be maintained. Generally, occurs whenever an edit spans a range boundary. * After this, startLine & endLine will be unusable (set to null). - * Also occurs when the document is deleted, thought startLine & endLine won't be modified + * Also occurs when the document is deleted, though startLine & endLine won't be modified * These events only ever occur in response to Document changes, so if you are already listening * to the Document, you could ignore the TextRange events and just read its updated value in your * own Document change handler. From d0217781910d81de18e8b9c4f102de080360a37a Mon Sep 17 00:00:00 2001 From: Dennis Kehrig Date: Tue, 9 Oct 2012 00:55:03 +0200 Subject: [PATCH 3/4] Use double quotes instead of single quotes --- src/editor/MultiRangeInlineEditor.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/editor/MultiRangeInlineEditor.js b/src/editor/MultiRangeInlineEditor.js index 949eed7333b..2c84b1d6cd9 100644 --- a/src/editor/MultiRangeInlineEditor.js +++ b/src/editor/MultiRangeInlineEditor.js @@ -471,7 +471,7 @@ define(function (require, exports, module) { */ MultiRangeInlineEditor.prototype._onLostContent = function (event, cause) { // Ignore when the editor's content got lost due to a deleted file, this is handled via TextRange's lostSync - if (cause && cause.type === 'deleted') { return; } + if (cause && cause.type === "deleted") { return; } // Else yield to the parent's implementation return this.parentClass._onLostContent.apply(this, arguments); }; From 4b9a6ed339c007e74ee6c9a9df4eb0be34fc7fcf Mon Sep 17 00:00:00 2001 From: Dennis Kehrig Date: Wed, 10 Oct 2012 17:45:19 +0200 Subject: [PATCH 4/4] Improved comments --- src/editor/InlineTextEditor.js | 2 +- src/editor/MultiRangeInlineEditor.js | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/editor/InlineTextEditor.js b/src/editor/InlineTextEditor.js index 10e37de296e..d767dd5228b 100644 --- a/src/editor/InlineTextEditor.js +++ b/src/editor/InlineTextEditor.js @@ -247,7 +247,7 @@ define(function (require, exports, module) { $lineNumber.text(inlineInfo.editor.getFirstVisibleLine() + 1); }); - // If Document's file is deleted, or Editor loses sync with Document + // If Document's file is deleted, or Editor loses sync with Document, delegate to this._onLostContent() $(inlineInfo.editor).on("lostContent", function () { self._onLostContent.apply(self, arguments); }); diff --git a/src/editor/MultiRangeInlineEditor.js b/src/editor/MultiRangeInlineEditor.js index 2c84b1d6cd9..04d43600424 100644 --- a/src/editor/MultiRangeInlineEditor.js +++ b/src/editor/MultiRangeInlineEditor.js @@ -467,10 +467,11 @@ define(function (require, exports, module) { }; /** - * If Document's file is deleted, or Editor loses sync with Document + * Overwrite InlineTextEditor's _onLostContent to do nothing if the document's file is deleted + * (deletes are handled via TextRange's lostSync). */ MultiRangeInlineEditor.prototype._onLostContent = function (event, cause) { - // Ignore when the editor's content got lost due to a deleted file, this is handled via TextRange's lostSync + // 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);