Better responsive layout for change metadata

Also fixes issue where table elements can’t be rendered in a dom-repeat template[1] on IE. [1] https://github.com/Polymer/polymer/issues/1567 Change-Id: I7c89c064ccd86eb49c152ddcd09529d3a51a0b1c
2016-02-23 17:28:50 -05:00
parent 406005736f
commit 09c8c24f98
7 changed files with 291 additions and 136 deletions
--- a/polygerrit-ui/app/elements/gr-account-label.html
+++ b/polygerrit-ui/app/elements/gr-account-label.html
@@ -26,7 +26,8 @@ limitations under the License.
      gr-avatar {
        height: 1.3em;
        width: 1.3em;
-        vertical-align: -.3em;
+        margin-right: .15em;
+        vertical-align: -.25em;
      }
      .text:hover {
        @apply(--gr-account-label-text-hover-style);
--- a/polygerrit-ui/app/elements/gr-account-link.html
+++ b/polygerrit-ui/app/elements/gr-account-link.html
@@ -21,7 +21,7 @@ limitations under the License.
  <template>
    <style>
      :host {
-        display: inline;
+        display: inline-block;
      }
      a {
        color: var(--default-text-color);
--- a/polygerrit-ui/app/elements/gr-change-metadata.html
+++ b/polygerrit-ui/app/elements/gr-change-metadata.html
@@ -0,0 +1,193 @@
+<!--
+Copyright (C) 2016 The Android Open Source Project
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+-->
+
+<link rel="import" href="../bower_components/polymer/polymer.html">
+<link rel="import" href="../behaviors/rest-client-behavior.html">
+<link rel="import" href="gr-account-link.html">
+<link rel="import" href="gr-date-formatter.html">
+<link rel="import" href="gr-reviewer-list.html">
+
+<script src="../scripts/fake-app.js"></script>
+
+<dom-module id="gr-change-metadata">
+  <template>
+    <style>
+      section:not(:first-of-type) {
+        margin-top: 1em;
+      }
+      .title,
+      .value {
+        display: block;
+      }
+      .title {
+        color: #666;
+        font-weight: bold;
+      }
+      .labelValueContainer:not(:first-of-type) {
+        margin-top: .25em;
+      }
+      .labelValueContainer .approved,
+      .labelValueContainer .notApproved {
+        display: inline-block;
+        padding: .1em .3em;
+        border-radius: 3px;
+      }
+      .labelValue {
+        display: inline-block;
+      }
+      .approved {
+        background-color: #d4ffd4;
+      }
+      .notApproved {
+        background-color: #ffd4d4;
+      }
+      @media screen and (max-width: 800px), screen and (min-width: 1200px) {
+        section {
+          display: flex;
+        }
+        section:not(:first-of-type) {
+          margin-top: .25em;
+        }
+        .title {
+          min-width: 9em;
+        }
+      }
+    </style>
+    <section>
+      <span class="title">Updated</span>
+      <span class="value">
+        <gr-date-formatter
+            date-str="[[change.updated]]"></gr-date-formatter>
+      </span>
+    </section>
+    <section>
+      <span class="title">Owner</span>
+      <span class="value">
+        <gr-account-link account="[[change.owner]]"></gr-account-link>
+      </span>
+    </section>
+    <section>
+      <span class="title">Reviewers</span>
+      <span class="value">
+        <gr-reviewer-list
+            change="[[change]]"
+            mutable="[[mutable]]"
+            suggest-from="[[serverConfig.suggest.from]]"></gr-reviewer-list>
+      </span>
+    </section>
+    <section>
+      <span class="title">Project</span>
+      <span class="value">[[change.project]]</span>
+    </section>
+    <section>
+      <span class="title">Branch</span>
+      <span class="value">[[change.branch]]</span>
+    </section>
+    <section>
+      <span class="title">Topic</span>
+      <span class="value">[[change.topic]]</span>
+    </section>
+    <section hidden$="[[_computeHideStrategy(_change)]]" hidden>
+      <span class="title">Strategy</span>
+      <span class="value">[[_computeStrategy(_change)]]</span>
+    </section>
+    <template is="dom-repeat"
+        items="[[_computeLabelNames(change.labels)]]" as="labelName">
+      <section>
+        <span class="title">[[labelName]]</span>
+        <span class="value">
+          <template is="dom-repeat"
+              items="[[_computeLabelValues(labelName, change.labels)]]"
+              as="label">
+            <div class="labelValueContainer">
+              <span class$="[[label.className]]">
+                <span class="labelValue">[[label.value]]</span>
+                <gr-account-link account="[[label.account]]"></gr-account-link>
+              </span>
+            </div>
+          </template>
+        </span>
+      </section>
+    </template>
+  </template>
+  <script>
+  (function() {
+    'use strict';
+
+    var SubmitTypeLabel = {
+      FAST_FORWARD_ONLY: 'Fast Forward Only',
+      MERGE_IF_NECESSARY: 'Merge if Necessary',
+      REBASE_IF_NECESSARY: 'Rebase if Necessary',
+      MERGE_ALWAYS: 'Always Merge',
+      CHERRY_PICK: 'Cherry Pick',
+    };
+
+    Polymer({
+      is: 'gr-change-metadata',
+
+      properties: {
+        change: Object,
+        mutable: Boolean,
+      },
+
+      behaviors: [
+        Gerrit.RESTClientBehavior,
+      ],
+
+      _computeHideStrategy: function(change) {
+        var open = change.status == this.ChangeStatus.NEW ||
+            change.status == this.ChangeStatus.DRAFT;
+        var current = !this._patchNum ||
+            change.revisions[change.current_revision]._number == this._patchNum;
+        return !(open && current);
+      },
+
+      _computeStrategy: function(change) {
+        return SubmitTypeLabel[change.submit_type];
+      },
+
+      _computeLabelNames: function(labels) {
+        return Object.keys(labels).sort();
+      },
+
+      _computeLabelValues: function(labelName, labels) {
+        var result = [];
+        var t = labels[labelName];
+        if (!t) { return result; }
+        var approvals = t.all || [];
+        approvals.forEach(function(label) {
+          if (label.value && label.value != labels[labelName].default_value) {
+            var labelClassName;
+            var labelValPrefix = '';
+            if (label.value > 0) {
+              labelValPrefix = '+';
+              labelClassName = 'approved';
+            } else if (label.value < 0) {
+              labelClassName = 'notApproved';
+            }
+            result.push({
+              value: labelValPrefix + label.value,
+              className: labelClassName,
+              account: label,
+            });
+          }
+        });
+        return result;
+      },
+    });
+  })();
+  </script>
+</dom-module>
--- a/polygerrit-ui/app/elements/gr-change-view.html
+++ b/polygerrit-ui/app/elements/gr-change-view.html
@@ -21,6 +21,7 @@ limitations under the License.
 <link rel="import" href="gr-ajax.html">
 <link rel="import" href="gr-button.html">
 <link rel="import" href="gr-change-actions.html">
+<link rel="import" href="gr-change-metadata.html">
 <link rel="import" href="gr-change-star.html">
 <link rel="import" href="gr-date-formatter.html">
 <link rel="import" href="gr-download-dialog.html">
@@ -30,7 +31,6 @@ limitations under the License.
 <link rel="import" href="gr-overlay.html">
 <link rel="import" href="gr-related-changes-list.html">
 <link rel="import" href="gr-reply-dialog.html">
-<link rel="import" href="gr-reviewer-list.html">

 <dom-module id="gr-change-view">
  <template>
@@ -106,32 +106,12 @@ limitations under the License.
        padding-top: 0;
      }
      .changeInfo-column:not(:last-of-type) {
-        margin-right: 2em;
+        margin-right: 1em;
+        padding-right: 1em;
      }
-      table {
-        border-collapse: collapse;
-      }
-      td {
-        padding: 2px 5px 2px 0;
-        vertical-align: top;
-      }
-      .changeMetadata-label {
-        font-weight: bold;
-      }
-      .labelValue:not(:first-of-type) {
-        margin-top: .25em;
-      }
-      .labelValue .approved,
-      .labelValue .notApproved {
-        display: inline-block;
-        padding: .1em .3em;
-        border-radius: 3px;
-      }
-      .approved {
-        background-color: #d4ffd4;
-      }
-      .notApproved {
-        background-color: #ffd4d4;
+      .changeMetadata {
+        border-right: 1px solid #ddd;
+        font-size: .9em;
      }
      gr-change-actions {
        margin-top: 1em;
@@ -139,28 +119,24 @@ limitations under the License.
      .commitMessage {
        font-family: var(--monospace-font-family);
        flex: 0 0 72ch;
+        margin-right: 2em;
+        margin-bottom: 1em;
      }
      .commitMessage h4 {
        font-family: var(--font-family);
        font-weight: bold;
        margin-bottom: .25em;
      }
+      .commitAndRelated {
+        align-content: flex-start;
+        display: flex;
+        flex: 1;
+        flex-wrap: wrap;
+      }
      gr-file-list {
        margin-bottom: 1em;
        padding: 0 var(--default-horizontal-margin);
      }
-      @media screen and (max-width: 80em) {
-        .changeInfo {
-          flex-wrap: wrap;
-        }
-        .changeInfo-column,
-        .changeInfo-column:not(:last-of-type) {
-          margin-right: 0;
-        }
-        .changeInfo-column:not(:first-of-type) {
-          margin-top: .5em;
-        }
-      }
      @media screen and (max-width: 60em) {
        .container {
          margin: .5em 0 !important;
@@ -211,12 +187,25 @@ limitations under the License.
          margin-left: 0 !important;
          margin-right: .5em;
        }
-        .changeInfo {
+        .changeInfo-column:not(:last-of-type) {
+          margin-right: 0;
+          padding-right: 0;
+        }
+        .changeInfo,
+        .commitAndRelated {
          flex-direction: column;
          flex-wrap: nowrap;
        }
+        .changeMetadata {
+          font-size: 1em;
+          border-right: none;
+          margin-bottom: 1em;
+          margin-top: .25em;
+          max-width: none;
+        }
        .commitMessage {
          flex: initial;
+          margin-right: 0;
        }
      }
    </style>
@@ -266,81 +255,28 @@ limitations under the License.
      </div>
      <section class="changeInfo">
        <div class="changeInfo-column changeMetadata">
-          <table>
-            <tr>
-              <td class="changeMetadata-label">Owner</td>
-              <td>
-                <gr-account-link account="[[_change.owner]]"></gr-account-link>
-              </td>
-            </tr>
-            <tr>
-              <td class="changeMetadata-label">Reviewers</td>
-              <td>
-                <gr-reviewer-list
-                    change="[[_change]]"
-                    mutable="[[_loggedIn]]"
-                    suggest-from="[[serverConfig.suggest.from]]"></gr-reviewer-list>
-              </td>
-            </tr>
-            <tr>
-              <td class="changeMetadata-label">Project</td>
-              <td>[[_change.project]]</td>
-            </tr>
-            <tr>
-              <td class="changeMetadata-label">Branch</td>
-              <td>[[_change.branch]]</td>
-            </tr>
-            <tr>
-              <td class="changeMetadata-label">Topic</td>
-              <td>[[_change.topic]]</td>
-            </tr>
-            <tr hidden$="[[_computeHideStrategy(_change)]]" hidden>
-              <td class="changeMetadata-label">Strategy</td>
-              <td>[[_computeStrategy(_change)]]</td>
-            </tr>
-            <tr>
-              <td class="changeMetadata-label">Updated</td>
-              <td>
-                <gr-date-formatter
-                    date-str="[[_change.updated]]"></gr-date-formatter>
-              </td>
-            </tr>
-            <template is="dom-repeat"
-                items="[[_computeLabelNames(_change.labels)]]" as="labelName">
-              <tr>
-                <td class="changeMetadata-label">[[labelName]]</td>
-                <td>
-                  <template is="dom-repeat"
-                      items="[[_computeLabelValues(labelName, _change.labels)]]"
-                      as="label">
-                    <div class="labelValue">
-                      <span class$="[[label.className]]">
-                        <span>[[label.value]]</span>
-                        <gr-account-link account="[[label.account]]"></gr-account-link>
-                      </span>
-                    </div>
-                  </template>
-                </td>
-              </tr>
-            </template>
-          </table>
+          <gr-change-metadata
+              change="[[_change]]"
+              mutable="[[_loggedIn]]"></gr-change-metadata>
          <gr-change-actions id="actions"
              actions="[[_change.actions]]"
              change-num="[[_changeNum]]"
              patch-num="[[_patchNum]]"
              on-reload-change="_handleReloadChange"></gr-change-actions>
        </div>
-        <div class="changeInfo-column commitMessage">
-          <h4>Commit message</h4>
-          <gr-linked-text pre
-              content="[[_commitInfo.message]]"
-              config="[[_projectConfig.commentlinks]]"></gr-linked-text>
-        </div>
-        <div class="changeInfo-column">
-          <gr-related-changes-list id="relatedChanges"
+        <div class="changeInfo-column commitAndRelated">
+          <div class="commitMessage">
+            <h4>Commit message</h4>
+            <gr-linked-text pre
+                content="[[_commitInfo.message]]"
+                config="[[_projectConfig.commentlinks]]"></gr-linked-text>
+          </div>
+          <div class="relatedChanges">
+            <gr-related-changes-list id="relatedChanges"
              change="[[_change]]"
              server-config="[[serverConfig]]"
              patch-num="[[_patchNum]]"></gr-related-changes-list>
+          </div>
        </div>
      </section>
      <gr-file-list id="fileList"
@@ -380,14 +316,6 @@ limitations under the License.
  (function() {
    'use strict';

-    var SubmitTypeLabel = {
-      FAST_FORWARD_ONLY: 'Fast Forward Only',
-      MERGE_IF_NECESSARY: 'Merge if Necessary',
-      REBASE_IF_NECESSARY: 'Rebase if Necessary',
-      MERGE_ALWAYS: 'Always Merge',
-      CHERRY_PICK: 'Cherry Pick',
-    };
-
    Polymer({
      is: 'gr-change-view',

@@ -648,18 +576,6 @@ limitations under the License.
        return this._allPatchSets[index] == patchNum;
      },

-      _computeHideStrategy: function(change) {
-        var open = change.status == this.ChangeStatus.NEW ||
-            change.status == this.ChangeStatus.DRAFT;
-        var current = !this._patchNum ||
-            change.revisions[change.current_revision]._number == this._patchNum;
-        return !(open && current);
-      },
-
-      _computeStrategy: function(change) {
-        return SubmitTypeLabel[change.submit_type];
-      },
-
      _computeLabelNames: function(labels) {
        return Object.keys(labels).sort();
      },
--- a/polygerrit-ui/app/test/gr-change-metadata-test.html
+++ b/polygerrit-ui/app/test/gr-change-metadata-test.html
@@ -0,0 +1,53 @@
+<!DOCTYPE html>
+<!--
+Copyright (C) 2015 The Android Open Source Project
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+-->
+
+<meta name="viewport" content="width=device-width, minimum-scale=1.0, initial-scale=1.0, user-scalable=yes">
+<title>gr-change-metadata</title>
+
+<script src="../bower_components/webcomponentsjs/webcomponents.min.js"></script>
+<script src="../bower_components/web-component-tester/browser.js"></script>
+<script src="../bower_components/page/page.js"></script>
+
+<link rel="import" href="../bower_components/iron-test-helpers/iron-test-helpers.html">
+<link rel="import" href="../elements/gr-change-metadata.html">
+<script src="../scripts/util.js"></script>
+
+<test-fixture id="basic">
+  <template>
+    <gr-change-metadata></gr-change-metadata>
+  </template>
+</test-fixture>
+
+<script>
+  suite('gr-change-metadata tests', function() {
+    var element;
+
+    setup(function() {
+      element = fixture('basic');
+    });
+
+    test('computed fields', function() {
+      assert.isFalse(element._computeHideStrategy({status: 'NEW'}));
+      assert.isFalse(element._computeHideStrategy({status: 'DRAFT'}));
+      assert.isTrue(element._computeHideStrategy({status: 'MERGED'}));
+      assert.isTrue(element._computeHideStrategy({status: 'ABANDONED'}));
+      assert.equal(element._computeStrategy({submit_type: 'CHERRY_PICK'}),
+          'Cherry Pick');
+    });
+
+  });
+</script>
--- a/polygerrit-ui/app/test/gr-change-view-test.html
+++ b/polygerrit-ui/app/test/gr-change-view-test.html
@@ -59,15 +59,6 @@ limitations under the License.
      server.restore();
    });

-    test('computed fields', function() {
-      assert.isFalse(element._computeHideStrategy({status: 'NEW'}));
-      assert.isFalse(element._computeHideStrategy({status: 'DRAFT'}));
-      assert.isTrue(element._computeHideStrategy({status: 'MERGED'}));
-      assert.isTrue(element._computeHideStrategy({status: 'ABANDONED'}));
-      assert.equal(element._computeStrategy({submit_type: 'CHERRY_PICK'}),
-          'Cherry Pick');
-    });
-
    test('keyboard shortcuts', function() {
      var showStub = sinon.stub(page, 'show');

--- a/polygerrit-ui/app/test/index.html
+++ b/polygerrit-ui/app/test/index.html
@@ -31,6 +31,7 @@ limitations under the License.
    'gr-change-actions-test.html',
    'gr-change-list-item-test.html',
    'gr-change-list-test.html',
+    'gr-change-metadata-test.html',
    'gr-change-star-test.html',
    'gr-change-view-test.html',
    'gr-confirm-dialog-test.html',