From 7a8c399a49c2f8f22bdae80f932ea4068a45ade1 Mon Sep 17 00:00:00 2001
From: Kasper Nilsson <kaspern@google.com>
Date: Wed, 13 Jun 2018 16:46:49 -0700
Subject: [PATCH] Rework whitespace usage for related changes

This change revamps the way the change view looks on screens > 50em but
< 75em wide (between 650px and 975px at default zoom).

- The change metadata is always displayed as a table
- The breakpoint at which the relation chain is moved below the commit
  message is increased from 60 to 75em
- Font sizes for the metadata and related changes are standardized to
  --font-size-small
- The border above the relation chain is always shown when the relation
  chain is not in the third column

Bug: Issue 8882
Change-Id: I173d325107d1bba5b3f6a8af6d2a09f524c33681
---
 .../gr-change-metadata.html                   | 34 +++++++------------
 .../change/gr-change-view/gr-change-view.html | 21 ++++++++----
 .../change/gr-change-view/gr-change-view.js   |  2 +-
 .../gr-change-view/gr-change-view_test.html   |  2 +-
 .../gr-related-changes-list.html              |  7 ----
 5 files changed, 29 insertions(+), 37 deletions(-)

diff --git a/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata.html b/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata.html
index 0585b2aadf..8118d775d8 100644
--- a/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata.html
+++ b/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata.html
@@ -40,20 +40,28 @@ limitations under the License.
   <template>
     <style include="gr-voting-styles"></style>
     <style include="shared-styles">
-      .hideDisplay {
-        display: none;
+      :host {
+        display: table;
+      }
+      section {
+        display: table-row;
+      }
+      section:not(:first-of-type) .title,
+      section:not(:first-of-type) .value {
+        padding-top: .5em;
       }
       section:not(:first-of-type) {
         margin-top: 1em;
       }
       .title,
       .value {
-        display: block;
+        display: table-cell;
       }
       .title {
         color: var(--deemphasized-text-color);
         font-family: var(--font-family-bold);
         max-width: 20em;
+        padding-right: .5em;
         word-break: break-word;
       }
       gr-account-link {
@@ -126,6 +134,7 @@ limitations under the License.
       .parentList gr-commit-info {
         display: inline-block;
       }
+      .hideDisplay,
       #parentNotCurrentMessage {
         display: none;
       }
@@ -133,25 +142,6 @@ limitations under the License.
         --arrow-color: #ffa62f;
         display: inline-block;
       }
-      @media screen and (max-width: 50em), screen and (min-width: 75em) {
-        :host {
-          display: table;
-        }
-        section {
-          display: table-row;
-        }
-        section:not(:first-of-type) .title,
-        section:not(:first-of-type) .value {
-          padding-top: .5em;
-        }
-        .title,
-        .value {
-          display: table-cell;
-        }
-        .title {
-          padding-right: .5em;
-        }
-      }
     </style>
     <gr-external-style id="externalStyle" name="change-metadata">
       <section>
diff --git 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
index b26d649715..64b3f2455f 100644
--- 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
@@ -116,13 +116,12 @@ limitations under the License.
         font-family: var(--font-family);
         margin-top: 1em;
       }
-      .changeInfo-column:not(:last-of-type) {
-        margin-right: 1em;
-        padding-right: 1em;
+      .changeInfo-column {
+        padding: 0 1em;
       }
       .changeMetadata {
         border-right: 1px solid var(--border-color);
-        font-size: .95rem;
+        font-size: var(--font-size-small);
         padding: 1em 0;
       }
       /* Prevent plugin text from overflowing. */
@@ -191,7 +190,7 @@ limitations under the License.
         overflow: hidden;
       }
       #relatedChanges {
-        font-size: .9rem;
+        font-size: var(--font-size-small);
       }
       #relatedChanges.collapsed {
         margin-bottom: 1.1em;
@@ -255,9 +254,19 @@ limitations under the License.
           max-width: var(--commit-message-max-width, 100ch);
         }
       }
+      #metadata {
+        padding-right: 1em;
+      }
       /* NOTE: If you update this breakpoint, also update the
       BREAKPOINT_RELATED_MED in the JS */
-      @media screen and (max-width: 60em) {
+      @media screen and (max-width: 75em) {
+        .relatedChanges {
+          padding: 0;
+        }
+        #relatedChanges {
+          border-top: 1px solid var(--border-color);
+          padding-top: 1em;
+        }
         #commitAndRelated {
           flex-direction: column;
           flex-wrap: nowrap;
diff --git 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
index d2b5ee3786..533545e143 100644
--- 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
@@ -32,7 +32,7 @@
   // These are the same as the breakpoint set in CSS. Make sure both are changed
   // together.
   const BREAKPOINT_RELATED_SMALL = '50em';
-  const BREAKPOINT_RELATED_MED = '60em';
+  const BREAKPOINT_RELATED_MED = '75em';
 
   // In the event that the related changes medium width calculation is too close
   // to zero, provide some height.
diff --git 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
index a2a2de7a11..9671711de7 100644
--- 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
@@ -1321,7 +1321,7 @@ limitations under the License.
         sandbox.stub(element, '_getOffsetHeight', () => 50);
         sandbox.stub(element, '_getLineHeight', () => 12);
         sandbox.stub(window, 'matchMedia', () => {
-          if (window.matchMedia.lastCall.args[0] === '(max-width: 60em)') {
+          if (window.matchMedia.lastCall.args[0] === '(max-width: 75em)') {
             return {matches: true};
           } else {
             return {matches: false};
diff --git a/polygerrit-ui/app/elements/change/gr-related-changes-list/gr-related-changes-list.html b/polygerrit-ui/app/elements/change/gr-related-changes-list/gr-related-changes-list.html
index e9d8ce8dfe..721e38d1a2 100644
--- a/polygerrit-ui/app/elements/change/gr-related-changes-list/gr-related-changes-list.html
+++ b/polygerrit-ui/app/elements/change/gr-related-changes-list/gr-related-changes-list.html
@@ -97,16 +97,9 @@ limitations under the License.
         .mobile {
           display: block;
         }
-        hr {
-          border: 0;
-          border-top: 1px solid var(--border-color);
-          height: 0;
-          margin-bottom: 1em;
-        }
       }
     </style>
     <div>
-      <hr class="mobile">
       <section class="relatedChanges" hidden$="[[!_relatedResponse.changes.length]]" hidden>
         <h4>Relation chain</h4>
         <template