pages/uninstall-abp.tmpl - Issue 29329984: Issue 3257 - Create uninstallation page in adblockplus.org

Delta Between Two Patch Sets: pages/uninstall-abp.tmpl

Issue 29329984: Issue 3257 - Create uninstallation page in adblockplus.org (Closed)

Left Patch Set: Removed the shuffling filter Created Nov. 17, 2015, 3:37 p.m.

Right Patch Set: Convert to array instead of cloning Created Dec. 3, 2015, 1:55 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=Adblock Plus has been uninstalled	1 title=Adblock Plus has been uninstalled

2 template=simple	2 template=simple

3	3

4 {% set reasons = [	4 {% set reasons = [

5 ("1v0", "reason-not-installed", "I didn't install Adblock Plus."),	5 ("1v0", "reason-not-installed", "I didn't install Adblock Plus."),

6 ("2v0", "reason-slowing-down", "Adblock Plus slowed down my browser."),	6 ("2v0", "reason-slowing-down", "Adblock Plus slowed down my browser."),

7 ("3v0", "reason-acceptable-ads", "I don't like the Acceptable Ads program.") ,	7 ("3v0", "reason-acceptable-ads", "I don't like the Acceptable Ads program."),

8 ("4v0", "reason-see-ads", "Adblock Plus didn't block all ads."),	8 ("4v0", "reason-see-ads", "Adblock Plus didn't block all ads."),

9 ("5v0", "reason-better-adblocker", "I found better ad blocking software."),	9 ("5v0", "reason-better-adblocker", "I found better ad blocking software."),

10 ("6v0", "reason-break-websites", "Adblock Plus breaks websites that I visit. ")	10 ("6v0", "reason-break-websites", "Adblock Plus breaks websites that I visit.")

11 ] %}	11 ] %}

12	12

13 <head>	13 <head>

14 <meta name="robots" content="noindex" />	14 <meta name="robots" content="noindex" />

15

16 <script type="text/javascript">	15 <script type="text/javascript">

17 (function()	16 (function()

18 {	17 {

19 function init()	18 function init()

20 {	19 {

21 var form = document.getElementById("reasons-form");	20 var form = document.getElementById("reasons-form");

	21

	22 // Create hidden input for GET parameters

22 window.location.search.substr(1).split("&").forEach(function(param)	23 window.location.search.substr(1).split("&").forEach(function(param)

23 {	24 {

24 if (!/.=./.test(param))	25 if (!/.=./.test(param))

25 return;	26 return;

26	27

27 var paramSplit = param.split("=");	28 var paramSplit = param.split("=");

28 var input = document.createElement("input");	29 var input = document.createElement("input");

29 input.setAttribute("type", "hidden");	30 input.setAttribute("type", "hidden");

30 input.setAttribute("name", paramSplit[0]);	31 input.setAttribute("name", paramSplit[0]);

31 input.setAttribute("value", paramSplit[1]);	32 input.setAttribute("value", paramSplit[1]);

32 form.appendChild(input);	33 form.appendChild(input);

33 });	34 });

34	35

	36 // Randomly add reasons

35 var reasonsContainer = document.getElementById("reasons");	37 var reasonsContainer = document.getElementById("reasons");

36 var reasons = reasonsContainer.cloneNode(true).getElementsByTagName("li");	38 var reasons = reasonsContainer.getElementsByTagName("li");

	39 reasons = Array.prototype.slice.call(reasons);

37 reasonsContainer.innerHTML = "";	40 reasonsContainer.innerHTML = "";

38 var reasons = Array.prototype.slice.call(reasons);	41 while (reasons.length)

39 while (reasons.length > 0)

40 {	42 {

41 var randomIndex = Math.floor(Math.random() * (reasons.length -1));	43 var randomIndex = Math.floor(Math.random() * (reasons.length -1));

42 var reasonElement = reasons.splice(randomIndex, 1)[0];	44 var reasonElement = reasons.splice(randomIndex, 1)[0];

43 reasonsContainer.appendChild(reasonElement);	45 reasonsContainer.appendChild(reasonElement);

44 }	46 }

45	47

46 document.getElementById("reason-other").addEventListener("change",	48 var reasonOtherCheckbox = document.getElementById("reason-other");

47 function()	49 reasonOtherCheckbox.addEventListener("change", function()

48 {	50 {

49 var textArea = document.getElementById("reason-other-input");	51 var textArea = document.getElementById("reason-other-input");

50 textArea.hasAttribute("class") ? textArea.removeAttribute("class") :	52 if (textArea.hasAttribute("class"))

51 textArea.setAttribute("class", "hidden");	53 textArea.removeAttribute("class")

	54 else

	55 textArea.setAttribute("class", "hidden");

52 }, false);	56 }, false);

53	57

54 document.getElementById("submit-form").addEventListener("click",	58 var submitButton = document.getElementById("submit-form");

55 function(event)	59 submitButton.addEventListener("click", function(event)

56 {	60 {

57 if (!document.querySelector("ul input:checked"))	61 if (!document.querySelector("ul input:checked"))

58 {	62 {

59 event.preventDefault();	63 event.preventDefault();

60 form.setAttribute("class", "error");	64 form.setAttribute("class", "error");

	65 }

	66 else

	67 {

	68 form.submit();

61 }	69 }

62 }, false);	70 }, false);

63 }	71 }

64 document.addEventListener("DOMContentLoaded", init, false);	72 document.addEventListener("DOMContentLoaded", init, false);

65 })();	73 })();

66 </script>	74 </script>

67 </head>	75 </head>

	76

68 <section class="highlighted">	77 <section class="highlighted">

69 <h1>{{"Please select the reason(s) why you uninstalled Adblock Plus:"\|translat e("reasons-header", "Form heading")}}</h1>	78 <h1>{{"Please select the reason(s) why you uninstalled Adblock Plus:"\|translat e("reasons-header", "Form heading")}}</h1>

70 <form id="reasons-form" action="uninstall-abp-submit" method="post">	79 <form id="reasons-form" action="uninstall-abp-submit" method="post">

71 <ul id="reasons">	80 <ul id="reasons">

72 {%- for reasonId, stringId, value in reasons %}	81 {%- for reasonId, stringId, value in reasons %}
kzar 2015/11/18 17:14:45 I think it would make more sense to add a to_json I think it would make more sense to add a to_json Jinja2 filter, and then use that to dump the reasons to a JavaScript array. You could then generate the shuffled elements directly in JavaScript, instead of putting them here and later removing them + appending them again in JavaScript. In fact, you could simply declare the list of reasons as a JSON array in the first place near the top of the file. Then you would no longer need the to_json filter and this file wouldn't even need to be a template at all. saroyanm 2015/11/18 18:00:18 Fare enough, I agree that this is not a good solut Show quoted text On 2015/11/18 17:14:45, kzar wrote: > I think it would make more sense to add a to_json Jinja2 filter, and then use > that to dump the reasons to a JavaScript array. You could then generate the > shuffled elements directly in JavaScript, instead of putting them here and later > removing them + appending them again in JavaScript. Fare enough, I agree that this is not a good solution I have now. Show quoted text > In fact, you could simply declare the list of reasons as a JSON array in the > first place near the top of the file. Then you would no longer need the to_json > filter and this file wouldn't even need to be a template at all. Not sure if in that case I can get translations for reasons, also the implementation should be supported by translation script to be able to upload strings to crowdin, this also applies to the first solution you suggested. kzar 2015/11/19 11:24:54 Make this page Markdown, you don't need it to be a Show quoted text On 2015/11/18 18:00:18, saroyanm wrote: > On 2015/11/18 17:14:45, kzar wrote: > > I think it would make more sense to add a to_json Jinja2 filter, and then use > > that to dump the reasons to a JavaScript array. You could then generate the > > shuffled elements directly in JavaScript, instead of putting them here and > later > > removing them + appending them again in JavaScript. > Fare enough, I agree that this is not a good solution I have now. > > > In fact, you could simply declare the list of reasons as a JSON array in the > > first place near the top of the file. Then you would no longer need the > to_json > > filter and this file wouldn't even need to be a template at all. > Not sure if in that case I can get translations for reasons, also the > implementation should be supported by translation script to be able to upload > strings to crowdin, this also applies to the first solution you suggested. Make this page Markdown, you don't need it to be a Jinja2 template. Do something like this: title=Adblock Plus has been uninstalled template=simple <head> <script type="text/javascript"> var reasons = [ ["1v0", "{{reason-not-installed I didn't install Adblock Plus.}}"], ["2v0", "{{reason-slowing-down Adblock Plus slowed down my browser.}}"], ["3v0", "{{reason-acceptable-ads I don't like the Acceptable Ads program.}}"], ["4v0", "{{reason-see-ads Adblock Plus didn't block all ads.}}"], ["5v0", "{{reason-better-adblocker I found better ad blocking software.}}"], ["6v0", "{{reason-break-websites Adblock Plus breaks websites that I visit.}}"] ]; ... </script> </head> <section class="highlighted"> <h1>{{reasons-header Please select the reason(s) why you uninstalled Adblock Plus:</h1> <form id="reasons-form" action="uninstall-abp-submit" method="post"> <ul id="reasons"></ul> ... </form> ... </section> Notes: - Yes quotes in translated strings are escaped, I checked. - My example uses HTML for markup, you should obviously only use it where required and use Markdown if possible. - Please create a function called shuffle to do the shuffling and use that to shuffle the array, instead of mixing that logic with the rest of the initialisation logic. kzar 2015/11/19 11:28:56 Also why not keep the string ID the same as the ID Show quoted text On 2015/11/19 11:24:54, kzar wrote: > On 2015/11/18 18:00:18, saroyanm wrote: > > On 2015/11/18 17:14:45, kzar wrote: > > > I think it would make more sense to add a to_json Jinja2 filter, and then > use > > > that to dump the reasons to a JavaScript array. You could then generate the > > > shuffled elements directly in JavaScript, instead of putting them here and > > later > > > removing them + appending them again in JavaScript. > > Fare enough, I agree that this is not a good solution I have now. > > > > > In fact, you could simply declare the list of reasons as a JSON array in the > > > first place near the top of the file. Then you would no longer need the > > to_json > > > filter and this file wouldn't even need to be a template at all. > > Not sure if in that case I can get translations for reasons, also the > > implementation should be supported by translation script to be able to upload > > strings to crowdin, this also applies to the first solution you suggested. > > Make this page Markdown, you don't need it to be a Jinja2 template. Do something > like this: > > title=Adblock Plus has been uninstalled > template=simple > > <head> > <script type="text/javascript"> > var reasons = [ > ["1v0", "{{reason-not-installed I didn't install Adblock Plus.}}"], > ["2v0", "{{reason-slowing-down Adblock Plus slowed down my browser.}}"], > ["3v0", "{{reason-acceptable-ads I don't like the Acceptable Ads > program.}}"], > ["4v0", "{{reason-see-ads Adblock Plus didn't block all ads.}}"], > ["5v0", "{{reason-better-adblocker I found better ad blocking > software.}}"], > ["6v0", "{{reason-break-websites Adblock Plus breaks websites that I > visit.}}"] > ]; > > ... > </script> > </head> > > <section class="highlighted"> > <h1>{{reasons-header Please select the reason(s) why you uninstalled Adblock > Plus:</h1> > > <form id="reasons-form" action="uninstall-abp-submit" method="post"> > <ul id="reasons"></ul> > > ... > </form> > ... > </section> > > Notes: > - Yes quotes in translated strings are escaped, I checked. > - My example uses HTML for markup, you should obviously only use it where > required and use Markdown if possible. > - Please create a function called shuffle to do the shuffling and use that to > shuffle the array, instead of mixing that logic with the rest of the > initialisation logic. Also why not keep the string ID the same as the ID the data guys will use? Instead of having "1v0" and "reason-not-installed" why not call both "reason-1v0-not-installed" or something? It seems like extra work for them to have to keep looking up string IDs like "1v0" -> "reason-not-installed" -> "blah". saroyanm 2015/11/19 17:00:17 Ohh wow, didn't considered that case, for some rea Show quoted text On 2015/11/19 11:24:54, kzar wrote: > On 2015/11/18 18:00:18, saroyanm wrote: > > On 2015/11/18 17:14:45, kzar wrote: > > > I think it would make more sense to add a to_json Jinja2 filter, and then > use > > > that to dump the reasons to a JavaScript array. You could then generate the > > > shuffled elements directly in JavaScript, instead of putting them here and > > later > > > removing them + appending them again in JavaScript. > > Fare enough, I agree that this is not a good solution I have now. > > > > > In fact, you could simply declare the list of reasons as a JSON array in the > > > first place near the top of the file. Then you would no longer need the > > to_json > > > filter and this file wouldn't even need to be a template at all. > > Not sure if in that case I can get translations for reasons, also the > > implementation should be supported by translation script to be able to upload > > strings to crowdin, this also applies to the first solution you suggested. > > Make this page Markdown, you don't need it to be a Jinja2 template. Do something > like this: > > title=Adblock Plus has been uninstalled > template=simple > > <head> > <script type="text/javascript"> > var reasons = [ > ["1v0", "{{reason-not-installed I didn't install Adblock Plus.}}"], > ["2v0", "{{reason-slowing-down Adblock Plus slowed down my browser.}}"], > ["3v0", "{{reason-acceptable-ads I don't like the Acceptable Ads > program.}}"], > ["4v0", "{{reason-see-ads Adblock Plus didn't block all ads.}}"], > ["5v0", "{{reason-better-adblocker I found better ad blocking > software.}}"], > ["6v0", "{{reason-break-websites Adblock Plus breaks websites that I > visit.}}"] > ]; > > ... > </script> > </head> > > <section class="highlighted"> > <h1>{{reasons-header Please select the reason(s) why you uninstalled Adblock > Plus:</h1> > > <form id="reasons-form" action="uninstall-abp-submit" method="post"> > <ul id="reasons"></ul> > > ... > </form> > ... > </section> > > Notes: > - Yes quotes in translated strings are escaped, I checked. > - My example uses HTML for markup, you should obviously only use it where > required and use Markdown if possible. > - Please create a function called shuffle to do the shuffling and use that to > shuffle the array, instead of mixing that logic with the rest of the > initialisation logic. Ohh wow, didn't considered that case, for some reason I was thinking that we can't have {{}} syntax in JavaScript, now I see your point, sorry for confusion indeed better solution. Please note that technically I'm not shuffling the array, but looping through array randomly, not sure if we need function for that, I've added comment to separate the implementation if it will work for you, otherwise I can have separate function for that if you insist. saroyanm 2015/11/19 17:00:17 It was requirement by data annalists to have reque Show quoted text On 2015/11/19 11:28:56, kzar wrote: > On 2015/11/19 11:24:54, kzar wrote: > > On 2015/11/18 18:00:18, saroyanm wrote: > > > On 2015/11/18 17:14:45, kzar wrote: > > > > I think it would make more sense to add a to_json Jinja2 filter, and then > > use > > > > that to dump the reasons to a JavaScript array. You could then generate > the > > > > shuffled elements directly in JavaScript, instead of putting them here and > > > later > > > > removing them + appending them again in JavaScript. > > > Fare enough, I agree that this is not a good solution I have now. > > > > > > > In fact, you could simply declare the list of reasons as a JSON array in > the > > > > first place near the top of the file. Then you would no longer need the > > > to_json > > > > filter and this file wouldn't even need to be a template at all. > > > Not sure if in that case I can get translations for reasons, also the > > > implementation should be supported by translation script to be able to > upload > > > strings to crowdin, this also applies to the first solution you suggested. > > > > Make this page Markdown, you don't need it to be a Jinja2 template. Do > something > > like this: > > > > title=Adblock Plus has been uninstalled > > template=simple > > > > <head> > > <script type="text/javascript"> > > var reasons = [ > > ["1v0", "{{reason-not-installed I didn't install Adblock Plus.}}"], > > ["2v0", "{{reason-slowing-down Adblock Plus slowed down my browser.}}"], > > ["3v0", "{{reason-acceptable-ads I don't like the Acceptable Ads > > program.}}"], > > ["4v0", "{{reason-see-ads Adblock Plus didn't block all ads.}}"], > > ["5v0", "{{reason-better-adblocker I found better ad blocking > > software.}}"], > > ["6v0", "{{reason-break-websites Adblock Plus breaks websites that I > > visit.}}"] > > ]; > > > > ... > > </script> > > </head> > > > > <section class="highlighted"> > > <h1>{{reasons-header Please select the reason(s) why you uninstalled Adblock > > Plus:</h1> > > > > <form id="reasons-form" action="uninstall-abp-submit" method="post"> > > <ul id="reasons"></ul> > > > > ... > > </form> > > ... > > </section> > > > > Notes: > > - Yes quotes in translated strings are escaped, I checked. > > - My example uses HTML for markup, you should obviously only use it where > > required and use Markdown if possible. > > - Please create a function called shuffle to do the shuffling and use that to > > shuffle the array, instead of mixing that logic with the rest of the > > initialisation logic. > > Also why not keep the string ID the same as the ID the data guys will use? > Instead of having "1v0" and "reason-not-installed" why not call both > "reason-1v0-not-installed" or something? It seems like extra work for them to > have to keep looking up string IDs like "1v0" -> "reason-not-installed" -> > "blah". It was requirement by data annalists to have request like: ?reason=1v0&reason=4v0&reason=2v0&other=some%20reason I can use 1v0, 2v0 and etc.. as Message ID, but not sure if it's okey to have meaningless Ids as messageId. Also it will require to keep in sync Message Ids and reason Ids together. kzar 2015/11/20 13:06:13 OK fair enough, thanks the latest version of this Show quoted text On 2015/11/19 17:00:17, saroyanm wrote: > On 2015/11/19 11:28:56, kzar wrote: > > On 2015/11/19 11:24:54, kzar wrote: > > > On 2015/11/18 18:00:18, saroyanm wrote: > > > > On 2015/11/18 17:14:45, kzar wrote: > > > > > I think it would make more sense to add a to_json Jinja2 filter, and > then > > > use > > > > > that to dump the reasons to a JavaScript array. You could then generate > > the > > > > > shuffled elements directly in JavaScript, instead of putting them here > and > > > > later > > > > > removing them + appending them again in JavaScript. > > > > Fare enough, I agree that this is not a good solution I have now. > > > > > > > > > In fact, you could simply declare the list of reasons as a JSON array in > > the > > > > > first place near the top of the file. Then you would no longer need the > > > > to_json > > > > > filter and this file wouldn't even need to be a template at all. > > > > Not sure if in that case I can get translations for reasons, also the > > > > implementation should be supported by translation script to be able to > > upload > > > > strings to crowdin, this also applies to the first solution you suggested. > > > > > > Make this page Markdown, you don't need it to be a Jinja2 template. Do > > something > > > like this: > > > > > > title=Adblock Plus has been uninstalled > > > template=simple > > > > > > <head> > > > <script type="text/javascript"> > > > var reasons = [ > > > ["1v0", "{{reason-not-installed I didn't install Adblock Plus.}}"], > > > ["2v0", "{{reason-slowing-down Adblock Plus slowed down my > browser.}}"], > > > ["3v0", "{{reason-acceptable-ads I don't like the Acceptable Ads > > > program.}}"], > > > ["4v0", "{{reason-see-ads Adblock Plus didn't block all ads.}}"], > > > ["5v0", "{{reason-better-adblocker I found better ad blocking > > > software.}}"], > > > ["6v0", "{{reason-break-websites Adblock Plus breaks websites that I > > > visit.}}"] > > > ]; > > > > > > ... > > > </script> > > > </head> > > > > > > <section class="highlighted"> > > > <h1>{{reasons-header Please select the reason(s) why you uninstalled > Adblock > > > Plus:</h1> > > > > > > <form id="reasons-form" action="uninstall-abp-submit" method="post"> > > > <ul id="reasons"></ul> > > > > > > ... > > > </form> > > > ... > > > </section> > > > > > > Notes: > > > - Yes quotes in translated strings are escaped, I checked. > > > - My example uses HTML for markup, you should obviously only use it where > > > required and use Markdown if possible. > > > - Please create a function called shuffle to do the shuffling and use that > to > > > shuffle the array, instead of mixing that logic with the rest of the > > > initialisation logic. > > > > Also why not keep the string ID the same as the ID the data guys will use? > > Instead of having "1v0" and "reason-not-installed" why not call both > > "reason-1v0-not-installed" or something? It seems like extra work for them to > > have to keep looking up string IDs like "1v0" -> "reason-not-installed" -> > > "blah". > > It was requirement by data annalists to have request like: > ?reason=1v0&reason=4v0&reason=2v0&other=some%20reason > > I can use 1v0, 2v0 and etc.. as Message ID, but not sure if it's okey to have > meaningless Ids as messageId. Also it will require to keep in sync Message Ids > and reason Ids together. OK fair enough, thanks the latest version of this page is looking a lot more sensible now.
73 <li>	82 <li>

74 <label>	83 <label>

75 <input type="checkbox" name="reason" value="{{reasonId}}" />	84 <input type="checkbox" name="reason" value="{{reasonId}}" />

76 <span>{{value\|translate(stringId, "One of the listed uninstallation re ason")}}</span>	85 {{value\|translate(stringId, "Uninstallation reason")}}

77 </label>	86 </label>

78 </li>	87 </li>

79 {%- endfor %}	88 {%- endfor %}

80 <li>	89 <li>

81 <label>	90 <label>

82 <input id="reason-other" type="checkbox" name="reason" value="0v0" />	91 <input id="reason-other" type="checkbox" name="reason" value="0v0" />

83 <span>{{"Other, namely..."\|translate("reason-other", "Last uninstallat ion reason")}}</span>	92 {{"Other, namely..."\|translate("reason-other", "Last uninstallation re ason")}}

84 </label>	93 </label>

85 <textarea id="reason-other-input" class="hidden" name="reasonOther" plac eholder="{{"Please explain why you are uninstalling Adblock Plus"\|translate("rea son-other-placeholder", "Textarea placeholder text, appears after selecting 'Oth er, namely...' option")}}"></textarea>	94 <textarea id="reason-other-input" class="hidden" name="reasonOther" plac eholder="{{"Please explain why you are uninstalling Adblock Plus"\|translate("rea son-other-placeholder", "Textarea placeholder text, appears after selecting 'Oth er, namely...' option")}}"></textarea>

86 </li>	95 </li>

87 </ul>	96 </ul>

88 <button id="submit-form" type="submit">{{"Submit"\|translate("submit", "Submi t button text")}}</button>	97 <button id="submit-form">{{"Submit"\|translate("submit", "Submit button text" )}}</button>

89 <span class="error-label">{{"Please select at least one of the options above "\|translate("error-msg", "Error message, is being shown after submission if no i tem selected")}}</span>	98 <span class="error-label">{{"Please select at least one of the options above "\|translate("error-msg", "Error message, is being shown after submission if no i tem selected")}}</span>

90 </form>	99 </form>

91 <p class="disclaimer">{{"By clicking Submit, you are sending your response to Adblock Plus. Please see our <a href='https://adblockplus.org/privacy'>privacy p olicy</a>."\|translate("disclaimer", "Disclaimer below form")}}</p>	100 <p class="disclaimer">

	101 {{"By clicking Submit, you are sending your response to Adblock Plus. Please see our <a href='/privacy'>privacy policy</a>."\|translate("disclaimer", "Discla imer below form")}}

	102 </p>

92 </section>	103 </section>

LEFT	RIGHT