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
This commit is contained in:
Kasper Nilsson
2017-08-08 14:04:54 -07:00
parent c1eb61280d
commit 5f1f0c6f4e
3 changed files with 73 additions and 9 deletions

View File

@@ -153,6 +153,9 @@ limitations under the License.
-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 @@ limitations under the License.
}
</style>
<gr-fixed-panel
class$="[[_computeContainerClass(_editLoaded)]]"
floating-disabled="[[_panelFloatingDisabled]]"
keep-on-scroll
ready-for-measure="[[!_loading]]">
@@ -210,10 +214,10 @@ limitations under the License.
<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 @@ limitations under the License.
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 @@ limitations under the License.
<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 @@ limitations under the License.
</div>
<div class="fileNav mobile">
<a class="mobileNavLink"
href$="[[_computeNavLinkURL(_change, _path, _fileList, -1, 1)]]">
href$="[[_computeNavLinkURL(_change, _path, _fileList, -1, 1)]]">
&lt;</a>
<div class="fullFileName mobile">[[_computeFileDisplayName(_path)]]
</div>

View File

@@ -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' : '';
},
});
})();

View File

@@ -494,6 +494,17 @@ limitations under the License.
});
});
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 @@ limitations under the License.
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>