UI Patches for Immediate fixes (#17911)

* UI Patches for Immediate fixes
* Design review feedback

Author: zoracon

chromium/images/icons/SettingsIcon.png

@@ -13,7 +13,7 @@ <h1 id="https-everywhere"><img src="../../images/banner-red.png" alt="HTTPS Ever
       <p data-i18n="cancel_he_blocking_explainer"></p>
 
       <p id="url-paragraph">
-        <span id="url-label">URL: </span><a href="#" id="url-value">PLACEHOLDER</a>
+        <span id="url-label">URL: </span><span id="url-value">PLACEHOLDER</span>
       </p>
 
       <form method="get" action="#" id="url-actions-form">

chromium/pages/cancel/index.html

@@ -11,10 +11,6 @@
     <span class="section-header-span inactive" data-show="update-channels-wrapper" data-i18n="options_updateChannels"></span>
 
     <div class="section-wrapper" id="general-settings-wrapper">
-      <div id="counter-wrapper" class="settings-wrapper">
-        <input type="checkbox" id="showCounter">
-        <label for="showCounter" data-i18n="menu_showCounter"></label>
-      </div>
       <div id="update-wrapper" class="settings-wrapper">
         <input type="checkbox" id="autoUpdateRulesets">
         <label for="autoUpdateRulesets" data-i18n="options_autoUpdateRulesets"></label>

chromium/pages/options/index.html

@@ -7,28 +7,21 @@
 
 document.addEventListener("DOMContentLoaded", () => {
 
-  const showCounter = document.getElementById("showCounter");
   const autoUpdateRulesets = document.getElementById("autoUpdateRulesets");
   const enableMixedRulesets = document.getElementById("enableMixedRulesets");
   const showDevtoolsTab = document.getElementById("showDevtoolsTab");
 
   const defaultOptions = {
-    showCounter: true,
     autoUpdateRulesets: true,
     enableMixedRulesets: false,
     showDevtoolsTab: true
   };
 
   sendMessage("get_option", defaultOptions, item => {
-    showCounter.checked = item.showCounter;
     autoUpdateRulesets.checked = item.autoUpdateRulesets;
     enableMixedRulesets.checked = item.enableMixedRulesets;
     showDevtoolsTab.checked = item.showDevtoolsTab;
 
-    showCounter.addEventListener("change", () => {
-      sendMessage("set_option", { showCounter: showCounter.checked });
-    });
-
     autoUpdateRulesets.addEventListener("change", () => {
       sendMessage("set_option", { autoUpdateRulesets: autoUpdateRulesets.checked });
     });

chromium/pages/options/ux.js

@@ -35,29 +35,23 @@ <h1 id="onoffswitch_label" data-i18n="menu_globalEnable"></h1>
       <h1 id="HttpNowhere__header" data-i18n="menu_encryptAllSitesEligible"></h1>
       <input aria-hidden="true" id="http-nowhere-checkbox" type="checkbox"><label id="http-nowhere-checkbox_label" aria-label="Toggle on or off" for="http-nowhere-checkbox"></label>
       <h2 id="HttpNowhere__explained" data-i18n="menu_httpNoWhereExplained"></h2>
-      <span id="HttpNowhere__see_more" class="see_more__arrow down" aria-label="Listen or See more explanation"></span>
-      <span id="HttpNowhere__see_more--prompt" class="see_more__prompt" data-i18n="menu_seeMore"></span>
-      <div class="see_more__content hide">
-        <p class="see_more--clarified" data-i18n="menu_httpNoWhereMore"></p>
-      </div>
     </section>
 
     <section id="RuleManagement" class="settings_block">
       <section id="settingsForThisSite">
         <h1 data-i18n="chrome_settings_for_this_site_header"></h1>
-        <img class="settingsForThisSite__gear-icon" src="../../images/icons/SettingsIcon.png" alt="gear icon" aria-hidden="true" >
         <h2 data-i18n="chrome_settings_for_this_site_subheader"></h2>
       </section>
-
-      <span id="RuleManagement__see_more" class="see_more__arrow down" aria-label="Listen or See more explanation"></span>
-      <span id="RuleManagement__see_more--prompt" class="see_more__prompt" data-i18n="menu_seeMore"></span>
-      <div class="see_more__content hide">
-        <p class="see_more--clarified" data-i18n="chrome_settings_for_this_site_explained"></p>
-
-        <section id="disableEnableSection">
+      <section id="disableEnableSection">
           <a href="javascript:void 0" id="disable-on-this-site" class="button" data-i18n="chrome_disable_on_this_site"></a>
           <a href="javascript:void 0" id="enable-on-this-site" class="button" data-i18n="chrome_enable_on_this_site"></a>
-        </section>
+      </section>
+      <div id="RuleManagement__see_more--prompt" class="see_more__prompt" aria-label="Listen or See more for explanation">
+        <span id="RuleManagement__see_more" class="see_more__arrow down"></span>
+        <span class="see_more__text" data-i18n="menu_seeMore"></span>
+      </div>
+      <div class="see_more__content hide">
+        <p class="see_more--clarified" data-i18n="chrome_settings_for_this_site_explained"></p>
 
         <section id="addRuleSection">
           <a href="javascript:void 0" id="add-rule-link" class="button" data-i18n="chrome_add_rule"></a>

chromium/pages/popup/index.html

@@ -18,7 +18,7 @@ body {
 header {
   background-color: var(--light-grey);
   color: var(--text-main);
-  display: block;
+  display: flex;
   padding: var(--space);
 }
 
@@ -27,12 +27,12 @@ header h1 {
 }
 
 header .logo-container {
-  width: 100%;
-  float: left;
+  margin: var(--space);
+  width: 85%;
 }
 
 header .logo-container img {
-  width: 70%;
+  width: 100%;
 }
 
 header small {
@@ -44,36 +44,41 @@ header small {
   width: 100%;
 }
 
-h1,
-h2,
-h3 {
-  font-size: 1em;
-}
-
 h1 {
+  font-size: 16px;
   margin: var(--space) auto
 }
 
 h2 {
   clear: both;
+  font-size: 12px;
   font-weight: normal;
   margin: 0;
 }
 
+h3 {
+  font-size: 16px;
+}
+
 h1,
 h2 {
   color: var(--text-main);
 }
 
 footer {
+  display: inline-flex;
+  margin: var(--space);
+}
+
+footer a.button {
   margin: var(--space);
 }
 
 /* Elements */
 .settings_block {
   display: inline-block;
-  padding: 20px 1em;
-  border-bottom: #000 solid 1px;
+  padding: 5px 1em 20px 1em;
+  border-bottom: var(--light-grey) solid 1px;
 }
 
 .button {
@@ -83,7 +88,7 @@ footer {
   font-size: 12px;
   font-weight: bold;
   margin: var(--space) auto;
-  padding: 15px;
+  padding: 10px;
   text-align: center;
   text-decoration: none;
 }
@@ -96,23 +101,23 @@ footer {
 .see_more__prompt {
   color: var(--text-main);
   cursor: pointer;
-  font-size: .9em;
+  font-size: 12px;
   float: right;
   margin: 5px 0;
 }
 
 .see_more__arrow {
   border: solid var(--darker-blue);
   border-width: 0 3px 3px 0;
-  cursor: pointer;
   float: right;
   margin-left: 10px;
-  margin-top: var(--space);
+  margin-top: 3px;
   padding: 3px;
 }
 
 .see_more__content.show p {
-  margin: 10px 0 0 0;
+  font-size: 12px;
+  margin: 10px 0;
 }
 
 .down {
@@ -196,7 +201,7 @@ input:checked+label:after {
 }
 
 .rule.checkbox label {
-  font-size: .9em;
+  font-size: 12px;
   font-weight: normal;
   height: 25px;
   text-indent: -285px;
@@ -208,6 +213,10 @@ input:checked+label:after {
   width: 15px;
 }
 
+#disable-on-this-site {
+  clear: both;
+}
+
 #disableButton {
   display: block;
 }
@@ -217,17 +226,17 @@ input:checked+label:after {
   float: left;
 }
 
-#settingsForThisSite h1 {
-  float: left;
+#RuleManagement {
+  padding: 5px 1em 10px 1em;
 }
 
-#settingsForThisSite img {
-  float: right;
-  width: 30px;
+#settingsForThisSite h1 {
+  float: left;
 }
 
 #addRuleSection label,
 #addRuleSection button {
+  font-size: 14px;
   width: 100%;
 }
 
@@ -247,11 +256,15 @@ input:checked+label:after {
 
 #HttpNowhere {
   display: block;
-  padding: 25px 1em;
+  padding: 5px 1em;
 }
 
 #StableRules h3 {
   float: left;
+  margin-left: 10px;
+}
+#StableRules h2 {
+  margin-left: 10px;
 }
 #StableRules #RuleManagement--counter {
   background: var(--darker-blue);
@@ -262,16 +275,16 @@ input:checked+label:after {
   font-size: 12px;
   font-weight: bold;
   height: 15px;
-  margin-top: 5px;
-  margin-right: 60%;
+  margin-top: 10px;
+  margin-right: 55%;
   padding: 5px;
   text-align: center;
   width: 15px;
 }
 
 /* Event based */
 .see_more--clarified {
-  font-size: .9em;
+  font-size: 12px;
 }
 
 /* Hide rules & options if the extension is off. */
@@ -302,14 +315,15 @@ section.rules {
 /* For "see more" content */
 .hide {
   height: 0;
-  visibility: hidden;
   opacity: 0;
   overflow: hidden;
   transition: 0s;
+  visibility: hidden;
 }
 
 .show {
-  visibility: visible;
+  clear: both;
   opacity: 1;
   transition: visibility 0s linear 0s, opacity 300ms;
+  visibility: visible;
 }

chromium/pages/popup/style.css

@@ -33,34 +33,30 @@ function toggleRuleLine(event) {
  * @description Toggles content for user to view rules and explanations for different modes
  */
 function toggleSeeMore(event) {
-  /*
-    @todo
-    probably could put this all in one div so two events aren't being read for this section
-  */
-  let target = event.target.parentNode.querySelector('.see_more__content');
-  let arrow, prompt;
-
-  if(event.target.classList.contains('see_more__arrow')) {
-    arrow  = event.target;
-    prompt = arrow.parentNode.querySelector('.see_more__prompt');
+  let target = event.target;
+  let content;
+
+  if (target !== this) {
+    content = document.querySelector('.see_more__content');
   } else {
-    // Reverse logic if user clicks text instead of the arrow.
-    prompt = event.target;
-    arrow = prompt.parentNode.querySelector('.see_more__arrow');
+    content = target.parentNode.querySelector('.see_more__content');
   }
 
+  let arrow  = target.parentNode.querySelector('.see_more__arrow');
+  let text = target.parentNode.querySelector('.see_more__text');
+
   if(arrow.classList.contains('down')) {
     arrow.classList.replace('down', 'up');
-    prompt.innerText = 'See less';
+    text.innerText = 'See less';
   } else if (arrow.classList.contains('up')) {
     arrow.classList.replace('up', 'down');
-    prompt.innerText = 'See more';
+    text.innerText = 'See more';
   }
 
-  if (target.classList.contains('hide')) {
-    target.classList.replace('hide', 'show');
-  } else if (target.classList.contains('show')) {
-    target.classList.replace('show', 'hide');
+  if (content.classList.contains('hide')) {
+    content.classList.replace('hide', 'show');
+  } else if (content.classList.contains('show')) {
+    content.classList.replace('show', 'hide');
   }
 }
 
@@ -193,7 +189,7 @@ function listRules(activeTab) {
       appendRulesToListDiv(unstableRules, e("UnstableRules"), 'unstable');
 
       // Add listener to capture the toggle event
-      e("RuleManagement").addEventListener("click", toggleRuleLine);
+      e("add-new-rule-button").addEventListener("click", toggleRuleLine);
     }
 
     // Only show the "Add a rule" section if we're on an HTTPS page
@@ -221,10 +217,6 @@ document.addEventListener("DOMContentLoaded", function () {
   updateEnabledDisabledUI();
   e('onoffswitch').addEventListener('click', toggleEnabledDisabled);
   e('http-nowhere-checkbox').addEventListener('click', toggleHttpNowhere, false);
-
-  e('HttpNowhere__see_more').addEventListener('click', toggleSeeMore);
-  e('RuleManagement__see_more').addEventListener('click', toggleSeeMore);
-  e('HttpNowhere__see_more--prompt').addEventListener('click', toggleSeeMore);
   e('RuleManagement__see_more--prompt').addEventListener('click', toggleSeeMore);
 
   e('reset-to-defaults').addEventListener('click', () => {

chromium/pages/popup/ux.js

@@ -9,25 +9,3 @@ def load_options(self):
     def test_options(self):
         self.load_options()
         self.assertEqual(self.driver.current_url, self.shim.options_url)
-
-    def test_show_counter(self):
-        if self.shim.browser_type == 'chrome':
-            raise unittest.SkipTest('broken on chrome')
-        selector = '#showCounter'
-        self.load_options()
-        sleep(3)
-
-        el = self.query_selector(selector)
-        self.assertTrue(el.is_selected())
-        el.click()
-
-        self.driver.refresh()
-        sleep(3)
-        el = self.query_selector(selector)
-        self.assertFalse(el.is_selected())
-        el.click()
-
-        self.driver.refresh()
-        sleep(3)
-        el = self.query_selector(selector)
-        self.assertTrue(el.is_selected())