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

Issue 29735569: Fixes #19 - Define how conditional text should be displayed on the index page (Closed)

Created:
March 28, 2018, 2:08 p.m. by ire
Modified:
April 5, 2018, 7:24 a.m.
Reviewers:
juliandoucette
Base URL:
https://hg.adblockplus.org/web.adblockplus.org
Visibility:
Public.

Description

Fixes #19 - Define how conditional text should be displayed on the index page Issue - https://gitlab.com/eyeo/websites/web.adblockplus.org/issues/19 Branch - index_page

Patch Set 1 #

Total comments: 1

Patch Set 2 : Rebased #

Total comments: 2

Patch Set 3 : Separate conditional blocks #

Unified diffs Side-by-side diffs Delta from patch set Stats (+21 lines, -2 lines) Patch
M includes/hero-download.html View 1 2 chunks +3 lines, -2 lines 0 comments Download
A includes/hero-download-conditional.tmpl View 1 2 1 chunk +10 lines, -0 lines 0 comments Download
M static/css/index.css View 1 1 chunk +8 lines, -0 lines 0 comments Download

Messages

Total messages: 6
ire
March 28, 2018, 2:09 p.m. (2018-03-28 14:09:22 UTC) #1
ire
Ready for review https://codereview.adblockplus.org/29735569/diff/29735570/static/css/index.css File static/css/index.css (right): https://codereview.adblockplus.org/29735569/diff/29735570/static/css/index.css#newcode17 static/css/index.css:17: * Browser-based styles See https://codereview.adblockplus.org/29727563/diff/29733602/static/css/index.css I'm ...
March 28, 2018, 2:11 p.m. (2018-03-28 14:11:09 UTC) #2
juliandoucette
Patch does not apply. Please rebase.
April 3, 2018, 1:41 p.m. (2018-04-03 13:41:59 UTC) #3
ire
On 2018/04/03 13:41:59, juliandoucette wrote: > Patch does not apply. Please rebase. Done
April 3, 2018, 4:23 p.m. (2018-04-03 16:23:23 UTC) #4
juliandoucette
LGTM + NIT https://codereview.adblockplus.org/29735569/diff/29741637/includes/hero-download-conditional.tmpl File includes/hero-download-conditional.tmpl (right): https://codereview.adblockplus.org/29735569/diff/29741637/includes/hero-download-conditional.tmpl#newcode4 includes/hero-download-conditional.tmpl:4: {% elif locale in ["zh_CN", "zh_TW"] ...
April 4, 2018, 10:50 a.m. (2018-04-04 10:50:26 UTC) #5
ire
April 5, 2018, 7:23 a.m. (2018-04-05 07:23:46 UTC) #6
https://codereview.adblockplus.org/29735569/diff/29741637/includes/hero-downl...
File includes/hero-download-conditional.tmpl (right):

https://codereview.adblockplus.org/29735569/diff/29741637/includes/hero-downl...
includes/hero-download-conditional.tmpl:4: {% elif locale in ["zh_CN", "zh_TW"]
%}
On 2018/04/04 10:50:26, juliandoucette wrote:
> NIT: I think that we should separate these conditional blocks to avoid
confusion
> about their text contents being related.

Done

Powered by Google App Engine
This is Rietveld