Merge "Revert "Ensure that quoted messages show up in the reply comment""
diff --git a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.ts b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.ts
index 22f1325..b540c89 100644
--- a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.ts
+++ b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.ts
@@ -257,6 +257,9 @@
@state() serverConfig?: ServerInfo;
@state()
+ patchsetLevelDraftMessage = '';
+
+ @state()
filterReviewerSuggestion: (input: Suggestion) => boolean;
@state()
@@ -379,7 +382,7 @@
patchsetLevelDraftIsResolved = true;
@state()
- patchsetLevelComment: UnsavedInfo | DraftInfo = this.createDraft('');
+ patchsetLevelComment?: UnsavedInfo | DraftInfo;
private readonly restApiService: RestApiService =
getAppContext().restApiService;
@@ -670,9 +673,7 @@
subscribe(
this,
() => this.getCommentsModel().patchsetLevelDrafts$,
- x => {
- if (x.length > 0) this.patchsetLevelComment = x[0];
- }
+ x => (this.patchsetLevelComment = x[0])
);
subscribe(
this,
@@ -763,7 +764,7 @@
changedProperties.has('mentionedUsersInUnresolvedDrafts') ||
changedProperties.has('includeComments') ||
changedProperties.has('labelsChanged') ||
- changedProperties.has('patchsetLevelComment') ||
+ changedProperties.has('patchsetLevelDraftMessage') ||
changedProperties.has('mentionedCCs')
) {
this.computeNewAttention();
@@ -915,11 +916,10 @@
}
// TODO: move to comment-util
- // Private but used in tests.
- createDraft(message: string): UnsavedInfo {
+ private createDraft(): UnsavedInfo {
return {
patch_set: this.latestPatchNum,
- message,
+ message: this.patchsetLevelDraftMessage,
unresolved: !this.patchsetLevelDraftIsResolved,
path: SpecialFilePath.PATCHSET_LEVEL_COMMENTS,
__unsaved: true,
@@ -927,6 +927,8 @@
}
private renderPatchsetLevelComment() {
+ if (!this.patchsetLevelComment)
+ this.patchsetLevelComment = this.createDraft();
return html`
<gr-comment
id="patchsetLevelComment"
@@ -936,10 +938,7 @@
this.patchsetLevelDraftIsResolved = !e.detail.value;
}}
@comment-text-changed=${(e: ValueChangedEvent<string>) => {
- const newMessage = e.detail.value;
- if (this.patchsetLevelComment.message === newMessage) return;
- this.patchsetLevelComment.message = newMessage;
- this.requestUpdate('patchsetLevelComment');
+ this.patchsetLevelDraftMessage = e.detail.value;
}}
.messagePlaceholder=${this.messagePlaceholder}
hide-header
@@ -1269,7 +1268,7 @@
this.focusOn(focusTarget);
if (quote?.length) {
// If a reply quote has been provided, use it.
- this.patchsetLevelComment = this.createDraft(quote);
+ this.patchsetLevelDraftMessage = quote;
}
if (this.restApiService.hasPendingDiffDrafts()) {
this.savingComments = true;
@@ -1282,7 +1281,7 @@
hasDrafts() {
return (
- !!this.patchsetLevelComment.message?.length ||
+ this.patchsetLevelDraftMessage.length > 0 ||
this.draftCommentThreads.length > 0
);
}
@@ -1470,8 +1469,8 @@
return;
}
+ this.patchsetLevelDraftMessage = '';
this.includeComments = true;
- this.patchsetLevelComment = this.createDraft('');
this.dispatchEvent(
new CustomEvent('send', {
composed: true,
@@ -2032,6 +2031,7 @@
computeSendButtonDisabled() {
if (
this.canBeStarted === undefined ||
+ this.patchsetLevelDraftMessage === undefined ||
this.reviewersMutated === undefined ||
this.labelsChanged === undefined ||
this.includeComments === undefined ||
@@ -2055,8 +2055,13 @@
const revotingOrNewVote = this.labelsChanged || existingVote;
const hasDrafts =
(this.includeComments && this.draftCommentThreads.length > 0) ||
- !!this.patchsetLevelComment?.message?.length;
- return !hasDrafts && !this.reviewersMutated && !revotingOrNewVote;
+ this.patchsetLevelDraftMessage.length > 0;
+ return (
+ !hasDrafts &&
+ !this.patchsetLevelDraftMessage.length &&
+ !this.reviewersMutated &&
+ !revotingOrNewVote
+ );
}
computePatchSetWarning() {
diff --git a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog_test.ts b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog_test.ts
index cef787a..acd1755 100644
--- a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog_test.ts
+++ b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog_test.ts
@@ -327,9 +327,7 @@
// Async tick is needed because iron-selector content is distributed and
// distributed content requires an observer to be set up.
await element.updateComplete;
- element.patchsetLevelComment = element.createDraft(
- 'I wholeheartedly disapprove'
- );
+ element.patchsetLevelDraftMessage = 'I wholeheartedly disapprove';
element.draftCommentThreads = [createCommentThread([createComment()])];
element.includeComments = true;
@@ -1056,9 +1054,7 @@
});
test('label picker', async () => {
- element.patchsetLevelComment = element.createDraft(
- 'I wholeheartedly disapprove'
- );
+ element.patchsetLevelDraftMessage = 'I wholeheartedly disapprove';
element.draftCommentThreads = [createCommentThread([createComment()])];
const saveReviewPromise = interceptSaveReview();
@@ -1079,7 +1075,7 @@
const review = await saveReviewPromise;
await element.updateComplete;
await waitUntil(() => element.disabled === false);
- assert.equal(element.patchsetLevelComment.message?.length, 0);
+ assert.equal(element.patchsetLevelDraftMessage.length, 0);
assert.deepEqual(review, {
drafts: 'PUBLISH_ALL_REVISIONS',
labels: {
@@ -1105,9 +1101,7 @@
// Async tick is needed because iron-selector content is distributed and
// distributed content requires an observer to be set up.
await element.updateComplete;
- element.patchsetLevelComment = element.createDraft(
- 'I wholeheartedly disapprove'
- );
+ element.patchsetLevelDraftMessage = 'I wholeheartedly disapprove';
const saveReviewPromise = interceptSaveReview();
@@ -1622,7 +1616,7 @@
assert.isFalse(element.attentionExpanded);
- element.patchsetLevelComment = element.createDraft('a test comment');
+ element.patchsetLevelDraftMessage = 'a test comment';
await element.updateComplete;
modifyButton.click();
@@ -2072,11 +2066,11 @@
const expectedError = new Error('test');
setup(() => {
- element.patchsetLevelComment = element.createDraft(expectedDraft);
+ element.patchsetLevelDraftMessage = expectedDraft;
});
function assertDialogOpenAndEnabled() {
- assert.strictEqual(expectedDraft, element.patchsetLevelComment.message);
+ assert.strictEqual(expectedDraft, element.patchsetLevelDraftMessage);
assert.isFalse(element.disabled);
}
@@ -2122,7 +2116,7 @@
// Mock canBeStarted
element.canBeStarted = true;
element.draftCommentThreads = [];
- element.patchsetLevelComment = element.createDraft('');
+ element.patchsetLevelDraftMessage = '';
element.reviewersMutated = false;
element.labelsChanged = false;
element.includeComments = false;
@@ -2136,7 +2130,7 @@
// Mock everything false
element.canBeStarted = false;
element.draftCommentThreads = [];
- element.patchsetLevelComment = element.createDraft('');
+ element.patchsetLevelDraftMessage = '';
element.reviewersMutated = false;
element.labelsChanged = false;
element.includeComments = false;
@@ -2150,7 +2144,7 @@
// Mock nonempty comment draft array; with sending comments.
element.canBeStarted = false;
element.draftCommentThreads = [{...createCommentThread([createComment()])}];
- element.patchsetLevelComment = element.createDraft('');
+ element.patchsetLevelDraftMessage = '';
element.reviewersMutated = false;
element.labelsChanged = false;
element.includeComments = true;
@@ -2164,7 +2158,7 @@
// Mock nonempty comment draft array; without sending comments.
element.canBeStarted = false;
element.draftCommentThreads = [{...createCommentThread([createComment()])}];
- element.patchsetLevelComment = element.createDraft('');
+ element.patchsetLevelDraftMessage = '';
element.reviewersMutated = false;
element.labelsChanged = false;
element.includeComments = false;
@@ -2179,7 +2173,7 @@
// Mock nonempty change message.
element.canBeStarted = false;
element.draftCommentThreads = [{...createCommentThread([createComment()])}];
- element.patchsetLevelComment = element.createDraft('test');
+ element.patchsetLevelDraftMessage = 'test';
element.reviewersMutated = false;
element.labelsChanged = false;
element.includeComments = false;
@@ -2194,7 +2188,7 @@
// Mock reviewers mutated.
element.canBeStarted = false;
element.draftCommentThreads = [{...createCommentThread([createComment()])}];
- element.patchsetLevelComment = element.createDraft('');
+ element.patchsetLevelDraftMessage = '';
element.reviewersMutated = true;
element.labelsChanged = false;
element.includeComments = false;
@@ -2209,7 +2203,7 @@
// Mock labels changed.
element.canBeStarted = false;
element.draftCommentThreads = [{...createCommentThread([createComment()])}];
- element.patchsetLevelComment = element.createDraft('');
+ element.patchsetLevelDraftMessage = '';
element.reviewersMutated = false;
element.labelsChanged = true;
element.includeComments = false;
@@ -2224,7 +2218,7 @@
// Whole dialog is disabled.
element.canBeStarted = false;
element.draftCommentThreads = [{...createCommentThread([createComment()])}];
- element.patchsetLevelComment = element.createDraft('');
+ element.patchsetLevelDraftMessage = '';
element.reviewersMutated = false;
element.labelsChanged = true;
element.includeComments = false;
@@ -2242,7 +2236,7 @@
).all = [account];
element.canBeStarted = false;
element.draftCommentThreads = [{...createCommentThread([createComment()])}];
- element.patchsetLevelComment = element.createDraft('');
+ element.patchsetLevelDraftMessage = '';
element.reviewersMutated = false;
element.labelsChanged = false;
element.includeComments = false;
@@ -2311,13 +2305,13 @@
queryAndAssert<GrComment>(element, '#patchsetLevelComment').messageText =
'hello';
- await waitUntil(() => element.patchsetLevelComment.message === 'hello');
+ await waitUntil(() => element.patchsetLevelDraftMessage === 'hello');
assert.isFalse(element.computeSendButtonDisabled());
queryAndAssert<GrComment>(element, '#patchsetLevelComment').messageText =
'';
- await waitUntil(() => element.patchsetLevelComment.message === '');
+ await waitUntil(() => element.patchsetLevelDraftMessage === '');
assert.isTrue(element.computeSendButtonDisabled());
});
@@ -2333,7 +2327,7 @@
patchsetLevelComment.messageText = 'hello world';
await waitUntil(
- () => element.patchsetLevelComment.message === 'hello world'
+ () => element.patchsetLevelDraftMessage === 'hello world'
);
const saveReviewPromise = interceptSaveReview();
@@ -2373,7 +2367,7 @@
patchsetLevelComment.messageText = 'hello world';
await waitUntil(
- () => element.patchsetLevelComment.message === 'hello world'
+ () => element.patchsetLevelDraftMessage === 'hello world'
);
assert.deepEqual(autoSaveStub.callCount, 0);
@@ -2396,7 +2390,7 @@
await waitUntil(() => element.draftCommentThreads.length === 1);
// patchset level draft as a reply is not loaded in patchsetLevel comment
- assert.equal(element.patchsetLevelComment.message, '');
+ assert.equal(element.patchsetLevelDraftMessage, '');
assert.deepEqual(element.draftCommentThreads[0].comments[0], draft);
});
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 de3d4604..721349f 100644
--- a/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.ts
+++ b/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.ts
@@ -976,16 +976,8 @@
override willUpdate(changed: PropertyValues) {
this.firstWillUpdate();
- if (changed.has('editing') || changed.has('comment')) {
- this.reflectCommentToInternalFields();
- }
if (changed.has('editing')) {
- // Parent components such as the reply dialog might be interested in whether
- // come of their child components are in editing mode.
- fire(this, 'comment-editing-changed', {
- editing: this.editing,
- path: this.comment?.path ?? '',
- });
+ this.onEditingChanged();
}
if (changed.has('unresolved')) {
// The <gr-comment-thread> component wants to change its color based on
@@ -1063,14 +1055,22 @@
throw new Error('unable to create preview fix event');
}
- private reflectCommentToInternalFields() {
- if (!this.editing) return;
- this.collapsed = false;
- this.messageText = this.comment?.message ?? '';
- this.unresolved = this.comment?.unresolved ?? true;
- this.originalMessage = this.messageText;
- this.originalUnresolved = this.unresolved;
- setTimeout(() => this.textarea?.putCursorAtEnd(), 1);
+ private onEditingChanged() {
+ if (this.editing) {
+ this.collapsed = false;
+ this.messageText = this.comment?.message ?? '';
+ this.unresolved = this.comment?.unresolved ?? true;
+ this.originalMessage = this.messageText;
+ this.originalUnresolved = this.unresolved;
+ setTimeout(() => this.textarea?.putCursorAtEnd(), 1);
+ }
+
+ // Parent components such as the reply dialog might be interested in whether
+ // come of their child components are in editing mode.
+ fire(this, 'comment-editing-changed', {
+ editing: this.editing,
+ path: this.comment?.path ?? '',
+ });
}
// private, but visible for testing
