Merge "Update change view header to new design"

2017-10-27 01:03:39 +00:00
parent fe2de97d76 9b03dd24f6
commit 3dad636aad
8 changed files with 151 additions and 86 deletions
--- a/polygerrit-ui/app/behaviors/rest-client-behavior/rest-client-behavior.html
+++ b/polygerrit-ui/app/behaviors/rest-client-behavior/rest-client-behavior.html
@@ -119,7 +119,7 @@ limitations under the License.
      return status === this.ChangeStatus.NEW;
    },

-    changeStatusString(change) {
+    changeStatuses(change) {
      const states = [];
      if (change.status === this.ChangeStatus.MERGED) {
        states.push('Merged');
@@ -131,7 +131,11 @@ limitations under the License.
      }
      if (change.work_in_progress) { states.push('WIP'); }
      if (change.is_private) { states.push('Private'); }
-      return states.join(', ');
+      return states;
+    },
+
+    changeStatusString(change) {
+      return this.changeStatuses(change).join(', ');
    },
  },
    Gerrit.BaseUrlBehavior,
--- a/polygerrit-ui/app/behaviors/rest-client-behavior/rest-client-behavior_test.html
+++ b/polygerrit-ui/app/behaviors/rest-client-behavior/rest-client-behavior_test.html
@@ -87,8 +87,10 @@ limitations under the License.
        labels: {},
        mergeable: true,
      };
-      const status = element.changeStatusString(change);
-      assert.equal(status, '');
+      const statuses = element.changeStatuses(change);
+      const statusString = element.changeStatusString(change);
+      assert.deepEqual(statuses, []);
+      assert.equal(statusString, '');
    });

    test('Merge conflict', () => {
@@ -102,8 +104,10 @@ limitations under the License.
        labels: {},
        mergeable: false,
      };
-      const status = element.changeStatusString(change);
-      assert.equal(status, 'Merge Conflict');
+      const statuses = element.changeStatuses(change);
+      const statusString = element.changeStatusString(change);
+      assert.deepEqual(statuses, ['Merge Conflict']);
+      assert.equal(statusString, 'Merge Conflict');
    });

    test('mergeable prop undefined', () => {
@@ -116,8 +120,10 @@ limitations under the License.
        status: 'NEW',
        labels: {},
      };
-      const status = element.changeStatusString(change);
-      assert.equal(status, '');
+      const statuses = element.changeStatuses(change);
+      const statusString = element.changeStatusString(change);
+      assert.deepEqual(statuses, []);
+      assert.equal(statusString, '');
    });

    test('Merged status', () => {
@@ -130,8 +136,10 @@ limitations under the License.
        status: 'MERGED',
        labels: {},
      };
-      const status = element.changeStatusString(change);
-      assert.equal(status, 'Merged');
+      const statuses = element.changeStatuses(change);
+      const statusString = element.changeStatusString(change);
+      assert.deepEqual(statuses, ['Merged']);
+      assert.equal(statusString, 'Merged');
    });

    test('Abandoned status', () => {
@@ -144,8 +152,10 @@ limitations under the License.
        status: 'ABANDONED',
        labels: {},
      };
-      const status = element.changeStatusString(change);
-      assert.equal(status, 'Abandoned');
+      const statuses = element.changeStatuses(change);
+      const statusString = element.changeStatusString(change);
+      assert.deepEqual(statuses, ['Abandoned']);
+      assert.equal(statusString, 'Abandoned');
    });

    test('Open status with private and wip', () => {
@@ -161,8 +171,10 @@ limitations under the License.
        labels: {},
        mergeable: true,
      };
-      const status = element.changeStatusString(change);
-      assert.equal(status, 'WIP, Private');
+      const statuses = element.changeStatuses(change);
+      const statusString = element.changeStatusString(change);
+      assert.deepEqual(statuses, ['WIP', 'Private']);
+      assert.equal(statusString, 'WIP, Private');
    });

    test('Merge conflict with private and wip', () => {
@@ -178,8 +190,10 @@ limitations under the License.
        labels: {},
        mergeable: false,
      };
-      const status = element.changeStatusString(change);
-      assert.equal(status, 'Merge Conflict, WIP, Private');
+      const statuses = element.changeStatuses(change);
+      const statusString = element.changeStatusString(change);
+      assert.deepEqual(statuses, ['Merge Conflict', 'WIP', 'Private']);
+      assert.equal(statusString, 'Merge Conflict, WIP, Private');
    });
  });
 </script>
--- 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
@@ -38,30 +38,33 @@ limitations under the License.
  <template>
    <style include="shared-styles">
      :host {
-        display: inline-block;
+        display: flex;
        font-family: var(--font-family);
      }
+      #mainContent,
      section {
-        display: inline-block;
+        display: flex;
      }
      gr-button,
      gr-dropdown {
-        margin-left: .5em;
+        /* px because don't have the same font size */
+        margin-left: 12px;
      }
      #actionLoadingMessage {
        color: #777;
      }
      @media screen and (max-width: 50em) {
-        :host,
+        #mainContent,
        section,
        gr-button,
        gr-dropdown {
          display: block;
+          flex: 1;
        }
        gr-button,
        gr-dropdown {
-          margin-bottom: .5em;
-          margin-left: 0;
+          /* px because don't have the same font size */
+          margin: 0 0 6px 0;
        }
        .confirmDialog {
          width: 90vw;
--- 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
@@ -27,6 +27,7 @@ limitations under the License.
 <link rel="import" href="../../shared/gr-account-link/gr-account-link.html">
 <link rel="import" href="../../shared/gr-button/gr-button.html">
 <link rel="import" href="../../shared/gr-change-star/gr-change-star.html">
+<link rel="import" href="../../shared/gr-change-status/gr-change-status.html">
 <link rel="import" href="../../shared/gr-date-formatter/gr-date-formatter.html">
 <link rel="import" href="../../shared/gr-editable-content/gr-editable-content.html">
 <link rel="import" href="../../shared/gr-js-api-interface/gr-js-api-interface.html">
@@ -55,9 +56,10 @@ limitations under the License.
      }
      .header {
        align-items: center;
-        background-color: var(--view-background-color);
+        background-color: #fafafa;
+        border-bottom: 1px solid #ddd;
        display: flex;
-        padding: .65em var(--default-horizontal-margin);
+        padding: .55em var(--default-horizontal-margin);
        z-index: 99;  /* Less than gr-overlay's backdrop */
      }
      .header .download {
@@ -71,12 +73,13 @@ limitations under the License.
        transition: box-shadow 250ms linear;
        width: 100%;
      }
-      .header.wip {
-        background-color: #fcfad6;
-        border-bottom: 1px solid #ddd;
-        margin-bottom: .5em;
+      gr-change-status {
+        display: initial;
+        margin: .1em .5em .1em 0;
      }
      .header-title {
+        align-items: center;
+        display: flex;
        flex: 1;
        font-size: 1.2em;
        font-family: var(--font-family-bold);
@@ -95,7 +98,7 @@ limitations under the License.
         https://github.com/Polymer/polymer/issues/2531 */
      .container section.changeInfo {
        display: flex;
-        padding: 0 var(--default-horizontal-margin);
+        padding: 1em var(--default-horizontal-margin);
      }
      .changeId {
        color: #666;
@@ -128,15 +131,13 @@ limitations under the License.
      .editCommitMessage {
        margin-top: 1em;
      }
+      .changeStatuses,
      .commitActions {
-        border-bottom: 1px solid #ddd;
+        align-items: center;
        display: flex;
-        justify-content: space-between;
-        margin-bottom: .5em;
-        padding-bottom: .5em;
      }
-      .reply {
-        margin-right: .5em;
+      .changeStatuses {
+        flex-wrap: wrap;
      }
      .mainChangeInfo {
        display: flex;
@@ -205,6 +206,9 @@ limitations under the License.
      .scrollable {
        overflow: auto;
      }
+      .text {
+        white-space: pre;
+      }
      @media screen and (min-width: 80em) {
        .commitMessage {
          max-width: var(--commit-message-max-width, 100ch);
@@ -221,9 +225,6 @@ limitations under the License.
          min-width: 0;
        }
      }
-      .patchInfo {
-        margin-top: 1em;
-      }
      /* NOTE: If you update this breakpoint, also update the
      BREAKPOINT_RELATED_SMALL in the JS */
      @media screen and (max-width: 50em) {
@@ -233,12 +234,14 @@ limitations under the License.
        .header {
          align-items: flex-start;
          flex-direction: column;
+          flex: 1;
          padding: .5em var(--default-horizontal-margin);
        }
        gr-change-star {
          vertical-align: middle;
        }
        .header-title {
+          flex-wrap: wrap;
          font-size: 1.1em;
        }
        gr-reply-dialog {
@@ -251,7 +254,8 @@ limitations under the License.
        .reply {
          display: block;
          margin-right: 0;
-          margin-bottom: .5em;
+          /* px because don't have the same font size */
+          margin-bottom: 6px;
        }
        .changeInfo-column:not(:last-of-type) {
          margin-right: 0;
@@ -273,7 +277,9 @@ limitations under the License.
          max-width: none;
        }
        .commitActions {
-          flex-direction: column;
+          display: block;
+          margin-top: 1em;
+          width: 100%;
        }
        .commitMessage {
          flex: initial;
@@ -292,32 +298,53 @@ limitations under the License.
        class="container"
        hidden$="{{_loading}}">
      <div class$="hideOnMobileOverlay [[_computeHeaderClass(_change)]]">
-        <span class="header-title">
+        <div class="header-title">
          <gr-change-star
              id="changeStar"
              change="{{_change}}" hidden$="[[!_loggedIn]]"></gr-change-star>
-          <a
-              aria-label$="[[_computeChangePermalinkAriaLabel(_change._number)]]"
-              href$="[[_computeChangeUrl(_change)]]">[[_change._number]]</a><!--
-       --><template is="dom-if" if="[[_changeStatus]]"><!--
-         --> (<!--
-         --><span
-                aria-label$="Change status: [[_changeStatus]]"
-                tabindex="0">[[_changeStatus]]</span><!--
-         --><template
+          <div class="changeStatuses">
+            <template is="dom-repeat" items="[[_changeStatuses]]" as="status">
+              <gr-change-status
+                  max-width="100"
+                  status="[[status]]"></gr-change-status>
+            </template>
+          </div>
+          <div class="changeText">
+            <span>[[_change._number]]</span>
+            <template
                is="dom-if"
                if="[[_computeShowCommitInfo(_changeStatus, _change.current_revision)]]">
-              as
-              <gr-commit-info
-                  change="[[_change]]"
-                  commit-info="[[_computeMergedCommitInfo(_change.current_revision, _change.revisions)]]"
-                  server-config="[[_serverConfig]]"></gr-commit-info><!--
-         --></template><!--
-         -->)<!--
-       --></template><!--
-       -->: [[_change.subject]]
-        </span>
-      </div>
+              <span class="text"> ([[_changeStatus]] as </span>
+                <gr-commit-info
+                    change="[[_change]]"
+                    commit-info="[[_computeMergedCommitInfo(_change.current_revision, _change.revisions)]]"
+                    server-config="[[_serverConfig]]"></gr-commit-info>)
+            </template>
+            <span class="text">: </span><span>[[_change.subject]]</span>
+          </div>
+        </div><!-- end header-title -->
+        <div class="commitActions" hidden$="[[!_loggedIn]]">
+          <gr-button
+              class="reply"
+              secondary
+              disabled="[[_replyDisabled]]"
+              on-tap="_handleReplyTap">[[_replyButtonLabel]]</gr-button>
+          <gr-change-actions id="actions"
+              change="[[_change]]"
+              has-parent="[[hasParent]]"
+              actions="[[_change.actions]]"
+              revision-actions="[[_currentRevisionActions]]"
+              change-num="[[_changeNum]]"
+              change-status="[[_change.status]]"
+              commit-num="[[_commitInfo.commit]]"
+              patch-num="[[computeLatestPatchNum(_allPatchSets)]]"
+              commit-message="[[_latestCommitMessage]]"
+              edit-loaded="[[_editLoaded]]"
+              edit-based-on-current-patch-set="[[hasEditBasedOnCurrentPatchSet(_allPatchSets)]]"
+              on-reload-change="_handleReloadChange"
+              on-download-tap="_handleOpenDownloadDialog"></gr-change-actions>
+        </div><!-- end commit actions -->
+      </div><!-- end header -->
      <section class="changeInfo">
        <div class="changeInfo-column changeMetadata hideOnMobileOverlay">
          <gr-change-metadata
@@ -333,27 +360,6 @@ limitations under the License.
          <div id="change_plugins"></div>
        </div>
        <div id="mainChangeInfo" class="changeInfo-column mainChangeInfo">
-          <div class="commitActions" hidden$="[[!_loggedIn]]">
-            <gr-button
-                class="reply"
-                secondary
-                disabled="[[_replyDisabled]]"
-                on-tap="_handleReplyTap">[[_replyButtonLabel]]</gr-button>
-            <gr-change-actions id="actions"
-                change="[[_change]]"
-                has-parent="[[hasParent]]"
-                actions="[[_change.actions]]"
-                revision-actions="[[_currentRevisionActions]]"
-                change-num="[[_changeNum]]"
-                change-status="[[_change.status]]"
-                commit-num="[[_commitInfo.commit]]"
-                patch-num="[[computeLatestPatchNum(_allPatchSets)]]"
-                commit-message="[[_latestCommitMessage]]"
-                edit-loaded="[[_editLoaded]]"
-                edit-based-on-current-patch-set="[[hasEditBasedOnCurrentPatchSet(_allPatchSets)]]"
-                on-reload-change="_handleReloadChange"
-                on-download-tap="_handleOpenDownloadDialog"></gr-change-actions>
-          </div>
          <hr class="mobile">
          <div id="commitAndRelated" class="hideOnMobileOverlay">
            <div class="commitContainer">
--- 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
@@ -187,6 +187,10 @@
        type: String,
        computed: 'changeStatusString(_change)',
      },
+      _changeStatuses: {
+        type: String,
+        computed: '_computeChangeStatusChips(_change)',
+      },
      _commitCollapsed: {
        type: Boolean,
        value: true,
@@ -328,6 +332,10 @@
      this._editingCommitMessage = false;
    },

+    _computeChangeStatusChips(change) {
+      return this.changeStatuses(change);
+    },
+
    _computeHideEditCommitMessage(loggedIn, editing, change) {
      if (!loggedIn || editing || change.status === this.ChangeStatus.MERGED) {
        return true;
--- 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
@@ -148,7 +148,7 @@ limitations under the License.
        element.$.replyDialog.fire('fullscreen-overlay-opened');
        assert.isTrue(element._handleHideBackgroundContent.called);
        assert.isTrue(element.$.mainContent.classList.contains('overlayOpen'));
-        assert.equal(getComputedStyle(element.$.actions).display, 'block');
+        assert.equal(getComputedStyle(element.$.actions).display, 'flex');
      });

      test('fullscreen-overlay-closed shows content', () => {
@@ -259,6 +259,37 @@ limitations under the License.
      });
    });

+    test('_changeStatuses', () => {
+      sandbox.stub(element, 'changeStatuses').returns(
+          ['Merged', 'WIP']);
+      element._loading = false;
+      element._change = {
+        change_id: 'Iad9dc96274af6946f3632be53b106ef80f7ba6ca',
+        revisions: {
+          rev2: {_number: 2},
+          rev1: {_number: 1},
+          rev13: {_number: 13},
+          rev3: {_number: 3},
+        },
+        current_revision: 'rev3',
+        labels: {
+          test: {
+            all: [],
+            default_value: 0,
+            values: [],
+            approved: {},
+          },
+        },
+      };
+      expectedStatuses = ['Merged', 'WIP'];
+      assert.deepEqual(element._changeStatuses, expectedStatuses);
+      assert.equal(element._changeStatus, expectedStatuses.join(', '));
+      flushAsynchronousOperations();
+      const statusChips = Polymer.dom(element.root)
+          .querySelectorAll('gr-change-status');
+      assert.equal(statusChips.length, 2);
+    });
+
    test('diff preferences open when open-diff-prefs is fired', () => {
      const overlayOpenStub = sandbox.stub(element.$.fileList,
          'openDiffPrefs');
--- a/polygerrit-ui/app/elements/shared/gr-change-star/gr-change-star.html
+++ b/polygerrit-ui/app/elements/shared/gr-change-star/gr-change-star.html
@@ -22,8 +22,7 @@ limitations under the License.
  <template>
    <style include="shared-styles">
      :host {
-        display: inline-block;
-        overflow: hidden;
+        display: flex;
      }
      .starButton {
        background-color: transparent;
--- a/polygerrit-ui/app/elements/shared/gr-change-status/gr-change-status.js
+++ b/polygerrit-ui/app/elements/shared/gr-change-status/gr-change-status.js
@@ -57,7 +57,7 @@
    },

    _toClassName(str) {
-      return str.toLowerCase().replace(' ', '-');
+      return str.toLowerCase().replace(/\s/g, '-');
    },

    _updateChipDetails(status) {