Disable reviewed checkbox in diff view for edit
Modifying the reviewed state of an edit patchset is an illogical action,
and should be disabled.
This change adds an _editLoaded property to gr-diff-view to
conditionally hide the reviewed checkbox. In addition, attempts to
modify the reviewed state are treated as a no-op.
TODO: Further utilize this approach to conditionally show buttons for
modifying an edit patchset. Will be done in a later change.
Bug: Issue 4437
Change-Id: I0ea98e49c5ec66e73f45fef9db5e3849d6e594df
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.html b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.html
index 2908f85..440f44b 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.html
@@ -153,6 +153,9 @@
-webkit-user-select: text;
user-select: text;
}
+ .editLoaded .hideOnEdit {
+ display: none;
+ }
@media screen and (max-width: 50em) {
header {
padding: .5em var(--default-horizontal-margin);
@@ -200,6 +203,7 @@
}
</style>
<gr-fixed-panel
+ class$="[[_computeContainerClass(_editLoaded)]]"
floating-disabled="[[_panelFloatingDisabled]]"
keep-on-scroll
ready-for-measure="[[!_loading]]">
@@ -210,10 +214,10 @@
<span>[[_change.subject]]</span>
<span class="dash">—</span>
<input id="reviewed"
- class="reviewed"
- type="checkbox"
- on-change="_handleReviewedChange"
- hidden$="[[!_loggedIn]]" hidden>
+ class="reviewed hideOnEdit"
+ type="checkbox"
+ on-change="_handleReviewedChange"
+ hidden$="[[!_loggedIn]]" hidden>
<div class="jumpToFileContainer desktop">
<gr-button link class="dropdown-trigger" id="trigger" on-tap="_showDropdownTapHandler">
<span>[[_computeFileDisplayName(_path)]]</span>
@@ -231,9 +235,9 @@
as="path"
initial-count="75">
<a href$="[[_computeDiffURL(_change, _patchRange.*, path)]]"
- selected$="[[_computeFileSelected(path, _path)]]"
- data-key-nav$="[[_computeKeyNav(path, _path, _fileList)]]"
- on-tap="_handleFileTap">[[_computeFileDisplayName(path)]]</a>
+ selected$="[[_computeFileSelected(path, _path)]]"
+ data-key-nav$="[[_computeKeyNav(path, _path, _fileList)]]"
+ on-tap="_handleFileTap">[[_computeFileDisplayName(path)]]</a>
</template>
</div>
</iron-dropdown>
@@ -277,7 +281,7 @@
<span class="download desktop">
<span class="separator">/</span>
<a class="downloadLink"
- href$="[[_computeDownloadLink(_changeNum, _patchRange, _path)]]">
+ href$="[[_computeDownloadLink(_changeNum, _patchRange, _path)]]">
Download
</a>
</span>
@@ -306,7 +310,7 @@
</div>
<div class="fileNav mobile">
<a class="mobileNavLink"
- href$="[[_computeNavLinkURL(_change, _path, _fileList, -1, 1)]]">
+ href$="[[_computeNavLinkURL(_change, _path, _fileList, -1, 1)]]">
<</a>
<div class="fullFileName mobile">[[_computeFileDisplayName(_path)]]
</div>
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.js b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.js
index 0f6a07d..c12f700 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.js
@@ -113,6 +113,10 @@
type: Boolean,
value: () => { return window.PANEL_FLOATING_DISABLED; },
},
+ _editLoaded: {
+ type: Boolean,
+ computed: '_computeEditLoaded(_patchRange.*)',
+ },
},
behaviors: [
@@ -205,6 +209,7 @@
},
_setReviewed(reviewed) {
+ if (this._editLoaded) { return; }
this.$.reviewed.checked = reviewed;
this._saveReviewedState(reviewed).catch(err => {
this.fire('show-alert', {message: ERR_REVIEW_STATUS});
@@ -767,5 +772,20 @@
return 'noOverflow';
}
},
+
+ /**
+ * @param {!Object} patchRangeRecord
+ */
+ _computeEditLoaded(patchRangeRecord) {
+ const patchRange = patchRangeRecord.base || {};
+ return this.patchNumEquals(patchRange.patchNum, this.EDIT_NAME);
+ },
+
+ /**
+ * @param {boolean} editLoaded
+ */
+ _computeContainerClass(editLoaded) {
+ return editLoaded ? 'editLoaded' : '';
+ },
});
})();
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.html b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.html
index 8ff75c1..49e82d4 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.html
@@ -494,6 +494,17 @@
});
});
+ test('file review status with edit loaded', () => {
+ const saveReviewedStub = sandbox.stub(element, '_saveReviewedState');
+
+ element._patchRange = {patchNum: element.EDIT_NAME};
+ flushAsynchronousOperations();
+
+ assert.isTrue(element._editLoaded);
+ element._setReviewed();
+ assert.isFalse(saveReviewedStub.called);
+ });
+
test('hash is determined from params', done => {
stub('gr-rest-api-interface', {
getDiffComments() { return Promise.resolve({}); },
@@ -746,5 +757,34 @@
assert.isFalse(upgradeStub.called);
});
});
+
+ test('_computeEditLoaded', () => {
+ const callCompute = range => element._computeEditLoaded({base: range});
+ assert.isFalse(callCompute({}));
+ assert.isFalse(callCompute({basePatchNum: 'PARENT', patchNum: 1}));
+ assert.isFalse(callCompute({basePatchNum: 'edit', patchNum: 1}));
+ assert.isTrue(callCompute({basePatchNum: 1, patchNum: 'edit'}));
+ });
+
+ suite('editLoaded behavior', () => {
+ setup(() => {
+ element._loggedIn = true;
+ });
+
+ const isVisible = el => {
+ assert.ok(el);
+ return getComputedStyle(el).getPropertyValue('display') !== 'none';
+ };
+
+ test('reviewed checkbox', () => {
+ element._patchRange = {patchNum: '1'};
+ // Reviewed checkbox should be shown.
+ assert.isTrue(isVisible(element.$.reviewed));
+ element.set('_patchRange.patchNum', element.EDIT_NAME);
+ flushAsynchronousOperations();
+
+ assert.isFalse(isVisible(element.$.reviewed));
+ });
+ });
});
</script>
