pages/android.tmpl - Issue 29766571: Fixes #55 - Updated install button on Samsung Browser for Android

Side by Side Diff: pages/android.tmpl

Issue 29766571: Fixes #55 - Updated install button on Samsung Browser for Android (Closed) Base URL: https://hg.adblockplus.org/web.adblockplus.org

Patch Set: Ready for review Created April 30, 2018, 9:53 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
	1 <head>

	2 <style>

	3 /* Hide TOS, install button, and platform buttons on not-samsung Android */
	ire 2018/05/01 08:04:25 Although I agree that these elements may not be ve Although I agree that these elements may not be very useful on Android, I don't think we should make this change without getting input on it's potential impact. Since it's not directly related to this particular issue, I would prefer you handle it separately. Also it seems Luiza mentioned the banner should remain? https://gitlab.com/eyeo/websites/web.adblockplus.org/issues/55#note_70890051 juliandoucette 2018/05/01 11:14:36 RE: Changing this without Product feedback I agre Show quoted text On 2018/05/01 08:04:25, ire wrote: > Although I agree that these elements may not be very useful on Android, I don't > think we should make this change without getting input on it's potential impact. > Since it's not directly related to this particular issue, I would prefer you > handle it separately. > > Also it seems Luiza mentioned the banner should remain? > https://gitlab.com/eyeo/websites/web.adblockplus.org/issues/55#note_70890051 RE: Changing this without Product feedback I agree. I should have made it clearer that I won't move forward without product feedback. Sorry about that. And I agree about handling this separately. But I'd prefer to handle it in two commits to one issue if you don't mind (since it's already part of the issue on gitlab). RE: Luiza banner or not Luiza changed her mind from option 1 to option 2 of those originally presented in my description. After that, I updated my description accordingly (no banner IIRC). ire 2018/05/02 08:06:59 Ack, that is fine. Show quoted text On 2018/05/01 11:14:36, juliandoucette wrote: > On 2018/05/01 08:04:25, ire wrote: > > Although I agree that these elements may not be very useful on Android, I > don't > > think we should make this change without getting input on it's potential > impact. > > Since it's not directly related to this particular issue, I would prefer you > > handle it separately. > > > > Also it seems Luiza mentioned the banner should remain? > > https://gitlab.com/eyeo/websites/web.adblockplus.org/issues/55#note_70890051 > > RE: Changing this without Product feedback > > I agree. I should have made it clearer that I won't move forward without product > feedback. Sorry about that. And I agree about handling this separately. But I'd > prefer to handle it in two commits to one issue if you don't mind (since it's > already part of the issue on gitlab). Ack, that is fine. Show quoted text > RE: Luiza banner or not > > Luiza changed her mind from option 1 to option 2 of those originally presented > in my description. After that, I updated my description accordingly (no banner > IIRC). In the comment I linked to above she wrote "Please let's keep the banner there for now." . Maybe confirm form her again so we are sure?
	4 .ua-android #terms-message,

	5 .ua-android #install,

	6 .android #other-platform

	7 {

	8 display: none;

	9 }

	10 </style>

	11 <script src="/js/vendor/bowser.js"></script>

	12 <script>

	13 document.addEventListener("DOMContentLoaded", function()

	14 {

	15 var locale = "{{ locale }}";

	16

	17 if (bowser instanceof Object && bowser.samsungBrowser)

	18 {

	19 // hide the Adblock Browser banner

	20 document

	21 .getElementById("content")

	22 .classList.remove("ua-android");
	ire 2018/05/01 08:04:25 Suggest: Add "ua-samsung" to the #content and hide Suggest: Add "ua-samsung" to the #content and hide/show the appropriate buttons etc through CSS instead juliandoucette 2018/05/01 11:14:36 Good idea. That will make testing easier. Show quoted text On 2018/05/01 08:04:25, ire wrote: > Suggest: Add "ua-samsung" to the #content and hide/show the appropriate buttons > etc through CSS instead Good idea. That will make testing easier.
	23

	24 // change the Android install link to Samsung Browser

	25 document

	26 .getElementById("install-android")

	27 .href = "https://play.google.com/store/apps/details?id=org.adblockplus .adblockplussbrowser";

	28

	29 // update the install button text in English only

	30 if (locale == "en")

	31 document

	32 .getElementById("install-android")

	33 .textContent = "Agree and Install for Samsung Browser";
	ire 2018/05/01 08:04:25 This text should be "Agree and Install for Samsung This text should be "Agree and Install for Samsung Internet" juliandoucette 2018/05/01 11:14:36 Good catch. Show quoted text On 2018/05/01 08:04:25, ire wrote: > This text should be "Agree and Install for Samsung Internet" Good catch.
	34 }

	35 });

	36 </script>

	37 </head>

	38

1 <? include index ?>	39 <? include index ?>

OLD	NEW

« no previous file with comments | « no previous file | static/js/vendor/bowser.js » ('j') | no next file with comments »