Decathlon / vitamin-web

Decathlon Design System UI components for web applications
https://decathlon.github.io/vitamin-web
Apache License 2.0
282 stars 76 forks source link

fix: make open property of VtmnAccordion reactive #1461

Closed Martin-Brandenburg closed 1 year ago

Martin-Brandenburg commented 1 year ago

Changes description

This is a bugfix to the VtmnAccordion component. The open prop was binded in the details element.

Context

The previous implementation only allows us to pass the open prop down, but changes are not reflected. In particular, we cannot let the parent of the VtmnAccordion know when it is open and when not. So this is just a missing feature, but: there is also a bug in the previous implementation: the line aria-expanded={open} means that the aria-expanded property is always set to the initial value. It will never be changed.

When the accordion is closed, it does not have the open prop and we have aria-expanded=false, which is correct:

Bildschirmfoto 2023-09-14 um 16 50 15

But when it is opened, the open prop is added, but aria-expanded=false stays.

Bildschirmfoto 2023-09-14 um 16 51 12

This pull request fixes this bug.

Checklist

Does this introduce a breaking change?

CLAassistant commented 1 year ago

CLA assistant check
All committers have signed the CLA.

lauthieb commented 1 year ago

Thanks @Martin-Brandenburg for this proposition! I'll let @thibault-mahe review it. @Tlahey feel free to check it out also if you can, please 🙏. @Martin-Brandenburg can you please sign the CLA? https://github.com/Decathlon/vitamin-web/pull/1461#issuecomment-1719618856