Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: make open property of VtmnAccordion reactive #1461

Merged
merged 1 commit into from
Sep 15, 2023
Merged

fix: make open property of VtmnAccordion reactive #1461

merged 1 commit into from
Sep 15, 2023

Conversation

Martin-Brandenburg
Copy link
Contributor

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

  • Make sure you are requesting to pull a topic/feature/bugfix branch. Please, don't request directly from your main!
  • Check commits & PR names matches our requested structure. It must follow the https://www.conventionalcommits.org pattern.
  • Check your code additions will fail neither code linting checks.
  • I have reviewed the submitted code.
  • I have tested on related showcases.
  • If it includes design changes, please ask for a review design-system-core-team-design GitHub team.

Does this introduce a breaking change?

  • No

@CLAassistant
Copy link

CLAassistant commented Sep 14, 2023

CLA assistant check
All committers have signed the CLA.

@lauthieb lauthieb requested a review from Tlahey September 14, 2023 14:56
@lauthieb lauthieb added bug 🐛 Something isn't working Svelte 🟠 Related to Svelte components library labels Sep 14, 2023
@lauthieb
Copy link
Member

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? #1461 (comment)

@lauthieb lauthieb merged commit a4612af into Decathlon:main Sep 15, 2023
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 Something isn't working Svelte 🟠 Related to Svelte components library
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants