Revert "Add Start Review button to Change View"
This reverts commit 8bd014c05c575430456cd595dd84485ec54aee29.
Reason for revert: We would like to hold this change back for a week or two, because we will need to properly announce it to our users on master. And we want to wait for a few other things to also land and announce them together. I will make sure that this is rolled forward within 14 days.
Change-Id: I30a5f2a7987c9dc0183c19784590b5f2cf1973d9
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js
index 00d3e27..d62ea04 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js
@@ -295,7 +295,7 @@
_replyButtonLabel: {
type: String,
value: 'Reply',
- computed: '_computeReplyButtonLabel(_diffDrafts.*)',
+ computed: '_computeReplyButtonLabel(_diffDrafts.*, _canStartReview)',
},
_selectedPatchSet: String,
_shownFileCount: Number,
@@ -863,23 +863,6 @@
this._openReplyDialog(this.$.replyDialog.FocusTarget.ANY);
}
- _handleReadyTap(e) {
- e.preventDefault();
- const button = e && e.target;
- if (button) {
- button.loading = true;
- }
- return this.$.restAPI.startReview(this._changeNum)
- .then(result => {
- this._reload(result);
- })
- .finally(() => {
- if (button) {
- button.loading = false;
- }
- });
- }
-
_handleOpenDiffPrefs() {
this.$.fileList.openDiffPrefs();
}
@@ -1312,12 +1295,16 @@
return result;
}
- _computeReplyButtonLabel(changeRecord) {
+ _computeReplyButtonLabel(changeRecord, canStartReview) {
// Polymer 2: check for undefined
- if ([changeRecord].some(arg => arg === undefined)) {
+ if ([changeRecord, canStartReview].some(arg => arg === undefined)) {
return 'Reply';
}
+ if (canStartReview) {
+ return 'Start review';
+ }
+
const drafts = (changeRecord && changeRecord.base) || {};
const draftCount = Object.keys(drafts)
.reduce((count, file) => count + drafts[file].length, 0);
@@ -1820,7 +1807,7 @@
_computeCanStartReview(change) {
return !!(change.actions && change.actions.ready &&
- change.actions.ready.enabled);
+ change.actions.ready.enabled);
}
_computeReplyDisabled() { return false; }
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_html.js b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_html.js
index 7e38f9b..b7fdbb7 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_html.js
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_html.js
@@ -371,9 +371,7 @@
<div class="commitContainer">
<div>
<gr-button id="replyBtn" class="reply" title="[[createTitle(Shortcut.OPEN_REPLY_DIALOG,
- ShortcutSection.ACTIONS)]]" hidden\$="[[!_loggedIn]]" primary\$="[[!_canStartReview]]" disabled="[[_replyDisabled]]" on-click="_handleReplyTap">[[_replyButtonLabel]]</gr-button>
- <gr-button id="startReviewBtn" class="startReview" title="Switches change state from 'Work in Progress' to 'Active'."
- hidden="[[!_canStartReview]]" primary\$="[[_canStartReview]]" on-click="_handleReadyTap">Start review</gr-button>
+ ShortcutSection.ACTIONS)]]" hidden\$="[[!_loggedIn]]" primary="" disabled="[[_replyDisabled]]" on-click="_handleReplyTap">[[_replyButtonLabel]]</gr-button>
</div>
<div id="commitMessage" class="commitMessage">
<gr-editable-content id="commitMessageEditor" editing="[[_editingCommitMessage]]" content="{{_latestCommitMessage}}" storage-key="[[_computeCommitMessageKey(_change._number, _change.current_revision)]]" remove-zero-width-space="" collapsed\$="[[_computeCommitMessageCollapsed(_commitCollapsed, _commitCollapsible)]]">
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.html b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.html
index 641a88f..24e2fbd 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.html
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.html
@@ -954,7 +954,7 @@
const getLabel = element._computeReplyButtonLabel;
assert.equal(getLabel(null, false), 'Reply');
- assert.equal(getLabel(null, true), 'Reply');
+ assert.equal(getLabel(null, true), 'Start review');
const changeRecord = {base: null};
assert.equal(getLabel(changeRecord, false), 'Reply');
@@ -970,7 +970,9 @@
});
test('start review button when owner of WIP change', () => {
- assert.equal(element.$.startReviewBtn.innerHTML, 'Start review');
+ assert.equal(
+ element._computeReplyButtonLabel(null, true),
+ 'Start review');
});
test('comment events properly update diff drafts', () => {
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 9435621..305505d 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
@@ -253,7 +253,7 @@
},
_sendDisabled: {
type: Boolean,
- computed: '_computeSendButtonDisabled(canBeStarted, ' +
+ computed: '_computeSendButtonDisabled(_sendButtonLabel, ' +
'draftCommentThreads, draft, _reviewersMutated, _labelsChanged, ' +
'_includeComments, disabled)',
observer: '_sendDisabledChanged',
@@ -853,8 +853,7 @@
}
_computeSendButtonLabel(canBeStarted) {
- return canBeStarted ? ButtonLabels.SEND + ' and ' +
- ButtonLabels.START_REVIEW : ButtonLabels.SEND;
+ return canBeStarted ? ButtonLabels.START_REVIEW : ButtonLabels.SEND;
}
_computeSendButtonTooltip(canBeStarted) {
@@ -866,11 +865,11 @@
}
_computeSendButtonDisabled(
- canBeStarted, draftCommentThreads, text, reviewersMutated,
+ buttonLabel, draftCommentThreads, text, reviewersMutated,
labelsChanged, includeComments, disabled) {
// Polymer 2: check for undefined
if ([
- canBeStarted,
+ buttonLabel,
draftCommentThreads,
text,
reviewersMutated,
@@ -882,7 +881,7 @@
}
if (disabled) { return true; }
- if (canBeStarted === true) { return false; }
+ if (buttonLabel === ButtonLabels.START_REVIEW) { return false; }
const hasDrafts = includeComments && draftCommentThreads.length;
return !hasDrafts && !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 5697090..f7361597 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
@@ -925,7 +925,7 @@
'Send');
assert.equal(
element._computeSendButtonLabel(true),
- 'Send and Start review');
+ 'Start review');
});
test('_handle400Error reviewrs and CCs', done => {
@@ -1108,11 +1108,10 @@
});
});
- test('_computeSendButtonDisabled_canBeStarted', () => {
+ test('_computeSendButtonDisabled', () => {
const fn = element._computeSendButtonDisabled.bind(element);
- // Mock canBeStarted
assert.isFalse(fn(
- /* canBeStarted= */ true,
+ /* buttonLabel= */ 'Start review',
/* draftCommentThreads= */ [],
/* text= */ '',
/* reviewersMutated= */ false,
@@ -1120,13 +1119,8 @@
/* includeComments= */ false,
/* disabled= */ false
));
- });
-
- test('_computeSendButtonDisabled_allFalse', () => {
- const fn = element._computeSendButtonDisabled.bind(element);
- // Mock everything false
assert.isTrue(fn(
- /* canBeStarted= */ false,
+ /* buttonLabel= */ 'Send',
/* draftCommentThreads= */ [],
/* text= */ '',
/* reviewersMutated= */ false,
@@ -1134,13 +1128,9 @@
/* includeComments= */ false,
/* disabled= */ false
));
- });
-
- test('_computeSendButtonDisabled_draftCommentsSend', () => {
- const fn = element._computeSendButtonDisabled.bind(element);
- // Mock nonempty comment draft array, with sending comments.
+ // Mock nonempty comment draft array, with seding comments.
assert.isFalse(fn(
- /* canBeStarted= */ false,
+ /* buttonLabel= */ 'Send',
/* draftCommentThreads= */ [{comments: [{__draft: true}]}],
/* text= */ '',
/* reviewersMutated= */ false,
@@ -1148,13 +1138,9 @@
/* includeComments= */ true,
/* disabled= */ false
));
- });
-
- test('_computeSendButtonDisabled_draftCommentsDoNotSend', () => {
- const fn = element._computeSendButtonDisabled.bind(element);
- // Mock nonempty comment draft array, without sending comments.
+ // Mock nonempty comment draft array, without seding comments.
assert.isTrue(fn(
- /* canBeStarted= */ false,
+ /* buttonLabel= */ 'Send',
/* draftCommentThreads= */ [{comments: [{__draft: true}]}],
/* text= */ '',
/* reviewersMutated= */ false,
@@ -1162,13 +1148,9 @@
/* includeComments= */ false,
/* disabled= */ false
));
- });
-
- test('_computeSendButtonDisabled_changeMessage', () => {
- const fn = element._computeSendButtonDisabled.bind(element);
// Mock nonempty change message.
assert.isFalse(fn(
- /* canBeStarted= */ false,
+ /* buttonLabel= */ 'Send',
/* draftCommentThreads= */ {},
/* text= */ 'test',
/* reviewersMutated= */ false,
@@ -1176,13 +1158,9 @@
/* includeComments= */ false,
/* disabled= */ false
));
- });
-
- test('_computeSendButtonDisabled_reviewersChanged', () => {
- const fn = element._computeSendButtonDisabled.bind(element);
// Mock reviewers mutated.
assert.isFalse(fn(
- /* canBeStarted= */ false,
+ /* buttonLabel= */ 'Send',
/* draftCommentThreads= */ {},
/* text= */ '',
/* reviewersMutated= */ true,
@@ -1190,13 +1168,9 @@
/* includeComments= */ false,
/* disabled= */ false
));
- });
-
- test('_computeSendButtonDisabled_labelsChanged', () => {
- const fn = element._computeSendButtonDisabled.bind(element);
// Mock labels changed.
assert.isFalse(fn(
- /* canBeStarted= */ false,
+ /* buttonLabel= */ 'Send',
/* draftCommentThreads= */ {},
/* text= */ '',
/* reviewersMutated= */ false,
@@ -1204,13 +1178,9 @@
/* includeComments= */ false,
/* disabled= */ false
));
- });
-
- test('_computeSendButtonDisabled_dialogDisabled', () => {
- const fn = element._computeSendButtonDisabled.bind(element);
// Whole dialog is disabled.
assert.isTrue(fn(
- /* canBeStarted= */ false,
+ /* buttonLabel= */ 'Send',
/* draftCommentThreads= */ {},
/* text= */ '',
/* reviewersMutated= */ false,
