Move gr-change-action buttons to be above commit message

Additionally: + Introduces a new, red secondary button style. Applied to the ‘Reply’ button. It is always highlighted. + Default button styles are gray and not just white. Change-Id: Idf12bcefac873fea65dc569405334d28ff667092
2016-08-17 14:55:31 -07:00
parent ed82bbcae8
commit ea82d1c0fb
5 changed files with 114 additions and 102 deletions
--- a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.html
+++ b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.html
@@ -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]]"
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.html
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.html
@@ -49,9 +49,12 @@ limitations under the License.
        align-items: center;
        background-color: var(--view-background-color);
        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);
@@ -64,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;
      }
@@ -106,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;
@@ -119,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;
@@ -155,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;
@@ -168,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;
@@ -199,6 +185,9 @@ limitations under the License.
          margin-top: .25em;
          max-width: none;
        }
+        .commitActions {
+          flex-direction: column;
+        }
        .commitMessage {
          flex: initial;
          margin-right: 0;
@@ -215,11 +204,6 @@ limitations under the License.
          <span class="changeStatus">[[_computeChangeStatus(_change, _patchRange.patchNum)]]</span>
        </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>
@@ -235,7 +219,6 @@ limitations under the License.
          </span>
        </span>
      </div>
-
      <section class="changeInfo">
        <div class="changeInfo-column changeMetadata">
          <gr-change-metadata
@@ -245,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>
--- 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
@@ -447,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) {
--- 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
@@ -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: {}});
      });

--- a/polygerrit-ui/app/elements/shared/gr-button/gr-button.html
+++ b/polygerrit-ui/app/elements/shared/gr-button/gr-button.html
@@ -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>