Merge changes Ib130fb07,Idf12bcef,I8a39995f,Ida1a4678

* changes:
  Polish up top-level dropdown menus
  Move gr-change-action buttons to be above commit message
  Remove topMargin since the header is no longer pinned
  Remove pinned header behavior
This commit is contained in:
Andrew Bonventre 2016-08-22 19:56:22 +00:00 committed by Gerrit Code Review
commit 3f8bf004d1
12 changed files with 159 additions and 207 deletions

View File

@ -32,19 +32,14 @@ limitations under the License.
<template>
<style>
:host {
display: block;
display: inline-block;
font-family: var(--font-family);
}
section {
margin-top: 1em;
}
.groupLabel {
color: #666;
margin-bottom: .15em;
text-align: center;
display: inline-block;
}
gr-button {
display: block;
margin-bottom: .5em;
margin-left: .5em;
}
gr-button:before {
content: attr(data-label);
@ -53,6 +48,15 @@ limitations under the License.
content: attr(data-loading-label);
}
@media screen and (max-width: 50em) {
:host,
section,
gr-button {
display: block;
}
gr-button {
margin-bottom: .5em;
margin-left: 0;
}
.confirmDialog {
width: 90vw;
}
@ -60,7 +64,6 @@ limitations under the License.
</style>
<div>
<section hidden$="[[!_actionCount(actions.*, _additionalActions.*)]]">
<div class="groupLabel">Change</div>
<template is="dom-repeat" items="[[_changeActionValues]]" as="action">
<gr-button title$="[[action.title]]"
primary$="[[action.__primary]]"
@ -73,7 +76,6 @@ limitations under the License.
</template>
</section>
<section hidden$="[[!_actionCount(_revisionActions.*, _additionalActions.*)]]">
<div class="groupLabel">Revision</div>
<template is="dom-repeat" items="[[_revisionActionValues]]" as="action">
<gr-button title$="[[action.title]]"
primary$="[[action.__primary]]"

View File

@ -45,18 +45,16 @@ limitations under the License.
color: #666;
padding: 1em var(--default-horizontal-margin);
}
.headerContainer {
height: 4.1em;
margin-bottom: .5em;
}
.header {
align-items: center;
background-color: var(--view-background-color);
border-bottom: 1px solid #ddd;
display: flex;
padding: 1em var(--default-horizontal-margin);
padding: .65em var(--default-horizontal-margin);
z-index: 99; /* Less than gr-overlay's backdrop */
}
.header .download {
margin-right: 1em;
}
.header.pinned {
border-bottom-color: transparent;
box-shadow: 0 1px 3px rgba(0, 0, 0, 0.3);
@ -69,24 +67,11 @@ limitations under the License.
flex: 1;
font-size: 1.2em;
font-weight: bold;
overflow: hidden;
text-overflow: ellipsis;
white-space: nowrap;
}
gr-change-star {
margin-right: .25em;
vertical-align: -.425em;
}
.download,
.patchSelectLabel {
margin-left: 1em;
}
.header select {
margin-left: .5em;
}
.header .reply {
margin-left: var(--default-horizontal-margin);
}
gr-reply-dialog {
width: 50em;
}
@ -111,12 +96,8 @@ limitations under the License.
padding-right: 1em;
}
.changeMetadata {
border-right: 1px solid #ddd;
font-size: .9em;
}
gr-change-actions {
margin-top: 1em;
}
.commitMessage {
font-family: var(--monospace-font-family);
flex: 0 0 72ch;
@ -124,15 +105,28 @@ limitations under the License.
margin-bottom: 1em;
overflow-x: hidden;
}
.commitMessage h4 {
font-family: var(--font-family);
font-weight: bold;
margin-bottom: .25em;
}
.commitMessage gr-linked-text {
--linked-text-white-space: pre;
overflow: auto;
}
.editCommitMessage {
margin-top: 1em;
}
.commitActions {
border-bottom: 1px solid #ddd;
display: flex;
justify-content: space-between;
margin-bottom: .5em;
padding-bottom: .5em;
}
.reply {
margin-right: .5em;
}
.mainChangeInfo {
display: flex;
flex: 1;
flex-direction: column;
}
.commitAndRelated {
align-content: flex-start;
display: flex;
@ -149,9 +143,6 @@ limitations under the License.
padding: 0 var(--default-horizontal-margin);
}
@media screen and (max-width: 50em) {
.headerContainer {
height: 5.15em;
}
.header {
align-items: flex-start;
flex-direction: column;
@ -163,12 +154,6 @@ limitations under the License.
.header-title {
font-size: 1.1em;
}
.header-actions {
align-items: center;
display: flex;
justify-content: space-between;
margin-top: .5em;
}
gr-reply-dialog {
min-width: initial;
width: 90vw;
@ -176,17 +161,10 @@ limitations under the License.
.download {
display: none;
}
.patchSelectLabel {
margin-left: 0;
margin-right: .5em;
}
.header select {
margin-left: 0;
margin-right: .5em;
}
.header .reply {
margin-left: 0;
margin-right: .5em;
.reply {
display: block;
margin-right: 0;
margin-bottom: .5em;
}
.changeInfo-column:not(:last-of-type) {
margin-right: 0;
@ -207,6 +185,9 @@ limitations under the License.
margin-top: .25em;
max-width: none;
}
.commitActions {
flex-direction: column;
}
.commitMessage {
flex: initial;
margin-right: 0;
@ -215,35 +196,28 @@ limitations under the License.
</style>
<div class="container loading" hidden$="{{!_loading}}">Loading...</div>
<div class="container" hidden$="{{_loading}}">
<div class="headerContainer">
<div class="header">
<span class="header-title">
<gr-change-star change="{{_change}}" hidden$="[[!_loggedIn]]"></gr-change-star>
<a href$="[[_computeChangePermalink(_change._number)]]">[[_change._number]]</a><span>:</span>
<span>[[_change.subject]]</span>
<span class="changeStatus">[[_computeChangeStatus(_change, _patchRange.patchNum)]]</span>
<div class="header">
<span class="header-title">
<gr-change-star change="{{_change}}" hidden$="[[!_loggedIn]]"></gr-change-star>
<a href$="[[_computeChangePermalink(_change._number)]]">[[_change._number]]</a><span>:</span>
<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 class="header-actions">
<gr-button hidden
class="reply"
primary$="[[_computeReplyButtonHighlighted(_diffDrafts.*)]]"
hidden$="[[!_loggedIn]]"
on-tap="_handleReplyTap">[[_replyButtonLabel]]</gr-button>
<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>
</span>
</div>
<section class="changeInfo">
<div class="changeInfo-column changeMetadata">
@ -254,34 +228,40 @@ limitations under the License.
mutable="[[_loggedIn]]"
on-show-reply-dialog="_handleShowReplyDialog">
</gr-change-metadata>
<gr-change-actions id="actions"
change="[[_change]]"
actions="[[_change.actions]]"
change-num="[[_changeNum]]"
patch-num="[[_patchRange.patchNum]]"
commit-info="[[_commitInfo]]"
on-reload-change="_handleReloadChange"></gr-change-actions>
</div>
<div class="changeInfo-column commitAndRelated">
<div class="commitMessage">
<h4>
Commit message
<div class="changeInfo-column mainChangeInfo">
<div class="commitActions" hidden$="[[!_loggedIn]]"">
<gr-button
class="reply"
secondary
on-tap="_handleReplyTap">[[_replyButtonLabel]]</gr-button>
<gr-change-actions id="actions"
change="[[_change]]"
actions="[[_change.actions]]"
change-num="[[_changeNum]]"
patch-num="[[_patchRange.patchNum]]"
commit-info="[[_commitInfo]]"
on-reload-change="_handleReloadChange"></gr-change-actions>
</div>
<div class="commitAndRelated">
<div class="commitMessage">
<gr-editable-content id="commitMessageEditor"
editing="[[_editingCommitMessage]]"
content="{{_commitInfo.message}}">
<gr-linked-text pre
content="[[_commitInfo.message]]"
config="[[_projectConfig.commentlinks]]"></gr-linked-text>
</gr-editable-content>
<gr-button link
class="editCommitMessage"
on-tap="_handleEditCommitMessage"
hidden$="[[_hideEditCommitMessage]]">Edit</gr-button>
</h4>
<gr-editable-content id="commitMessageEditor"
editing="[[_editingCommitMessage]]"
content="{{_commitInfo.message}}">
<gr-linked-text pre
content="[[_commitInfo.message]]"
config="[[_projectConfig.commentlinks]]"></gr-linked-text>
</gr-editable-content>
</div>
<div class="relatedChanges">
<gr-related-changes-list id="relatedChanges"
change="[[_change]]"
patch-num="[[_patchRange.patchNum]]"></gr-related-changes-list>
</div>
<div class="relatedChanges">
<gr-related-changes-list id="relatedChanges"
change="[[_change]]"
patch-num="[[_patchRange.patchNum]]"></gr-related-changes-list>
</div>
</div>
</div>
</section>

View File

@ -78,8 +78,6 @@
value: false,
},
_loading: Boolean,
_headerContainerEl: Object,
_headerEl: Object,
_projectConfig: Object,
_replyButtonLabel: {
type: String,
@ -98,10 +96,6 @@
'_paramsAndChangeChanged(params, _change)',
],
ready: function() {
this._headerEl = this.$$('.header');
},
attached: function() {
this._getLoggedIn().then(function(loggedIn) {
this._loggedIn = loggedIn;
@ -114,34 +108,6 @@
this._handleCommitMessageSave.bind(this));
this.addEventListener('editable-content-cancel',
this._handleCommitMessageCancel.bind(this));
this.listen(window, 'scroll', '_handleBodyScroll');
},
detached: function() {
this.unlisten(window, 'scroll', '_handleBodyScroll');
},
_handleBodyScroll: function(e) {
var containerEl = this._headerContainerEl ||
this.$$('.headerContainer');
// Calculate where the header is relative to the window.
var top = containerEl.offsetTop;
for (var offsetParent = containerEl.offsetParent;
offsetParent;
offsetParent = offsetParent.offsetParent) {
top += offsetParent.offsetTop;
}
// The element may not be displayed yet, in which case do nothing.
if (top == 0) { return; }
this._headerEl.classList.toggle('pinned', window.scrollY >= top);
},
_resetHeaderEl: function() {
var el = this._headerEl || this.$$('.header');
this._headerEl = el;
el.classList.remove('pinned');
},
_handleEditCommitMessage: function(e) {
@ -327,9 +293,6 @@
};
this._reload().then(function() {
this.$.messageList.topMargin = this._headerEl.offsetHeight;
this.$.fileList.topMargin = this._headerEl.offsetHeight;
// Allow the message list to render before scrolling.
this.async(function() {
this._maybeScrollToMessage();
@ -484,11 +447,6 @@
return result;
},
_computeReplyButtonHighlighted: function(changeRecord) {
var drafts = (changeRecord && changeRecord.base) || {};
return Object.keys(drafts).length > 0;
},
_computeReplyButtonLabel: function(changeRecord) {
var drafts = (changeRecord && changeRecord.base) || {};
var draftCount = Object.keys(drafts).reduce(function(count, file) {
@ -640,8 +598,6 @@
]);
}.bind(this);
this._resetHeaderEl();
if (this._patchRange.patchNum) {
return reloadPatchNumDependentResources().then(function() {
return detailCompletes;

View File

@ -111,22 +111,21 @@ limitations under the License.
});
});
test('reply button is highlighted when there are drafts', function() {
test('reply button has updated count when there are drafts', function() {
var replyButton = element.$$('gr-button.reply');
assert.ok(replyButton);
assert.isFalse(replyButton.hasAttribute('primary'));
assert.equal(replyButton.textContent, 'Reply');
element._diffDrafts = null;
assert.isFalse(replyButton.hasAttribute('primary'));
assert.equal(replyButton.textContent, 'Reply');
element._diffDrafts = {};
assert.isFalse(replyButton.hasAttribute('primary'));
assert.equal(replyButton.textContent, 'Reply');
element._diffDrafts = {
'file1.txt': [{}],
'file2.txt': [{}, {}],
};
assert.isTrue(replyButton.hasAttribute('primary'));
assert.equal(replyButton.textContent, 'Reply (3)');
});
@ -354,7 +353,7 @@ limitations under the License.
test('topic is coalesced to null', function(done) {
sinon.stub(element, '_changeChanged');
sinon.stub(element.$.restAPI, 'getChangeDetail', function(num) {
sinon.stub(element.$.restAPI, 'getChangeDetail', function() {
return Promise.resolve({id: '123456789', labels: {}});
});

View File

@ -181,9 +181,7 @@ limitations under the License.
</template>
<gr-rest-api-interface id="restAPI"></gr-rest-api-interface>
<gr-storage id="storage"></gr-storage>
<gr-diff-cursor
id="cursor"
fold-offset-top="[[topMargin]]"></gr-diff-cursor>
<gr-diff-cursor id="cursor"></gr-diff-cursor>
</template>
<script src="gr-file-list.js"></script>
</dom-module>

View File

@ -27,7 +27,6 @@
drafts: Object,
revisions: Object,
projectConfig: Object,
topMargin: Number,
selectedIndex: {
type: Number,
notify: true,
@ -352,7 +351,7 @@
}
// Don't scroll if it's already in view.
if (top > window.pageYOffset + this.topMargin &&
if (top > window.pageYOffset &&
top < window.pageYOffset + window.innerHeight - el.clientHeight) {
return;
}

View File

@ -29,7 +29,6 @@
},
comments: Object,
projectConfig: Object,
topMargin: Number,
showReplyButtons: {
type: Boolean,
value: false,
@ -52,7 +51,7 @@
offsetParent = offsetParent.offsetParent) {
top += offsetParent.offsetTop;
}
window.scrollTo(0, top - this.topMargin);
window.scrollTo(0, top);
this._highlightEl(el);
},

View File

@ -45,34 +45,38 @@ limitations under the License.
.links {
margin-left: 1em;
}
.links ul {
.links .menuContainer {
display: none;
}
.links > li {
cursor: default;
display: inline-block;
margin-left: 1em;
padding: .4em 0;
padding: .5em 0;
position: relative;
}
.links li:hover ul {
.links li:hover .menuContainer,
.links li:active .menuContainer {
background-color: #fff;
border-radius: 3px;
box-shadow: 0 1px 1px rgba(0, 0, 0, .3);
display: block;
left: -.75em;
left: -.5em;
padding: .5em 0;
position: absolute;
top: 2em;
top: 2.45em;
z-index: 1000;
}
.links li ul li a:link,
.links li ul li a:visited {
color: #00e;
display: block;
padding: .5em .75em;
padding: .3em 1em;
text-decoration: none;
white-space: nowrap;
}
.links li ul li:hover a {
.links li ul li:hover a,
.links li ul li:active a {
background-color: var(--selection-background-color);
}
.linksTitle {
@ -87,7 +91,8 @@ limitations under the License.
height: 0;
position: absolute;
right: 0;
top: calc(50% - .1em);
top: calc(50% - .05em);
transition: border-top-color 200ms;
width: 0;
}
.links li:hover .downArrow {
@ -137,11 +142,13 @@ limitations under the License.
<span class="linksTitle">
[[linkGroup.title]] <i class="downArrow"></i>
</span>
<ul>
<template is="dom-repeat" items="[[linkGroup.links]]" as="link">
<li><a href$="[[link.url]]">[[link.name]]</a></li>
</template>
</ul>
<div class="menuContainer">
<ul>
<template is="dom-repeat" items="[[linkGroup.links]]" as="link">
<li><a href$="[[link.url]]">[[link.name]]</a></li>
</template>
</ul>
</div>
</li>
</template>
</ul>

View File

@ -23,7 +23,6 @@ limitations under the License.
id="cursorManager"
scroll="keep-visible"
cursor-target-class="target-row"
fold-offset-top="[[foldOffsetTop]]"
target="{{diffRow}}"></gr-cursor-manager>
</template>
<script src="gr-diff-cursor.js"></script>

View File

@ -54,11 +54,6 @@
},
},
foldOffsetTop: {
type: Number,
value: 0,
},
/**
* If set, the cursor will attempt to move to the line number (instead of
* the first chunk) the next time the diff renders. It is set back to null

View File

@ -22,7 +22,7 @@ limitations under the License.
<template strip-whitespace>
<style>
:host {
background-color: #fff;
background-color: #f5f5f5;
border: 1px solid #d1d2d3;
border-radius: 2px;
box-sizing: border-box;
@ -30,10 +30,10 @@ limitations under the License.
cursor: pointer;
display: inline-block;
font-family: var(--font-family);
font-size: 13px;
font-size: 12px;
font-weight: bold;
outline-width: 0;
padding: .3em .65em;
padding: .4em .85em;
position: relative;
text-align: center;
-moz-user-select: none;
@ -44,10 +44,17 @@ limitations under the License.
:host([hidden]) {
display: none;
}
:host([primary]),
:host([secondary]) {
color: #fff;
}
:host([primary]) {
background-color: #4d90fe;
border-color: #3079ed;
color: #fff;
}
:host([secondary]) {
background-color: #d14836;
border-color: transparent;
}
:host([small]) {
font-size: 12px;
@ -74,25 +81,44 @@ limitations under the License.
:host([loading][disabled]) {
cursor: wait;
}
:host(:focus),
:host(:hover) {
border-color: #666;
:host(:focus:not([primary]:not[secondary])),
:host(:hover:not([primary]:not[secondary])) {
background-color: #f8f8f8;
border-color: #aaa;
}
:host(:active) {
border-color: #d1d2d3;
color: #aaa;
}
:host([primary]:focus),
:host([secondary]:focus),
:host([primary]:active),
:host([secondary]:active) {
color: #fff;
}
:host([primary]:focus) {
border-color: #fff;
box-shadow: 0 0 1px #00f;
}
:host([primary]:hover) {
background-color: #4d90fe;
border-color: #00F;
}
:host([primary]:active),
:host([secondary]:active) {
box-shadow: none;
}
:host([primary]:active) {
border-color: #0c2188;
box-shadow: none;
color: #fff;
}
:host([secondary]:focus) {
box-shadow: 0 0 1px #f00;
}
:host([secondary]:hover) {
background-color: #c53727;
border: 1px solid #b0281a;
}
:host([secondary]:active) {
border-color: #941c0c;
}
:host([primary][loading]),
:host([primary][disabled]) {
@ -100,6 +126,7 @@ limitations under the License.
border-color: transparent;
color: #fff;
}
</style>
<content></content>
</template>

View File

@ -63,15 +63,6 @@
type: String,
value: ScrollBehavior.NEVER,
},
/**
* When using the 'keep-visible' scroll behavior, set an offset to the top
* of the window for what is considered above the upper fold.
*/
foldOffsetTop: {
type: Number,
value: 0,
},
},
detached: function() {
@ -214,7 +205,7 @@
}
if (this.scroll === ScrollBehavior.KEEP_VISIBLE &&
top > window.pageYOffset + this.foldOffsetTop &&
top > window.pageYOffset &&
top < window.pageYOffset + window.innerHeight) { return; }
// Scroll the element to the middle of the window. Dividing by a third