-
Notifications
You must be signed in to change notification settings - Fork 32
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
[Feature][Collapsible] - Allow uncontrolled behavior #51
Comments
Having a component that is both controlled and uncontrolled can lead to many issues, and can become a maintenance nightmare. This is discouraged in the official React docs (they refer to it as a "mistake" no less) and I've also written an article about why it should be avoided - Don’t Mix Controlled and Uncontrolled Values In React. Moreover, if we decide to take it upon ourselves to provide this functionality out of the box, we will have to consider every possible behavior that the user may want to have. For example, what if the consumer would like the trigger to take place on hover instead of click? What if he wants it to only show or only hide, instead of toggle? That said, I don't think there's much to gain from it anyway. Instead of adding 2 new props ( const {visible, toggle} = useVisibilityState();
<div onClick={toggle}>Toggle</div>
<Collapsible expanded={visible}>
Lorem ipsum dolor sit amet
</Collapsible> As for having a large number of collapsibles, the above code can be turned into a component and used as many times as needed. |
I wanted to use Collapsible in some story in Storybook which is written in MDX format, and I wanted to place there lots of collapsible components without managing any state on an MDX file. State makes managing MDX stories more difficult. Normally I use: <details>
<summary>click to toggle</summary>
content here
</details> But it's not as visually pleasing as the Collapsible 🙂 Without having it manage its own open/close state I don't think MDX use-case is very practical |
I see, how about creating a small component for that? Something like: const Details = ({children, title}) => {
const {visible, toggle} = useVisibilityState();
return (
<div className='details'>
<div className='details-header' onClick={toggle}>
{title}
</div>
<Collapsible expanded={visible}>
{children}
</Collapsible>
</div>
);
}; Which you can then use like: <Details title='click to toggle'>
content here
</Details> |
I was considering creating a Github repository only for Storybook-specific components, for common things |
Currently the
<Collapsible/>
may only work as a controlled component by using theexpanded
prop.Sometimes there is no need for outside control and it is wished for the component to show/hide content when clicked, so another prop can be introduced:
defaultExpanded
for the initial state, and from there the component will handle the state internally, as long as theexpanded
prop is not defined.Also, this requires another sub-component, which I suggest something like
Collapsible.Trigger
which will act as the "clicking" area that shows/hides content, this will also give extra control where the triggering content is rendered relative to the collapsible content.Alternatively, add ability to place the
<Trigger>
inside:Then it will be possible to clone & place it outside of the actual collapsible wrapper, so it will always be visible.
This will free developers from creating their own states which is especially time consuming when wanting to use a large number of
Collapsible
componentsThe text was updated successfully, but these errors were encountered: