Issue 4912 - Update media coverage links on eyeo.com

includes/index/media.tmpl

Index: includes/index/media.tmpl
===================================================================
new file mode 100644
--- /dev/null
+++ b/includes/index/media.tmpl
@@ -0,0 +1,44 @@
+<h1 class="heading centered">
+  <span>Media coverage:</span>
+</h1>
+
+{% set media = [

[juliandoucette, 2017/08/01 23:07:33]

NIT: I think "media" is a little too general / misleading. Can you be more
specific? e.g. media_coverage, media_coverage_links, coverage_links

[ire, 2017/08/02 10:35:59]

Done.

+  {
+    "url": "http://www.mediapost.com/publications/article/289691/adblock-plus-comes-to-new-york.html",
+    "publication": "Media Post",

[juliandoucette, 2017/08/01 23:07:33]

NIT: Are these "publication"s? Suggest: author, publisher, from

[ire, 2017/08/02 10:35:58]

I think "publisher" works better. Author implies the individual who wrote the
article, whereas this is the institution publishing the article. 

Done.

+    "image": "media-post"
+  },
+  {
+    "url": "https://www.nytimes.com/2015/08/20/technology/personaltech/ad-blockers-and-the-nuisance-at-the-heart-of-the-modern-web.html",
+    "publication": "The New York Times",
+    "image": "new-york-times"
+  },
+  {
+    "url": "https://www.wsj.com/articles/adblock-plus-chief-till-faida-says-consumers-are-fed-up-with-current-online-ads-1462981668",
+    "publication": "The Wall Street Journal",
+    "image": "wall-street-journal"
+  },
+  {
+    "url": "https://techcrunch.com/2016/05/09/adblock-plus-closes-in-on-a-billion-downloads/",
+    "publication": "TechCrunch",
+    "image": "techcrunch"
+  },
+  {
+    "url": "http://www.businessinsider.com/theres-nothing-wrong-about-the-way-adblock-plus-makes-money-2015-9",
+    "publication": "Business Insider",
+    "image": "business-insider"
+  }
+] %}
+
+{% macro media_item(item) -%}
+  <a href="{{ item.url }}" target="_blank">

[juliandoucette, 2017/08/01 23:07:33]

NIT: I think we should describe the action (opening a publisher's article in a
new window) on the element that triggers it (the title of this anchor tag) and
use the image titles and alts to describe the images (see comments below).

[ire, 2017/08/02 10:35:58]

Done.

+    <img class="publication-logo" alt="{{ item.publication }} (Opens in a new window)" src="/images/coverage/{{ item.image }}-1x.png" srcset="/images/coverage/{{ item.image }}-2x.png 2x">

[juliandoucette, 2017/08/01 23:07:33]

NIT: I think alt text is supposed to describe the image. In this case, the image
is a publisher's logo e.g. "{{ item.publication }} logo". I think the
publisher's name makes a better title.

[ire, 2017/08/02 10:35:58]

Agreed on using "{{ item.publication }} logo".

I'm not sure the benefit of adding the publisher's name as the title as well
though. Seems like duplication of information.

[juliandoucette, 2017/08/02 20:31:12]

Acknowledged.

+    <img class="external-link-icon" src="/images/icon-external-link.png" srcset="/images/icon-external-link.svg 2x" alt="">

[ire, 2017/08/01 10:04:06]

I purposefully left a null alternative text here because:

1. I didn't think it made sense for the external link image to have the
alternative text, "Opens in a new window", so I moved it to the logo alternative
text like you suggested
2. Since the "Opens in a new window" text will be read out by the screen reader,
I didn't think it was necessary for a screen reader to also read “External
link”, or whatever alternative text would be placed here

[juliandoucette, 2017/08/01 23:07:33]

Acknowledged.

I would suggest something like:

alt: External link icon
title: Open featured {{ item.publication }} article in a new window

[ire, 2017/08/02 10:35:59]

Makes sense. I changed the alt, but added the title attribute on the anchor
element instead

[juliandoucette, 2017/08/02 20:31:12]

Acknowledged.

+  </a>
+{% endmacro %}
+
+<ul id="media-links">
+  {% for item in media %}
+    <li>{{ media_item(item) }}</li>
+  {% endfor %}
+</ul>