From c117502f22f7dc772454ec99ba44613f906eaa50 Mon Sep 17 00:00:00 2001
From: Lauri Eskola <lauri.eskola@acquia.com>
Date: Fri, 5 Feb 2021 16:54:58 +0200
Subject: [PATCH] Issue #3173014 by mherchel, morganlyndel, paulocs, raman.b,
 adamzimmermann, komalk, proeung, lauriii, alexpott, catch, volkswagenchick,
 kostyashupenko: Address code feedback for Olivero's header.pcss.css partials

---
 .../components/navigation/nav-secondary.css   | 58 ++++++-------------
 .../navigation/nav-secondary.pcss.css         | 18 +-----
 ...av-button-wide.css => wide-nav-expand.css} | 42 +++++++-------
 ...wide.pcss.css => wide-nav-expand.pcss.css} | 14 ++---
 .../{header.css => site-header.css}           |  6 +-
 .../{header.pcss.css => site-header.pcss.css} |  5 +-
 .../css/layout/region-secondary-menu.css      | 33 +++++++++++
 .../css/layout/region-secondary-menu.pcss.css | 30 ++++++++++
 core/themes/olivero/js/scripts.es6.js         |  2 +-
 core/themes/olivero/js/scripts.js             |  2 +-
 core/themes/olivero/olivero.libraries.yml     |  5 +-
 .../olivero/templates/layout/page.html.twig   | 16 ++---
 .../layout/region--secondary-menu.html.twig   | 13 ++++-
 13 files changed, 137 insertions(+), 107 deletions(-)
 rename core/themes/olivero/css/components/navigation/{nav-button-wide.css => wide-nav-expand.css} (62%)
 rename core/themes/olivero/css/components/navigation/{nav-button-wide.pcss.css => wide-nav-expand.pcss.css} (86%)
 rename core/themes/olivero/css/components/{header.css => site-header.css} (98%)
 rename core/themes/olivero/css/components/{header.pcss.css => site-header.pcss.css} (98%)
 create mode 100644 core/themes/olivero/css/layout/region-secondary-menu.css
 create mode 100644 core/themes/olivero/css/layout/region-secondary-menu.pcss.css

diff --git a/core/themes/olivero/css/components/navigation/nav-secondary.css b/core/themes/olivero/css/components/navigation/nav-secondary.css
index fdb9339e6c35..7eb3e8453bc7 100644
--- a/core/themes/olivero/css/components/navigation/nav-secondary.css
+++ b/core/themes/olivero/css/components/navigation/nav-secondary.css
@@ -7,15 +7,9 @@
 
 /**
  * @file
- * Nav Secondary.
+ * Secondary navigation styling.
  */
 
-.secondary-nav__wrapper {
-  display: flex;
-  margin-top: 2.25rem;
-  margin-bottom: 2.25rem;
-}
-
 .secondary-nav {
   letter-spacing: 0.02em;
   font-size: 0.875rem;
@@ -23,35 +17,35 @@
 }
 
 [dir="ltr"] .secondary-nav ul.menu {
-    margin-left: 0;
+    margin-left: 0
 }
 
 [dir="rtl"] .secondary-nav ul.menu {
-    margin-right: 0;
+    margin-right: 0
 }
 
 [dir="ltr"] .secondary-nav ul.menu {
-    margin-right: 0;
+    margin-right: 0
 }
 
 [dir="rtl"] .secondary-nav ul.menu {
-    margin-left: 0;
+    margin-left: 0
 }
 
 [dir="ltr"] .secondary-nav ul.menu {
-    padding-left: 0;
+    padding-left: 0
 }
 
 [dir="rtl"] .secondary-nav ul.menu {
-    padding-right: 0;
+    padding-right: 0
 }
 
 [dir="ltr"] .secondary-nav ul.menu {
-    padding-right: 0;
+    padding-right: 0
 }
 
 [dir="rtl"] .secondary-nav ul.menu {
-    padding-left: 0;
+    padding-left: 0
 }
 
 .secondary-nav ul.menu {
@@ -77,11 +71,11 @@
     }
 
 [dir="ltr"] .secondary-nav ul.menu li:not(:last-child) {
-        margin-right: 1.6875rem;
+        margin-right: 1.6875rem
 }
 
 [dir="rtl"] .secondary-nav ul.menu li:not(:last-child) {
-        margin-left: 1.6875rem;
+        margin-left: 1.6875rem
 }
 
 .secondary-nav ul.menu a:not(.button--primary) {
@@ -115,43 +109,29 @@
         }
 
 @media (min-width: 75rem) {
-    body:not(.is-always-mobile-nav) .secondary-nav__wrapper {
-      justify-content: flex-end;
-      margin: 0
-
-      /* If the secondary nav is the first item within the header, it does not need left separator. */
-    }
-      body:not(.is-always-mobile-nav) .secondary-nav__wrapper:first-child .secondary-nav:before {
-        content: none;
-      }
-
     [dir="ltr"] body:not(.is-always-mobile-nav) .secondary-nav {
-      margin-left: 1.125rem;
+      margin-left: 1.125rem
   }
-
     [dir="rtl"] body:not(.is-always-mobile-nav) .secondary-nav {
-      margin-right: 1.125rem;
+      margin-right: 1.125rem
   }
-
     [dir="ltr"] body:not(.is-always-mobile-nav) .secondary-nav {
-      padding-left: 2.25rem;
+      padding-left: 2.25rem
   }
-
     [dir="rtl"] body:not(.is-always-mobile-nav) .secondary-nav {
-      padding-right: 2.25rem;
+      padding-right: 2.25rem
   }
-
     body:not(.is-always-mobile-nav) .secondary-nav {
       position: relative;
       display: flex
     }
 
       [dir="ltr"] body:not(.is-always-mobile-nav) .secondary-nav:before {
-        left: 0;
+        left: 0
   }
 
       [dir="rtl"] body:not(.is-always-mobile-nav) .secondary-nav:before {
-        right: 0;
+        right: 0
   }
 
       body:not(.is-always-mobile-nav) .secondary-nav:before {
@@ -165,10 +145,10 @@
       }
 
       [dir="ltr"] body:not(.is-always-mobile-nav) .secondary-nav ul.menu li:not(:last-child) {
-        margin-right: 2.25rem;
+        margin-right: 2.25rem
   }
 
       [dir="rtl"] body:not(.is-always-mobile-nav) .secondary-nav ul.menu li:not(:last-child) {
-        margin-left: 2.25rem;
+        margin-left: 2.25rem
   }
   }
diff --git a/core/themes/olivero/css/components/navigation/nav-secondary.pcss.css b/core/themes/olivero/css/components/navigation/nav-secondary.pcss.css
index 91a3e15503bf..faf29f312d8e 100644
--- a/core/themes/olivero/css/components/navigation/nav-secondary.pcss.css
+++ b/core/themes/olivero/css/components/navigation/nav-secondary.pcss.css
@@ -1,16 +1,10 @@
 /**
  * @file
- * Nav Secondary.
+ * Secondary navigation styling.
  */
 
 @import "../../base/variables.pcss.css";
 
-.secondary-nav__wrapper {
-  display: flex;
-  margin-block-start: var(--sp2);
-  margin-block-end: var(--sp2);
-}
-
 .secondary-nav {
   letter-spacing: 0.02em;
   font-size: var(--font-size-s);
@@ -79,16 +73,6 @@
 
 body:not(.is-always-mobile-nav) {
   @media (--nav) {
-    & .secondary-nav__wrapper {
-      justify-content: flex-end;
-      margin: 0;
-
-      /* If the secondary nav is the first item within the header, it does not need left separator. */
-      &:first-child .secondary-nav:before {
-        content: none;
-      }
-    }
-
     & .secondary-nav {
       position: relative;
       display: flex;
diff --git a/core/themes/olivero/css/components/navigation/nav-button-wide.css b/core/themes/olivero/css/components/navigation/wide-nav-expand.css
similarity index 62%
rename from core/themes/olivero/css/components/navigation/nav-button-wide.css
rename to core/themes/olivero/css/components/navigation/wide-nav-expand.css
index 435fb853a782..518bf6827d29 100644
--- a/core/themes/olivero/css/components/navigation/nav-button-wide.css
+++ b/core/themes/olivero/css/components/navigation/wide-nav-expand.css
@@ -7,16 +7,16 @@
 
 /**
  * @file
- * Nav Button Wide.
+ * Button which expands the navigation at wide viewport widths.
  */
 
-.nav-primary__button {
+.wide-nav-expand {
   display: none
 }
 
 @media (min-width: 75rem) {
 
-.nav-primary__button {
+.wide-nav-expand {
     display: flex;
     visibility: hidden;
     flex-shrink: 0;
@@ -31,26 +31,26 @@
     background-color: #2494db
 }
 
-    .nav-primary__button:focus {
+    .wide-nav-expand:focus {
       border: solid 1px transparent; /* Will show in IE/Edge high contrast mode. */
     }
   }
 
 @media (min-width: 75rem) {
 
-body:not(.is-always-mobile-nav) .js-fixed .nav-primary__button {
+body:not(.is-always-mobile-nav) .js-fixed .wide-nav-expand {
     visibility: visible
 }
   }
 
 @media (min-width: 75rem) {
 
-body.is-always-mobile-nav .nav-primary__button {
+body.is-always-mobile-nav .wide-nav-expand {
     visibility: hidden
 }
   }
 
-.nav-primary__icon {
+.wide-nav-expand__icon {
   position: relative;
   width: 2.25rem;
   height: 1.3125rem;
@@ -60,22 +60,22 @@ body.is-always-mobile-nav .nav-primary__button {
   transform-style: preserve-3d
 }
 
-.nav-primary__icon > span {
+.wide-nav-expand__icon > span {
     display: block;
     height: 0;
     /* Intentionally not using CSS logical properties. */
     border-top: solid 3px #fff
   }
 
-[dir="ltr"] .nav-primary__icon > span:nth-child(1) {
+[dir="ltr"] .wide-nav-expand__icon > span:nth-child(1) {
       left: 0
 }
 
-[dir="rtl"] .nav-primary__icon > span:nth-child(1) {
+[dir="rtl"] .wide-nav-expand__icon > span:nth-child(1) {
       right: 0
 }
 
-.nav-primary__icon > span:nth-child(1) {
+.wide-nav-expand__icon > span:nth-child(1) {
       position: absolute;
       top: 0;
       width: 100%;
@@ -84,15 +84,15 @@ body.is-always-mobile-nav .nav-primary__button {
       background-color: #fff;
     }
 
-[dir="ltr"] .nav-primary__icon > span:nth-child(2) {
+[dir="ltr"] .wide-nav-expand__icon > span:nth-child(2) {
       left: 0
 }
 
-[dir="rtl"] .nav-primary__icon > span:nth-child(2) {
+[dir="rtl"] .wide-nav-expand__icon > span:nth-child(2) {
       right: 0
 }
 
-.nav-primary__icon > span:nth-child(2) {
+.wide-nav-expand__icon > span:nth-child(2) {
       position: absolute;
       top: 0.5625rem;
       width: 100%;
@@ -101,15 +101,15 @@ body.is-always-mobile-nav .nav-primary__button {
       background-color: #fff;
     }
 
-[dir="ltr"] .nav-primary__icon > span:nth-child(3) {
+[dir="ltr"] .wide-nav-expand__icon > span:nth-child(3) {
       left: 0
 }
 
-[dir="rtl"] .nav-primary__icon > span:nth-child(3) {
+[dir="rtl"] .wide-nav-expand__icon > span:nth-child(3) {
       right: 0
 }
 
-.nav-primary__icon > span:nth-child(3) {
+.wide-nav-expand__icon > span:nth-child(3) {
       position: absolute;
       top: auto;
       bottom: 0;
@@ -119,20 +119,20 @@ body.is-always-mobile-nav .nav-primary__button {
       background-color: #fff;
     }
 
-.js-fixed .nav-primary__icon {
+.js-fixed .wide-nav-expand__icon {
   opacity: 1;
 }
 
-[aria-expanded="true"] .nav-primary__icon > span:nth-child(1) {
+[aria-expanded="true"] .wide-nav-expand__icon > span:nth-child(1) {
     top: 0.5625rem;
     transform: rotate(-45deg);
   }
 
-[aria-expanded="true"] .nav-primary__icon > span:nth-child(2) {
+[aria-expanded="true"] .wide-nav-expand__icon > span:nth-child(2) {
     opacity: 0;
   }
 
-[aria-expanded="true"] .nav-primary__icon > span:nth-child(3) {
+[aria-expanded="true"] .wide-nav-expand__icon > span:nth-child(3) {
     top: 0.5625rem;
     transform: rotate(45deg);
   }
diff --git a/core/themes/olivero/css/components/navigation/nav-button-wide.pcss.css b/core/themes/olivero/css/components/navigation/wide-nav-expand.pcss.css
similarity index 86%
rename from core/themes/olivero/css/components/navigation/nav-button-wide.pcss.css
rename to core/themes/olivero/css/components/navigation/wide-nav-expand.pcss.css
index f4abc3124aa5..5c7d9b8806d4 100644
--- a/core/themes/olivero/css/components/navigation/nav-button-wide.pcss.css
+++ b/core/themes/olivero/css/components/navigation/wide-nav-expand.pcss.css
@@ -1,11 +1,11 @@
 /**
  * @file
- * Nav Button Wide.
+ * Button which expands the navigation at wide viewport widths.
  */
 
 @import "../../base/variables.pcss.css";
 
-.nav-primary__button {
+.wide-nav-expand {
   display: none;
 
   @media (--nav) {
@@ -28,19 +28,19 @@
   }
 }
 
-body:not(.is-always-mobile-nav) .js-fixed .nav-primary__button {
+body:not(.is-always-mobile-nav) .js-fixed .wide-nav-expand {
   @media (--nav) {
     visibility: visible;
   }
 }
 
-body.is-always-mobile-nav .nav-primary__button {
+body.is-always-mobile-nav .wide-nav-expand {
   @media (--nav) {
     visibility: hidden;
   }
 }
 
-.nav-primary__icon {
+.wide-nav-expand__icon {
   position: relative;
   width: var(--sp2);
   height: 21px;
@@ -87,11 +87,11 @@ body.is-always-mobile-nav .nav-primary__button {
   }
 }
 
-.js-fixed .nav-primary__icon {
+.js-fixed .wide-nav-expand__icon {
   opacity: 1;
 }
 
-[aria-expanded="true"] .nav-primary__icon {
+[aria-expanded="true"] .wide-nav-expand__icon {
   & > span:nth-child(1) {
     inset-block-start: 9px;
     transform: rotate(-45deg);
diff --git a/core/themes/olivero/css/components/header.css b/core/themes/olivero/css/components/site-header.css
similarity index 98%
rename from core/themes/olivero/css/components/header.css
rename to core/themes/olivero/css/components/site-header.css
index 4ca8e6538c53..b900e7a9bf6c 100644
--- a/core/themes/olivero/css/components/header.css
+++ b/core/themes/olivero/css/components/site-header.css
@@ -7,11 +7,9 @@
 
 /**
  * @file
- * Main Header.
+ * Site header.
  */
 
-/* Header */
-
 .site-header {
   position: relative
 }
@@ -25,7 +23,7 @@
 }
   }
 
-.header__left {
+.site-header__initial {
   position: relative;
   z-index: 102;
   display: flex;
diff --git a/core/themes/olivero/css/components/header.pcss.css b/core/themes/olivero/css/components/site-header.pcss.css
similarity index 98%
rename from core/themes/olivero/css/components/header.pcss.css
rename to core/themes/olivero/css/components/site-header.pcss.css
index d1f1606677da..adc7b381aa5d 100644
--- a/core/themes/olivero/css/components/header.pcss.css
+++ b/core/themes/olivero/css/components/site-header.pcss.css
@@ -1,11 +1,10 @@
 /**
  * @file
- * Main Header.
+ * Site header.
  */
 
 @import "../base/variables.pcss.css";
 
-/* Header */
 .site-header {
   position: relative;
 
@@ -16,7 +15,7 @@
   }
 }
 
-.header__left {
+.site-header__initial {
   position: relative;
   z-index: 102;
   display: flex;
diff --git a/core/themes/olivero/css/layout/region-secondary-menu.css b/core/themes/olivero/css/layout/region-secondary-menu.css
new file mode 100644
index 000000000000..e187d2b19825
--- /dev/null
+++ b/core/themes/olivero/css/layout/region-secondary-menu.css
@@ -0,0 +1,33 @@
+/*
+ * DO NOT EDIT THIS FILE.
+ * See the following change record for more information,
+ * https://www.drupal.org/node/3084859
+ * @preserve
+ */
+
+/**
+ * @file
+ * Secondary menu region layout styling.
+ */
+
+.region--secondary-menu {
+  display: flex;
+  margin-top: 2.25rem;
+  margin-bottom: 2.25rem
+}
+
+.region--secondary-menu > * {
+    margin-bottom: 0;
+  }
+
+@media (min-width: 75rem) {
+    body:not(.is-always-mobile-nav) .region--secondary-menu {
+      justify-content: flex-end;
+      margin: 0
+
+      /* If the secondary nav is the first item within the header, it does not need left separator. */
+    }
+      body:not(.is-always-mobile-nav) .region--secondary-menu:first-child .secondary-nav:before {
+        content: none;
+      }
+  }
diff --git a/core/themes/olivero/css/layout/region-secondary-menu.pcss.css b/core/themes/olivero/css/layout/region-secondary-menu.pcss.css
new file mode 100644
index 000000000000..d1d88e0f1561
--- /dev/null
+++ b/core/themes/olivero/css/layout/region-secondary-menu.pcss.css
@@ -0,0 +1,30 @@
+/**
+ * @file
+ * Secondary menu region layout styling.
+ */
+
+@import "../base/variables.pcss.css";
+
+.region--secondary-menu {
+  display: flex;
+  margin-block-start: var(--sp2);
+  margin-block-end: var(--sp2);
+
+  & > * {
+    margin-block-end: 0;
+  }
+}
+
+body:not(.is-always-mobile-nav) {
+  @media (--nav) {
+    & .region--secondary-menu {
+      justify-content: flex-end;
+      margin: 0;
+
+      /* If the secondary nav is the first item within the header, it does not need left separator. */
+      &:first-child .secondary-nav:before {
+        content: none;
+      }
+    }
+  }
+}
diff --git a/core/themes/olivero/js/scripts.es6.js b/core/themes/olivero/js/scripts.es6.js
index 1b59cb699a0e..a485d814a609 100644
--- a/core/themes/olivero/js/scripts.es6.js
+++ b/core/themes/olivero/js/scripts.es6.js
@@ -16,7 +16,7 @@
 
   Drupal.olivero.isDesktopNav = isDesktopNav;
 
-  const wideNavButton = document.querySelector('.nav-primary__button');
+  const wideNavButton = document.querySelector('.wide-nav-expand');
   const siteHeaderFixable = document.querySelector('.site-header__fixable');
 
   function wideNavIsOpen() {
diff --git a/core/themes/olivero/js/scripts.js b/core/themes/olivero/js/scripts.js
index 0c2237448b2c..7292acf12a11 100644
--- a/core/themes/olivero/js/scripts.js
+++ b/core/themes/olivero/js/scripts.js
@@ -14,7 +14,7 @@
   }
 
   Drupal.olivero.isDesktopNav = isDesktopNav;
-  var wideNavButton = document.querySelector('.nav-primary__button');
+  var wideNavButton = document.querySelector('.wide-nav-expand');
   var siteHeaderFixable = document.querySelector('.site-header__fixable');
 
   function wideNavIsOpen() {
diff --git a/core/themes/olivero/olivero.libraries.yml b/core/themes/olivero/olivero.libraries.yml
index 6eebb2750bec..679f9e2caf70 100644
--- a/core/themes/olivero/olivero.libraries.yml
+++ b/core/themes/olivero/olivero.libraries.yml
@@ -15,6 +15,7 @@ global-styling:
       css/layout/region.css: {}
       css/layout/region-content.css: {}
       css/layout/region-hero.css: {}
+      css/layout/region-secondary-menu.css: {}
       css/layout/social-bar.css: {}
       css/layout/views.css: {}
     component:
@@ -32,7 +33,6 @@ global-styling:
       css/components/form-text.css: {}
       css/components/form-textarea.css: {}
       css/components/form-select.css: {}
-      css/components/header.css: {}
       css/components/header-buttons-mobile.css: {}
       css/components/header-navigation.css: {}
       css/components/header-site-branding.css: {}
@@ -40,7 +40,7 @@ global-styling:
       css/components/links.css: {}
       css/components/messages.css: {}
       css/components/navigation/nav-button-mobile.css: {}
-      css/components/navigation/nav-button-wide.css: {}
+      css/components/navigation/wide-nav-expand.css: {}
       css/components/navigation/nav-primary-button.css: {}
       css/components/navigation/nav-primary.css: {}
       css/components/navigation/nav-primary-wide.css: {}
@@ -48,6 +48,7 @@ global-styling:
       css/components/node.css: {}
       css/components/node-teaser.css: {}
       css/components/page-title.css: {}
+      css/components/site-header.css: {}
       css/components/skip-link.css: {}
       css/components/pager.css: {}
       css/components/table.css: {}
diff --git a/core/themes/olivero/templates/layout/page.html.twig b/core/themes/olivero/templates/layout/page.html.twig
index f850022749c3..00c399a69355 100644
--- a/core/themes/olivero/templates/layout/page.html.twig
+++ b/core/themes/olivero/templates/layout/page.html.twig
@@ -50,13 +50,13 @@
   <div id="page">
 
     {% if page.header or page.primary_menu or page.secondary_menu %}
-      <header id="header" class="header site-header" role="banner">
+      <header id="header" class="site-header" role="banner">
 
         {# Gets fixed by JS at wide widths. #}
         <div class="site-header__fixable fixable">
-          <div class="header__left">
-            <button class="nav-primary__button" aria-controls="site-header__inner" aria-label="Toggle navigation" aria-expanded="false">
-              <span class="nav-primary__icon">
+          <div class="site-header__initial">
+            <button class="wide-nav-expand" aria-controls="site-header__inner" aria-label="{{ 'Toggle navigation'|t }}" aria-expanded="false">
+              <span class="wide-nav-expand__icon">
                 <span></span>
                 <span></span>
                 <span></span>
@@ -79,14 +79,8 @@
                 </div>
 
                 <div id="header-nav" class="header-nav" data-menu-open="false">
-
                   {{ page.primary_menu }}
-
-                  {% if page.secondary_menu %}
-                    <div class="secondary-nav__wrapper">
-                      {{ page.secondary_menu }}
-                    </div>
-                  {% endif %}
+                  {{ page.secondary_menu }}
                 </div>
               {% endif %}
             </div>
diff --git a/core/themes/olivero/templates/layout/region--secondary-menu.html.twig b/core/themes/olivero/templates/layout/region--secondary-menu.html.twig
index 32ade151fe6e..ce7312194fe3 100644
--- a/core/themes/olivero/templates/layout/region--secondary-menu.html.twig
+++ b/core/themes/olivero/templates/layout/region--secondary-menu.html.twig
@@ -13,4 +13,15 @@
  */
 #}
 
-{{ content }}
+{%
+  set classes = [
+    'region',
+    'region--' ~ region|clean_class,
+  ]
+%}
+
+{% if content %}
+  <div{{ attributes.addClass(classes) }}>
+    {{ content }}
+  </div>
+{% endif %}
-- 
GitLab