themesberg / flowbite-react

Official React components built for Flowbite and Tailwind CSS
https://flowbite-react.com
MIT License
1.89k stars 423 forks source link

Conditional Button in a Button.Group breaks application #900

Open emre88tosun opened 1 year ago

emre88tosun commented 1 year ago

Steps to reproduce

  1. Add a conditional Button in a Button.Group

`

{ someCondition && } </Button.Group>`

Current behavior

When a conditional Button is added in a Button.Group it throws

Uncaught Error: React.cloneElement(...): The argument must be a React element, but you passed null. at Object.cloneElement (react.development.js:879:11) at Object.cloneElementWithValidation (react.development.js:2413:33) at React2.cloneElement (whyDidYouRender.js:283:48) at ButtonGroup.js:7:74 at react.development.js:1158:17 at mapIntoArray (react.development.js:1049:23) at mapIntoArray (react.development.js:1099:23) at Object.mapChildren [as map] (react.development.js:1157:3) at ButtonGroup.js:7:42 at mountMemo (react-dom.development.js:17225:19)

Expected behavior

It just should or not render the conditional button

Context

Trying to add just some conditional buttons in a button group

rluders commented 1 year ago

I can confirm that it still happening, and it happens 'cause of the cloneElement that expects to get a ReactElement<ButtonProps> as the children.

CodingWatchCollector commented 1 year ago

Possible solution is to use ternary operator and React.FC, like this: { someCondition ? <Button>Conditional button</Button> : <></>}

Or is it better to handle this case in the ButtonGroup component? It could use filter before map to get rid of nullish elements.

uzegonemad commented 1 year ago

I am experiencing this with the Accordion component as well. The workaround above does work, but considering this syntax for conditional rendering is commonly used, it would be more ideal to address this in the library instead of in userland.

Repro:

<Accordion>
    <Accordion.Panel>
        <Accordion.Title>Title</Accordion.Title>
        <Accordion.Content>Content</Accordion.Content>
    </Accordion.Panel>
    {false === true && (
        <Accordion.Panel>
            <Accordion.Title>Title 2</Accordion.Title>
            <Accordion.Content>Content 2</Accordion.Content>
        </Accordion.Panel>
    )}
</Accordion>
Stupidism commented 9 months ago

I'm using pnpm patch to workaround this.

image