Amphtml: Add `clear` action for `amp-selector`

[aghassemi]

Background

amp-selector is an AMP Extended Component that presents a list of options and lets the user choose one or many options.

You can see a demo of this component in action on AMP By Example website.

The Issue

Currently there is no way to clear the selection in a single or multi-select amp-selector. This is a useful feature and we like to expose it as an AMP Action on the component, for instance we like to support the following code:

<button on="tap:mySelector.clear">Clear Selection</button>
<amp-selector id="mySelector" layout="container" multiple>
  <div>Option One</div>
  <div>Option Two</div>
  <div>Option Three</div>
  <div>Option Four</div>
</amp-selector>

Implementation strategy

amp-selector already has private methods for clearing selection, we just need to expose them as an action, test and document it.

• [ ] Register a new action named clear using this.registerAction method inside amp-selector.js
• [ ] Have the action call the existing clearAllSelections_ method.
• [ ] Add tests (one for single and one for multi-select modes) to test-amp-selector.js
• [ ] Updateamp-selector.md to document the new feature.
• [ ] Create a separate PR for AMP By Example to demo the new feature on their page by updating https://github.com/ampproject/amp-by-example/blob/master/src/20_Components/amp-selector.html

Step by step

• [ ] Claim this issue by adding a comment below. Please only claim this bug if you plan on starting work in the next day or so.
• [ ] If you aren't too familiar with Git/GitHub, see the Getting Started End-to-End Guide for an intro to Git & GitHub, and how to get a copy of the code. You can also refer to the Quick Start Guide for the necessary setup steps with less explanation than the End-to-End guide.
• [ ] Follow the instructions for building AMP.
• [ ] [Create a Git branch](https://github.com/ampproject/amphtml/blob/master/contributing/getting-started-e2e.md#create-a-git-branch) for making your changes.
• [ ] Follow the Implementation strategy from above.
• [ ] [Commit your changes](https://github.com/ampproject/amphtml/blob/master/contributing/getting-started-e2e.md#edit-files-and-commit-them) frequently.
• [ ] [Push your changes to GitHub](https://github.com/ampproject/amphtml/blob/master/contributing/getting-started-e2e.md#push-your-changes-to-your-github-fork).
• [ ] [Create a Pull Request](https://github.com/ampproject/amphtml/blob/master/contributing/getting-started-e2e.md#send-a-pull-request-ie-request-a-code-review). Mention closes Issue #10475 in the description.
• [ ] Assign @aghassemi as the reviewer
• [ ] [Respond to your reviewer's comments](https://github.com/ampproject/amphtml/blob/master/contributing/getting-started-e2e.md#respond-to-pull-request-comments) (if any).

Once approved, your changes will be merged. ⚡⚡⚡Congrats on making your first contribution to the AMP Project!⚡⚡⚡ You'll be able to see it live across the web soon!

Thanks, and we hope to see more contributions from you soon.

Questions?

If you have questions ask in this issue or on your Pull Request (mentioning @aghassemi) or see the How to get help section of the Getting Started guide.

[mianuddin]

I want to try out implementing this as my first open source contribution, could I claim it?

[aghassemi]

@mianuddin Go for it, thanks for contributing.

[mianuddin]

@aghassemi What's the best way to go about testing the action? I went through the tests for other extensions but could not find a similar instance for reference.

[aghassemi]

@mianuddin For a good e2e test:

• [ ] Use getSelector to create a multi-select amp-selector similar to other tests but also pass an id in the attributes
• [ ] pre-select a few of its options using ('ampSelector.children[1].setAttribute('selected', '');')
• [ ] create an actual button (using win.document.createElement) and set it's on attribute to call the selector's clear action ( similar to the sample code ).
• [ ] Append the button to win.body
• [ ] Use JS to click the button (btn.click() will do)
• [ ] Assert the children that initially got the selected attribute now don't have it anymore.

[mianuddin]

Sounds great! I'll try it out right now. I have most of that already done, but I was working with TriggerSpy based on what I saw in other tests. Thanks for the clear breakdown!

[mianuddin]

@aghassemi I just made both of my pull requests. It won't allow me to add you as a reviewer, so I mentioned you in the comments.

• https://github.com/ampproject/amphtml/pull/10545
• https://github.com/ampproject/amp-by-example/pull/900

[jridgewell]

Closed by https://github.com/ampproject/amphtml/pull/10545.