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

accordion group inconsistent callback value #194

Open
david-nathanael opened this issue Sep 20, 2023 · 4 comments
Open

accordion group inconsistent callback value #194

david-nathanael opened this issue Sep 20, 2023 · 4 comments

Comments

@david-nathanael
Copy link

Trying to work with an accordion group in controlled mode but having issues getting the right value on the onExpandedChange callback.

https://stackblitz.com/edit/nextjs-hu1mpy?file=app%2Fpage.tsx

If i click on Accordion 1 then Accordion 2 then Accordion 1 then Accordion 2
i should expect to always have expanded = true in the onExpandedChange callback right?

right now it would show true, true, false, false

is this the expected behaviour ?

Thanks

@garronej
Copy link
Collaborator

Sorry about that, I can't review the issue right now but thank you for taking the time to produce a reproduction repo.

I think only one accordeon tab is supposed to be open at once but I might be wrong though:

https://main--ds-gouv.netlify.app/example/component/accordion/

Un regroupement de ces éléments peut être fait avec un wrapper .fr-accordions-group. Lors de l’ouverture d’un item au sein d’un même groupe, les autres items se fermeront automatiquement.

Il est possible d’utiliser plusieurs groupes au sein d’une même page, et ceux-si fonctionneront de manière totalement indépendante, c’est-à-dire qu’ouvrir un item d’un groupe fermera les items de ce même groupe, sans interagir sur les groupes annexes de la page. Pour cela il faut s’assurer que chaque id de collapse est unique.

https://www.systeme-de-design.gouv.fr/elements-d-interface/composants/accordeon/

@ddecrulle
Copy link
Collaborator

I think I understand. In the case of an accordion group, the states of the accordions are dependent on each other. When you click on the second accordion, it closes the first one. This action is performed by the JavaScript in the dsfr, without changing the value in the React component. Therefore, when you click on it again, the value retains its old value, which explains what is logged.

@david-nathanael
Copy link
Author

I think only one accordeon tab is supposed to be open at once but I might be wrong though:

Yes that is the behaviour i am looking for, should only open an accordion at a time. @ddecrulle resumed it well

@garronej
Copy link
Collaborator

If I click on Accordion 1, then Accordion 2, then Accordion 1, and then Accordion 2, should I always expect expanded = true in the onExpandedChange callback?

Not quite. While you are correct that the component is currently flawed, the onExpandedChange callback should actually be called with false when an accordion is automatically closed by another accordion in the same group. This is currently not happening.

@ddecrulle, since this is your code, would you be open to fixing it?
At the moment, the component does not account for the fact that an accordion can be automatically closed by the @gouvfr/dsfr JS code.
This leads to issues in the controlled mode when multiple accordions are used within a group.
To address this, we should dynamically observe changes, as we do for many other components.
Creating a React wrapper for @gouvfr/dsfr presents challenges, especially in controlled mode. Unlike building the component from scratch, we can't we have to accomodate with the fact that @gouvfr/dsfr is handeling most of the logic and that we can't and don't want to overrule it.
This is evident, for example, in this code which we use to track the open/closed state of a modal.
If we where coding the Modal ourselvs we would just have const [isOpent, setIsOpent] = useState(false).
But since we don't have direct control over the modal—the core JS does—we can only modify HTML attributes and observe the changes applied by the core JS.
The situation with the Accordion component is similar.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants