pages/download.tmpl - Issue 29722640: Fixes #22 - Add download page

Delta Between Two Patch Sets: pages/download.tmpl

Issue 29722640: Fixes #22 - Add download page (Closed) Base URL: https://hg.adblockplus.org/web.adblockplus.org

Left Patch Set: Addressed comments #3 Created March 15, 2018, 9:30 a.m.

Right Patch Set: Addressed comments #17 Created April 3, 2018, 3:19 p.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

LEFT	RIGHT
1 title=Download Adblock Plus	1 title=Download Adblock Plus
saroyanm 2018/03/19 19:04:07 I assume having 2 steps installation from the home I assume having 2 steps installation from the homepage in comparison to previous 1 click installation from the homepage is temporary, something we plan to change ? Also in some cases it's 3 step installation. ire 2018/03/20 07:25:26 This download page is more of a fallback in case t Show quoted text On 2018/03/19 19:04:07, saroyanm wrote: > I assume having 2 steps installation from the homepage in comparison to previous > 1 click installation from the homepage is temporary, something we plan to change > ? > > Also in some cases it's 3 step installation. This download page is more of a fallback in case the appropriate browser isn't detected. See https://codereview.adblockplus.org/29727563/ saroyanm 2018/03/23 18:33:38 Noted Show quoted text On 2018/03/20 07:25:26, ire wrote: > On 2018/03/19 19:04:07, saroyanm wrote: > > I assume having 2 steps installation from the homepage in comparison to > previous > > 1 click installation from the homepage is temporary, something we plan to > change > > ? > > > > Also in some cases it's 3 step installation. > > This download page is more of a fallback in case the appropriate browser isn't > detected. > > See https://codereview.adblockplus.org/29727563/ Noted
2 description=Get Adblock Plus for free on Firefox, Chrome, Opera, Safari, Android and iOS	2 description=Get Adblock Plus for free on Firefox, Chrome, Opera, Safari, Android and iOS

3 template=minimal	3 template=minimal

4	4

5 <head>	5 <head>

6 <style>	6 <style>

7 header img {	7 header img {

8 width: 4em;	8 width: 4em;

9 height: auto;	9 height: auto;

10 }	10 }

11 </style>	11 </style>

12 </head>	12 </head>

13	13

14 {% set supported_devices = [	14 {% set supported_devices = [

15 {	15 {

16 "id": "desktop",	16 "id": "desktop",

17 "icons": ["device-desktop.png", "device-desktop-2x.png"],	17 "icons": { "1x": "device-desktop.png", "2x": "device-desktop-2x.png" },

18 "icon_alt": "Computer monitor and mouse icon",	18 "icon_alt": "Computer monitor and mouse icon",

19 "title": "Block ads on any desktop browser",	19 "title": "Block ads on any desktop browser",

20 "description": "By clicking any of the links below, you agree to our <a href =\"terms\">Terms of Use</a>.",	20 "description": "By clicking any of the links below, you agree to our " + "t erms" \| linkify + "Terms of Use</a>.",
saroyanm 2018/03/19 19:04:07 Detail: We used to use a special function when we Detail: We used to use a special function when we were creating a relative links called "linkify". This function suppose to generate link and set hreflang attribute as well. ire 2018/03/20 07:25:25 Good suggestion! Done. Show quoted text On 2018/03/19 19:04:07, saroyanm wrote: > Detail: We used to use a special function when we were creating a relative links > called "linkify". This function suppose to generate link and set hreflang > attribute as well. Good suggestion! Done.
21 "platforms": [	21 "platforms": [

22 { "name": "Chrome", "url": "https://chrome.google.com/webstore/detail/cfhd ojbkjhnklbpkdaibdccddilifddb" },	22 { "name": "Chrome", "url": "https://chrome.google.com/webstore/detail/cfhd ojbkjhnklbpkdaibdccddilifddb" },
saroyanm 2018/03/19 19:04:07 Is that intentional that we are not using inline i Is that intentional that we are not using inline installation -> https://developer.chrome.com/webstore/inline_installation ? Make sense to check for other extensions as well. ire 2018/03/20 07:25:25 I will bring this up with Jeen Show quoted text On 2018/03/19 19:04:07, saroyanm wrote: > Is that intentional that we are not using inline installation -> > https://developer.chrome.com/webstore/inline_installation ? > > Make sense to check for other extensions as well. I will bring this up with Jeen ire 2018/03/20 14:40:58 From Jeen's response it seems like the links here Show quoted text On 2018/03/20 07:25:25, ire wrote: > On 2018/03/19 19:04:07, saroyanm wrote: > > Is that intentional that we are not using inline installation -> > > https://developer.chrome.com/webstore/inline_installation ? > > > > Make sense to check for other extensions as well. > > I will bring this up with Jeen > From Jeen's response it seems like the links here are the correct ones. saroyanm 2018/03/23 18:33:38 Yes, but in order the inline installation for the Show quoted text On 2018/03/20 14:40:58, ire wrote: > On 2018/03/20 07:25:25, ire wrote: > > On 2018/03/19 19:04:07, saroyanm wrote: > > > Is that intentional that we are not using inline installation -> > > > https://developer.chrome.com/webstore/inline_installation ? > > > > > > Make sense to check for other extensions as well. > > > > I will bring this up with Jeen > > > > From Jeen's response it seems like the links here are the correct ones. Yes, but in order the inline installation for the desktop product to work we should include <link href="https://chrome.google.com/webstore/detail/cfhdojbkjhnklbpkdaibdccddilifddb" rel="chrome-webstore-item"> right ? Which seems like is missing here. For other webstores that should be different ex. for Firefox, but make sense to be sure that the installation is 1 step, until this was intentional (according to Jeen's comment this wasn't ?) ire 2018/03/26 07:29:07 Okay I think I was mixing up two different things Show quoted text On 2018/03/23 18:33:38, saroyanm wrote: > On 2018/03/20 14:40:58, ire wrote: > > On 2018/03/20 07:25:25, ire wrote: > > > On 2018/03/19 19:04:07, saroyanm wrote: > > > > Is that intentional that we are not using inline installation -> > > > > https://developer.chrome.com/webstore/inline_installation ? > > > > > > > > Make sense to check for other extensions as well. > > > > > > I will bring this up with Jeen > > > > > > > From Jeen's response it seems like the links here are the correct ones. > > Yes, but in order the inline installation for the desktop product to work we > should include <link > href="https://chrome.google.com/webstore/detail/cfhdojbkjhnklbpkdaibdccddilifddb" > rel="chrome-webstore-item"> right ? Which seems like is missing here. > > For other webstores that should be different ex. for Firefox, but make sense to > be sure that the installation is 1 step, until this was intentional (according > to Jeen's comment this wasn't ?) Okay I think I was mixing up two different things here, sorry about that. I don't think we are doing any browser detection on the download page, since it's main function is as a fallback for when we don't detect the browser from the homepage. Because of that, I don't think the inline installers are being used here, we are just linking to the various web stores. On the homepage, where we are doing the browser detection, is where I think the one-click installers are being used. I will double-check this with Jeen though, as I don't think her response was clear. ire 2018/03/26 09:44:27 So Jeen confirmed that we are not using inline ins Show quoted text On 2018/03/26 07:29:07, ire wrote: > On 2018/03/23 18:33:38, saroyanm wrote: > > On 2018/03/20 14:40:58, ire wrote: > > > On 2018/03/20 07:25:25, ire wrote: > > > > On 2018/03/19 19:04:07, saroyanm wrote: > > > > > Is that intentional that we are not using inline installation -> > > > > > https://developer.chrome.com/webstore/inline_installation ? > > > > > > > > > > Make sense to check for other extensions as well. > > > > > > > > I will bring this up with Jeen > > > > > > > > > > From Jeen's response it seems like the links here are the correct ones. > > > > Yes, but in order the inline installation for the desktop product to work we > > should include <link > > > href="https://chrome.google.com/webstore/detail/cfhdojbkjhnklbpkdaibdccddilifddb" > > rel="chrome-webstore-item"> right ? Which seems like is missing here. > > > > For other webstores that should be different ex. for Firefox, but make sense > to > > be sure that the installation is 1 step, until this was intentional (according > > to Jeen's comment this wasn't ?) > > Okay I think I was mixing up two different things here, sorry about that. > > I don't think we are doing any browser detection on the download page, since > it's main function is as a fallback for when we don't detect the browser from > the homepage. Because of that, I don't think the inline installers are being > used here, we are just linking to the various web stores. On the homepage, where > we are doing the browser detection, is where I think the one-click installers > are being used. > > I will double-check this with Jeen though, as I don't think her response was > clear. So Jeen confirmed that we are not using inline installations on the download page, just the homepage. See https://gitlab.com/eyeo/specs/spec/merge_requests/135#note_65041707
23 { "name": "Firefox", "url": "https://update.adblockplus.org/latest/adblock plusfirefox.xpi" },	23 { "name": "Firefox", "url": "https://update.adblockplus.org/latest/adblock plusfirefox.xpi" },

24 { "name": "Internet Explorer", "url": "https://update.adblockplus.org/late st/adblockplusie.exe" },	24 { "name": "Internet Explorer", "url": "https://update.adblockplus.org/late st/adblockplusie.exe" },

25 { "name": "Safari", "url": "https://update.adblockplus.org/latest/adblockp lussafari.safariextz" },	25 { "name": "Safari", "url": "https://update.adblockplus.org/latest/adblockp lussafari.safariextz" },

26 { "name": "Edge (beta)", "url": "https://www.microsoft.com/store/p/adblock -plus/9nblggh4r9nz" },	26 { "name": "Edge (beta)", "url": "https://www.microsoft.com/store/p/adblock -plus/9nblggh4r9nz" },

27 { "name": "Opera", "url": "https://addons.opera.com/extensions/details/ope ra-adblock/?display=en-US" },	27 { "name": "Opera", "url": "https://addons.opera.com/extensions/details/ope ra-adblock/" },
saroyanm 2018/03/19 19:04:07 Detail: No need to use language specific version o Detail: No need to use language specific version of the page. ire 2018/03/20 07:25:26 Done. Show quoted text On 2018/03/19 19:04:07, saroyanm wrote: > Detail: No need to use language specific version of the page. Done.
28 { "name": "Maxthon", "url": "http://www.maxthon.com/mx5/features/ad-blocke r/" },	28 { "name": "Maxthon", "url": "http://www.maxthon.com/mx5/features/ad-blocke r/" },

29 { "name": "Yandex Browser", "url": "https://chrome.google.com/webstore/det ail/cfhdojbkjhnklbpkdaibdccddilifddb" }	29 { "name": "Yandex Browser", "url": "https://chrome.google.com/webstore/det ail/cfhdojbkjhnklbpkdaibdccddilifddb" }

30 ]	30 ]

31 },	31 },

32 {	32 {

33 "id": "mobile",	33 "id": "mobile",

34 "icons": ["device-mobile.png", "device-mobile.svg"],	34 "icons": { "1x": "device-mobile.png", "2x": "device-mobile.svg" },

35 "icon_alt": "Mobile icon",	35 "icon_alt": "Mobile icon",

36 "title": "Block ads in your mobile browser",	36 "title": "Block ads in your mobile browser",

37 "platforms": [	37 "platforms": [

38 { "name": "Samsung Internet (Android)", "url": "https://play.google.com/st ore/apps/details?id=org.adblockplus.adblockplussbrowser&hl" },	38 { "name": "Samsung Internet (Android)", "url": "https://play.google.com/st ore/apps/details?id=org.adblockplus.adblockplussbrowser&hl" },

39 { "name": "Safari (iOS)", "url": "https://itunes.apple.com/app/adblock-plu s-abp-remove-ads-browse-faster-without-tracking/id1028871868?mt=8" },	39 { "name": "Safari (iOS)", "url": "https://itunes.apple.com/app/adblock-plu s-abp-remove-ads-browse-faster-without-tracking/id1028871868?mt=8" },

40 { "name": "Firefox", "url": "https://addons.mozilla.org/android/addon/adbl ock-plus/" }	40 { "name": "Firefox", "url": "https://addons.mozilla.org/android/addon/adbl ock-plus/" }

41 ]	41 ]

42 },	42 },

43 {	43 {

44 "id": "abb",	44 "id": "abb",

45 "icons": ["abb-icon.png", "abb-icon-2x.png"],	45 "icons": { "1x": "abb-icon.png", "2x": "abb-icon-2x.png" },
saroyanm 2018/03/19 19:04:07 The small icon is 500px on 500px, as far as I can The small icon is 500px on 500px, as far as I can see we are using 90x90px on the page, either make sense to use 180px for both size, or adjust accordingly, until we don't show the larger icon on this page. ire 2018/03/20 07:25:26 You're right. i just copied the images from the sp Show quoted text On 2018/03/19 19:04:07, saroyanm wrote: > The small icon is 500px on 500px, as far as I can see we are using 90x90px on > the page, either make sense to use 180px for both size, or adjust accordingly, > until we don't show the larger icon on this page. You're right. i just copied the images from the spec but I should have resized them. Done.
46 "icon_alt": "Adblock Browser logo",	46 "icon_alt": "Adblock Browser logo",

47 "title": "Block ads in a dedicated ad-blocking browser",	47 "title": "Block ads in a dedicated ad-blocking browser",

48 "platforms": [	48 "platforms": [

49 { "name": "Adblock Browser (Android)", "url": "https://play.google.com/sto re/apps/details?id=org.adblockplus.browser" },	49 { "name": "Adblock Browser (Android)", "url": "https://play.google.com/sto re/apps/details?id=org.adblockplus.browser" },
saroyanm 2018/03/19 19:04:07 We can use redirection links instead: https://adbl We can use redirection links instead: https://adblockplus.org/redirect?link=adblock_browser_android_store and https://adblockplus.org/redirect?link=adblock_browser_ios_store Make sense to find out which of this items should also use redirection links, I mean which of the items that are not using inline installation. ire 2018/03/20 07:25:26 Will bring this up with Jeen Show quoted text On 2018/03/19 19:04:07, saroyanm wrote: > We can use redirection links instead: > https://adblockplus.org/redirect?link=adblock_browser_android_store > and > https://adblockplus.org/redirect?link=adblock_browser_ios_store > > Make sense to find out which of this items should also use redirection links, I > mean which of the items that are not using inline installation. Will bring this up with Jeen
50 { "name": "Adblock Browser (iOS)", "url": "https://itunes.apple.com/my/app /adblock-browser/id1015653330?mt=8&ign-itsct=1015653330-1015653330&ign-itscg=017 6&ign-mpt=uo%3D4" }	50 { "name": "Adblock Browser (iOS)", "url": "https://itunes.apple.com/my/app /adblock-browser/id1015653330?mt=8&ign-itsct=1015653330-1015653330&ign-itscg=017 6&ign-mpt=uo%3D4" }

51 ]	51 ]

52 }	52 }

53 ] %}	53 ] %}

54	54

55 <header class="bg-primary section content text-center">	55 <header class="bg-primary section content text-center">

56 <div class="container">	56 <div class="container">

57 <img height="90px"	57 <img height="90px"

58 width="90px"	58 width="90px"

59 src="/img/download-icon.png"	59 src="/img/download-icon.png"

60 srcset="/img/download-icon.svg 2x"	60 srcset="/img/download-icon.svg 2x"

61 alt="{{ "White down arrow" \| translate("download-icon", "Alt text") }}" >	61 alt="{{ "White down arrow" \| translate("download-icon", "Alt text") }}" >

62 <h1>{{ "Get Adblock Plus products on these devices" \| translate("download-he ading", "Heading") }}</h1>	62 <h1>{{ "Get Adblock Plus products on these devices" \| translate("download-he ading", "Heading") }}</h1>

63 <p>{{ "Supported platforms" \| translate("download-subheading", "Subheading") }}</p>	63 <p>{{ "Supported platforms" \| translate("download-subheading", "Subheading") }}</p>

64 </div>	64 </div>

65 </header>	65 </header>

66	66

67 <div class="item-group container text-center content">	67 <div id="supported-devices" class="item-group container text-center content">

68 <div class="row">	68 <div class="row">

69 {% for device in supported_devices %}	69 {% for device in supported_devices %}

70 <div class="column one-third">	70 <div class="column one-third">

71 <img	71 <img

72 height="90px"	72 height="90px"

73 width="90px"	73 width="90px"

74 src="/img/{{ device.icons[0] }}"	74 src="/img/{{ device.icons['1x'] }}"

75 srcset="/img/{{ device.icons[1] }} 2x"	75 srcset="/img/{{ device.icons['2x'] }} 2x"

76 alt="{{ device.icon_alt \| translate(device.id + "-icon-alt", "Alt text") }}">	76 alt="{{ device.icon_alt \| translate(device.id + "-icon-alt", "Alt text") }}">

77 <h2>{{ device.title \| translate(device.id + "-title", "Heading") }}</h2>	77 <h2>{{ device.title \| translate(device.id + "-title", "Heading") }}</h2>

78 <div>	78 <div>

79 {% if device.description %}	79 {% if device.description %}

80 <p>{{ device.description \| safe \| translate(device.id + "-description", "Paragraph") }}</p>	80 <p><small>{{ device.description \| safe \| translate(device.id + "-descrip tion", "Paragraph") }}</small></p>

81 {% endif %}	81 {% endif %}

82 <ul class="horizontal-list">	82 <ul class="horizontal-list">

83 {% for platform in device.platforms %}	83 {% for platform in device.platforms %}

84 <li>	84 <li>

85 {% set platform_identifier = device.id + "-product-name-" + loop.i ndex \| string %}	85 {% set platform_identifier = device.id + "-product-name-" + loop.i ndex \| string %}

86 <a href="{{ platform.url }}">{{ platform.name \| translate(platform _identifier, "Product name") }}</a>{{ ", " if loop.index != loop.length }}	86 <a href="{{ platform.url }}">{{ platform.name \| translate(platform _identifier, "Product name") }}</a>{{ ", " if loop.index != loop.length }}
saroyanm 2018/03/19 19:04:06 Detail: There are some Browser names that are loca Detail: There are some Browser names that are localized, for example in Russian it's Яндекс.Браузер https://browser.yandex.ru/, but I'm not sure if we would like to localize our product names, ex.: Adblock Browser and there might be some other product names we would like to avoid mistake translations. Probable make sense to ask Tamara and if so maybe we can make a use of the fix tags as well. ire 2018/03/20 07:25:25 I will bring this up with Tamara Show quoted text On 2018/03/19 19:04:06, saroyanm wrote: > Detail: There are some Browser names that are localized, for example in Russian > it's Яндекс.Браузер https://browser.yandex.ru/, but I'm not sure if we would > like to localize our product names, ex.: Adblock Browser and there might be some > other product names we would like to avoid mistake translations. > > Probable make sense to ask Tamara and if so maybe we can make a use of the fix > tags as well. I will bring this up with Tamara ire 2018/03/20 14:40:58 According to Tamara we should not be using the fix Show quoted text On 2018/03/20 07:25:25, ire wrote: > On 2018/03/19 19:04:06, saroyanm wrote: > > Detail: There are some Browser names that are localized, for example in > Russian > > it's Яндекс.Браузер https://browser.yandex.ru/, but I'm not sure if we would > > like to localize our product names, ex.: Adblock Browser and there might be > some > > other product names we would like to avoid mistake translations. > > > > Probable make sense to ask Tamara and if so maybe we can make a use of the fix > > tags as well. > > I will bring this up with Tamara According to Tamara we should not be using the fix tags here. saroyanm 2018/03/23 18:33:39 Acknowledged. Show quoted text On 2018/03/20 14:40:58, ire wrote: > On 2018/03/20 07:25:25, ire wrote: > > On 2018/03/19 19:04:06, saroyanm wrote: > > > Detail: There are some Browser names that are localized, for example in > > Russian > > > it's Яндекс.Браузер https://browser.yandex.ru/, but I'm not sure if we would > > > like to localize our product names, ex.: Adblock Browser and there might be > > some > > > other product names we would like to avoid mistake translations. > > > > > > Probable make sense to ask Tamara and if so maybe we can make a use of the > fix > > > tags as well. > > > > I will bring this up with Tamara > > According to Tamara we should not be using the fix tags here. Acknowledged.
87 </li>	87 </li>

88 {% endfor %}	88 {% endfor %}

89 </ul>	89 </ul>

90 </div>	90 </div>

91 </div>	91 </div>

92 {% endfor %}	92 {% endfor %}

93 </div>	93 </div>

94 </div>	94 </div>

LEFT	RIGHT