WebUI: cr-radio-group, select radio button with focus when enter/space pressed

Bug: 899187
Change-Id: Ib640ceeec2abeb1f6ad1411cc10633018f85e303
Reviewed-on: https://chromium-review.googlesource.com/c/1308076
Commit-Queue: Esmael El-Moslimany <aee@chromium.org>
Reviewed-by: Demetrios Papadopoulos <dpapad@chromium.org>
Cr-Commit-Position: refs/heads/master@{#604160}

chrome/test/data/webui/cr_elements/cr_radio_group_test.js

@@ -15,7 +15,7 @@ suite('cr-radio-group', () => {
     document.body.innerHTML = `
         <cr-radio-group>
           <cr-radio-button name="1"></cr-radio-button>
-          <cr-radio-button name="2"></cr-radio-button>
+          <cr-radio-button name="2"><input></input></cr-radio-button>
           <cr-radio-button name="3"><a></a></cr-radio-button>
         </cr-radio-group>`;
     radioGroup = document.body.querySelector('cr-radio-group');
@@ -44,7 +44,8 @@ suite('cr-radio-group', () => {
    * @param {Element=} target
    */
   function press(key, target) {
-    MockInteractions.pressAndReleaseKeyOn(target || radioGroup, -1, [], key);
+    target = target || radioGroup.querySelector('[name="1"]');
+    MockInteractions.pressAndReleaseKeyOn(target, -1, [], key);
   }
 
   /**
@@ -76,6 +77,7 @@ suite('cr-radio-group', () => {
   });
 
   test('key events when initially nothing checked', () => {
+    const firstRadio = radioGroup.querySelector('[name="1"]');
     press('Enter');
     checkSelected(1);
     radioGroup.selected = '';
@@ -161,4 +163,29 @@ suite('cr-radio-group', () => {
     a.click();
     checkSelected(1);
   });
+
+  test('radios with input', () => {
+    const input = radioGroup.querySelector('input');
+    assertTrue(!!input);
+    noneSelectedOneFocusable(1);
+    press('Enter', input);
+    press(' ', input);
+    noneSelectedOneFocusable(1);
+    input.click();
+    checkSelected(2);
+    radioGroup.querySelector('[name="1"]').click();
+    press('Enter', input);
+    press(' ', input);
+    checkSelected(1);
+    input.click();
+    checkSelected(2);
+  });
+
+  test('select the radio that has focus when space or enter pressed', () => {
+    noneSelectedOneFocusable(1);
+    press('Enter', radioGroup.querySelector('[name="3"]'));
+    checkSelected(3);
+    press(' ', radioGroup.querySelector('[name="2"]'));
+    checkSelected(2);
+  });
 });

ui/webui/resources/cr_elements/cr_radio_group/cr_radio_group.js

@@ -33,8 +33,12 @@
         value: 'cr-radio-button, controlled-radio-button',
       },
 
+      /**
+       * @type {!RegExp}
+       * @private
+       */
       selectableRegExp_: {
-        type: Object,
+        value: Object,
         computed: 'computeSelectableRegExp_(selectable)',
       },
     },
@@ -118,31 +122,24 @@
         radio.focus();
     },
 
-    /** @private */
-    computeSelectableRegExp_: function() {
-      const tags = this.selectable.split(', ').join('|');
-      return new RegExp(`^(${tags})$`, 'i');
-    },
-
-    /**
-     * @param {!EventTarget} target
-     * @return {boolean}
-     * @private
-     */
-    isRadioButton_: function(target) {
-      return this.selectableRegExp_.test(target.tagName);
-    },
-
     /**
      * @param {!KeyboardEvent} event
      * @private
      */
     onKeyDown_: function(event) {
-      if (event.path.some(target => /^(a|(cr-|)input)$/i.test(target.tagName)))
+      if (this.disabled)
+        return;
+
+      if (event.ctrlKey || event.shiftKey || event.metaKey || event.altKey)
         return;
 
-      if (this.disabled || event.ctrlKey || event.shiftKey || event.metaKey ||
-          event.altKey) {
+      const targetElement = /** @type {!Element} */ (event.target);
+      if (!this.buttons_.includes(targetElement))
+        return;
+
+      if (event.key == ' ' || event.key == 'Enter') {
+        event.preventDefault();
+        this.select_(/** @type {!Element} */ (event.target));
         return;
       }
 
@@ -150,18 +147,16 @@
       if (enabledRadios.length == 0)
         return;
 
-      const lastSelection = enabledRadios.findIndex(radio => radio.checked);
       let selectedIndex;
       const max = enabledRadios.length - 1;
-      if (lastSelection == -1 && (event.key == ' ' || event.key == 'Enter')) {
-        selectedIndex = 0;
-      } else if (event.key == 'Home') {
+      if (event.key == 'Home') {
         selectedIndex = 0;
       } else if (event.key == 'End') {
         selectedIndex = max;
       } else if (this.deltaKeyMap_.has(event.key)) {
         const delta = this.deltaKeyMap_.get(event.key);
         // If nothing selected, start from the first radio then add |delta|.
+        const lastSelection = enabledRadios.findIndex(radio => radio.checked);
         selectedIndex = Math.max(0, lastSelection) + delta;
         selectedIndex = Math.min(max, Math.max(0, selectedIndex));
       } else {
@@ -177,17 +172,27 @@
       }
     },
 
+    /**
+     * @return {!RegExp}
+     * @private
+     */
+    computeSelectableRegExp_: function() {
+      const tags = this.selectable.split(', ').join('|');
+      return new RegExp(`^(${tags})$`, 'i');
+    },
+
     /**
      * @param {!Event} event
      * @private
      */
     onClick_: function(event) {
-      if (event.path.some(target => /^a$/i.test(target.tagName)))
+      const path = event.composedPath();
+      if (path.some(target => /^a$/i.test(target.tagName)))
         return;
-      const target = event.path.find(n => this.isRadioButton_(n));
-      const name = `${target.name}`;
-      if (target && !target.disabled && this.selected != name)
-        this.selected = name;
+      const target = /** @type {!Element} */ (
+          path.find(n => this.selectableRegExp_.test(n.tagName)));
+      if (target && this.buttons_.includes(target))
+        this.select_(/** @type {!Element} */ (target));
     },
 
     /** @private */
@@ -195,8 +200,9 @@
       // TODO(crbug.com/738611): After migration to Polymer 2, remove
       // Polymer 1 references.
       this.buttons_ = Polymer.DomIf ?
-          this.$$('slot').assignedNodes({flatten: true})
-              .filter(n => this.isRadioButton_(n)) :
+          this.$$('slot')
+              .assignedNodes({flatten: true})
+              .filter(n => this.selectableRegExp_.test(n.tagName)) :
           this.queryAllEffectiveChildren(this.selectable);
       this.buttonEventTracker_.removeAll();
       this.buttons_.forEach(el => {
@@ -208,6 +214,19 @@
       this.update_();
     },
 
+    /**
+     * @param {!Element} button
+     * @private
+     */
+    select_: function(button) {
+      if (!isEnabled(button))
+        return;
+
+      const name = `${button.name}`;
+      if (this.selected != name)
+        this.selected = name;
+    },
+
     /** @private */
     update_: function() {
       if (!this.buttons_)