Guard comment textarea and overlays behind dom-if templates
Until the thread list was added, we rarely rendered a large number of
diff comments at once. A diff would add them incrementally in an
asynchronous loop, and even then, their number was reduced by being
filtered to to the given path and patch range.
Rendering all comments at once in the thread list exposed inefficiencies
in instantiating gr-diff-comments, and, for changes with many comments,
could temporarily lockup the browser.
The biggest cause of slowness in comment instantiation was the textarea
for editing. Because existing comments are not in an edit state by
default (...and because only the final comment in a thread can be edited
anyway ... assuming the user even chooses to edit ... assuming there
even is an authenticated user...) it's more efficient to put the
textarea in a dom-if until it's needed.
The overlays are also expensive but infrequently used. These are put
inside a dom-if as well.
Change-Id: I50702afe2b178221110ad3200c5e3862a7722c5d
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment.js b/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment.js
index 9e5ae87..8612011 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment.js
@@ -124,6 +124,22 @@
},
_savingMessage: String,
+
+ _enableOverlay: {
+ type: Boolean,
+ value: false,
+ },
+
+ /**
+ * Property for storing references to overlay elements. When the overlays
+ * are moved to Gerrit.getRootElement() to be shown they are no-longer
+ * children, so they can't be queried along the tree, so they are stored
+ * here.
+ */
+ _overlays: {
+ type: Object,
+ value: () => ({}),
+ },
},
observers: [
@@ -155,7 +171,31 @@
detached() {
this.cancelDebouncer('fire-update');
- this.$.editTextarea.closeDropdown();
+ if (this.textarea) {
+ this.textarea.closeDropdown();
+ }
+ },
+
+ get textarea() {
+ return this.$$('#editTextarea');
+ },
+
+ get confirmDeleteOverlay() {
+ if (!this._overlays.confirmDelete) {
+ this._enableOverlay = true;
+ Polymer.dom.flush();
+ this._overlays.confirmDelete = this.$$('#confirmDeleteOverlay');
+ }
+ return this._overlays.confirmDelete;
+ },
+
+ get confirmDiscardOverlay() {
+ if (!this._overlays.confirmDiscard) {
+ this._enableOverlay = true;
+ Polymer.dom.flush();
+ this._overlays.confirmDiscard = this.$$('#confirmDiscardOverlay');
+ }
+ return this._overlays.confirmDiscard;
},
_computeShowHideText(collapsed) {
@@ -272,9 +312,6 @@
_editingChanged(editing, previousValue) {
this.$.container.classList.toggle('editing', editing);
- if (editing) {
- this.$.editTextarea.putCursorAtEnd();
- }
if (this.comment && this.comment.id) {
this.$$('.cancel').hidden = !editing;
}
@@ -285,6 +322,12 @@
// To prevent event firing on comment creation.
this._fireUpdate();
}
+ if (editing) {
+ this.async(() => {
+ Polymer.dom.flush();
+ this.textarea.putCursorAtEnd();
+ }, 1);
+ }
},
_computeLinkToComment(comment) {
@@ -437,7 +480,7 @@
this._discardDraft();
return;
}
- this._openOverlay(this.$.confirmDiscardOverlay);
+ this._openOverlay(this.confirmDiscardOverlay);
},
_handleConfirmDiscard(e) {
@@ -475,7 +518,7 @@
},
_closeConfirmDiscardOverlay() {
- this._closeOverlay(this.$.confirmDiscardOverlay);
+ this._closeOverlay(this.confirmDiscardOverlay);
},
_getSavingMessage(numPending) {
@@ -593,11 +636,11 @@
},
_handleCommentDelete() {
- this._openOverlay(this.$.confirmDeleteOverlay);
+ this._openOverlay(this.confirmDeleteOverlay);
},
_handleCancelDeleteComment() {
- this._closeOverlay(this.$.confirmDeleteOverlay);
+ this._closeOverlay(this.confirmDeleteOverlay);
},
_openOverlay(overlay) {
@@ -613,9 +656,11 @@
},
_handleConfirmDeleteComment() {
+ const dialog =
+ this.confirmDeleteOverlay.querySelector('#confirmDeleteComment');
this.$.restAPI.deleteComment(
- this.changeNum, this.patchNum, this.comment.id,
- this.$.confirmDeleteComment.message).then(newComment => {
+ this.changeNum, this.patchNum, this.comment.id, dialog.message)
+ .then(newComment => {
this._handleCancelDeleteComment();
this.comment = newComment;
});
