Disable send button when user has made no changes
Bug: Issue 6336
Change-Id: I9a3003f0084141d72d899ac6d01d89de79a90704
diff --git a/polygerrit-ui/app/elements/change/gr-label-score-row/gr-label-score-row.js b/polygerrit-ui/app/elements/change/gr-label-score-row/gr-label-score-row.js
index d5506ad..20fd147 100644
--- a/polygerrit-ui/app/elements/change/gr-label-score-row/gr-label-score-row.js
+++ b/polygerrit-ui/app/elements/change/gr-label-score-row/gr-label-score-row.js
@@ -16,6 +16,13 @@
Polymer({
is: 'gr-label-score-row',
+
+ /**
+ * Fired when any label is changed.
+ *
+ * @event labels-changed
+ */
+
properties: {
/**
* @type {{ name: string }}
@@ -96,6 +103,7 @@
// nothing and then to the new item.
if (!e.target.selectedItem) { return; }
this._selectedValueText = e.target.selectedItem.getAttribute('title');
+ this.fire('labels-changed');
},
_computeAnyPermittedLabelValues(permittedLabels, label) {
diff --git a/polygerrit-ui/app/elements/change/gr-label-score-row/gr-label-score-row_test.html b/polygerrit-ui/app/elements/change/gr-label-score-row/gr-label-score-row_test.html
index 537bd25..da450e1 100644
--- a/polygerrit-ui/app/elements/change/gr-label-score-row/gr-label-score-row_test.html
+++ b/polygerrit-ui/app/elements/change/gr-label-score-row/gr-label-score-row_test.html
@@ -103,6 +103,8 @@
});
test('label picker', () => {
+ const labelsChangedHandler = sandbox.stub();
+ element.addEventListener('labels-changed', labelsChangedHandler);
assert.ok(element.$$('iron-selector'));
MockInteractions.tap(element.$$(
'gr-button[value="-1"]'));
@@ -112,6 +114,7 @@
.textContent.trim(), '-1');
assert.strictEqual(
element.$.selectedValueLabel.textContent.trim(), 'bad');
+ assert.isTrue(labelsChangedHandler.called);
});
test('correct item is selected', () => {
diff --git a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.html b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.html
index 2f9f78c..24673ff 100644
--- a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.html
+++ b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.html
@@ -241,6 +241,7 @@
id="labelScores"
account="[[_account]]"
change="[[change]]"
+ on-labels-changed="_handleLabelsChanged"
permitted-labels=[[permittedLabels]]></gr-label-scores>
</section>
<section class="draftsContainer" hidden$="[[_computeHideDraftList(diffDrafts)]]">
@@ -265,7 +266,7 @@
<section>
<gr-button
primary
- disabled="[[_isState(knownLatestState, 'not-latest')]]"
+ disabled="[[_computeSendButtonDisabled(knownLatestState, _sendButtonLabel, diffDrafts, draft, _reviewersMutated, _labelsChanged)]]"
class="action send"
on-tap="_sendTapHandler">[[_sendButtonLabel]]</gr-button>
<template is="dom-if" if="[[canBeStarted]]">
diff --git a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.js b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.js
index 752f15a..9f4a79a 100644
--- a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.js
+++ b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.js
@@ -34,6 +34,11 @@
NOT_LATEST: 'not-latest',
};
+ const ButtonLabels = {
+ START_REVIEW: 'Start review',
+ SEND: 'Send',
+ };
+
// TODO(logan): Remove once the fix for issue 6841 is stable on
// googlesource.com.
const START_REVIEW_MESSAGE = 'This change is ready for review.';
@@ -175,6 +180,14 @@
computed: '_computeCCsEnabled(serverConfig)',
},
_savingComments: Boolean,
+ _reviewersMutated: {
+ type: Boolean,
+ value: false,
+ },
+ _labelsChanged: {
+ type: Boolean,
+ value: false,
+ },
},
FocusTarget,
@@ -268,12 +281,14 @@
_ccsChanged(splices) {
if (splices && splices.indexSplices) {
+ this._reviewersMutated = true;
this._processReviewerChange(splices.indexSplices, ReviewerTypes.CC);
}
},
_reviewersChanged(splices) {
if (splices && splices.indexSplices) {
+ this._reviewersMutated = true;
this._processReviewerChange(splices.indexSplices,
ReviewerTypes.REVIEWER);
let key;
@@ -768,6 +783,11 @@
});
},
+ _handleLabelsChanged() {
+ this._labelsChanged = Object.keys(
+ this.$.labelScores.getLabelValues()).length !== 0;
+ },
+
_isState(knownLatestState, value) {
return knownLatestState === value;
},
@@ -778,7 +798,7 @@
},
_computeSendButtonLabel(canBeStarted) {
- return canBeStarted ? 'Start review' : 'Send';
+ return canBeStarted ? ButtonLabels.START_REVIEW : ButtonLabels.SEND;
},
_computeCCsEnabled(serverConfig) {
@@ -788,5 +808,17 @@
_computeSavingLabelClass(savingComments) {
return savingComments ? 'saving' : '';
},
+
+ _computeSendButtonDisabled(knownLatestState, buttonLabel, drafts, text,
+ reviewersMutated, labelsChanged) {
+ if (this._isState(knownLatestState, LatestPatchState.NOT_LATEST)) {
+ return true;
+ }
+ if (buttonLabel === ButtonLabels.START_REVIEW) {
+ return false;
+ }
+ return !(drafts.length || text.length || reviewersMutated ||
+ labelsChanged);
+ },
});
})();
diff --git a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog_test.html b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog_test.html
index 0f90019..9a83259 100644
--- a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog_test.html
+++ b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog_test.html
@@ -55,6 +55,8 @@
stub('gr-rest-api-interface', {
getConfig() { return Promise.resolve({}); },
getAccount() { return Promise.resolve({}); },
+ getChange() { return Promise.resolve({}); },
+ getChangeSuggestedReviewers() { return Promise.resolve([]); },
});
element = fixture('basic');
@@ -1007,5 +1009,20 @@
});
});
});
+
+ test('_computeSendButtonDisabled', () => {
+ const fn = element._computeSendButtonDisabled.bind(element);
+ assert.isTrue(fn('not-latest'));
+ assert.isFalse(fn('latest', 'Start review'));
+ assert.isTrue(fn('latest', 'Send', [], '', false, false));
+ // Mock nonempty comment draft array.
+ assert.isFalse(fn('latest', 'Send', ['test'], '', false, false));
+ // Mock nonempty change message.
+ assert.isFalse(fn('latest', 'Send', [], 'test', false, false));
+ // Mock reviewers mutated.
+ assert.isFalse(fn('latest', 'Send', [], '', true, false));
+ // Mock labels changed.
+ assert.isFalse(fn('latest', 'Send', [], '', false, true));
+ });
});
</script>
