Issue 5326 - General tab (HTML, strings and functionality)

new-options.html

Index: new-options.html
===================================================================
--- a/new-options.html
+++ b/new-options.html
@@ -22,7 +22,6 @@
     <title class="i18n_options_page_title"></title>
     <link rel="stylesheet" href="skin/common.css">
     <link rel="stylesheet" href="skin/new-options.css">
-    <style id="search-style"></style>
     <script src="ext/common.js"></script>
     <script src="ext/content.js"></script>
     <script src="common.js"></script>
@@ -87,85 +86,71 @@
 
         <!-- General tab content -->
         <div id="content-general" role="tabpanel" aria-labelledby="tab-general">
-          <div>
-            <h1 class="i18n_options_blocking_title"></h1>
-            <div class="hbox">
-              <div id="blocking-languages">
-                <div class="option-name">
-                  <span class="i18n_options_language_title"></span>
-                  <span class="i18n_options_readMore"
-                      data-tooltip="options_language_title_tooltip"
-                      data-tooltip-image="skin/tooltips/block.png"></span>
-                </div>
-                <ul id="blocking-languages-table" class="table list">
-                  <template>
-                    <button data-action="toggle-disable-subscription" role="checkbox" class="control" title="options_control_toggle_title"></button>
-                    <label class="display"></label>
-                    <button data-action="remove-subscription" class="delete control" title="options_control_remove_title"></button>
-                  </template>
-                </ul>
-                <div class="controls">
-                  <button data-action="open-dialog" data-dialog="language">
-                    <span class="icon icon-add"></span>
-                    <span class="i18n_options_language_add"></span>
-                  </button>
-                </div>
+          <h1 class="i18n_options_tab_general"></h1>
+          <p class="i18n_options_general_description"></p>
+          <section>
+            <h2 class="i18n_options_privacy_header"></h2>
+            <ul id="recommend-security-list-table" class="table list">
+              <template>
+                <button data-action="toggle-remove-subscription" role="checkbox" class="control"></button>
+                <label class="display"></label>
+                <span class="i18n_options_readMore" data-tooltip="options_more_%value%_tooltip"></span>
+              </template>
+            </ul>
+          </section>
+          <section>
+            <h2 class="i18n_options_aa_header"></h2>
+            <p class="i18n_options_aa_description"></p>
+            <form id="acceptable-ads">
+              <div>
+                <input data-action="enable-aa" type="radio" name="acceptable-ads" value="tracking">

[Thomas Greiner, 2017/08/09 18:14:48]

Detail: I noticed that you created three messages ("block-all", "enable-aa" and
"enable-privacy-aa"") which do pretty much the same thing (i.e. change the
status of Acceptable Ads).

Therefore I'd recommend grouping them together into a single message and
determine what to do based on the value. For that I'd recommend using more
useful values such as "ads", "ads,privacy", "privacy" and "".
Thereby we'd also avoid using the word "tracking" because it implies that it's
about allowing tracking rather than allowing ads.

e.g.
<input
  data-action="switch-acceptable"
  type="radio"
  name="acceptable-ads"
  value="ads,privacy">

[saroyanm, 2017/08/14 14:00:08]

I'll do in your suggested way.

[saroyanm, 2017/08/16 14:17:35]

Done, I used none, for the block all option value, to match the value using CSS
selectors easily.

+                <label class="i18n_options_aa_tracking_label"></label>
+                <p id="enable-aa-description" class="i18n_options_aa_tracking_description_1"></p>
+                <p class="i18n_options_aa_tracking_description_2"></p>
               </div>
-              <div id="further-blocking">
-                <div class="option-name">
-                  <strong class="i18n_options_more_title"></strong>
-                  <span class="i18n_options_readMore"
-                      data-tooltip="options_more_title_tooltip"
-                      data-tooltip-image="skin/tooltips/more.png"
-                      data-tooltip-flip="horizontal"></span>
-                </div>
-                <div id="custom-wrapper">
-                  <ul id="recommend-list-table" class="table list">
-                    <template>
-                      <button data-action="toggle-remove-subscription" role="checkbox" class="control"></button>
-                      <div class="fill">
-                        <label class="display"
-                            data-tooltip="options_more_%value%_tooltip"
-                            data-tooltip-native></label>
-                      </div>
-                    </template>
-                  </ul>
-                  <ul id="custom-list-table" class="table list">
-                    <template>
-                      <button data-action="toggle-disable-subscription" role="checkbox" class="control" title="options_control_toggle_title"></button>
-                      <label class="display"></label>
-                      <button data-action="remove-subscription" class="delete control" title="options_control_remove_title"></button>
-                    </template>
-                  </ul>
-                </div>
-                <div class="controls">
-                  <button data-action="open-dialog" data-dialog="custom">
-                    <span class="icon icon-add"></span>
-                    <span class="i18n_options_more_add"><span>
-                  </button>
-                </div>
+              <div>
+                <input data-action="enable-privacy-aa" type="radio" name="acceptable-ads" value="no-tracking">
+                <label class="i18n_options_aa_no_tracking_label"></label>
+                <p class="i18n_options_aa_no_tracking_description"></p>
+                <p id="no-dnt" class="i18n_options_aa_no_dnt_notification"></p>

[Thomas Greiner, 2017/08/09 18:14:48]

Suggestion: Can we drop the "no" in "no-dnt" and
"options_aa_no_dnt_notification"? Because it could make it easier to understand
and there's no other DNT-related notification that we have anyway.

[saroyanm, 2017/08/14 14:00:08]

Acknowledged.

[saroyanm, 2017/08/16 14:17:35]

Done.

+              </div>
+              <div>
+                <input data-action="block-all" type="radio" name="acceptable-ads" value="disable">
+                <label class="i18n_options_aa_disable_label"></label>
+              </div>
+            </form>
+          </section>
+          <section>
+            <h2 class="i18n_options_language_header"></h2>
+            <p class="i18n_options_language_description"></p>
+            <div id="blocking-languages">
+              <ul id="blocking-languages-table" class="table list">
+                <template>
+                  <label class="display"></label>
+                  <button data-single="hidden" data-action="remove-subscription" class="delete control" title="options_control_remove_title"></button>
+                  <button data-single="visible" data-action="save-change-subscription,open-dialog" data-dialog="language-change" class="i18n_options_language_change"></button>
+                </template>
+              </ul>
+              <div class="controls">
+                <button data-action="open-dialog" data-dialog="language-add">
+                  <span class="i18n_options_language_add"></span>
+                </button>
               </div>
             </div>
-          </div>
-          <div>
-            <h1 class="i18n_options_exceptions_title"></h1>
-            <div class="hbox">
-              <div id="acceptable-ads">
-                <div class="option-name">
-                  <strong class="i18n_options_acceptableAds_title"></strong>
-                  <span class="i18n_options_readMore"
-                      data-tooltip="options_acceptableAds_title_tooltip"
-                      data-tooltip-image="skin/tooltips/acceptable-ads.png"></span>
-                </div>
-                <ul id="acceptableads-table" class="table list">
-                  <template>
-                    <button data-action="toggle-remove-subscription" role="checkbox" class="control"></button>
-                    <label class="display"></label>
-                  </template>
-                </ul>
-              </div>
-            </div>
-          </div>
+            <p class="i18n_options_language_tip"></p>
+          </section>
+          <section>
+            <h2 class="i18n_options_more_filters_header"></h2>
+            <p class="i18n_options_more_filters_description"></p>
+            <ul id="custom-list-table" class="table list">
+              <template>
+                <label class="display"></label>
+                <span class="i18n_options_readMore" data-tooltip="options_more_%value%_tooltip"></span>
+                <button data-action="remove-subscription" class="delete control" title="options_control_remove_title"></button>
+              </template>
+            </ul>
+            <p class="i18n_options_more_filters_note"></p>
+          </section>
         </div>
 
         <!-- Whitelist tab content -->
@@ -344,36 +329,32 @@
       <header>
         <span id="dialog-title">
           <span id="dialog-title-custom" class="i18n_options_dialog_custom_title"></span>
-          <span id="dialog-title-language" class="i18n_options_dialog_language_title"></span>
+          <span id="dialog-title-language-add" class="i18n_options_dialog_language_title"></span>
+          <span id="dialog-title-language-change" class="i18n_options_dialog_language_title"></span>
           <span id="dialog-title-predefined" class="i18n_options_dialog_predefined_title"></span>
         </span>
         <button id="dialog-close" class="i18n_options_close focus-first" data-action="close-dialog"></button>
       </header>
       <div id="dialog-body" class="content">
         <!-- Add language subscription -->
-        <div id="dialog-content-language" class="dialog-content">
-          <div class="dialog-content-block">
-            <h3 class="i18n_options_dialog_language_added"></h3>
-            <ul id="blocking-languages-dialog-table" class="table list">
-              <template>
+        <div id="dialog-content-language-add" class="dialog-content">
+          <ul id="all-lang-table-add" class="table list">
+            <template>
+              <button data-action="add-language-subscription,close-dialog" class="control" role="checkbox">
                 <label class="display"></label>
-              </template>
-            </ul>
-          </div>
-          <div id="other-language" class="dialog-content-block">
-            <div>
-              <label for="find-language" class="i18n_options_dialog_language_other"></label>
-              <input type="search" id="find-language" class="default-focus" />
-            </div>
-            <ul id="all-lang-table" class="table list">
-              <template>
-                <button data-action="add-language-subscription" class="button-add control">
-                  +<span class="i18n_options_button_add"></span>
-                </button>
+              </button>
+            </template>
+          </ul>
+        </div>
+        <!-- Change language subscription -->

[Thomas Greiner, 2017/08/09 18:14:48]

This is pretty much exactly the same dialog as the one above. The only
difference is the button action so it'd be great if you could avoid this
duplication.

[saroyanm, 2017/08/14 14:00:08]

agree

[saroyanm, 2017/08/16 14:17:35]

Done.

+        <div id="dialog-content-language-change" class="dialog-content">
+          <ul id="all-lang-table-change" class="table list">
+            <template>
+              <button data-action="change-language-subscription,close-dialog" class="control" role="checkbox">
                 <label class="display"></label>
-              </template>
-            </ul>
-          </div>
+              </button>
+            </template>
+          </ul>
         </div>
         <!-- Add custom subscription -->
         <div id="dialog-content-custom" class="dialog-content">