Left: | ||
Right: |
LEFT | RIGHT |
---|---|
1 <h1 class="heading centered"> | 1 {% set media_coverage = [ |
2 <span>Media coverage:</span> | |
3 </h1> | |
4 | |
5 {% set media = [ | |
juliandoucette
2017/08/01 23:07:33
NIT: I think "media" is a little too general / mis
ire
2017/08/02 10:35:59
Done.
| |
6 { | 2 { |
7 "url": "http://www.mediapost.com/publications/article/289691/adblock-plus-co mes-to-new-york.html", | 3 "url": "http://www.mediapost.com/publications/article/289691/adblock-plus-co mes-to-new-york.html", |
8 "publication": "Media Post", | 4 "publisher": "Media Post", |
juliandoucette
2017/08/01 23:07:33
NIT: Are these "publication"s? Suggest: author, pu
ire
2017/08/02 10:35:58
I think "publisher" works better. Author implies t
| |
9 "image": "media-post" | 5 "image": "media-post" |
10 }, | 6 }, |
11 { | 7 { |
12 "url": "https://www.nytimes.com/2015/08/20/technology/personaltech/ad-blocke rs-and-the-nuisance-at-the-heart-of-the-modern-web.html", | 8 "url": "https://www.nytimes.com/2015/08/20/technology/personaltech/ad-blocke rs-and-the-nuisance-at-the-heart-of-the-modern-web.html", |
13 "publication": "The New York Times", | 9 "publisher": "The New York Times", |
14 "image": "new-york-times" | 10 "image": "new-york-times" |
15 }, | 11 }, |
16 { | 12 { |
17 "url": "https://www.wsj.com/articles/adblock-plus-chief-till-faida-says-cons umers-are-fed-up-with-current-online-ads-1462981668", | 13 "url": "https://www.wsj.com/articles/adblock-plus-chief-till-faida-says-cons umers-are-fed-up-with-current-online-ads-1462981668", |
18 "publication": "The Wall Street Journal", | 14 "publisher": "The Wall Street Journal", |
19 "image": "wall-street-journal" | 15 "image": "wall-street-journal" |
20 }, | 16 }, |
21 { | 17 { |
22 "url": "https://techcrunch.com/2016/05/09/adblock-plus-closes-in-on-a-billio n-downloads/", | 18 "url": "https://techcrunch.com/2016/05/09/adblock-plus-closes-in-on-a-billio n-downloads/", |
23 "publication": "TechCrunch", | 19 "publisher": "TechCrunch", |
24 "image": "techcrunch" | 20 "image": "techcrunch" |
25 }, | 21 }, |
26 { | 22 { |
27 "url": "http://www.businessinsider.com/theres-nothing-wrong-about-the-way-ad block-plus-makes-money-2015-9", | 23 "url": "http://www.businessinsider.com/theres-nothing-wrong-about-the-way-ad block-plus-makes-money-2015-9", |
28 "publication": "Business Insider", | 24 "publisher": "Business Insider", |
29 "image": "business-insider" | 25 "image": "business-insider" |
30 } | 26 } |
31 ] %} | 27 ] %} |
32 | 28 |
33 {% macro media_item(item) -%} | 29 {% macro media_coverage_item(item) -%} |
34 <a href="{{ item.url }}" target="_blank"> | 30 <a href="{{ item.url }}" target="_blank" title="Open featured {{ item.publicat ion }} article in a new window"> |
juliandoucette
2017/08/01 23:07:33
NIT: I think we should describe the action (openin
ire
2017/08/02 10:35:58
Done.
| |
35 <img class="publication-logo" alt="{{ item.publication }} (Opens in a new wi ndow)" src="/images/coverage/{{ item.image }}-1x.png" srcset="/images/coverage/{ { item.image }}-2x.png 2x"> | 31 <img class="publisher-logo" alt="{{ item.publisher }} logo" src="/images/cov erage/{{ 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
ire
2017/08/02 10:35:58
Agreed on using "{{ item.publication }} logo".
I'
juliandoucette
2017/08/02 20:31:12
Acknowledged.
| |
36 <img class="external-link-icon" src="/images/icon-external-link.png" srcset= "/images/icon-external-link.svg 2x" alt=""> | 32 <img class="external-link-icon" src="/images/icon-external-link.png" srcset= "/images/icon-external-link.svg 2x" alt="External link icon"> |
ire
2017/08/01 10:04:06
I purposefully left a null alternative text here b
juliandoucette
2017/08/01 23:07:33
Acknowledged.
I would suggest something like:
al
ire
2017/08/02 10:35:59
Makes sense. I changed the alt, but added the titl
juliandoucette
2017/08/02 20:31:12
Acknowledged.
| |
37 </a> | 33 </a> |
38 {% endmacro %} | 34 {% endmacro %} |
39 | 35 |
36 <h1 class="heading"> | |
37 <span>Media coverage:</span> | |
38 </h1> | |
39 | |
40 <ul id="media-links"> | 40 <ul id="media-links"> |
41 {% for item in media %} | 41 {% for item in media_coverage %} |
42 <li>{{ media_item(item) }}</li> | 42 <li>{{ media_coverage_item(item) }}</li> |
43 {% endfor %} | 43 {% endfor %} |
44 </ul> | 44 </ul> |
45 | |
46 <small>(Links open in a new window)</small> | |
juliandoucette
2017/08/02 20:31:13
NIT:
- I think it's better to put labels above (to
| |
LEFT | RIGHT |