Convert es-accordion to angle bracket component

[MelSumner]

For the next iteration of these components, we should be using angle-bracket components.

To Do

• Convert es-accordion to be a template-only/angle bracket component
• Update documentation (in dummy app)

Supporting Resources

• Video: Angle Bracket Component Invocation in Ember.js
• Video: Template-only components in Ember.js

[LaSylv]

I'd like to give it a go.

How would you approch this one ?

[alexeykostevich]

@MelSumner , here are some links for convenience:

• es-accordion source
• es-accordion docs

Currently, es-accordion is not used in ember-learn projects.

It seems that the component is not a fully generic implementation of an accordion pattern and is designed for the specific use case:

Taking into account that the team is updating the Ember.js design (https://github.com/emberjs/rfcs/blob/master/text/0425-website-redesign.md), would it make sense to re-implement it in a more generic way and make it consistent with the current design language?

I would be glad to help with this if we have the agreement on its design.

[MelSumner]

It would definitely make sense. We want this component for use in projects that haven't been fully realized yet. If you want to take it on that would be great!! 👍

[alexeykostevich]

Ok, let's do this! I'm starting working on a new <EsAccordion />.

[alexeykostevich]

@MelSumner , I've finished the draft version of a new <EsAccordion /> component and will create the PR shortly.

Overall, the component will support:

• Single and multiple expanded panels simultaneously (configurable)
• Custom templates for a panel header
• Custom templates for panel content
• DDAU-driven binding for currently expanded panels
• Accessibility based the W3 Accordion example
• Very basic CSS animation

<EsAccordion
  @multi={{true}}
  @expanded={{value}}
  @expandedChange={{fn (mut value)}} as |accordion|
>
  <accordion.panel @name="panel-1" as |accordionPanel|>
    <accordionPanel.header @title="Panel 1 Header" />
    <accordionPanel.content>
      Lorem ipsum dolor...
    </accordionPanel.content>
  </accordion.panel>
  <accordion.panel @name="panel-2" as |accordionPanel|>
    <accordionPanel.header @title="Panel 2 Header" />
    <accordionPanel.content>
      Lorem ipsum dolor...
    </accordionPanel.content>
  </accordion.panel>
  <accordion.panel @name="panel-3" as |accordionPanel|>
    <accordionPanel.header>
      Panel 3 Header with custom content 🐹
    </accordionPanel.header>
    <accordionPanel.content>
      Lorem ipsum dolor...
    </accordionPanel.content>
  </accordion.panel>
</EsAccordion>

{{value}} is expanded.

Here is how its design looks like (the first panel is focused):

So, the component should be generic enough to support different use cases. If we need an accordion to show, let’s say, releases in multiple places, we can implement a lightweight specific <EsReleaseAccordion /> on top of <EsAccordion />.

@MelSumner,, @pichfl , please, let me know if you have any major concerns about the features, API or design.

Thank you!

[alexeykostevich]

#301 removes not documented components including the legacy <EsAccordion />. So, it would be helpful to get it merged first before creating a PR with a new Octane version of the component.

[pichfl]

@alexeykostevich that looks good. I haven't done any design on this so far as it wasn't brought to my attention this would be a required module. Even so, it is an excellent application of the design styles I created 👍

[mansona]

@pichfl the reason this wasn't brought to your attention was that there is currently nowhere in any of our apps that actually uses an accordion 🙈

We have something in the planned designs where we will need collapsible headers in the sidebar but I personally think that this ought to be implemented in the sidebar component and not in a separate accordion component.

@alexeykostevich you're right #301 does remove this and personally I think we shouldn't be adding it back until we have a known use case where we will need accordions in 2 places (i.e. other than just the sidebar component). I'm sorry I've had a super busy weekend so I missed the conversation starting back up on this issue, I really hope we don't end up with any wasted effort on this 🙈

[alexeykostevich]

@mansona , thank you for the details! No worries at all, I’ve probably rushed a bit with this 🙈

I totally agree that collapsible headers should be implemented in the <EsSidebar /> component itself. So, <EsAccordion /> is not needed for this functionality.

With regards to <EsAccordion /> component, it is ready but without tests so far. However, I'm all for avoiding pushing unused code just for future needs if we don't have any plans for it. We know that this future might never comes but the maintenance cost will be there 🙂

So, let's do this. I've pushed the implementation of a <EsAccordion /> into my forked repository. If we ever need this component, we can create a PR and merge it from here: upgrade...alexeykostevich:feature/accordion

As I've mentioned, please, pay attention, that the implementation doesn't currently include tests -- I could add them when needed.

Please, let me know if you have any concerns.

[MelSumner]

@alexeykostevich this looks great! I'll give it a review later today. In the meantime, if you wanted to add some tests (especially try out some ones that would confirm that the keyboard interactions work as expected) that would be great! Great job!