Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code
Issues
All Issues

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.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « no previous file | static/css/simple.css » ('j') | static/css/simple.css » ('J')
Expand Comments ('e') | Collapse Comments ('c') | Show Comments 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')

Powered by Google App Engine
This is Rietveld