Update delete comment dialog to require a reason.
With this change the submit button is disabled if the reason is empty
and textarea is focused on show.
Google-Bug-Id: b/263759160
Google-Bug-Id: b/263745303
Release-Notes: skip
Change-Id: I3f0216612bb6bd8192f55dede4ac70200b8f51a0
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 b4083f9..090dfef 100644
--- a/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.ts
+++ b/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.ts
@@ -130,6 +130,9 @@
@query('#confirmDeleteModal')
confirmDeleteModal?: HTMLDialogElement;
+ @query('#confirmDeleteCommentDialog')
+ confirmDeleteDialog?: GrConfirmDeleteCommentDialog;
+
@property({type: Object})
comment?: Comment;
@@ -940,7 +943,7 @@
return html`
<dialog id="confirmDeleteModal" tabindex="-1">
<gr-confirm-delete-comment-dialog
- id="confirmDeleteComment"
+ id="confirmDeleteCommentDialog"
@confirm=${this.handleConfirmDeleteComment}
@cancel=${this.closeDeleteCommentModal}
>
@@ -1262,6 +1265,9 @@
private openDeleteCommentModal() {
this.confirmDeleteModal?.showModal();
+ whenVisible(this.confirmDeleteDialog!, () => {
+ this.confirmDeleteDialog!.resetFocus();
+ });
}
private closeDeleteCommentModal() {
@@ -1274,10 +1280,7 @@
*/
// private, but visible for testing
async handleConfirmDeleteComment() {
- const dialog = this.confirmDeleteModal?.querySelector(
- '#confirmDeleteComment'
- ) as GrConfirmDeleteCommentDialog | null;
- if (!dialog || !dialog.message) {
+ if (!this.confirmDeleteDialog || !this.confirmDeleteDialog.message) {
throw new Error('missing confirm delete dialog');
}
assertIsDefined(this.changeNum, 'changeNum');
@@ -1286,7 +1289,7 @@
await this.getCommentsModel().deleteComment(
this.changeNum,
this.comment,
- dialog.message
+ this.confirmDeleteDialog.message
);
this.closeDeleteCommentModal();
}
diff --git a/polygerrit-ui/app/elements/shared/gr-comment/gr-comment_test.ts b/polygerrit-ui/app/elements/shared/gr-comment/gr-comment_test.ts
index 6c3caed..ec9c875 100644
--- a/polygerrit-ui/app/elements/shared/gr-comment/gr-comment_test.ts
+++ b/polygerrit-ui/app/elements/shared/gr-comment/gr-comment_test.ts
@@ -128,7 +128,7 @@
</div>
</gr-endpoint-decorator>
<dialog id="confirmDeleteModal" tabindex="-1">
- <gr-confirm-delete-comment-dialog id="confirmDeleteComment">
+ <gr-confirm-delete-comment-dialog id="confirmDeleteCommentDialog">
</gr-confirm-delete-comment-dialog>
</dialog>
`
@@ -171,7 +171,7 @@
</div>
</gr-endpoint-decorator>
<dialog id="confirmDeleteModal" tabindex="-1">
- <gr-confirm-delete-comment-dialog id="confirmDeleteComment">
+ <gr-confirm-delete-comment-dialog id="confirmDeleteCommentDialog">
</gr-confirm-delete-comment-dialog>
</dialog>
`
@@ -247,7 +247,7 @@
</div>
</gr-endpoint-decorator>
<dialog id="confirmDeleteModal" tabindex="-1">
- <gr-confirm-delete-comment-dialog id="confirmDeleteComment">
+ <gr-confirm-delete-comment-dialog id="confirmDeleteCommentDialog">
</gr-confirm-delete-comment-dialog>
</dialog>
`
@@ -349,7 +349,7 @@
</div>
</gr-endpoint-decorator>
<dialog id="confirmDeleteModal" tabindex="-1">
- <gr-confirm-delete-comment-dialog id="confirmDeleteComment">
+ <gr-confirm-delete-comment-dialog id="confirmDeleteCommentDialog">
</gr-confirm-delete-comment-dialog>
</dialog>
`
@@ -438,7 +438,7 @@
</div>
</gr-endpoint-decorator>
<dialog id="confirmDeleteModal" tabindex="-1">
- <gr-confirm-delete-comment-dialog id="confirmDeleteComment">
+ <gr-confirm-delete-comment-dialog id="confirmDeleteCommentDialog">
</gr-confirm-delete-comment-dialog>
</dialog>
`
@@ -523,7 +523,7 @@
assertIsDefined(element.confirmDeleteModal, 'confirmDeleteModal');
const dialog = queryAndAssert<GrConfirmDeleteCommentDialog>(
element.confirmDeleteModal,
- '#confirmDeleteComment'
+ '#confirmDeleteCommentDialog'
);
dialog.message = 'removal reason';
await element.updateComplete;
diff --git a/polygerrit-ui/app/elements/shared/gr-confirm-delete-comment-dialog/gr-confirm-delete-comment-dialog.ts b/polygerrit-ui/app/elements/shared/gr-confirm-delete-comment-dialog/gr-confirm-delete-comment-dialog.ts
index b6512b3..285a41a 100644
--- a/polygerrit-ui/app/elements/shared/gr-confirm-delete-comment-dialog/gr-confirm-delete-comment-dialog.ts
+++ b/polygerrit-ui/app/elements/shared/gr-confirm-delete-comment-dialog/gr-confirm-delete-comment-dialog.ts
@@ -75,6 +75,7 @@
override render() {
return html` <gr-dialog
confirm-label="Delete"
+ ?disabled=${this.message === ''}
@confirm=${this.handleConfirmTap}
@cancel=${this.handleCancelTap}
>
diff --git a/polygerrit-ui/app/elements/shared/gr-confirm-delete-comment-dialog/gr-confirm-delete-comment-dialog_test.ts b/polygerrit-ui/app/elements/shared/gr-confirm-delete-comment-dialog/gr-confirm-delete-comment-dialog_test.ts
index bd84ac4..b7551c1 100644
--- a/polygerrit-ui/app/elements/shared/gr-confirm-delete-comment-dialog/gr-confirm-delete-comment-dialog_test.ts
+++ b/polygerrit-ui/app/elements/shared/gr-confirm-delete-comment-dialog/gr-confirm-delete-comment-dialog_test.ts
@@ -7,6 +7,7 @@
import {fixture, html, assert} from '@open-wc/testing';
import {GrConfirmDeleteCommentDialog} from './gr-confirm-delete-comment-dialog';
import './gr-confirm-delete-comment-dialog';
+import {GrDialog} from '../gr-dialog/gr-dialog';
suite('gr-confirm-delete-comment-dialog tests', () => {
let element: GrConfirmDeleteCommentDialog;
@@ -17,7 +18,10 @@
);
});
- test('render', () => {
+ test('render', async () => {
+ element.message = 'Just cause';
+ await element.updateComplete;
+
// prettier and shadowDom string disagree about wrapping in <p> tag.
assert.shadowDom.equal(
element,
@@ -43,4 +47,13 @@
`
);
});
+
+ test('dialog is disabled when message is empty', async () => {
+ element.message = '';
+ await element.updateComplete;
+
+ assert.isTrue(
+ (element.shadowRoot!.querySelector('gr-dialog') as GrDialog).disabled
+ );
+ });
});