Merge "unlock scroll if hovercard detached"
diff --git a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog_test.js b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog_test.js
index 358ebb0..c70678f 100644
--- a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog_test.js
+++ b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog_test.js
@@ -1508,7 +1508,9 @@
.querySelector('gr-button.send'));
assert.isFalse(sendStub.called);
- element.draftCommentThreads = [{comments: [{__draft: true}]}];
+ element.draftCommentThreads = [{comments: [
+ {__draft: true, path: 'test', line: 1, patch_set: 1},
+ ]}];
flushAsynchronousOperations();
MockInteractions.tap(element.shadowRoot
@@ -1521,7 +1523,9 @@
// computed to false.
element.draftCommentThreads = [];
assert.equal(element.getFocusStops().end, element.$.cancelButton);
- element.draftCommentThreads = [{comments: [{__draft: true}]}];
+ element.draftCommentThreads = [{comments: [
+ {__draft: true, path: 'test', line: 1, patch_set: 1},
+ ]}];
assert.equal(element.getFocusStops().end, element.$.sendButton);
});
diff --git a/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.ts b/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.ts
index d240f2c..209b2c9 100644
--- a/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.ts
+++ b/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.ts
@@ -159,10 +159,10 @@
*/
@property({type: Number})
- changeNum!: number;
+ changeNum?: number;
@property({type: Object, notify: true, observer: '_commentChanged'})
- comment!: Comment | RobotComment;
+ comment?: Comment | RobotComment;
@property({type: Array})
comments?: (Comment | RobotComment)[];
@@ -186,7 +186,7 @@
hasChildren?: boolean;
@property({type: String})
- patchNum!: PatchSetNum;
+ patchNum?: PatchSetNum;
@property({type: Boolean})
showActions?: boolean;
@@ -275,18 +275,6 @@
reporting = appContext.reportingService;
/** @override */
- ready() {
- super.ready();
- if (
- this.changeNum === undefined ||
- this.patchNum === undefined ||
- this.comment === undefined
- ) {
- throw new Error('Not all required properties are defined.');
- }
- }
-
- /** @override */
attached() {
super.attached();
this.$.restAPI.getAccount().then(account => {
@@ -460,7 +448,7 @@
resComment.__draft = true;
// Maintain the ephemeral draft ID for identification by other
// elements.
- if (this.comment.__draftID) {
+ if (this.comment?.__draftID) {
resComment.__draftID = this.comment.__draftID;
}
resComment.__commentSide = this.commentSide;
@@ -482,8 +470,11 @@
// prior to it being saved.
this.cancelDebouncer('store');
- if (!this.comment.path || this.comment.line === undefined)
+ if (!this.comment?.path || this.comment.line === undefined)
throw new Error('Cannot erase Draft Comment');
+ if (this.changeNum === undefined) {
+ throw new Error('undefined changeNum');
+ }
this.$.storage.eraseDraftComment({
changeNum: this.changeNum,
patchNum: this._getPatchNum(),
@@ -504,12 +495,13 @@
@observe('comment', 'comments.*')
_computeHasHumanReply() {
- if (!this.comment || !this.comments) return;
+ const comment = this.comment;
+ if (!comment || !this.comments) return;
// hide please fix button for robot comment that has human reply
this._hasHumanReply = this.comments.some(
c =>
c.in_reply_to &&
- c.in_reply_to === this.comment.id &&
+ c.in_reply_to === comment.id &&
!(c as RobotComment).robot_id
);
}
@@ -582,7 +574,11 @@
return isAdmin && !draft ? 'showDeleteButtons' : '';
}
- _computeSaveDisabled(draft: string, comment: Comment, resolved?: boolean) {
+ _computeSaveDisabled(
+ draft: string,
+ comment: Comment | undefined,
+ resolved?: boolean
+ ) {
// If resolved state has changed and a msg exists, save should be enabled.
if (!comment || (comment.unresolved === resolved && draft)) {
return false;
@@ -628,15 +624,21 @@
return;
}
+ const patchNum = this.comment.patch_set
+ ? this.comment.patch_set
+ : this._getPatchNum();
this.debounce(
'store',
() => {
const message = this._messageText;
- if (!this.comment.path || this.comment.line === undefined)
+ if (!this.comment?.path || this.comment.line === undefined)
throw new Error('missing path or line in comment');
+ if (this.changeNum === undefined) {
+ throw new Error('undefined changeNum');
+ }
const commentLocation: StorageLocation = {
changeNum: this.changeNum,
- patchNum: this._getPatchNum(),
+ patchNum,
path: this.comment.path,
line: this.comment.line,
range: this.comment.range,
@@ -656,7 +658,7 @@
_handleAnchorClick(e: Event) {
e.preventDefault();
- if (!this.comment.line) {
+ if (!this.comment?.line) {
return;
}
this.dispatchEvent(
@@ -673,7 +675,7 @@
_handleEdit(e: Event) {
e.preventDefault();
- if (!this.comment.message) throw new Error('message undefined');
+ if (!this.comment?.message) throw new Error('message undefined');
this._messageText = this.comment.message;
this.editing = true;
this.reporting.recordDraftInteraction();
@@ -686,7 +688,7 @@
if (this.disabled) {
return;
}
- const timingLabel = this.comment.id
+ const timingLabel = this.comment?.id
? REPORT_UPDATE_DRAFT
: REPORT_CREATE_DRAFT;
const timer = this.reporting.getTimer(timingLabel);
@@ -700,7 +702,7 @@
e.preventDefault();
if (
- !this.comment.message ||
+ !this.comment?.message ||
this.comment.message.trim().length === 0 ||
!this.comment.id
) {
@@ -773,6 +775,7 @@
}
_discardDraft() {
+ if (!this.comment) return Promise.reject(new Error('undefined comment'));
if (!this.comment.__draft) {
return Promise.reject(new Error('Cannot discard a non-draft comment.'));
}
@@ -871,7 +874,10 @@
this._handleFailedDraftRequest();
}
- _saveDraft(draft: Comment) {
+ _saveDraft(draft?: Comment) {
+ if (!draft || this.changeNum === undefined || this.patchNum === undefined) {
+ throw new Error('undefined draft or changeNum or patchNum');
+ }
this._showStartRequest();
return this.$.restAPI
.saveDiffDraft(this.changeNum, this.patchNum, draft)
@@ -893,6 +899,9 @@
}
_deleteDraft(draft: Comment) {
+ if (this.changeNum === undefined || this.patchNum === undefined) {
+ throw new Error('undefined changeNum or patchNum');
+ }
this._showStartRequest();
return this.$.restAPI
.deleteDiffDraft(this.changeNum, this.patchNum, draft)
@@ -907,7 +916,11 @@
}
_getPatchNum(): PatchSetNum {
- return this.isOnParent() ? ('PARENT' as PatchSetNum) : this.patchNum;
+ const patchNum = this.isOnParent()
+ ? ('PARENT' as PatchSetNum)
+ : this.patchNum;
+ if (patchNum === undefined) throw new Error('patchNum undefined');
+ return patchNum;
}
@observe('changeNum', 'patchNum', 'comment')
@@ -932,8 +945,7 @@
comment.message ||
comment.__otherEditing ||
!comment.path ||
- !comment.line ||
- !comment.range
+ !comment.line
) {
if (comment) delete comment.__otherEditing;
return;
@@ -958,6 +970,9 @@
// Modify payload instead of this.comment, as this.comment is passed from
// the parent by ref.
const payload = this._getEventPayload();
+ if (!payload.comment) {
+ throw new Error('comment not defined in payload');
+ }
payload.comment.unresolved = !this.$.resolvedCheckbox.checked;
this.dispatchEvent(
new CustomEvent('comment-update', {
@@ -1007,6 +1022,13 @@
if (!dialog || !dialog.message) {
throw new Error('missing confirm delete dialog');
}
+ if (
+ !this.comment ||
+ this.changeNum === undefined ||
+ this.patchNum === undefined
+ ) {
+ throw new Error('undefined comment or changeNum or patchNum');
+ }
this.$.restAPI
.deleteComment(
this.changeNum,
diff --git a/polygerrit-ui/app/elements/shared/gr-comment/gr-comment_test.js b/polygerrit-ui/app/elements/shared/gr-comment/gr-comment_test.js
index 1299b90..a9a1d32 100644
--- a/polygerrit-ui/app/elements/shared/gr-comment/gr-comment_test.js
+++ b/polygerrit-ui/app/elements/shared/gr-comment/gr-comment_test.js
@@ -149,6 +149,7 @@
email: 'tenn1sballchaser@aol.com',
},
line: 5,
+ path: 'test',
};
flush(() => {
assert.isTrue(loadSpy.called);
@@ -368,11 +369,13 @@
test('failed save draft request', done => {
element.draft = true;
+ element.changeNum = 1;
+ element.patchNum = 1;
const updateRequestStub = sinon.stub(element, '_updateRequestToast');
const diffDraftStub =
sinon.stub(element.$.restAPI, 'saveDiffDraft').returns(
Promise.resolve({ok: false}));
- element._saveDraft();
+ element._saveDraft({id: 'abc_123'});
flush(() => {
let args = updateRequestStub.lastCall.args;
assert.deepEqual(args, [0, true]);
@@ -384,7 +387,7 @@
.querySelector('.save')), 'save is visible');
diffDraftStub.returns(
Promise.resolve({ok: true}));
- element._saveDraft();
+ element._saveDraft({id: 'abc_123'});
flush(() => {
args = updateRequestStub.lastCall.args;
assert.deepEqual(args, [0]);
@@ -402,11 +405,13 @@
test('failed save draft request with promise failure', done => {
element.draft = true;
+ element.changeNum = 1;
+ element.patchNum = 1;
const updateRequestStub = sinon.stub(element, '_updateRequestToast');
const diffDraftStub =
sinon.stub(element.$.restAPI, 'saveDiffDraft').returns(
Promise.reject(new Error()));
- element._saveDraft();
+ element._saveDraft({id: 'abc_123'});
flush(() => {
let args = updateRequestStub.lastCall.args;
assert.deepEqual(args, [0, true]);
@@ -418,7 +423,7 @@
.querySelector('.save')), 'save is visible');
diffDraftStub.returns(
Promise.resolve({ok: true}));
- element._saveDraft();
+ element._saveDraft({id: 'abc_123'});
flush(() => {
args = updateRequestStub.lastCall.args;
assert.deepEqual(args, [0]);
