Issue 5635 - Implement website-default navbar component

static/scss/_navbar.scss

 // This file is part of help.eyeo.com.
 // Copyright (C) 2017 Eyeo GmbH
 //
 // help.eyeo.com is free software: you can redistribute it and/or modify
 // it under the terms of the GNU General Public License as published by
 // the Free Software Foundation, either version 3 of the License, or
 // (at your option) any later version.
 //
 // help.eyeo.com is distributed in the hope that it will be useful,
 // but WITHOUT ANY WARRANTY; without even the implied warranty of
 // MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
 // GNU General Public License for more details.
 //
 // You should have received a copy of the GNU General Public License
 // along with help.eyeo.com.  If not, see <http://www.gnu.org/licenses/>.
 
 /*******************************************************************************
  * Navbar component
  ******************************************************************************/
 
 .navbar
 {
-  padding-top: $small-space;

[juliandoucette, 2017/10/31 15:18:38]

- This doesn't apply to new abp.org
- This will prevent full-height hover/focus states within

-  padding-bottom: $small-space;
-  color: $primary-light;

[juliandoucette, 2017/10/31 15:18:38]

Suggest: we apply a bg utility class in HTML or extend a utility class in CSS
instead

[ire, 2017/11/02 11:15:27]

Great idea! Done.

-  background-color: $primary;
   @extend .clearfix;

[juliandoucette, 2017/10/31 15:18:38]

This may be overkill because the .container within is already clearfixed?

[ire, 2017/11/02 11:15:27]

.container isn't clearfixed?

 }
 
-.navbar a,
-.navbar button

[juliandoucette, 2017/10/31 15:18:38]

See comment above about background utility classes. On that note, we should
probably style links within background utility classes.

[ire, 2017/11/02 11:15:26]

Links are already styled because of the default `color: inherit`. The problem
here is the button, but i'm not sure buttons should be (by default) styled for
the background utility classes. This seems more like something specific to the
navbar. We may not even have text in the navbar button, it'll likely be an icon.


For now I'm setting `color: inherit` on the navbar button.

+/* .navbar .navbar-container
+ ****************************************************************************/
+
+.navbar-container
 {
-  color: $primary-light;
+  @if ($add-horizontal-navbar-padding)
+  {
+    @extend .container;
+  }
+  @else
+  {
+    @include container($container-width, 0px);
+  }
 }
 
 /* .navbar .navbar-branding
  ****************************************************************************/
 
 .navbar-branding

[juliandoucette, 2017/10/31 15:18:37]

Perhaps a float utility class or and ID class would be more appropriate if there
is nothing else unique about a navbar brand.

[ire, 2017/11/02 11:15:27]

Done.

 {
-  float: left;
+  padding-top: $navbar-padding + ($navbar-padding / 4);
+  padding-bottom: $navbar-padding + ($navbar-padding / 4);
+  line-height: 1;
+
+  @if ($add-horizontal-navbar-padding != true)
+  {
+    padding-right: $navbar-padding;
+    padding-left: $navbar-padding;
+  }
 }
 
-[dir="rtl"] .navbar-branding
+.navbar-branding img
 {
-  float: right;
+  height: $navbar-branding-height;
+  vertical-align: text-top;
 }
 
 /* .navbar .toggle-navbar-collapse
  ****************************************************************************/
 
 .toggle-navbar-collapse
 {
   display: none;
-  float: right;
-  border: 0;
-  background-color: transparent;
-  text-align: right;
-  cursor: pointer;
-}
+  padding-top: ($navbar-branding-height / 2) + ($navbar-padding / 2);
+  padding-bottom: ($navbar-branding-height / 2) + ($navbar-padding / 2);
 
-[dir="rtl"] .toggle-navbar-collapse
-{
-  float: left;

[juliandoucette, 2017/10/31 15:18:37]

What about text-align?

[ire, 2017/11/02 11:15:26]

The other text-align isn't actually needed. So I'm removing it.

+  @if ($add-horizontal-navbar-padding != true)
+  {
+    padding-right: $navbar-padding;
+    padding-left: $navbar-padding;
+  }
 }
 
 .js .toggle-navbar-collapse
 {
   display: block;
 
   @media (min-width: $tablet-breakpoint)
   {
     display: none;
   }
 }
 
-.js .toggle-navbar-collapse .close-label

[juliandoucette, 2017/10/31 15:18:38]

I think that we will always use an icon; not a label.

[ire, 2017/11/02 11:15:27]

We could have separate icons for open and close?

Or are you suggesting that the name be close-icon instead of close-label?

-{
-  display: none;
-}
-
-.expanded .toggle-navbar-collapse .open-label
-{
-  display: none;
-}
-
-.expanded .toggle-navbar-collapse .close-label
-{
-  display: inline;
-}
-
 /* .navbar .navbar-collapse
  ****************************************************************************/
 
 .navbar-collapse
 {
-  display: block;

[juliandoucette, 2017/10/31 15:18:37]

I think that this may be unnecessary because we are unlikely to add
.navbar-collapse to an inline or inline-block element?

[ire, 2017/11/02 11:15:27]

Done.

   clear: both;
-  width: 100%;

[juliandoucette, 2017/10/31 15:18:38]

I think that this may be unnecessary because this element is probably already
display:block;

[ire, 2017/11/02 11:15:27]

Done.

-  padding-top: $small-space;

[juliandoucette, 2017/10/31 15:18:37]

I think that this may be unnecessary because the padding-top will probably be
provided by the margin of the child elements.

[ire, 2017/11/02 11:15:26]

Done.

 
   @media (min-width: $tablet-breakpoint)
   {
     float: right;
     clear: none;
-    width: auto;

[juliandoucette, 2017/10/31 15:18:38]

I think that this will become unnecessary if I am right about the width above
being unnecessary. 

The same applies to the line below (except about padding not width).

[ire, 2017/11/02 11:15:27]

Done.

-    padding-top: 0;
   }
 }
 
 [dir="rtl"] .navbar-collapse
 {
   float: left;
 }
 
 .js .navbar-collapse
 {
   display: none;
 
   @media (min-width: $tablet-breakpoint)
   {
     display: block;
   }
 }
 
 .expanded .navbar-collapse

[juliandoucette, 2017/10/31 15:18:37]

Suggest: Use aria expanded (unless you want to do that separately)

[ire, 2017/11/02 11:15:27]

I think I will do that separately when we implement aria-haspopup or whatever
the correct role it would be here

 {
   display: block;
+  width: 100%;
+
+  @media (min-width: $tablet-breakpoint)
+  {
+    width: auto;
+  }
 }
+
+/* .navbar .navbar-nav
+ ****************************************************************************/
+
+@if ($add-horizontal-navbar-padding)
+{
+  .navbar .navbar-nav
+  {
+    padding-bottom: $navbar-padding / 2;
+
+    @media (min-width: $tablet-breakpoint)
+    {
+      margin-right: -($navbar-padding / 2);
+      margin-left: -($navbar-padding / 2);
+      padding-bottom: 0;
+    }
+  }
+}
+
+.navbar .navbar-nav li
+{
+  @media (min-width: $tablet-breakpoint)
+  {
+    display: inline-block;
+  }
+}
+
+.navbar .navbar-nav a
+{
+  display: block;
+  padding-top: $navbar-padding / 2;
+  padding-bottom: $navbar-padding / 2;
+
+  @if ($add-horizontal-navbar-padding != true)
+  {
+    padding-right: $navbar-padding;
+    padding-left: $navbar-padding;
+  }
+
+  @media (min-width: $tablet-breakpoint)
+  {
+    padding-top: ($navbar-branding-height / 2) + ($navbar-padding / 2);
+    padding-right: $navbar-padding / 2;
+    padding-bottom: ($navbar-branding-height / 2) + ($navbar-padding / 2);
+    padding-left: $navbar-padding / 2;
+  }
+}