Amphtml: Add `expand`, `collapse`, `toggle` actions to `amp-accordion`

Created on 8 Aug 2017  Â·  19Comments  Â·  Source: ampproject/amphtml

amp-accordion is an AMP Extended Component that allows you to display collapsible and expandable content sections. Sections toggle between expanded/collapsed states as user taps on the header section.

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

The Issue

Currently there is no way to programmatically expand, collapse, toggle a section of amp-accordion. 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="myAccordion.collapse(section=s1)">Collapse section 1</button>
<button on="myAccordion.expand(section=s2)">Expand section 2</button>
<button on="myAccordion.toggle(section=s3)">Toggle section 3</button>

<amp-accordion id="myAccordion">
  <section id="s1" expanded>
    <h4>Section 1</h4>
    <p>Content.</p>
  </section>
  <section id="s2" >
    <h4>Section 2</h4>
    <p>Content.</p>
  </section>
  <section id="s3">
    <h4>Section 3</h4>
    <p>Content.</p>
  </section>
</amp-accordion>

Implementation strategy

amp-accordion already has code that handles these actions but the code is tied to click event.

  • [ ] Move the toggle logic out of onHeaderPicked_ method and into a new private method toggle_(section, opt_expand)
  • [ ] Register new actions named expand, collapse, toggle using this.registerAction
  • [ ] Have the actions code:

    • [ ] parse the arguments,

    • [ ] find the section parameter which is the id of the section to act on

    • [ ] find the section element with that id and call the newly created toggle_(section, opt_expand)

      method.

    • [ ] If section parameter is not provided, apply to method to ALL sections (e.g. on=myAccordion.expand() will expand all sections)

  • [ ] Add tests
  • [ ] Updateamp-accordion.md to document the new feature.
  • [ ] (Optional), update the AmpByExample for amp-accordion to show-case the new features.

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 #10826 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.

GFI Claimed! Soon good first issue

Most helpful comment

UPDATE: Sorry, I figured this out and edited my post, but left in "but I am unable". Change now to "I am able". I assumed you would just pass the section name, but you need to pass "section=

". Glad this now works. Very useful addition to accordion.

I am happy to say that with the prod release you mentioned, this code works, and I am able to pass a specific section to toggle.

<button on="prop-accordion.toggle(section=first);">Open</button>
<amp-accordion disable-session-states id="prop-accordion" class="prop-accordion">
<section id="first">
<h2>Title</h2>
<div class="content">something here</div>
</section>
</amp-accordion>

Hope this helps if anyone else is trying to make this work.

Thanks @aghassemi

Thom

All 19 comments

@aghassemi there's any logic/argument if i want to collapse just one and close the others section?

thanks!

We can make the ID given to the functions optional, if none given, it acts on all sections

Hi @aghassemi I was just working on this. How do I compile it and run it every time after I add a module?
Edit: This is my first contribution :)

Did you follow the instructions in https://github.com/ampproject/amphtml/blob/master/contributing/getting-started-e2e.md#building-amp-and-starting-a-local-server?

@jridgewell Hey, I followed this correctly and it works just fine.
I just wanted to know if I made a change in the amp-accordion.js file where would it reflect after building it?

They get built into dist/v0/amp-accordion*

@abhinavralhan If you run gulp watch it will keep rebuilding as you change the code. I suggest adding the sample example code from above to standard-actions.amp.html in examples so you can manually test and see your code working ( and it will be great to have it in examples anyway )

Hi @abhinavralhan are you still working on this issue?

@aghassemi Quick clarification, what data type does the opt_expand parameter for the toggle_ method accept and what is it's purpose?

@mianuddin it's a boolean. Works similar to Web's toggle on classList. Maybe opt_force would have been a better name. Essentially if true, it means expand and if false it means collapse if not provided it means opposite of current state.

I see this issue is closed, but I cannot find documentation on this and cannot get it to work. Pardon my newness to this process, but would love to have this functionality on my site.

@td234 Hello!
Here's the documentation for the actions, it should hopefully be all you need:

@mianuddin Hi!

Either I was not clear in my question or I just don't understand.

I have read that documentation several times. This thread talks about programmatically expanding accordion sections using something like "on=myAccordion.expand('sectionname')". I am wondering if this process has been implemented. When I try to use the format as suggested in the "Implementation strategy" above I get this:

SAMPLE CODE

<button on="prop-accordion.expand();">Open</button>
<amp-accordion disable-session-states id="prop-accordion" class="prop-accordion">
<section id="first">
<h2>Title</h2>
<div class="content">something here</div>
</section>
</amp-accordion>

RESULT

Invalid action definition in ​ : [ prop-accordion.expand(); ] ; expected [:]

If I change it to "on:tap:prop-accordion.toggle()" I get this:

Method not found: toggle in n {element: amp-accordion#prop-accordion.prop-accordion.i-amphtml-element.i-amphtml-layout-container.i-amphtml-l…, layout_: "container", layoutWidth_: 375, inViewport_: true, win: Window, …}

Please let me know if I am missing something.

Thom

@td234 Syntax is <button on="tap:myAccordion.toggle()">.
Please see https://codepen.io/aghassemi/pen/baLKqd for a working sample as reference.

@aghassemi
Okay, this is really making me feel stupid. I did try that syntax in the second example I sent, I just typed in wrong in the message above. I have

<button on="tap:prop-accordion.toggle();">Open</button>
for this accordion.
<amp-accordion disable-session-states class="prop-accordion" id="prop-accordion">

I still get the error. I even get the error on the Codepen page you sent me!

https://codepen.io/aghassemi/pen/baLKqd

Method not found: collapse in n {element: amp-accordion#prop-accordion.prop-accordion.i-amphtml-element.i-amphtml-layout-container.i-amphtml-l…, layout_: "container", layoutWidth_: 1137, inViewport_: true, win: Window, …}
(anonymous) @ console_runner-079c09a0e3b9ff743e39ee2d5637b9216b3545af0de366d4b9aad9dc87e26bfd.js:1
Wd @ url.js:345
(anonymous) @ viewer-impl.js:167
f.assert @ log.js:325
f.executeAction @ v0.js:173
Wg.b.re @ custom-element.js:663
Wg.b.enqueAction @ custom-element.js:641
th @ custom-element.js:1430
k @ custom-element.js:1388
(anonymous) @ custom-element.js:1408
sh @ custom-element.js:1364
f.trigger @ custom-element.js:1272
(anonymous) @ custom-element.js:1140
log.js:319 Uncaught Error: Action execution failed: AMP-ACCORDION collapse: Method not found: collapse in [object Object]​​​

I tried the Codepen in both Safari and Chrome. I must really be missing something.

Thom

@td234 What's happening is that this feature is only available in Dev Channel (https://cdn.ampproject.org/experiments.html) at the moment. So if you enable Dev Channel and refresh it would work. Good news is that this feature will make it to prod release later today and will work without enabling Dev Channel.

@aghassemi

Good news is that this feature will make it to prod release later today and will work without enabling Dev Channel.

That's the best news I have heard all day!

Is there a place to see all the new features added to each prod release?

UPDATE: Sorry, I figured this out and edited my post, but left in "but I am unable". Change now to "I am able". I assumed you would just pass the section name, but you need to pass "section=

". Glad this now works. Very useful addition to accordion.

I am happy to say that with the prod release you mentioned, this code works, and I am able to pass a specific section to toggle.

<button on="prop-accordion.toggle(section=first);">Open</button>
<amp-accordion disable-session-states id="prop-accordion" class="prop-accordion">
<section id="first">
<h2>Title</h2>
<div class="content">something here</div>
</section>
</amp-accordion>

Hope this helps if anyone else is trying to make this work.

Thanks @aghassemi

Thom

@td234 not sure what's happening, seems to work form me: I updated https://codepen.io/aghassemi/pen/baLKqd with section specific toggles, please take a look, may shed a light on the issue.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

choumx picture choumx  Â·  50Comments

ericlindley-g picture ericlindley-g  Â·  117Comments

lisotton picture lisotton  Â·  52Comments

vockalimo picture vockalimo  Â·  49Comments

ericlindley-g picture ericlindley-g  Â·  60Comments