templates/product-home.tmpl - Issue 29516622: Issue 5511 - Create Product Help Home Template for help.eyeo.com

Side by Side Diff: 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

Patch Set: Rebase Created Sept. 11, 2017, 2:46 p.m.

Left:
Right:

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

Jump to:

View unified diff | Download patch

OLD	NEW
(Empty)
	1 {% extends "templates/default" %}

	2

	3 {% block body %}

	4 <main id="main" class="wrapper">

	5 <h1 class="h1 section ta-center">
	juliandoucette 2017/09/12 13:21:58 NIT: I think it's confusing to add a class named " NIT: I think it's confusing to add a class named "section" to a header because a header logically belongs to a "section". I think that this indicates that this class should be named less specifically or this heading should belong to a component that styles headings differently. I'm leaning towards the latter e.g. .product-heading ire 2017/09/19 10:27:21 Okay, I like the .product-heading way. Done. Show quoted text On 2017/09/12 13:21:58, juliandoucette wrote: > NIT: I think it's confusing to add a class named "section" to a header because a > header logically belongs to a "section". I think that this indicates that this > class should be named less specifically or this heading should belong to a > component that styles headings differently. I'm leaning towards the latter e.g. > .product-heading 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") }}">
	juliandoucette 2017/09/12 13:21:59 NIT: I prefer string interpolation in python e.g. NIT: I prefer string interpolation in python e.g. '%s-logo-alt' % product_id ire 2017/09/19 10:27:22 I actually find it more confusing, particularly be Show quoted text On 2017/09/12 13:21:59, juliandoucette wrote: > NIT: I prefer string interpolation in python e.g. '%s-logo-alt' % product_id I actually find it more confusing, particularly because we're not using it everywhere (e.h. the image urls) juliandoucette 2017/09/22 13:29:36 Acknowledged. Please fix the spacing then. Show quoted text On 2017/09/19 10:27:22, ire wrote: > On 2017/09/12 13:21:59, juliandoucette wrote: > > NIT: I prefer string interpolation in python e.g. '%s-logo-alt' % product_id > > I actually find it more confusing, particularly because we're not using it > everywhere (e.h. the image urls) Acknowledged. Please fix the spacing then.
	7

	8 {{ products[product_id].full_name + " Help Center" \| translate(product_id+"- help-home-title", "Page title") }}
	juliandoucette 2017/09/12 13:21:58 NIT: I suggest you {% set product = products[produ NIT: I suggest you {% set product = products[product_id] %} so that you can {{ product.full_name }} (or equivalent) ire 2017/09/19 10:27:20 Good idea. Done. Show quoted text On 2017/09/12 13:21:58, juliandoucette wrote: > NIT: I suggest you {% set product = products[product_id] %} so that you can {{ > product.full_name }} (or equivalent) Good idea. Done.
	9 </h1>

	10

	11 {% set popular_topics = get_pages_metadata({ "product_id": product_id, "templa te": "article", "popular": "true" }) %}

	12 {% if popular_topics %}

	13 <section class="card {{product_id}}-card section">

	14 <h2 class="h4 card-heading">
	juliandoucette 2017/09/12 13:21:59 NIT: If all cards of the same type have the same f NIT: If all cards of the same type have the same font-size then it doesn't make sense to set their font-size using a separate class? ire 2017/09/19 10:27:22 Done. Show quoted text On 2017/09/12 13:21:59, juliandoucette wrote: > NIT: If all cards of the same type have the same font-size then it doesn't make > sense to set their font-size using a separate class? 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") }}">

	16

	17 {{ "Popular Topics" \| translate("popular-topics-title", "Section title") } }

	18 </h2>

	19

	20 <ul class="underlined-list row">
	juliandoucette 2017/09/12 13:21:59 Note: Clever use of row and columns here :) Note: Clever use of row and columns here :) ire 2017/09/19 10:27:20 Thanks :) Show quoted text On 2017/09/12 13:21:59, juliandoucette wrote: > Note: Clever use of row and columns here :) Thanks :)
	21 {% for article in popular_topics %}

	22 <li class="column one-half">

	23 <a href="{{article["page"]}}">
	juliandoucette 2017/09/12 13:21:59 NIT: Inconsistent use of space in {{ }} NIT: Inconsistent use of space in {{ }}
	24 {{ article.title \| translate( get_page_slug( article["page"]) + "-titl e", "Article title") }}

	25 </a>

	26 </li>

	27 {% endfor %}

	28 </ul>

	29 </section>

	30 {% endif %}

	31

	32 <div class="row">

	33 {% for category in products[product_id].help_categories %}

	34 {% 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 NIT: I think that it's unnecessary to test against the template key/value here? ire 2017/09/19 10:27:22 Done. Show quoted text On 2017/09/12 13:21:58, juliandoucette wrote: > NIT: I think that it's unnecessary to test against the template key/value here? Done.
	35 {% if articles and category.name != 'Popular Topics' %}

	36 <section class="section column one-half">
	juliandoucette 2017/09/12 13:21:58 Note: If you agree with my comment above about man Note: If you agree with my comment above about manually setting the font-size of a card then it may logically follow to componentize this section. This section is basically the inside of a card like the one above. Which suggests that we could use the same single class for both. ire 2017/09/19 10:27:20 I agree about the font-size, but in the case it is Show quoted text On 2017/09/12 13:21:58, juliandoucette wrote: > Note: If you agree with my comment above about manually setting the font-size of > a card then it may logically follow to componentize this section. This section > is basically the inside of a card like the one above. Which suggests that we > could use the same single class for both. I agree about the font-size, but in the case it is a bit unclear where the "component" should go. Because of where I am applying the columns, I can't use the same structure for both sections. juliandoucette 2017/09/22 13:29:36 Acknowledged. Show quoted text On 2017/09/19 10:27:20, ire wrote: > On 2017/09/12 13:21:58, juliandoucette wrote: > > Note: If you agree with my comment above about manually setting the font-size > of > > a card then it may logically follow to componentize this section. This section > > is basically the inside of a card like the one above. Which suggests that we > > could use the same single class for both. > > I agree about the font-size, but in the case it is a bit unclear where the > "component" should go. Because of where I am applying the columns, I can't use > the same structure for both sections. Acknowledged.
	37 <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") }}">

	39

	40 {{category.name \| translate( category.slug + "-category-title", "Categor y title")}}

	41 </h2>

	42

	43 <ul class="underlined-list">

	44 {% for article in articles %}

	45 <li>

	46 <a href="{{article["page"]}}">

	47 {{ article.title \| translate( get_page_slug( article["page"]) + "-ti tle", "Article title") }}

	48 </a>

	49 </li>

	50 {% endfor %}

	51 </ul>

	52 </section>

	53 {% endif %}

	54 {% endfor %}

	55 </div>

	56

	57 </main>

	58 {% endblock %}

OLD	NEW

« static/scss/layout/_grid.scss ('K') | « templates/default.tmpl ('k') | no next file » | no next file with comments »