Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code

Delta Between Two Patch Sets: templates/product-home.tmpl

Issue 29516622: Issue 5511 - Create Product Help Home Template for help.eyeo.com (Closed) Base URL: https://hg.adblockplus.org/help.eyeo.com
Left Patch Set: Rebase Created Sept. 11, 2017, 2:46 p.m.
Right Patch Set: Addressed final NITs Created Sept. 25, 2017, 7:24 a.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
« no previous file with change/comment | « templates/default.tmpl ('k') | no next file » | no next file with change/comment »
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
LEFTRIGHT
1 {% extends "templates/default" %} 1 {% extends "templates/default" %}
2 2
3 {% set product = products[product_id] %}
4
3 {% block body %} 5 {% block body %}
4 <main id="main" class="wrapper"> 6 <main id="main" class="container">
5 <h1 class="h1 section ta-center"> 7 <h1 class="product-heading">
juliandoucette 2017/09/12 13:21:58 NIT: I think it's confusing to add a class named "
ire 2017/09/19 10:27:21 Okay, I like the .product-heading way. Done.
6 <img class="heading-icon" src="/img/png/logo-{{product_id}}.png" srcset="/im g/svg/logo-{{product_id}}.svg 2x" alt="{{ products[product_id].full_name+" Logo" | translate( product_id+"-logo-alt", "Image alt text") }}"> 8 <img class="heading-icon" src="/img/png/logo-{{ product_id }}.png" srcset="/ img/svg/logo-{{ product_id }}.svg 2x" alt="{{ product.full_name+" Logo" | transl ate( product_id+"-logo-alt", "Image alt text") }}">
juliandoucette 2017/09/12 13:21:59 NIT: I prefer string interpolation in python e.g.
ire 2017/09/19 10:27:22 I actually find it more confusing, particularly be
juliandoucette 2017/09/22 13:29:36 Acknowledged. Please fix the spacing then.
7 9
8 {{ products[product_id].full_name + " Help Center" | translate(product_id+"- help-home-title", "Page title") }} 10 {{ product.full_name + " Help Center" | translate(product_id+"-help-home-tit le", "Page title") }}
juliandoucette 2017/09/12 13:21:58 NIT: I suggest you {% set product = products[produ
ire 2017/09/19 10:27:20 Good idea. Done.
9 </h1> 11 </h1>
10 12
11 {% set popular_topics = get_pages_metadata({ "product_id": product_id, "templa te": "article", "popular": "true" }) %} 13 {% set popular_topics = get_pages_metadata({ "product_id": product_id, "templa te": "article", "popular": "true" }) %}
12 {% if popular_topics %} 14 {% if popular_topics %}
13 <section class="card {{product_id}}-card section"> 15 <section class="card {{ product_id }}-card section">
14 <h2 class="h4 card-heading"> 16 <h2 class="card-heading">
juliandoucette 2017/09/12 13:21:59 NIT: If all cards of the same type have the same f
ire 2017/09/19 10:27:22 Done.
15 <img class="heading-icon" src="/img/png/popular-icon.png" srcset="/img/svg /popular-icon.svg 2x" alt="{{ "Popular Icon" | translate("popular-icon-alt", "Im age alt text") }}"> 17 <img class="heading-icon" src="/img/png/popular-icon.png" srcset="/img/svg /popular-icon.svg 2x" alt="{{ "Popular Icon" | translate("popular-icon-alt", "Im age alt text") }}">
16 18
17 {{ "Popular Topics" | translate("popular-topics-title", "Section title") } } 19 {{ "Popular Topics" | translate("popular-topics-title", "Section title") } }
18 </h2> 20 </h2>
19 21
20 <ul class="underlined-list row"> 22 <ul class="underlined-list row">
juliandoucette 2017/09/12 13:21:59 Note: Clever use of row and columns here :)
ire 2017/09/19 10:27:20 Thanks :)
21 {% for article in popular_topics %} 23 {% for article in popular_topics %}
22 <li class="column one-half"> 24 <li class="column one-half">
23 <a href="{{article["page"]}}"> 25 <a href="{{ article["page"] }}">
juliandoucette 2017/09/12 13:21:59 NIT: Inconsistent use of space in {{ }}
24 {{ article.title | translate( get_page_slug( article["page"]) + "-titl e", "Article title") }} 26 {{ article.title | translate( get_page_name( article["page"]) + "-titl e", "Article title") }}
25 </a> 27 </a>
26 </li> 28 </li>
27 {% endfor %} 29 {% endfor %}
28 </ul> 30 </ul>
29 </section> 31 </section>
30 {% endif %} 32 {% endif %}
31 33
32 <div class="row"> 34 <div class="row">
33 {% for category in products[product_id].help_categories %} 35 {% for category in product.help_categories %}
34 {% set articles = get_pages_metadata({ "product_id": product_id, "template": "article", "category": category.name }) %} 36 {% set articles = get_pages_metadata({ "product_id": product_id, "template": "article", "category": category.name }) %}
juliandoucette 2017/09/12 13:21:58 NIT: I think that it's unnecessary to test against
ire 2017/09/19 10:27:22 Done.
35 {% if articles and category.name != 'Popular Topics' %} 37 {% if articles and category.name != 'Popular Topics' %}
36 <section class="section column one-half"> 38 <section class="section column one-half">
juliandoucette 2017/09/12 13:21:58 Note: If you agree with my comment above about man
ire 2017/09/19 10:27:20 I agree about the font-size, but in the case it is
juliandoucette 2017/09/22 13:29:36 Acknowledged.
37 <h2 class="h4"> 39 <h2 class="h4">
38 <img class="heading-icon" src="/img/png/{{category.icon}}.png" srcset="/ img/svg/{{category.icon}}.svg 2x" alt="{{ category.name+" Icon" | translate(cate gory.icon+"-alt", "Image alt text") }}"> 40 <img class="heading-icon" src="/img/png/{{ category.icon }}.png" srcset= "/img/svg/{{ category.icon }}.svg 2x" alt="{{ category.name+" Icon" | translate( category.icon+"-alt", "Image alt text") }}">
39 41
40 {{category.name | translate( category.slug + "-category-title", "Categor y title")}} 42 {{ category.name | translate( category.slug + "-category-title", "Catego ry title") }}
41 </h2> 43 </h2>
42 44
43 <ul class="underlined-list"> 45 <ul class="underlined-list">
44 {% for article in articles %} 46 {% for article in articles %}
45 <li> 47 <li>
46 <a href="{{article["page"]}}"> 48 <a href="{{ article["page"] }}">
47 {{ article.title | translate( get_page_slug( article["page"]) + "-ti tle", "Article title") }} 49 {{ article.title | translate( get_page_name( article["page"]) + "-ti tle", "Article title") }}
48 </a> 50 </a>
49 </li> 51 </li>
50 {% endfor %} 52 {% endfor %}
51 </ul> 53 </ul>
52 </section> 54 </section>
53 {% endif %} 55 {% endif %}
54 {% endfor %} 56 {% endfor %}
55 </div> 57 </div>
56 58
57 </main> 59 </main>
58 {% endblock %} 60 {% endblock %}
LEFTRIGHT

Powered by Google App Engine
This is Rietveld