Bug 1639177 - Align checkbox in What's New panel to start r=Gijs,fluent-reviewers
authorAndrei Oprea <andrei.br92@gmail.com>
Thu, 21 May 2020 09:45:37 +0000
changeset 531408 63e434e9fec084b475d7b6e57ee07e881ab8ad46
parent 531407 a291f439f35a9a96948de0bc6acc07bbf3a8625c
child 531409 1c58f39177c0ea2e3bbd22fbb92fab39f8c9d2e0
push id37439
push userbtara@mozilla.com
push dateThu, 21 May 2020 21:49:34 +0000
treeherdermozilla-central@92c11f0bf14b [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersGijs, fluent-reviewers
bugs1639177
milestone78.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1639177 - Align checkbox in What's New panel to start r=Gijs,fluent-reviewers Differential Revision: https://phabricator.services.mozilla.com/D75943
browser/components/customizableui/content/panelUI.inc.xhtml
browser/components/newtab/lib/ToolbarPanelHub.jsm
browser/components/newtab/test/unit/lib/ToolbarPanelHub.test.js
browser/locales/en-US/browser/appmenu.ftl
browser/locales/en-US/browser/browserContext.ftl
browser/themes/shared/customizableui/panelUI.inc.css
--- a/browser/components/customizableui/content/panelUI.inc.xhtml
+++ b/browser/components/customizableui/content/panelUI.inc.xhtml
@@ -984,22 +984,20 @@
         <toolbaritem id="PanelUI-whatsNew-content"
                      orient="vertical"
                      smoothscroll="false">
           <html:div id="PanelUI-whatsNew-message-container" role="document">
             <!-- What's New messages will be rendered here -->
           </html:div>
         </toolbaritem>
       </vbox>
-      <toolbarbutton class="subviewbutton panel-subview-footer"
-                     onclick="ToolbarPanelHub.toggleWhatsNewPref(event)">
-        <checkbox id="panelMenu-toggleWhatsNew"
-                  class="panelMenu-toggleWhatsNew-checkbox"
-                  data-l10n-id="cfr-whatsnew-panel-footer-checkbox"/>
-      </toolbarbutton>
+      <checkbox id="panelMenu-toggleWhatsNew"
+                class="panelMenu-toggleWhatsNew-checkbox"
+                oncommand="ToolbarPanelHub.toggleWhatsNewPref(event)"
+                data-l10n-id="whatsnew-panel-footer-checkbox"/>
     </panelview>
   </panelmultiview>
 </panel>
 
 <html:template id="extensionNotificationTemplate">
   <panel id="extension-notification-panel"
          class="popup-notification-panel panel-no-padding"
          role="group"
--- a/browser/components/newtab/lib/ToolbarPanelHub.jsm
+++ b/browser/components/newtab/lib/ToolbarPanelHub.jsm
@@ -80,23 +80,17 @@ class _ToolbarPanelHub {
     return this._getMessages({
       template: "whatsnew_panel_message",
       triggerId: "whatsNewPanelOpened",
       returnAll: true,
     });
   }
 
   toggleWhatsNewPref(event) {
-    event.stopPropagation();
-    event.preventDefault();
-    const [checkbox] = event.target.getElementsByTagName("checkbox");
-    const value = checkbox.checked;
-
-    checkbox.checked = !value;
-    Preferences.set(WHATSNEW_ENABLED_PREF, !value);
+    Preferences.set(WHATSNEW_ENABLED_PREF, event.target.checked);
   }
 
   maybeInsertFTL(win) {
     win.MozXULElement.insertFTLIfNeeded("browser/newtab/asrouter.ftl");
     win.MozXULElement.insertFTLIfNeeded("browser/branding/brandings.ftl");
     win.MozXULElement.insertFTLIfNeeded("browser/branding/sync-brand.ftl");
   }
 
--- a/browser/components/newtab/test/unit/lib/ToolbarPanelHub.test.js
+++ b/browser/components/newtab/test/unit/lib/ToolbarPanelHub.test.js
@@ -186,79 +186,30 @@ describe("ToolbarPanelHub", () => {
 
     assert.calledOnce(instance.enableAppmenuButton);
   });
   it("should unregisterCallback on uninit()", () => {
     instance.uninit();
     assert.calledTwice(everyWindowStub.unregisterCallback);
   });
   describe("#toggleWhatsNewPref", () => {
-    it("should prevent clicks from bubbling", () => {
+    it("should call Preferences.set() with the checkbox value", () => {
       let checkbox = {};
-      let event = {
-        target: {
-          getElementsByTagName: fakeDocument.getElementsByTagName
-            .withArgs("checkbox")
-            .returns([checkbox]),
-        },
-        stopPropagation: sinon.stub(),
-        preventDefault: sinon.stub(),
-      };
-
-      instance.toggleWhatsNewPref(event);
-
-      assert.calledOnce(event.stopPropagation);
-      assert.calledOnce(event.preventDefault);
-    });
-    it("should call Preferences.set() with true when the checkbox is checked", () => {
-      let checkbox = {};
-      let event = {
-        target: {
-          getElementsByTagName: fakeDocument.getElementsByTagName
-            .withArgs("checkbox")
-            .returns([checkbox]),
-        },
-        stopPropagation: sinon.stub(),
-        preventDefault: sinon.stub(),
-      };
+      let event = { target: checkbox };
       // checkbox starts false
       checkbox.checked = false;
 
       // toggling the checkbox to set the value to true
       instance.toggleWhatsNewPref(event);
 
       assert.calledOnce(preferencesStub.set);
       assert.calledWith(
         preferencesStub.set,
         "browser.messaging-system.whatsNewPanel.enabled",
-        true
-      );
-    });
-    it("should call Preferences.set() with false when the checkbox is unchecked", () => {
-      let checkbox = {};
-      let event = {
-        target: {
-          getElementsByTagName: fakeDocument.getElementsByTagName
-            .withArgs("checkbox")
-            .returns([checkbox]),
-        },
-        stopPropagation: sinon.stub(),
-        preventDefault: sinon.stub(),
-      };
-      // checkbox starts true
-      checkbox.checked = true;
-
-      // toggling the checkbox to set the value to false
-      instance.toggleWhatsNewPref(event);
-
-      assert.calledOnce(preferencesStub.set);
-      assert.calledWith(
-        preferencesStub.set,
-        "browser.messaging-system.whatsNewPanel.enabled",
-        false
+        checkbox.checked
       );
     });
   });
   describe("#enableAppmenuButton", () => {
     it("should registerCallback on enableAppmenuButton() if there are messages", async () => {
       await instance.init(waitForInitializedStub, {
         getMessages: sandbox.stub().resolves([{}, {}]),
       });
--- a/browser/locales/en-US/browser/appmenu.ftl
+++ b/browser/locales/en-US/browser/appmenu.ftl
@@ -12,8 +12,16 @@ appmenuitem-zoom-enlarge =
   .label = Zoom in
 appmenuitem-zoom-reduce =
   .label = Zoom out
 
 ## Firefox Account toolbar button and Sync panel in App menu.
 
 fxa-toolbar-sync-now =
     .label = Sync Now
+
+## What's New panel in App menu.
+
+# Checkbox displayed at the bottom of the What's New panel, allowing users to
+# enable/disable What's New notifications.
+whatsnew-panel-footer-checkbox =
+  .label = Notify about new features
+  .accesskey = f
--- a/browser/locales/en-US/browser/browserContext.ftl
+++ b/browser/locales/en-US/browser/browserContext.ftl
@@ -382,13 +382,8 @@ main-context-menu-inspect-element =
     .accesskey = Q
 
 main-context-menu-inspect-a11y-properties =
     .label = Inspect Accessibility Properties
 
 main-context-menu-eme-learn-more =
     .label = Learn more about DRM…
     .accesskey = D
-
-# Checkbox displayed at the bottom of the What's New panel, allowing users to 
-# enable/disable What's New notifications.
-cfr-whatsnew-panel-footer-checkbox = 
-  .label = Notify about new features    
--- a/browser/themes/shared/customizableui/panelUI.inc.css
+++ b/browser/themes/shared/customizableui/panelUI.inc.css
@@ -1869,29 +1869,41 @@ panelview[mainview] #PanelUI-whatsNew-co
   height: 43em;
 }
 
 #PanelUI-whatsNew .panel-subview-body {
   padding-top: 0;
 }
 
 #PanelUI-whatsNew .panelMenu-toggleWhatsNew-checkbox {
-  margin-inline-start: -80px;
+  background-color: var(--arrowpanel-dimmed);
+  border-top: 1px solid var(--panel-separator-color);
+  padding-inline-start: 18px;
+  min-height: 41px;
+}
+
+#PanelUI-whatsNew .panelMenu-toggleWhatsNew-checkbox:hover,
+#PanelUI-whatsNew .panelMenu-toggleWhatsNew-checkbox:focus {
+  background-color: var(--arrowpanel-dimmed-further);
+}
+
+#PanelUI-whatsNew .panelMenu-toggleWhatsNew-checkbox:active {
+  background-color: var(--arrowpanel-dimmed-even-further);
 }
 
 /* These checkbox styles have been pulled from `common.inc.css` and
    duplicated here to ensure they apply correctly. See
    https://searchfox.org/mozilla-central/rev/7908ce29657cfd623993046bd8e38664e1c0b28e/toolkit/themes/shared/in-content/common.inc.css#559 */
 #PanelUI-whatsNew checkbox {
   -moz-appearance: none;
   height: 30px;
-  margin: 2px 0;
+  margin: 0;
 }
 
-#PanelUI-whatsNew checkbox[checked].panelMenu-toggleWhatsNew-checkbox {
+#PanelUI-whatsNew checkbox[checked] {
   list-style-image: url("chrome://global/skin/icons/check.svg");
   -moz-context-properties: fill;
   fill: #2292d0;
 }
 
 #PanelUI-whatsNew .checkbox-check {
   -moz-appearance: none;
   width: 20px;
@@ -1901,16 +1913,20 @@ panelview[mainview] #PanelUI-whatsNew-co
   border-radius: 2px;
   margin: 0;
   margin-inline-end: 10px;
   background-color: #FFF;
   background-position: center;
   background-repeat: no-repeat;
 }
 
+#PanelUI-whatsNew checkbox:hover .checkbox-check {
+  border-color: #0a84ff;
+}
+
 #PanelUI-whatsNew .checkbox-icon {
   display: none;
 }
 
 #PanelUI-whatsNew .whatsNew-message {
   cursor: pointer;
   margin: 0 12px;
   padding: 0;