includes/layout/header.tmpl - Issue 29485575: Issue 5385 - Create Site Header Component for Help Center

Delta Between Two Patch Sets: includes/layout/header.tmpl

Issue 29485575: Issue 5385 - Create Site Header Component for Help Center (Closed)

Left Patch Set: Created July 10, 2017, 4:08 p.m.

Right Patch Set: Include site variables before website-defaults Created Sept. 1, 2017, 12:11 p.m.

Left:
Right:

Use n/p to move between diff chunks; N/P to move between comments.

Jump to:

Left: Side by side diff | Download
Right: Side by side diff | Download

LEFT	RIGHT
1 {#
juliandoucette 2017/07/14 14:11:28 I'm not sure if we need this - based on my discus 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 no Show quoted text On 2017/07/14 14:11:28, juliandoucette wrote: > I'm not sure if we need this - based on my discussion with Kai that I forwarded > to you. What do you think? 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.
2 # This file is part of help.eyeo.com.

3 # Copyright (C) 2016 Eyeo GmbH

4 #

5 # help.eyeo.com is free software: you can redistribute it and/or modify

6 # it under the terms of the GNU General Public License as published by

7 # the Free Software Foundation, either version 3 of the License, or

8 # (at your option) any later version.

9 #

10 # help.eyeo.com is distributed in the hope that it will be useful,

11 # but WITHOUT ANY WARRANTY; without even the implied warranty of

12 # MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the

13 # GNU General Public License for more details.

14 #

15 # You should have received a copy of the GNU General Public License

16 # along with help.eyeo.com. If not, see <http://www.gnu.org/licenses/>.

17 #}

18

19 <header id="site-header" class="navbar">	1 <header id="site-header" class="navbar">

20 <div class="navbar-wrapper cf">	2 <div class="navbar-wrapper">
juliandoucette 2017/07/14 14:11:28 cf? cf? ire 2017/07/17 09:25:52 clearfix. Will change it to be the full word. Show quoted text On 2017/07/14 14:11:28, juliandoucette wrote: > cf? clearfix. Will change it to be the full word.
21 <h1 class="site-title">	3 <h1 id="site-title">

22 {{ "index" \| linkify() }}	4 {{ "index" \| linkify() }}
juliandoucette 2017/07/14 14:11:28 You are using linkify correctly here - but I don't 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. Show quoted text On 2017/07/14 14:11:28, juliandoucette wrote: > 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">. Seen comment below.
23 <span class="sr-only">eyeo Help</span>	5 <img src="/img/png/eyeo-help.png" srcset="/img/svg/eyeo-help.svg 2x" alt ="{{ "eyeo Help" \| translate("site-title", "Image alt text") }}">
juliandoucette 2017/07/14 14:11:28 Why span.sr-only and not img.alt? 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 Show quoted text On 2017/07/14 14:11:28, juliandoucette wrote: > Why span.sr-only and not img.alt? 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.
24 <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... 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 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 Show quoted text On 2017/07/14 14:11:28, juliandoucette wrote: > 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) Got it. The issue with these images was actually a problem with the SVG. Fixed it.
25 </a>	6 </a>

26 </h1>	7 </h1>

27 <a href="/" id="toggle-searchform" class="tablet-and-mobile-only">	8
ire 2017/07/10 16:18:02 I am thinking that this would link to a DuckDuck G 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 collap Show quoted text On 2017/07/10 16:18:02, ire wrote: > 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. 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. Show quoted text On 2017/07/14 14:11:28, juliandoucette wrote: > On 2017/07/10 16:18:02, ire wrote: > > 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. > > 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. Done. I've changed this to a button as well.
28 <img src="img/png/searchIcon.png" srcset="img/png/searchIcon.png, img/svg/ searchIcon.svg 2x" alt="Toggle Search Bar">	9 <button class="toggle-navbar-collapse unstyled phablet-and-mobile-only">
juliandoucette 2017/07/14 14:11:28 Text is not translated (I'm guessing you did this 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 Show quoted text On 2017/07/14 14:11:28, juliandoucette wrote: > Text is not translated (I'm guessing you did this on purpose since this is not > the only occurrence). Yes sorry i should have mentioned. I was trying to do this incrementally so translations would come after. I've added them now though.
29 </a>	10 <img src="/img/png/search-icon.png" srcset="/img/svg/search-icon.svg 2x" a lt="{{ "Toggle Search Bar" \| translate("search-toggle-alt", "Image alt text") }} ">

30 <div id="site-search">	11 </button>

31 <? include searchform ?>	12

32 <a id="product-website-link" href="https://adblockplus.org/">Adblock Plus< /a>	13 <div class="navbar-collapse">
ire 2017/07/10 16:18:01 This will be displayed conditionally depending on 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 Show quoted text On 2017/07/10 16:18:01, ire wrote: > This will be displayed conditionally depending on the type of page. 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 searchfor Show quoted text On 2017/07/14 14:11:29, juliandoucette wrote: > On 2017/07/10 16:18:01, ire wrote: > > This will be displayed conditionally depending on the type of page. > > 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. 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.
	14 <form id="search-form" action="https://duckduckgo.com" method="GET">

	15 <label for="search" class="sr-only">{{ "Search Adblock Plus Help" \| tran slate("search-form-label", "Input label") }}</label>

	16 <input id="search" name="q" type="search" placeholder="{{ "Search Adbloc k Plus Help" \| translate("search-form-label", "Input label") }}">

	17 <input type="hidden" name="sites" value="adblockplus.org">

	18 <button type="submit">

	19 <img src="/img/png/search-icon.png" srcset="/img/svg/search-icon.svg 2 x" alt="{{ "Search" \| translate("search-icon-alt", "Image alt text") }}">

	20 </button>

	21 </form>

	22

	23 <nav id="product-website-link">

	24 <a href="https://adblockplus.org/">

	25 {{ "Adblock Plus" \| translate("abp-product-link", "Link label") }}<img src="/img/png/external-icon.png" srcset="/img/svg/external-icon.svg 2x" alt="{{ "External link icon" \| translate("external-link-icon", "Image alt text") }}">

	26 </a>

	27 </nav>

33 </div>	28 </div>

34 </div>	29 </div>

35 </header>	30 </header>

LEFT	RIGHT