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

Fix issues with updating inline editor when not visible #5899

Merged
merged 4 commits into from
Nov 9, 2013
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
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
24 changes: 17 additions & 7 deletions src/editor/Editor.js
Original file line number Diff line number Diff line change
Expand Up @@ -1137,6 +1137,12 @@ define(function (require, exports, module) {
var deferred = new $.Deferred(),
self = this;

function finishRemoving() {
self._codeMirror.removeLineWidget(inlineWidget.info);
self._removeInlineWidgetInternal(inlineWidget);
deferred.resolve();
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Same comment again, this time in the pull request:

JSLint error on this line: Unexpected '(space)'.

Copy link
Author

Choose a reason for hiding this comment

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

Doh, fixed.


if (!inlineWidget.closePromise) {
var lineNum = this._getInlineWidgetLineNumber(inlineWidget);

Expand All @@ -1146,13 +1152,17 @@ define(function (require, exports, module) {
// the other stuff in _removeInlineWidgetInternal to wait until then).
self._removeInlineWidgetFromList(inlineWidget);

AnimationUtils.animateUsingClass(inlineWidget.htmlContent, "animating")
.done(function () {
self._codeMirror.removeLineWidget(inlineWidget.info);
self._removeInlineWidgetInternal(inlineWidget);
deferred.resolve();
});
inlineWidget.$htmlContent.height(0);
// If we're not visible (in which case the widget will have 0 client height),
// don't try to do the animation, because nothing will happen and we won't get
// called back right away. (The animation would happen later when we switch
// back to the editor.)
if (self.isFullyVisible()) {
AnimationUtils.animateUsingClass(inlineWidget.htmlContent, "animating")
.done(finishRemoving);
inlineWidget.$htmlContent.height(0);
} else {
finishRemoving();
}
inlineWidget.closePromise = deferred.promise();
}
return inlineWidget.closePromise;
Expand Down
28 changes: 18 additions & 10 deletions src/editor/MultiRangeInlineEditor.js
Original file line number Diff line number Diff line change
Expand Up @@ -239,11 +239,8 @@ define(function (require, exports, module) {
// compute scrollbars
this.$relatedContainer.height(this.$related.height());

// Update the position of the selected marker now that we're laid out, and then
// set it to animate for future updates.
this._updateSelectedMarker().done(function () {
self.$selectedMarker.addClass("animate");
});
// Set the initial position of the selected marker now that we're laid out.
this._updateSelectedMarker(false);

// Call super
MultiRangeInlineEditor.prototype.parentClass.onAdded.apply(this, arguments);
Expand Down Expand Up @@ -316,7 +313,7 @@ define(function (require, exports, module) {
// ensureVisibility is set to false because we don't want to scroll the main editor when the user selects a view
this.sizeInlineWidgetToContents(true, false);

this._updateSelectedMarker();
this._updateSelectedMarker(true);
}

this._updateCommands();
Expand Down Expand Up @@ -378,7 +375,7 @@ define(function (require, exports, module) {
// If the selected range is below, we need to update the index
if (index < this._selectedRangeIndex) {
this._selectedRangeIndex--;
this._updateSelectedMarker();
this._updateSelectedMarker(true);
}

if (this._ranges.length === 1) {
Expand Down Expand Up @@ -434,7 +431,7 @@ define(function (require, exports, module) {
this._updateCommands();
};

MultiRangeInlineEditor.prototype._updateSelectedMarker = function () {
MultiRangeInlineEditor.prototype._updateSelectedMarker = function (animate) {
if (this._selectedRangeIndex < 0) {
return new $.Deferred().resolve().promise();
}
Expand All @@ -449,8 +446,10 @@ define(function (require, exports, module) {
itemTop = $rangeItem.position().top,
scrollTop = self.$relatedContainer.scrollTop();

self.$selectedMarker.css("top", itemTop);
self.$selectedMarker.height($rangeItem.outerHeight());
self.$selectedMarker
.toggleClass("animate", animate)
.css("top", itemTop)
.height($rangeItem.outerHeight());

if (containerHeight <= 0) {
return;
Expand Down Expand Up @@ -614,6 +613,15 @@ define(function (require, exports, module) {
}
};

/**
* Called when the editor containing the inline is made visible. Updates UI based on
* state that might have changed while the editor was hidden.
*/
MultiRangeInlineEditor.prototype.onParentShown = function () {
MultiRangeInlineEditor.prototype.parentClass.onParentShown.apply(this, arguments);
this._updateSelectedMarker(false);
};

/**
* Refreshes the height of the inline editor and all child editors.
* @override
Expand Down