Noissue - Merged index_page into master

This merge and review was created for the purpose of syncing translations. It
should not be published as-is even if it looks good because it hasn't been
tested yet. Please see additional notes below.

[juliandoucette, 2018-07-30 05:01:08]

This merge and review was created for the purpose of syncing translations. It
should not be published as-is even if it looks good because it hasn't been
tested yet. Please see additional notes below.

https://codereview.adblockplus.org/29841563/diff/29841659/globals/browsers.py
File globals/browsers.py (right):

https://codereview.adblockplus.org/29841563/diff/29841659/globals/browsers.py...
globals/browsers.py:58: 'id': 'samsung',
Detail: Added for translation purposes only. We don't support Samsung Internet
properly yet.

https://codereview.adblockplus.org/29841563/diff/29841659/includes/index.tmpl
File includes/index.tmpl (right):

https://codereview.adblockplus.org/29841563/diff/29841659/includes/index.tmpl...
includes/index.tmpl:18: {{ ("Agree and Install for " + browser.name) |
translate(("agree-and-install-for-" + browser.id), "button label") }}
Detail: I fixed these strings as described in solution two in
https://gitlab.com/eyeo/websites/web.adblockplus.org/issues/109

https://codereview.adblockplus.org/29841563/diff/29841659/static/css/main.css
File static/css/main.css (right):

https://codereview.adblockplus.org/29841563/diff/29841659/static/css/main.css...
static/css/main.css:813: .hide
Detail: I decided to keep this class while merging although it may not be used
anywhere (I didn't check).

[juliandoucette, 2018-07-30 16:17:44]

Patchset 3 also moves the install button and it's dependencies out of
hero-download.html. I did this so that we can share this button across pages and
resolve the issues outlined in
https://gitlab.com/eyeo/websites/web.adblockplus.org/issues/109.

https://codereview.adblockplus.org/29841563/diff/29842752/includes/hero-downl...
File includes/hero-download-conditional.tmpl (right):

https://codereview.adblockplus.org/29841563/diff/29842752/includes/hero-downl...
includes/hero-download-conditional.tmpl:10: <p class="show-on-msie">{{ "Adblock
Plus for Internet Explorer is supported by eyeo GmbH on the following editions
of Windows 10 – Windows 10 Pro, Windows 10 Education and Windows 10 Enterprise.
Adblock Plus for Internet Explorer is supported on the in-market supported
servicing branches of Windows 10 including - Current Branch, Current Branch for
Business and the following Long-Term Servicing Branch: Windows 10." |
translate("msie-conditional-message") }}</p>
Detail: Correction reported in IRC

https://codereview.adblockplus.org/29841563/diff/29842752/static/poland.html
File static/poland.html (right):

https://codereview.adblockplus.org/29841563/diff/29842752/static/poland.html#...
static/poland.html:93: <p class="show-on-msie">Adblock Plus for Internet
Explorer is supported on the in-market supported servicing branches of Windows
10 including - Current Branch, Current Branch for Business and the following
Long-Term Servicing Branch: Windows 10.</p>
Detail: Correction reported in IRC

[juliandoucette, 2018-07-30 16:46:14]

https://codereview.adblockplus.org/29841563/diff/29842954/includes/features.html
File includes/features.html (right):

https://codereview.adblockplus.org/29841563/diff/29842954/includes/features.h...
includes/features.html:21: <p>{{ feature-safe-body With Adblock Plus avoiding
tracking and malware is easy. Blocking intrusive ads reduces the risk of
"malvertising" infections. Blocking tracking stops companies following your
online activity. }}</p>
Detail: Correction reported in IRC

https://codereview.adblockplus.org/29841563/diff/29842954/includes/install-bu...
File includes/install-button-templates.tmpl (right):

https://codereview.adblockplus.org/29841563/diff/29842954/includes/install-bu...
includes/install-button-templates.tmpl:11: {% for browser in browsers %}
FIXME / Detail: I added these (temporarily) because "install-for" strings were
not being uploaded to crowdin.

https://codereview.adblockplus.org/29841563/diff/29842954/pages/download.tmpl
File pages/download.tmpl (right):

https://codereview.adblockplus.org/29841563/diff/29842954/pages/download.tmpl...
pages/download.tmpl:36: "title": "Block ads on your mobile browser",
Detail: Correction reported on IRC

[ire, 2018-07-31 00:10:14]

Thanks Julian! Here's my first look through:

https://codereview.adblockplus.org/29841563/diff/29841659/globals/browsers.py
File globals/browsers.py (right):

https://codereview.adblockplus.org/29841563/diff/29841659/globals/browsers.py...
globals/browsers.py:58: 'id': 'samsung',
On 2018/07/30 05:01:06, juliandoucette wrote:
> Detail: Added for translation purposes only. We don't support Samsung Internet
> properly yet.

Acknowledged.

https://codereview.adblockplus.org/29841563/diff/29841659/includes/index.tmpl
File includes/index.tmpl (right):

https://codereview.adblockplus.org/29841563/diff/29841659/includes/index.tmpl...
includes/index.tmpl:18: {{ ("Agree and Install for " + browser.name) |
translate(("agree-and-install-for-" + browser.id), "button label") }}
On 2018/07/30 05:01:06, juliandoucette wrote:
> Detail: I fixed these strings as described in solution two in
> https://gitlab.com/eyeo/websites/web.adblockplus.org/issues/109

Acknowledged.

https://codereview.adblockplus.org/29841563/diff/29841659/static/css/main.css
File static/css/main.css (right):

https://codereview.adblockplus.org/29841563/diff/29841659/static/css/main.css...
static/css/main.css:813: .hide
On 2018/07/30 05:01:06, juliandoucette wrote:
> Detail: I decided to keep this class while merging although it may not be used
> anywhere (I didn't check).

I checked and it is being used on at least the coding style page.

https://codereview.adblockplus.org/29841563/diff/29842752/includes/hero-downl...
File includes/hero-download-conditional.tmpl (right):

https://codereview.adblockplus.org/29841563/diff/29842752/includes/hero-downl...
includes/hero-download-conditional.tmpl:10: <p class="show-on-msie">{{ "Adblock
Plus for Internet Explorer is supported by eyeo GmbH on the following editions
of Windows 10 – Windows 10 Pro, Windows 10 Education and Windows 10 Enterprise.
Adblock Plus for Internet Explorer is supported on the in-market supported
servicing branches of Windows 10 including - Current Branch, Current Branch for
Business and the following Long-Term Servicing Branch: Windows 10." |
translate("msie-conditional-message") }}</p>
On 2018/07/30 16:17:42, juliandoucette wrote:
> Detail: Correction reported in IRC

Acknowledged.

https://codereview.adblockplus.org/29841563/diff/29842954/includes/features.html
File includes/features.html (right):

https://codereview.adblockplus.org/29841563/diff/29842954/includes/features.h...
includes/features.html:21: <p>{{ feature-safe-body With Adblock Plus avoiding
tracking and malware is easy. Blocking intrusive ads reduces the risk of
"malvertising" infections. Blocking tracking stops companies following your
online activity. }}</p>
On 2018/07/30 16:46:12, juliandoucette wrote:
> Detail: Correction reported in IRC

Acknowledged.

https://codereview.adblockplus.org/29841563/diff/29842954/includes/install-bu...
File includes/install-button-templates.tmpl (right):

https://codereview.adblockplus.org/29841563/diff/29842954/includes/install-bu...
includes/install-button-templates.tmpl:9: {% endfor %}
NIT/Suggest: Is there a need to end and start 3 different for loops? Could we
just have all the templates within the same for loop

https://codereview.adblockplus.org/29841563/diff/29842954/includes/install-bu...
includes/install-button-templates.tmpl:11: {% for browser in browsers %}
On 2018/07/30 16:46:12, juliandoucette wrote:
> FIXME / Detail: I added these (temporarily) because "install-for" strings were
> not being uploaded to crowdin.

Acknowledged.

https://codereview.adblockplus.org/29841563/diff/29842954/locales/en/install-...
File locales/en/install-button.json (right):

https://codereview.adblockplus.org/29841563/diff/29842954/locales/en/install-...
locales/en/install-button.json:4: "message": "By clicking the Agree and Install
button below, you agree to our <a href='terms'>Terms of Use</a>."
For some reason when the language is not English, this link appears as plain
text (i.e. not an actual link)

https://codereview.adblockplus.org/29841563/diff/29842954/pages/download.tmpl
File pages/download.tmpl (right):

https://codereview.adblockplus.org/29841563/diff/29842954/pages/download.tmpl...
pages/download.tmpl:36: "title": "Block ads on your mobile browser",
On 2018/07/30 16:46:12, juliandoucette wrote:
> Detail: Correction reported on IRC

Acknowledged.

https://codereview.adblockplus.org/29841563/diff/29842954/static/css/main.css
File static/css/main.css (right):

https://codereview.adblockplus.org/29841563/diff/29842954/static/css/main.css...
static/css/main.css:261: .bg-secondary
NIT: This is a bg-* utility, not fg-*

https://codereview.adblockplus.org/29841563/diff/29842954/static/css/main.css...
static/css/main.css:941: color: #fff;
We also need to style the :visited pseudo-class because the text appears as red
for me

[juliandoucette, 2018-08-02 20:51:59]

Thanks Ire! I'll try to update this later today.

https://codereview.adblockplus.org/29841563/diff/29841659/static/css/main.css
File static/css/main.css (right):

https://codereview.adblockplus.org/29841563/diff/29841659/static/css/main.css...
static/css/main.css:813: .hide
On 2018/07/31 00:10:04, ire wrote:
> I checked and it is being used on at least the coding style page.

Thanks!

https://codereview.adblockplus.org/29841563/diff/29842954/locales/en/install-...
File locales/en/install-button.json (right):

https://codereview.adblockplus.org/29841563/diff/29842954/locales/en/install-...
locales/en/install-button.json:4: "message": "By clicking the Agree and Install
button below, you agree to our <a href='terms'>Terms of Use</a>."
On 2018/07/31 00:10:08, ire wrote:
> For some reason when the language is not English, this link appears as plain
> text (i.e. not an actual link)

I noticed that too. I'm not sure that will be the case when translations are
added. But it's a bug either way. And I'm not sure what to do about it yet.

Thanks!

https://codereview.adblockplus.org/29841563/diff/29842954/static/css/main.css
File static/css/main.css (right):

https://codereview.adblockplus.org/29841563/diff/29842954/static/css/main.css...
static/css/main.css:261: .bg-secondary
On 2018/07/31 00:10:10, ire wrote:
> NIT: This is a bg-* utility, not fg-*

Good catch :D

https://codereview.adblockplus.org/29841563/diff/29842954/static/css/main.css...
static/css/main.css:941: color: #fff;
On 2018/07/31 00:10:12, ire wrote:
> We also need to style the :visited pseudo-class because the text appears as
red
> for me

Good catch!

[juliandoucette, 2018-08-03 05:47:03]

Closed in favour of
https://gitlab.com/eyeo/websites/web.adblockplus.org/merge_requests/11