AccordionTab: is being removed from DOM on closing Tab

[kingmauri]

Describe the bug

When using the accordion component, the tab is removed from the DOM once it is closed. This could be problematic, for example, when forms and input fields are used as content, as the content of the input field is deleted when closing and reopening. Of course, one could store the content of the field in a useState (as in the example), but the problem with form validation persists because only the fields of the current tab are validated, not all of them. The entire form cannot be validated if there are form fields in the different accordion tabs.

I compared the component with other libraries like PrimeVue and PrimeNG, where the content of the DOM is NOT deleted. Is there a simple way to prevent the accordion tab from being removed from the DOM when the tab is closed?

Reproducer

https://stackblitz.com/edit/rdg6zx-hjxnyv?file=src%2FApp.jsx

PrimeReact version

10.6.4

React version

17.x

Language

TypeScript

Build / Runtime

Vite

Browser(s)

No response

Steps to reproduce the behavior

No response

Expected behavior

No response

[jerlam06]

I completely agree, this makes the accordion non usable in most cases. The choice should be given to the user, if possible. Or by default, the content should be hidden but not removed from the DOM.

[sja-cslab]

I disagree. See my comment in @jerlam06 issue #6755

Even at react.dev docs they do it the way PrimeReact is doing it.

[melloware]

I tend to agree with @sja-cslab here. Removing something from the DOM is always the preferred approach as their is less in the DOM tree not being used.

[cesarve77]

I think that should be a developer’s (use case) decision. With the current approach, there is no workaround to keep the state of the tab content. However, if it is a developer’s decision, they can unmount the tab content as they wish.

Currently, the Accordion only works to show static content, which goes against React’s dynamic nature.

Regarding this comment in @jerlam06’s issue #6755, it is not the same case. The current React implementation does not allow you to close the tab while keeping dynamic tab content that retains its state.

[sja-cslab]

@cesarve77 I stand my point - lift the state up and you always know the state of whatever component you try to use no matter if it's rendered or not.

I can totally see jerlams and your point but for me it's neither "static only" or "against React's dynamic nature" it's just a way an implementation should look like in that case.

The reason I stand my ground is because we've used accodions too and did not have such problems. PrimeReact is a component library - those show what you pass in, no less no more. If you've complex data and problems passing them around - think about using redux or any other library that's exactly doing this.

[jerlam06]

@sja-cslab No, the easiest way is just not to use primereact, as a developer working on a codebase that was there years before I arrived at the company, I can't just refacto thousands of line of code for the only sake of being able to use a stupid accordean component. This conversation is being ridiculous. Your opinion is only based on your own experience, which is the best case scenario...which is not the case for everyone.

[cesarve77]

In my case, I have a completely encapsulated uncontrolled component (its state cannot be lifted). It’s not about complex data; the issue is that it’s uncontrolled.

What I kindly ask is that you don’t make the decision on your own. If you can discuss it with someone else on your team, I would really appreciate it, and, ultimately, include in the documentation that the Accordion component only works correctly with controlled components, as it does not maintain the state of any component introduced inside it since it gets unmounted every time the Accordion is closed. This way, the discussion would be settled.

Thank you.

[cesarve77]

@sja-cslab No, the easiest way is just not to use primereact, as a developer working on a codebase that was there years before I arrived at the company, I can't just refacto thousands of line of code for the only sake of being able to use a stupid accordean component. This conversation is being ridiculous. Your opinion is only based on your own experience, which is the best case scenario...which is not the case for everyone.

It’s probably easier to recreate the Accordion component based on the existing CSS. I’m currently in the early stages of developing a large-scale project, and switching libraries would likely delay us by about two months, which isn’t much considering we estimate it will take 2.5 years to complete. What worries me is what other kinds of issues like this we might encounter.

Out of curiosity, I reviewed Accordion components from other popular libraries, and all of them work in a way that would suit us. However, switching libraries doesn’t guarantee that we won’t face other problems with other components.

Thinking it through, the issue isn’t the component itself; it’s the approach. When developing a public library, the goal should be to cover most use cases, of course without resorting to bad practices. In this case, I don’t believe leaving it up to the developer to decide whether or not to unmount the tab’s content is a bad practice. If it were, the solution would be to make unmounting the default behavior and provide a property, such as _veryBadPractice_keepContentMounted, so the developer can ultimately make the choice.

I would definitely expect this issue to be resolved before considering switching libraries.

[sja-cslab]

@cesarve77 I'm not sure if you'll get any answers to this from PrimeTek. Currently they're working on a new PrimeReact Version which should be, based on their words, a complete remake of the current lib. A first preview should be available this year.

Edit:

In my case, I have a completely encapsulated uncontrolled component (its state cannot be lifted). It’s not about complex data; the issue is that it’s uncontrolled.

What I kindly ask is that you don’t make the decision on your own. If you can discuss it with someone else on your team, I would really appreciate it, and, ultimately, include in the documentation that the Accordion component only works correctly with controlled components, as it does not maintain the state of any component introduced inside it since it gets unmounted every time the Accordion is closed. This way, the discussion would be settled.

Thank you.

I don't decide anything just telling you my experience and my opininion. As far as I can see you did not tell anything about an uncontrolled component before. I that case I can't tell you anything because I'm not using uncontrolled things.

[sja-cslab]

@sja-cslab No, the easiest way is just not to use primereact, as a developer working on a codebase that was there years before I arrived at the company, I can't just refacto thousands of line of code for the only sake of being able to use a stupid accordean component. This conversation is being ridiculous. Your opinion is only based on your own experience, which is the best case scenario...which is not the case for everyone.

@jerlam06 Of course it's based on my experience, else I would not have started to comment at all. And as said i totally see your point so please don't get me wrong and dont start getting rude. I'm just saying that there're ways to handle this and that a refactor of the current implementation wont happen (maybe the implementation will change in coming version, I dont know).

[cesarve77]

Thanks, I am not trying to be rude my english maybe does not help, (or the translation),

[sja-cslab]

@cesarve77 you wasn't rude at all dont worry, was not talking to you ;)

[jerlam06]

@sja-cslab Sorry I did not mean to be rude, I lost my temper. I understand the proper way would be to lift the state up, but in our case it is not possible for multiple reasons. The only solutions for us are: allow the dev to choose if he wants to unmount the content or just hide it, or use an alternative solution (other lib maybe). Now, I will respect the final word on this matter ;)

[sja-cslab]

@cesarve77 @jerlam06 Don't forget that i'm just as part of the Prime-Community as you're. My words are only opinions. @melloware added a discussion tag to this issue so I started exactly that discussion and because of that, we've some more information that we didn't have before. E.g. use of uncontrolled components.

Based on that information we (the community) or PrimeTek may get another view on that.

[jerlam06]

@sja-cslab @cesarve77 After a little investigation, I found that MUI, antd, ChakraUI, SyncFusion and SemanticUI components libraries do not remove the content of the DOM for the accordeon. And other libs such as Blueprint, have a prop to keep children mounted on collapse. Now, I rest my case.