Issue 5385 - Create Site Header Component for Help Center

includes/layout/header.tmpl

+{#

[juliandoucette, 2017/07/14 14:11:28]

I'm not sure if we need this -  based on my discussion with Kai that I forwarded
to you. What do you think?

[ire, 2017/07/17 09:25:53]

Yeah you're right. I'll remove it. 

On another note I think we should have a policy about this written somewhere, so
it's clear which types of files we have decided to put the note. For example,
based on the discussion, I'm still not sure if SCSS files should have the header
or not. 

+ # This file is part of help.eyeo.com.
+ # Copyright (C) 2016 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/>.
+ #}
+
+<header id="site-header" class="navbar">
+  <div class="navbar-wrapper cf">

[juliandoucette, 2017/07/14 14:11:28]

cf?

[ire, 2017/07/17 09:25:52]

clearfix. Will change it to be the full word.

+    <h1 class="site-title">
+      {{ "index" | linkify() }}

[juliandoucette, 2017/07/14 14:11:28]

You are using linkify correctly here - but I don't think this template needs to
be a template because there is no template logic inside of it. If this were HTML
then you could just <a href="index">.

[ire, 2017/07/17 09:25:53]

Seen comment below.

+        <span class="sr-only">eyeo Help</span>

[juliandoucette, 2017/07/14 14:11:28]

Why span.sr-only and not img.alt?

[ire, 2017/07/17 09:25:53]

So I looked more into this and I had misunderstood a way screen readers work.
Basically, I thought that there would be a difference in the way a screen reader
would read out plain text vs the alternative text for an image. e.g. in this
case I thought it would say something like "image, eyeo Help", instead of just
"eyeo Help". So I was trying to avoid that by having an empty alt text (which
would make the SR just skip over the image) and having the label for the site be
in plain text. But I just tried out both using Chrome Vox and it looks like the
SR just reads out the alt text without saying its an image beforehand.

Long story short, I've changed it to using the alt text instead.

+        <img src="img/png/eyeo-help.png" srcset="img/png/eyeo-help.png, img/svg/eyeo-help.svg 2x" alt="">

[juliandoucette, 2017/07/14 14:11:28]

These images are not resolving correctly for me... Are they for you?

In my experience:
- src=**/* will convert to src=/en/**/*
- locales/en/**/* will convert to /en/**/*
- /en/**/* will not resolve to /**/*

(Place language neutral resources in static/**/* and language dependant
resources in locales/*/**/* and adjust paths accordingly)

[juliandoucette, 2017/07/14 14:11:28]

Missing img.alt - if one was not provided then you may write it yourself if you
feel confident.

[ire, 2017/07/17 09:25:53]

Got it. The issue with these images was actually a problem with the SVG. Fixed
it.

+      </a>
+    </h1>
+    <a href="/" id="toggle-searchform" class="tablet-and-mobile-only">

[ire, 2017/07/10 16:18:02]

I am thinking that this would link to a DuckDuck Go page (specific for our
site), which will provide a fallback for users without JavaScript. I haven't
looked into whether that is something possible yet though.

[juliandoucette, 2017/07/14 14:11:28]

I think it's a better idea to just show the collapsed navigation by default on
tablet/mobile and hide it if JavaScript is enabled.

[ire, 2017/07/17 09:25:53]

Done. I've changed this to a button as well.

+      <img src="img/png/searchIcon.png" srcset="img/png/searchIcon.png, img/svg/searchIcon.svg 2x" alt="Toggle Search Bar">

[juliandoucette, 2017/07/14 14:11:28]

Text is not translated (I'm guessing you did this on purpose since this is not
the only occurrence).

[ire, 2017/07/17 09:25:53]

Yes sorry i should have mentioned. I was trying to do this incrementally so
translations would come after. I've added them now though.

+    </a>
+    <div id="site-search">
+      <? include searchform ?>
+      <a id="product-website-link" href="https://adblockplus.org/">Adblock Plus</a>

[ire, 2017/07/10 16:18:01]

This will be displayed conditionally depending on the type of page.

[juliandoucette, 2017/07/14 14:11:29]

Disregard my comment above about .tmpl vs html if you want to add logic for this
here instead of inside another include. I'm guessing this will become something
like?

{% if product %}
  <a ...>
    {{ get_string("global", product) }}
  </a>
{% endif %}

This is how we have been doing global strings so far (I'm not sure that's a good
thing :D) - by creating a locale file manually in the default language and using
get_string to retrieve a string from it.

[ire, 2017/07/17 09:25:53]

Yes this was my intention (same with the searchform.tmpl), although I hadn't
figured out exactly how to execute yet. I will likely ask some followup
questions on this via IRC.

+    </div>
+  </div>
+</header>