From 489319a3a6fca74315e011ed25b6cb309ee4ebc0 Mon Sep 17 00:00:00 2001
From: Nolan Lawson <nolan@nolanlawson.com>
Date: Fri, 20 Sep 2019 23:17:57 -0700
Subject: [PATCH] fix: improve nav height on tiny screens, use variables
 (#1503)

---
 src/routes/_components/NavItem.html           |  5 +--
 src/routes/_components/NavItemIcon.html       |  8 +---
 .../compose/ComposeStickyButton.html          | 12 +----
 src/routes/_components/list/List.html         | 11 +++--
 .../_components/status/Notification.html      |  3 +-
 src/routes/_components/status/Status.html     |  8 +---
 src/routes/_utils/getMainTopMargin.js         | 10 -----
 src/scss/fonts.scss                           | 23 ++++++++++
 src/scss/global.scss                          | 33 ++------------
 src/scss/variables.scss                       | 45 +++++++++++++++++++
 10 files changed, 85 insertions(+), 73 deletions(-)
 delete mode 100644 src/routes/_utils/getMainTopMargin.js
 create mode 100644 src/scss/fonts.scss
 create mode 100644 src/scss/variables.scss

diff --git a/src/routes/_components/NavItem.html b/src/routes/_components/NavItem.html
index 39ec22af..0635c187 100644
--- a/src/routes/_components/NavItem.html
+++ b/src/routes/_components/NavItem.html
@@ -25,7 +25,7 @@
   }
 
   .nav-icon-and-label {
-    padding: 15px 20px;
+    padding: var(--nav-icon-pad-v) var(--nav-icon-pad-h);
     display: flex;
     justify-content: center;
     align-items: center;
@@ -94,9 +94,6 @@
     .main-nav-link .nav-link-label {
       display: none;
     }
-    .nav-icon-and-label {
-      padding: 20px 0;
-    }
   }
 </style>
 <script>
diff --git a/src/routes/_components/NavItemIcon.html b/src/routes/_components/NavItemIcon.html
index 565f3045..59e8526f 100644
--- a/src/routes/_components/NavItemIcon.html
+++ b/src/routes/_components/NavItemIcon.html
@@ -15,8 +15,8 @@
   }
 
   :global(.nav-link-svg-wrapper, .nav-link-svg) {
-    width: 20px;
-    height: 20px;
+    width: var(--nav-icon-width);
+    height: var(--nav-icon-height);
   }
 
   .nav-link-badge {
@@ -57,10 +57,6 @@
   }
 
   @media (max-width: 991px) {
-    :global(.nav-link-svg-wrapper, .nav-link-svg) {
-      width: 25px;
-      height: 25px;
-    }
     .nav-link-badge {
       transform: translate(15px, 7px);
       margin: 2px 1px 4px;
diff --git a/src/routes/_components/compose/ComposeStickyButton.html b/src/routes/_components/compose/ComposeStickyButton.html
index c4d19656..7c2db601 100644
--- a/src/routes/_components/compose/ComposeStickyButton.html
+++ b/src/routes/_components/compose/ComposeStickyButton.html
@@ -25,19 +25,9 @@
 
   :global(.compose-box-button-sticky, .compose-box-button-fixed) {
     z-index: 5000;
-    top: 52px; /* padding-top for .main-content plus 10px */
+    top: calc(var(--main-content-pad-top) + var(--sticky-pad-top));
   }
 
-  @media (max-width: 767px) {
-    :global(.compose-box-button-sticky, .compose-box-button-fixed) {
-      top: 57px; /* padding-top for .main-content plus 5px */
-    }
-  }
-  @media (max-width: 991px) {
-    :global(.compose-box-button-sticky, .compose-box-button-fixed) {
-      top: 62px; /* padding-top for .main-content plus 10px */
-    }
-  }
   @supports (-webkit-overflow-scrolling: touch) {
     .compose-box-button-sticky {
       /* disable sticky positioning on iOS due to
diff --git a/src/routes/_components/list/List.html b/src/routes/_components/list/List.html
index ab207f56..1e4eff47 100644
--- a/src/routes/_components/list/List.html
+++ b/src/routes/_components/list/List.html
@@ -19,7 +19,12 @@
   import ListLazyItem from './ListLazyItem.html'
   import { listStore } from './listStore'
   import { getScrollContainer } from '../../_utils/scrollContainer'
-  import { getMainTopMargin } from '../../_utils/getMainTopMargin'
+
+  function getScrollTopOffset () {
+    const style = getComputedStyle(document.documentElement)
+    return parseInt(style.getPropertyValue('--main-content-pad-top'), 10) +
+      parseInt(style.getPropertyValue('--status-pad-v'), 10)
+  }
 
   export default {
     oncreate () {
@@ -44,9 +49,9 @@
           const element = document.getElementById(`list-item-${scrollToItem}`)
           requestAnimationFrame(() => {
             console.log('scrolling element into view')
-            // TODO: this is hacky
             element.scrollIntoView(true)
-            getScrollContainer().scrollTop -= (getMainTopMargin() + 10) // 10 is the status-article padding top
+            const scrollTopOffset = getScrollTopOffset()
+            getScrollContainer().scrollTop -= scrollTopOffset
             this.fire('initialized')
           })
         } else {
diff --git a/src/routes/_components/status/Notification.html b/src/routes/_components/status/Notification.html
index c338d9a1..0df641c8 100644
--- a/src/routes/_components/status/Notification.html
+++ b/src/routes/_components/status/Notification.html
@@ -20,14 +20,13 @@
 {/if}
 <style>
   .notification-article {
+    padding: var(--status-pad-v) var(--status-pad-h);
     width: 560px;
     max-width: calc(100vw - 40px);
-    padding: 10px 20px;
     border-bottom: 1px solid var(--main-border);
   }
   @media (max-width: 767px) {
     .notification-article {
-      padding: 10px 10px;
       max-width: calc(100vw - 20px);
       width: 580px;
     }
diff --git a/src/routes/_components/status/Status.html b/src/routes/_components/status/Status.html
index dfcb6289..32bb78f1 100644
--- a/src/routes/_components/status/Status.html
+++ b/src/routes/_components/status/Status.html
@@ -49,7 +49,7 @@
 
 <style>
   .status-article {
-    padding: 10px 20px;
+    padding: var(--status-pad-v) var(--status-pad-h);
     display: grid;
     grid-template-areas:
         "header  header       header        header"
@@ -98,12 +98,6 @@
     grid-template-columns: min-content 1fr;
     grid-template-rows: repeat(7, max-content);
   }
-
-  @media (max-width: 767px) {
-    .status-article {
-      padding: 10px 10px;
-    }
-  }
 </style>
 <script>
   import StatusSidebar from './StatusSidebar.html'
diff --git a/src/routes/_utils/getMainTopMargin.js b/src/routes/_utils/getMainTopMargin.js
deleted file mode 100644
index 87bf742f..00000000
--- a/src/routes/_utils/getMainTopMargin.js
+++ /dev/null
@@ -1,10 +0,0 @@
-// should be kept in sync with global.scss
-export function getMainTopMargin () {
-  if (matchMedia('(max-width: 767px)').matches) {
-    return 62
-  } else if (matchMedia('(max-width: 991px').matches) {
-    return 52
-  } else {
-    return 42
-  }
-}
diff --git a/src/scss/fonts.scss b/src/scss/fonts.scss
new file mode 100644
index 00000000..cca0995d
--- /dev/null
+++ b/src/scss/fonts.scss
@@ -0,0 +1,23 @@
+@font-face {
+  font-family: PinaforeRegular;
+  src: local("BlinkMacSystemFont"),
+  local("Segoe UI"),
+  local("Roboto"),
+  local("Oxygen-Sans"),
+  local("Ubuntu"),
+  local("Cantarell"),
+  local("Fira Sans"),
+  local("Droid Sans"),
+  local("Helvetica Neue");
+}
+
+@font-face {
+  font-family: PinaforeEmoji;
+  src: local("Apple Color Emoji"),
+  local("Segoe UI Emoji"),
+  local("Segoe UI Symbol"),
+  local("Twemoji Mozilla"),
+  local("Noto Color Emoji"),
+  local("EmojiOne Color"),
+  local("Android Emoji");
+}
diff --git a/src/scss/global.scss b/src/scss/global.scss
index 8ccb25b6..451dd92b 100644
--- a/src/scss/global.scss
+++ b/src/scss/global.scss
@@ -1,26 +1,5 @@
-@font-face {
-  font-family: PinaforeRegular;
-  src: local("BlinkMacSystemFont"),
-    local("Segoe UI"),
-    local("Roboto"),
-    local("Oxygen-Sans"),
-    local("Ubuntu"),
-    local("Cantarell"),
-    local("Fira Sans"),
-    local("Droid Sans"),
-    local("Helvetica Neue");
-}
-
-@font-face {
-  font-family: PinaforeEmoji;
-  src: local("Apple Color Emoji"),
-    local("Segoe UI Emoji"),
-    local("Segoe UI Symbol"),
-    local("Twemoji Mozilla"),
-    local("Noto Color Emoji"),
-    local("EmojiOne Color"),
-    local("Android Emoji");
-}
+@import './fonts.scss';
+@import './variables.scss';
 
 body {
   margin: 0;
@@ -40,13 +19,7 @@ body {
     // fixes iOS Safari horizontal scrolling (see https://github.com/nolanlawson/pinafore/issues/667)
     overflow-x: hidden;
   }
-  padding-top: 42px;
-  @media (max-width: 991px) {
-    padding-top: 52px;
-  }
-  @media (max-width: 767px) {
-    padding-top: 62px;
-  }
+  padding-top: var(--main-content-pad-top);
 }
 
 main {
diff --git a/src/scss/variables.scss b/src/scss/variables.scss
new file mode 100644
index 00000000..a1f7efd7
--- /dev/null
+++ b/src/scss/variables.scss
@@ -0,0 +1,45 @@
+:root {
+
+  //
+  // Vertical and horizontal padding for the status-article element (Status.html, Notification.html)
+  //
+  --status-pad-v: 10px;
+  --status-pad-h: 20px;
+
+  @media (max-width: 767px) {
+    --status-pad-h: 10px;
+  }
+
+  //
+  // Nav and .main-content offset. This is important for things like the sticky header (ComposeStickyButton.html)
+  // as well as deciding how to scroll to the nth item in a list (List.html).
+  //
+
+  --nav-icon-pad-v: 15px;
+  --nav-icon-pad-h: 20px;
+  --nav-icon-width: 20px;
+  --nav-icon-height: 20px;
+  --main-content-pad-top: 42px;
+  --sticky-pad-top: 10px;
+
+  @media (max-width: 991px) {
+    --nav-icon-pad-v: 20px;
+    --nav-icon-pad-h: 10px;
+    --nav-icon-width: 25px;
+    --nav-icon-height: 25px;
+    --main-content-pad-top: 52px;
+  }
+
+  @media (max-width: 767px) {
+    --main-content-pad-top: 62px;
+    --sticky-pad-top: 0px;
+  }
+
+  @media (max-width: 240px) {
+    --nav-icon-pad-v: 10px;
+    --nav-icon-pad-h: 0px;
+    --main-content-pad-top: 42px;
+    --sticky-pad-top: 0px;
+  }
+
+}