Issue 5385 - Create Site Header Component for Help Center

static/scss/layout/_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
+{
+  padding-top: $md;
+  padding-bottom: $md;
+  color: $white;

[juliandoucette, 2017/08/23 14:39:09]

I don't think `.navbar` should have color because "navbar" just describes a
layout component which can be different colors (e.g. all of our product websites
will have different colour navbars). I suggest using an ID or a more specific
class for this e.g.

#site-header
{
  color: ...
  background: color: ...
}

or 

.navbar-light, .navbar-dark, .navbar-primary, .navbar-secondary (shades, colors)

[ire, 2017/08/25 08:27:13]

I agree with you that "navbar" is more a description on layout. Here is the
issue:

1. There are only two "navbars" in this site (header and footer), and they both
have this colour
2. If I move this colour to a different class, there will be an additional class
(e..g navbar-primary) being applied to both of these elements (or the same style
to both IDs). Considering that there is no other navbar in this site, it seems
like an abstraction that is not necessary
3. I can imagine the "navbar" class being moved to website-defaults. That class
would not have a colour applied (or would have some default colour to be
overriden). But that doesn't stop us from applying the colour we want for
navbars across a particular site to the same "navbar" class (sort of like an
extension)

What do you think?

[juliandoucette, 2017/08/29 16:17:27]

I think you have the right idea. If we don't add color to `.navbar` then we will
end up extending `.navbar` by class or ID per website or adding an additional
class. I think that we could optimize this by providing default colors based on
the primary color scheme e.g. we could set .navbar background to
$primary-background etc or do this by proxy by $navbar-background with default
of $primary-background etc.

[ire, 2017/09/01 10:57:20]

I think this is a good idea we can implement when we add the navbar to
website-defaults.

+  background-color: $black;
+}
+
+.navbar a

[juliandoucette, 2017/08/23 14:39:09]

See comment above.

[ire, 2017/08/25 08:27:12]

Acknowledged.

+{
+  color: $white;
+}
+
+.navbar-wrapper

[juliandoucette, 2017/08/23 14:39:09]

See comment
[here](https://codereview.adblockplus.org/29485575/diff/29523654/static/scss/base/_variables.scss#newcode68)

[ire, 2017/08/25 08:27:13]

Acknowledged.

+{
+  width: $navbar-width;
+  max-width: 100%;
+  margin-right: auto;
+  margin-left: auto;
+  padding-right: $md;
+  padding-left: $md;
+}