<amp-selector> element toggle the 'selected' attribute (#13553)

* WIP

* Finalize implementation and tests

* Implementation and tests

* Make cvializ@ suggestions - change input argument name and clean up code

* Cater for large negative numbers and add tests to check for that too

* Rewrite code and tests to be cleaner

* Move mod functionality to helper and add tests too

* Remove unnecessary  attribute logic

* Add assert to ensure index is > 0

* Add additional assert

* Linter error

extensions/amp-selector/0.1/amp-selector.js

@@ -125,6 +125,16 @@ export class AmpSelector extends AMP.BaseElement {
       const delta = (args && args['delta'] !== undefined) ? args['delta'] : 1;
       this.select_(delta);
     }, ActionTrust.LOW);
+
+    this.registerAction('toggle', invocation => {
+      const args = invocation.args;
+      user().assert(args['index'] >= 0, '\'index\' must be greater than 0');
+      user().assert(args['index'] < this.options_.length, '\'index\' must be ' +
+        'less than the length of options in the <amp-selector>');
+      if (args && args['index'] !== undefined) {
+        this.toggle_(args['index'], args['value']);
+      }
+    }, ActionTrust.LOW);
   }
 
   /** @override */
@@ -331,6 +341,34 @@ export class AmpSelector extends AMP.BaseElement {
     }
   }
 
+  /**
+   * Handles toggle action.
+   * @param {number} index
+   * @param {boolean=} opt_value
+   */
+  toggle_(index, opt_value) {
+    // Change the selection to the next element in the specified direction.
+    // The selection should loop around if the user attempts to go one
+    // past the beginning or end.
+    const indexCurrentStatus = this.options_[index].hasAttribute('selected');
+    const indexFinalStatus =
+      opt_value !== undefined ? opt_value : !indexCurrentStatus;
+    const selectedIndex = this.options_.indexOf(this.selectedOptions_[0]);
+
+    if (indexFinalStatus === indexCurrentStatus) {
+      return;
+    }
+
+    // There is a change of the `selected` attribute for the element
+    if (selectedIndex !== index) {
+      this.setSelection_(this.options_[index]);
+      this.clearSelection_(this.options_[selectedIndex]);
+    } else {
+      this.clearSelection_(this.options_[index]);
+    }
+  }
+
+
   /**
    * Handles selectUp events.
    * @param {number} delta

extensions/amp-selector/0.1/test/test-amp-selector.js

@@ -631,6 +631,84 @@ describes.realWin('amp-selector', {
       expect(setInputsSpy).calledTwice;
     });
 
+    it('should trigger `toggle` action even when no `value` argument is' +
+      ' provided to the function', () => {
+      const ampSelector = getSelector({
+        config: {
+          count: 5,
+          selectedCount: 1,
+        },
+      });
+      ampSelector.build();
+      const impl = ampSelector.implementation_;
+
+      expect(ampSelector.hasAttribute('multiple')).to.be.false;
+      expect(ampSelector.children[0].hasAttribute('selected')).to.be.true;
+
+      // Test the case where the element to be `selected` and the currently
+      // selected element are different
+      let args = {'index': 2};
+      impl.executeAction(
+          {method: 'toggle', args, satisfiesTrust: () => true});
+      expect(ampSelector.children[0].hasAttribute('selected')).to.be.false;
+      expect(ampSelector.children[2].hasAttribute('selected')).to.be.true;
+
+      // Test the case where the element to be `selected` and the currently
+      // selected element are the same
+      args = {'index': 2};
+      impl.executeAction(
+          {method: 'toggle', args, satisfiesTrust: () => true});
+      expect(ampSelector.children[2].hasAttribute('selected')).to.be.false;
+    });
+
+    it('should trigger `toggle` action even with specified `value`' +
+      ' argument', () => {
+      const ampSelector = getSelector({
+        config: {
+          count: 5,
+          selectedCount: 1,
+        },
+      });
+      ampSelector.build();
+      const impl = ampSelector.implementation_;
+
+      expect(ampSelector.hasAttribute('multiple')).to.be.false;
+      expect(ampSelector.children[0].hasAttribute('selected')).to.be.true;
+
+      // Test the case where the element to be `selected` and the currently
+      // selected element are different
+      let args = {'index': 2, 'value': true};
+      impl.executeAction(
+          {method: 'toggle', args, satisfiesTrust: () => true});
+      expect(ampSelector.children[0].hasAttribute('selected')).to.be.false;
+      expect(ampSelector.children[2].hasAttribute('selected')).to.be.true;
+
+      // Test the case where the element to be `selected` and the currently
+      // selected element are the same
+      args = {'index': 2, 'value': true};
+      impl.executeAction(
+          {method: 'toggle', args, satisfiesTrust: () => true});
+      expect(ampSelector.children[2].hasAttribute('selected')).to.be.true;
+
+      // Test the case where the element to be removed as `selected` and
+      // the currently selected element are the same
+      args = {'index': 2, 'value': false};
+      impl.executeAction(
+          {method: 'toggle', args, satisfiesTrust: () => true});
+      expect(ampSelector.children[2].hasAttribute('selected')).to.be.false;
+
+      // Test the case where the element to be removed as `selected`
+      // is different from the currently `selected` element
+      ampSelector.children[0].setAttribute('selected', '');
+      expect(ampSelector.children[0].hasAttribute('selected')).to.be.true;
+
+      args = {'index': 2, 'value': false};
+      impl.executeAction(
+          {method: 'toggle', args, satisfiesTrust: () => true});
+      expect(ampSelector.children[0].hasAttribute('selected')).to.be.true;
+      expect(ampSelector.children[2].hasAttribute('selected')).to.be.false;
+    });
+
     it('should trigger `select` action when user selects an option', () => {
       const ampSelector = getSelector({
         config: {
@@ -740,7 +818,6 @@ describes.realWin('amp-selector', {
       expect(ampSelector.children[1].hasAttribute('selected')).to.be.true;
     });
 
-
     describe('keyboard-select-mode', () => {
 
       it('should have `none` mode by default', () => {

spec/amp-actions-and-events.md

@@ -380,6 +380,10 @@ event.response</pre></td>
     <td><code>selectDown(delta=INTEGER)</code></td>
     <td>Moves the selection down by the value of `delta`. The default `delta` is set to -1.</td>
   </tr>
+  <tr>
+    <td><code>toggle(index=INTEGER, value=BOOLEAN)</code></td>
+    <td>Sets the selected element's `selected` attribute if value is 'true', otherwise removes the attribute</td>
+  </tr>
 </table>
 
 ### amp-sidebar