fix(expandable): use aria-expanded attribute - TWIG-103

[papegaill]

PR description

Please drop a few lines about the PR: what it does, how to test it, etc.

QA Checklist

In order to ensure a safe and quick review, please check that your PR follow those guidelines:

• I have put the vanilla component as devDependencies
• I have put the specs package as devDependencies
• I have added the components directly used in the twig file (with include or embed) as dependencies
• My component is listed in @ecl-twig/ec-components's dependencies
• My variables naming follow the guidelines (snake case for twig)
• I have provided tests
• I have provided documentation (for the "notes" tab)
• If my local yarn.lock contains changes, I have committed it
• I have given my PR the proper label (pr: review needed to indicate that I'm done and now waiting for a review ,pr: wip to indicate that I'm actively working on it ...)

[planctus]

I think you overdid, here...in ECL the only thing was the addition of the default aria-expanded="false".

[planctus]

the aria-expanded attribute is handled by javascript in this component, so we just set it initially as false (at least this is the way it is done in ECL) and then is js playing with that. I see that by doing like you did we could offer the possibility of using the component with a "not-collapsed" state initially, but this is not what the ECL definition of the component does, so i would not introduce it here on our side as an extra parameter.

[papegaill]

Agree but this was already the case before the actual PR, I was also a bit confused with this, see the parameter here:

ecl-twig/src/ec/packages/ec-component-expandable/expandable.html.twig

Line 8 in 292c833

- "expanded" (boolean) (default: false)

Anyway, I have removed the expanded parameter now.

[planctus]

Same as before, this is handled by javascript, we set it as hidden and then js will play with that. I would have honestly considered this as an improvement in case it was meant to provide a non javascript solution with the collapsed button hidden and the content revealed, but it's not the case, so to stick to the definition of the component as we have it ECL we set this as hidden by default and then we leave the task to the js.

[papegaill]

Same as my previous comment this was already implemented... See:

ecl-twig/src/ec/packages/ec-component-expandable/expandable.html.twig

Line 66 in 292c833

{% if _expanded %}

Updated now.

[planctus]

@papegaill , i had a confirmation on the ECL side about this, personally speaking i would make this as an expandable/collapsible thing, but the collapsible "use case" is already covered by the accordion where we say not to use it with less than three pieces of information, so the single "collapsible" is not an use case, therefore no need to handle any logic on our side.