Disable the 'Send' button when a comment is being edited

Since we have threaded comments in the reply dialog, it's pretty
easy to update your comment reply from within that dialog. To avoid
losing edits, we should disable the 'Send' button when the user is
currently editing a comment.

Change-Id: I7e7bc99597f4827dc7f19d45053c53dd463372e5
(cherry picked from commit 31f3f10d4b)
This commit is contained in:
Tao Zhou
2020-04-27 16:15:29 +02:00
committed by Paladox none
parent b203c26544
commit 443d37f76d
4 changed files with 51 additions and 13 deletions

View File

@@ -244,11 +244,15 @@ class GrReplyDialog extends mixinBehaviors( [
type: String,
value: '',
},
_commentEditing: {
type: Boolean,
value: false,
},
_sendDisabled: {
type: Boolean,
computed: '_computeSendButtonDisabled(_sendButtonLabel, ' +
'draftCommentThreads, draft, _reviewersMutated, _labelsChanged, ' +
'_includeComments, disabled)',
'_includeComments, disabled, _commentEditing)',
observer: '_sendDisabledChanged',
},
draftCommentThreads: {
@@ -279,6 +283,10 @@ class GrReplyDialog extends mixinBehaviors( [
this._getAccount().then(account => {
this._account = account || {};
});
this.addEventListener('comment-editing-changed', e => {
this._commentEditing = e.detail;
});
}
/** @override */
@@ -876,7 +884,7 @@ class GrReplyDialog extends mixinBehaviors( [
_computeSendButtonDisabled(
buttonLabel, draftCommentThreads, text, reviewersMutated,
labelsChanged, includeComments, disabled) {
labelsChanged, includeComments, disabled, commentEditing) {
// Polymer 2: check for undefined
if ([
buttonLabel,
@@ -886,11 +894,12 @@ class GrReplyDialog extends mixinBehaviors( [
labelsChanged,
includeComments,
disabled,
commentEditing,
].some(arg => arg === undefined)) {
return undefined;
}
if (disabled) { return true; }
if (commentEditing || disabled) { return true; }
if (buttonLabel === ButtonLabels.START_REVIEW) { return false; }
const hasDrafts = includeComments && draftCommentThreads.length;
return !hasDrafts && !text.length && !reviewersMutated && !labelsChanged;

View File

@@ -1146,7 +1146,8 @@ suite('gr-reply-dialog tests', () => {
/* reviewersMutated= */ false,
/* labelsChanged= */ false,
/* includeComments= */ false,
/* disabled= */ false
/* disabled= */ false,
/* commentEditing= */ false
));
assert.isTrue(fn(
/* buttonLabel= */ 'Send',
@@ -1155,7 +1156,8 @@ suite('gr-reply-dialog tests', () => {
/* reviewersMutated= */ false,
/* labelsChanged= */ false,
/* includeComments= */ false,
/* disabled= */ false
/* disabled= */ false,
/* commentEditing= */ false
));
// Mock nonempty comment draft array, with seding comments.
assert.isFalse(fn(
@@ -1165,7 +1167,8 @@ suite('gr-reply-dialog tests', () => {
/* reviewersMutated= */ false,
/* labelsChanged= */ false,
/* includeComments= */ true,
/* disabled= */ false
/* disabled= */ false,
/* commentEditing= */ false
));
// Mock nonempty comment draft array, without seding comments.
assert.isTrue(fn(
@@ -1175,7 +1178,8 @@ suite('gr-reply-dialog tests', () => {
/* reviewersMutated= */ false,
/* labelsChanged= */ false,
/* includeComments= */ false,
/* disabled= */ false
/* disabled= */ false,
/* commentEditing= */ false
));
// Mock nonempty change message.
assert.isFalse(fn(
@@ -1185,7 +1189,8 @@ suite('gr-reply-dialog tests', () => {
/* reviewersMutated= */ false,
/* labelsChanged= */ false,
/* includeComments= */ false,
/* disabled= */ false
/* disabled= */ false,
/* commentEditing= */ false
));
// Mock reviewers mutated.
assert.isFalse(fn(
@@ -1195,7 +1200,8 @@ suite('gr-reply-dialog tests', () => {
/* reviewersMutated= */ true,
/* labelsChanged= */ false,
/* includeComments= */ false,
/* disabled= */ false
/* disabled= */ false,
/* commentEditing= */ false
));
// Mock labels changed.
assert.isFalse(fn(
@@ -1205,7 +1211,8 @@ suite('gr-reply-dialog tests', () => {
/* reviewersMutated= */ false,
/* labelsChanged= */ true,
/* includeComments= */ false,
/* disabled= */ false
/* disabled= */ false,
/* commentEditing= */ false
));
// Whole dialog is disabled.
assert.isTrue(fn(
@@ -1215,7 +1222,18 @@ suite('gr-reply-dialog tests', () => {
/* reviewersMutated= */ false,
/* labelsChanged= */ true,
/* includeComments= */ false,
/* disabled= */ true
/* disabled= */ true,
/* commentEditing= */ false
));
assert.isTrue(fn(
/* buttonLabel= */ 'Send',
/* draftCommentThreads= */ {},
/* text= */ '',
/* reviewersMutated= */ false,
/* labelsChanged= */ true,
/* includeComments= */ false,
/* disabled= */ false,
/* commentEditing= */ true
));
});

View File

@@ -108,6 +108,12 @@ class GrComment extends mixinBehaviors( [
* @event comment-update
*/
/**
* Fired when editing status changed.
*
* @event comment-editing-changed
*/
/**
* Fired when the comment's timestamp is tapped.
*
@@ -256,6 +262,11 @@ class GrComment extends mixinBehaviors( [
}
_onEditingChange(editing) {
this.dispatchEvent(new CustomEvent('comment-editing-changed', {
detail: !!editing,
bubbles: true,
composed: true,
}));
if (!editing) return;
// visibility based on cache this will make sure we only and always show
// a tip once every Math.max(a day, period between creating comments)

View File

@@ -772,12 +772,12 @@ suite('gr-comment tests', () => {
element.flushDebouncer('fire-update');
element.flushDebouncer('store');
assert.equal(dispatchEventStub.lastCall.args[0].type, 'comment-update'),
assert.isTrue(dispatchEventStub.calledOnce);
assert.isTrue(dispatchEventStub.calledTwice);
element._messageText = 'good news, everyone!';
element.flushDebouncer('fire-update');
element.flushDebouncer('store');
assert.isTrue(dispatchEventStub.calledOnce);
assert.isTrue(dispatchEventStub.calledTwice);
MockInteractions.tap(element.shadowRoot
.querySelector('.save'));