Unified Diff: pages/uninstalled.tmpl
Issue 29348067:
Issue 4049 - reinstall button and adbockers list added to uninstallation page (Closed)
Patch Set: Fixed some syntax issues
Created July 20, 2016, 3:55 p.m.
Use n/p to move between diff chunks;
N/P to move between comments.
« no previous file with comments
|
« no previous file
|
static/css/simple.css » ('j')
|
static/css/simple.css » ('J')
Expand Comments ('e') | Collapse Comments ('c') | Hide Comments ('s')
Expand Comments ('e') | Collapse Comments ('c') | Hide Comments ('s')
| Index: pages/uninstalled.tmpl |
| =================================================================== |
| --- a/pages/uninstalled.tmpl |
| +++ b/pages/uninstalled.tmpl |
| @@ -2,12 +2,12 @@ |
| template=simple |
| {% set 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.") |
| + ("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.", ["AdBlock", "uBlock", "uBlock Origin", "Ghostery"]), |
| + ("6v0", "reason-break-websites", "Adblock Plus breaks websites that I visit.", "") |
| ] %} |
| <head> |
| @@ -35,7 +35,7 @@ |
| // Randomly add reasons |
| var reasonsContainer = document.getElementById("reasons"); |
| - var reasons = reasonsContainer.getElementsByTagName("li"); |
| + var reasons = document.querySelectorAll("#reasons > li"); |
| reasons = Array.prototype.slice.call(reasons); |
| reasonsContainer.innerHTML = ""; |
| while (reasons.length) |
| @@ -45,11 +45,25 @@ |
| reasonsContainer.appendChild(reasonElement); |
| } |
| - var reasonOtherCheckbox = document.getElementById("reason-other"); |
| - var reasonOther = document.getElementById("reason-other-container"); |
| - reasonOtherCheckbox.addEventListener("change", function() |
| + reasonsContainer.addEventListener("click", function(event) |
| { |
| - reasonOther.classList.toggle("hidden"); |
| + var toggleView = event.target.getAttribute("toggle-view"); |
| + if (toggleView) |
| + document.getElementById(toggleView).classList.toggle("hidden"); |
| + }, false); |
| + |
| + var adblockersList = document.querySelector("#adblockers select"); |
| + adblockersList.addEventListener("change", function() |
| + { |
| + var selectedOption = adblockersList[adblockersList.selectedIndex]; |
| + var element = selectedOption.getAttribute("show-element"); |
| + if (element) |
| + document.getElementById(element).classList.remove("hidden"); |
|
juliandoucette
2016/07/28 23:05:41
classList is not supported < IE11
saroyanm
2016/08/24 11:48:41
Uninstallation page is only being shown to the Chr
juliandoucette
2016/09/24 16:49:33
Acknowledged.
|
| + else |
| + { |
| + element = selectedOption.getAttribute("hide-element"); |
| + document.getElementById(element).classList.add("hidden"); |
| + } |
| }, false); |
| var reasonOtherInput = document.getElementById("reason-other-input"); |
| @@ -80,30 +94,51 @@ |
| </script> |
| </head> |
| +<section class="clear"> |
|
juliandoucette
2016/07/28 23:05:42
I suggest changing this <strong> to a heading or t
saroyanm
2016/08/24 11:48:42
Fare enough, done.
juliandoucette
2016/09/24 16:49:33
I should have suggested changing this section to a
|
| + <strong>{{"Did you uninstall Adblock Plus by accident?"|translate("reinstall-headline", "Text next to the Reinstallation button")}}</strong> |
| + {{"index"|linkify}}{{"Reinstall Now"|translate("reinstall", "Reinstall button text")}}</a> |
| +</section> |
| <section class="highlighted"> |
| <h1>{{"Please select the reason(s) why you uninstalled Adblock Plus:"|translate("reasons-header", "Form heading")}}</h1> |
| <form id="reasons-form" action="uninstalled-submit" method="get"> |
| <ul id="reasons"> |
| - {%- for reasonId, stringId, value in reasons %} |
| + {%- for reasonId, stringId, value, adblockers in reasons %} |
| <li> |
| <label> |
| - <input type="checkbox" name="reason" value="{{reasonId}}" /> |
| + <input type="checkbox" name="reason" {% if adblockers %} toggle-view="adblockers" {% endif %} value="{{reasonId}}" /> |
| {{value|translate(stringId, "Uninstallation reason")}} |
| </label> |
| + {%- if adblockers %} |
| + <ul class="hidden" id="adblockers"> |
|
juliandoucette
2016/07/28 23:05:41
Why use a <ul> here?
Perhaps a fieldset would be
saroyanm
2016/08/24 11:48:41
Done.
|
| + <li> |
| + <label> |
|
juliandoucette
2016/07/28 23:05:41
Why use a label if there is no label text?
saroyanm
2016/08/24 11:48:42
Done.
|
| + <select> |
| + {%- for name in adblockers %} |
| + <option name="otherAdblockerPredefined" hide-element="other-adblocker" value="{{name}}">{{name}}</option> |
|
juliandoucette
2016/07/28 23:05:41
Invalid attribute "hide-element". See https://deve
saroyanm
2016/08/24 11:48:42
Done.
|
| + {%- endfor %} |
| + <option name="otherAdblockerPredefined" show-element="other-adblocker" value="Other">{{"Other"|translate("other", "Option in list of Adblockers")}}</option> |
|
juliandoucette
2016/07/28 23:05:41
Invalid attribute "show-element". See https://deve
saroyanm
2016/08/24 11:48:41
Done.
|
| + </select> |
| + <input id="other-adblocker" class="hidden" type="text" name="otherAdblocker" /> |
| + </label> |
| + </li> |
| + </ul> |
| + {%- endif %} |
| </li> |
| {%- endfor %} |
| <li> |
| <label> |
| - <input id="reason-other" type="checkbox" name="reason" value="0v0" /> |
| + <input id="reason-other" toggle-view="reason-other-container" type="checkbox" name="reason" value="0v0" /> |
|
juliandoucette
2016/07/28 23:05:42
Invalid attribute "toggle-view". See https://devel
saroyanm
2016/08/24 11:48:41
Done.
|
| {{"Other, namely..."|translate("reason-other", "Last uninstallation reason")}} |
| </label> |
| - <div id="reason-other-container" class="hidden"> |
| - <textarea id="reason-other-input" name="reasonOther" maxlength="300" placeholder="{{"Please explain why you are uninstalling Adblock Plus"|translate("reason-other-placeholder", "Textarea placeholder text, appears after selecting 'Other, namely...' option")}}"></textarea> |
| - <div> |
| - {{"Characters remaining:"|translate("countdown-text", "Text of character limit counter appears after selecting 'Other, namely...' option")}} |
| - <span id="characters-countdown"></span> |
| - </div> |
| - </div> |
| + <ul id="reason-other-container" class="hidden"> |
|
juliandoucette
2016/07/28 23:05:41
Why use a ul here?
Perhaps a fieldset would be mo
juliandoucette
2016/07/28 23:27:54
See https://www.w3.org/TR/html5/forms.html#the-fie
saroyanm
2016/08/24 11:48:41
I kinda was thinking that fieldset should require
|
| + <li> |
| + <textarea id="reason-other-input" name="reasonOther" maxlength="300" placeholder="{{"Please explain why you are uninstalling Adblock Plus"|translate("reason-other-placeholder", "Textarea placeholder text, appears after selecting 'Other, namely...' option")}}"></textarea> |
| + <div> |
| + {{"Characters remaining:"|translate("countdown-text", "Text of character limit counter appears after selecting 'Other, namely...' option")}} |
| + <span id="characters-countdown"></span> |
| + </div> |
| + </li> |
| + </ul> |
| </li> |
| </ul> |
| <button id="submit-form">{{"Submit"|translate("submit", "Submit button text")}}</button> |
« no previous file with comments
|
« no previous file
|
static/css/simple.css » ('j')
|
static/css/simple.css » ('J')
