[App Management] Pin to Shelf now toggles on key press.

The pin to shelf item now toggles it's state when a "space" or
"enter" keypress is registered.

Bug: 987585
Change-Id: I2f6125ce496dfc32aa0b94be0e779b065a01d9ac
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1718454Reviewed-by: calamity <calamity@chromium.org>
Commit-Queue: Jeevan Shikaram <jshikaram@chromium.org>
Cr-Commit-Position: refs/heads/master@{#685104}

chrome/browser/resources/app_management/BUILD.gn

@@ -256,6 +256,7 @@ if (!is_android) {
   js_library("toggle_row") {
     deps = [
       ":types",
+      "//ui/webui/resources/cr_elements/cr_toggle:cr_toggle",
     ]
   }
 

chrome/browser/resources/app_management/pin_to_shelf_item.html

@@ -9,10 +9,11 @@
       }
     </style>
     <app-management-toggle-row
-      label_="$i18n{pinToShelf}"
-      managed_$="[[isManaged_(app_)]]"
-      policy-label_="$i18n{pinControlledByPolicy}"
-      value_$="[[getValue_(app_)]]">
+      id="toggle-row"
+      label="$i18n{pinToShelf}"
+      managed$="[[isManaged_(app_)]]"
+      policy-label="$i18n{pinControlledByPolicy}"
+      value$="[[getValue_(app_)]]">
     </app-management-toggle-row>
   </template>
   <script src="pin_to_shelf_item.js"></script>

chrome/browser/resources/app_management/pin_to_shelf_item.js

@@ -32,9 +32,9 @@ Polymer({
     },
   },
 
-  ready: function() {
-    // capture the onClick event before it reaches the toggle.
-    this.addEventListener('click', this.onClick_, true);
+  listeners: {
+    click: 'onClick_',
+    change: 'toggleSetting_',
   },
 
   /**
@@ -75,21 +75,22 @@ Polymer({
     return app.isPolicyPinned === OptionalBool.kTrue;
   },
 
-  /**
-   * @param {Event} event
-   * @private
-   */
-  onClick_: function(event) {
-    event.stopPropagation();
-
-    // Disabled
-    if (this.isManaged_(this.app_)) {
-      return;
-    }
-
+  toggleSetting_: function() {
+    const newState =
+        assert(app_management.util.toggleOptionalBool(this.app_.isPinned));
+    assert(
+        app_management.util.convertOptionalBoolToBool(newState) ===
+        this.$['toggle-row'].isChecked());
     app_management.BrowserProxy.getInstance().handler.setPinned(
         this.app_.id,
-        assert(app_management.util.toggleOptionalBool(this.app_.isPinned)),
+        newState,
     );
   },
+
+  /**
+   * @private
+   */
+  onClick_: function() {
+    this.$['toggle-row'].click();
+  },
 });

chrome/browser/resources/app_management/toggle_row.html

@@ -27,27 +27,27 @@
     </style>
 
     <div id="left-content" class="horizontal-align">
-      <template is="dom-if" if="[[icon_]]">
-        <iron-icon id="icon" icon="[[icon_]]"></iron-icon>
+      <template is="dom-if" if="[[icon]]">
+        <iron-icon id="icon" icon="[[icon]]"></iron-icon>
       </template>
-      <div id="label">[[label_]]</div>
+      <div id="label">[[label]]</div>
     </div>
     <div id="right-content" class="horizontal-align">
       <iron-icon id="policy-indicator"
         icon="cr:domain"
         tabindex="0"
         aria-describedby="tooltip"
-        hidden$="[[!managed_]]">
+        hidden$="[[!managed]]">
       </iron-icon>
       <paper-tooltip id="tooltip"
         for="policy-indicator"
         position="top"
         fit-to-visible-bounds>
-        [[policyLabel_]]
+        [[policyLabel]]
       </paper-tooltip>
       <cr-toggle id="toggle"
-        checked="[[value_]]"
-        disabled$="[[managed_]]">
+        checked="[[value]]"
+        disabled$="[[managed]]">
       </cr-toggle>
     </div>
   </template>

chrome/browser/resources/app_management/toggle_row.js

@@ -7,28 +7,43 @@ Polymer({
   properties: {
     /**
      * @type {string}
-     * @private
      */
-    icon_: String,
+    icon: String,
     /**
      * @type {string}
-     * @private
      */
-    label_: String,
+    label: String,
     /**
      * @type {boolean}
-     * @private
      */
-    managed_: {type: Boolean, value: false, reflectToAttribute: true},
+    managed: {type: Boolean, value: false, reflectToAttribute: true},
     /**
      * @type {string}
-     * @private
      */
-    policyLabel_: String,
+    policyLabel: String,
     /**
      * @type {boolean}
-     * @private
      */
-    value_: {type: Boolean, value: false, reflectToAttribute: true},
+    value: {type: Boolean, value: false, reflectToAttribute: true},
+  },
+
+  listeners: {
+    click: 'onClick_',
+  },
+
+  /**
+   * @returns {boolean} true if the toggle is checked.
+   */
+  isChecked: function() {
+    return this.$.toggle.checked;
+  },
+
+  /**
+   * @param {MouseEvent} event
+   * @private
+   */
+  onClick_: function(event) {
+    event.stopPropagation();
+    this.$['toggle'].click();
   },
 });

chrome/browser/resources/app_management/util.js

@@ -242,9 +242,25 @@ cr.define('app_management.util', function() {
     }
   }
 
+  /**
+   * @param {OptionalBool} optionalBool
+   * @returns {boolean}
+   */
+  function convertOptionalBoolToBool(optionalBool) {
+    switch (optionalBool) {
+      case OptionalBool.kTrue:
+        return true;
+      case OptionalBool.kFalse:
+        return false;
+      default:
+        assertNotReached();
+    }
+  }
+
   return {
     addIfNeeded: addIfNeeded,
     alphabeticalSort: alphabeticalSort,
+    convertOptionalBoolToBool: convertOptionalBoolToBool,
     createEmptyState: createEmptyState,
     createInitialState: createInitialState,
     createPermission: createPermission,

chrome/test/data/webui/app_management/pwa_permission_view_test.js

@@ -25,6 +25,12 @@ suite('<app-management-pwa-permission-view>', function() {
     await fakeHandler.flushPipesForTesting();
   }
 
+  function getSelectedAppFromStore() {
+    const storeData = app_management.Store.getInstance().data;
+    const selectedAppId = storeData.currentPage.selectedAppId;
+    return storeData.apps[selectedAppId];
+  }
+
   setup(async function() {
     fakeHandler = setupFakeHandler();
     replaceStore();
@@ -66,4 +72,29 @@ suite('<app-management-pwa-permission-view>', function() {
     await checkToggle('CONTENT_SETTINGS_TYPE_MEDIASTREAM_CAMERA');
     await checkToggle('CONTENT_SETTINGS_TYPE_MEDIASTREAM_MIC');
   });
+
+  test('Pin to shelf toggle', async function() {
+    const pinToShelfItem = pwaPermissionView.$['pin-to-shelf-setting'];
+    const toggle = pinToShelfItem.$['toggle-row'].$.toggle;
+
+    assertFalse(toggle.checked);
+    assertEquals(
+        toggle.checked,
+        app_management.util.convertOptionalBoolToBool(
+            getSelectedAppFromStore().isPinned));
+    pinToShelfItem.click();
+    await fakeHandler.flushPipesForTesting();
+    assertTrue(toggle.checked);
+    assertEquals(
+        toggle.checked,
+        app_management.util.convertOptionalBoolToBool(
+            getSelectedAppFromStore().isPinned));
+    pinToShelfItem.click();
+    await fakeHandler.flushPipesForTesting();
+    assertFalse(toggle.checked);
+    assertEquals(
+        toggle.checked,
+        app_management.util.convertOptionalBoolToBool(
+            getSelectedAppFromStore().isPinned));
+  });
 });

ui/webui/resources/cr_elements/cr_toggle/cr_toggle.js

@@ -143,12 +143,6 @@ Polymer({
     e.stopPropagation();
     e.preventDefault();
 
-    // Ignore case where 'click' handler is triggered while disabled. Can happen
-    // via calling the click() method.
-    if (this.disabled) {
-      return;
-    }
-
     // User gesture has already been taken care of inside |pointermove|
     // handlers, Do nothing here.
     if (this.handledInPointerMove_) {
@@ -165,12 +159,17 @@ Polymer({
    * @private
    */
   toggleState_: function(fromKeyboard) {
-    this.checked = !this.checked;
+    // Ignore cases where the 'click' or 'keypress' handlers are triggered while
+    // disabled.
+    if (this.disabled) {
+      return;
+    }
 
     if (!fromKeyboard) {
       this.hideRipple_();
     }
 
+    this.checked = !this.checked;
     this.fire('change', this.checked);
   },