Move patch set and download actions to the file list

+ This is part one of a few changes that aim to ease confusion
  with users of Gerrit. Most notably, that there is a lot of
  information in the change view that doesn’t change across
  patch sets, but being on an old patch set can cause issues
  if the user wants to set a label or submit a change.
+ This change moves the patch set dropdown and download
  buttons to be in the file list area. Further changes such
  as moving the commit info and not adjusting the top-level
  commit message upon patch set change (since you can see
  old commit messages using the file list) are coming.

As a note, this is an experiment based on heavy feedback but
is no way set in stone. We can get early feedback from our
internal dogfooders on these changes and iterate as we go.

Change-Id: Iaa1506cf7d24d55c9786b967ed63726bcf777826
This commit is contained in:
Andrew Bonventre
2016-08-22 17:17:13 -04:00
parent 3f8bf004d1
commit 04a830cac7
3 changed files with 62 additions and 53 deletions

View File

@@ -79,17 +79,11 @@ limitations under the License.
color: #999;
text-transform: capitalize;
}
section {
margin: 10px 0;
padding: 10px var(--default-horizontal-margin);
}
/* Strong specificity here is needed due to
https://github.com/Polymer/polymer/issues/2531 */
.container section.changeInfo {
border-bottom: 1px solid #ddd;
display: flex;
margin-top: 0;
padding-top: 0;
padding: 0 var(--default-horizontal-margin);
}
.changeInfo-column:not(:last-of-type) {
margin-right: 1em;
@@ -138,9 +132,17 @@ limitations under the License.
font-size: .9em;
overflow: hidden;
}
.patchInfo {
border: 1px solid #ddd;
margin: 1em var(--default-horizontal-margin);
}
.patchInfo-header,
gr-file-list {
margin-bottom: 1em;
padding: 0 var(--default-horizontal-margin);
padding: .5em calc(var(--default-horizontal-margin) / 2);
}
.patchInfo-header {
background-color: #f6f6f6;
border-bottom: 1px solid #ebebeb;
}
@media screen and (max-width: 50em) {
.header {
@@ -158,7 +160,7 @@ limitations under the License.
min-width: initial;
width: 90vw;
}
.download {
.downloadContainer {
display: none;
}
.reply {
@@ -203,21 +205,6 @@ limitations under the License.
<span>[[_change.subject]]</span>
<span class="changeStatus">[[_computeChangeStatus(_change, _patchRange.patchNum)]]</span>
</span>
<span class="header-actions">
<gr-button class="download" on-tap="_handleDownloadTap">Download</gr-button>
<span>
<label class="patchSelectLabel" for="patchSetSelect">Patch set</label>
<select id="patchSetSelect" on-change="_handlePatchChange">
<template is="dom-repeat" items="{{_allPatchSets}}" as="patchNumber">
<option value$="[[patchNumber]]" selected$="[[_computePatchIndexIsSelected(index, _patchRange.patchNum)]]">
<span>[[patchNumber]]</span>
/
<span>[[_computeLatestPatchNum(_allPatchSets)]]</span>
</option>
</template>
</select>
</span>
</span>
</div>
<section class="changeInfo">
<div class="changeInfo-column changeMetadata">
@@ -265,15 +252,37 @@ limitations under the License.
</div>
</div>
</section>
<gr-file-list id="fileList"
change="[[_change]]"
change-num="[[_changeNum]]"
patch-range="[[_patchRange]]"
comments="[[_comments]]"
drafts="[[_diffDrafts]]"
revisions="[[_change.revisions]]"
projectConfig="[[_projectConfig]]"
selected-index="{{viewState.selectedFileIndex}}"></gr-file-list>
<section class="patchInfo">
<div class="patchInfo-header">
<span>
<label class="patchSelectLabel" for="patchSetSelect">Patch set</label>
<select id="patchSetSelect" on-change="_handlePatchChange">
<template is="dom-repeat" items="{{_allPatchSets}}" as="patchNumber">
<option value$="[[patchNumber]]" selected$="[[_computePatchIndexIsSelected(index, _patchRange.patchNum)]]">
<span>[[patchNumber]]</span>
/
<span>[[_computeLatestPatchNum(_allPatchSets)]]</span>
</option>
</template>
</select>
</span>
<span class="downloadContainer">
/
<gr-button link
class="download"
on-tap="_handleDownloadTap">Download</gr-button>
</span>
</div>
<gr-file-list id="fileList"
change="[[_change]]"
change-num="[[_changeNum]]"
patch-range="[[_patchRange]]"
comments="[[_comments]]"
drafts="[[_diffDrafts]]"
revisions="[[_change.revisions]]"
projectConfig="[[_projectConfig]]"
selected-index="{{viewState.selectedFileIndex}}"></gr-file-list>
</section>
<gr-messages-list id="messageList"
change-num="[[_changeNum]]"
messages="[[_change.messages]]"

View File

@@ -184,17 +184,17 @@ limitations under the License.
labels: {},
};
flushAsynchronousOperations();
var selectEl = element.$$('.header select');
var selectEl = element.$$('.patchInfo-header select');
assert.ok(selectEl);
var optionEls =
Polymer.dom(element.root).querySelectorAll('.header option');
var optionEls = Polymer.dom(element.root).querySelectorAll(
'.patchInfo-header option');
assert.equal(optionEls.length, 4);
assert.isFalse(
element.$$('.header option[value="1"]').hasAttribute('selected'));
assert.isTrue(
element.$$('.header option[value="2"]').hasAttribute('selected'));
assert.isFalse(
element.$$('.header option[value="3"]').hasAttribute('selected'));
assert.isFalse(element.$$('.patchInfo-header option[value="1"]')
.hasAttribute('selected'));
assert.isTrue(element.$$('.patchInfo-header option[value="2"]')
.hasAttribute('selected'));
assert.isFalse(element.$$('.patchInfo-header option[value="3"]')
.hasAttribute('selected'));
assert.equal(optionEls[3].value, 13);
var showStub = sinon.stub(page, 'show');
@@ -236,17 +236,17 @@ limitations under the License.
labels: {},
};
flushAsynchronousOperations();
var selectEl = element.$$('.header select');
var selectEl = element.$$('.patchInfo-header select');
assert.ok(selectEl);
var optionEls =
Polymer.dom(element.root).querySelectorAll('.header option');
var optionEls = Polymer.dom(element.root).querySelectorAll(
'.patchInfo-header option');
assert.equal(optionEls.length, 4);
assert.isFalse(
element.$$('.header option[value="1"]').hasAttribute('selected'));
assert.isTrue(
element.$$('.header option[value="2"]').hasAttribute('selected'));
assert.isFalse(
element.$$('.header option[value="3"]').hasAttribute('selected'));
assert.isFalse(element.$$('.patchInfo-header option[value="1"]')
.hasAttribute('selected'));
assert.isTrue(element.$$('.patchInfo-header option[value="2"]')
.hasAttribute('selected'));
assert.isFalse(element.$$('.patchInfo-header option[value="3"]')
.hasAttribute('selected'));
assert.equal(optionEls[3].value, 13);
var showStub = sinon.stub(page, 'show');

View File

@@ -20,7 +20,7 @@ limitations under the License.
--selection-background-color: #ebf5fb;
--default-text-color: #000;
--view-background-color: #fff;
--default-horizontal-margin: 1.25rem;
--default-horizontal-margin: 1rem;
--font-family: -apple-system, BlinkMacSystemFont, 'Segoe UI', Roboto, Oxygen, Ubuntu, Cantarell, 'Open Sans', 'Helvetica Neue', sans-serif;
--monospace-font-family: 'Source Code Pro', Menlo, 'Lucida Console', Monaco, monospace;