feat(Accordion): Added support for bordered, display large … #6085
Conversation
…and mutilple body content
|
PF4 preview: https://patternfly-react-pr-6085.surge.sh |
| @@ -18,6 +19,8 @@ export interface AccordionContentProps extends React.HTMLProps<HTMLDivElement> { | |||
| 'aria-label'?: string; | |||
| /** Component to use as content container */ | |||
| component?: React.ElementType; | |||
| /** Flag indicating content is custom. Expanded content Body wrapper will be removed from children. This allows multiple bodies to be rendered as content. */ | |||
| isCustomContent?: React.ReactNode; | |||
There was a problem hiding this comment.
This is an interesting way to handle this. I think we don't usually conditionally expect the consumer to wrap elements in a body component. I wonder if it's be more consistent if we allowed people to just pass a single child, or an array of children? and we wrap things in body components for them?
There was a problem hiding this comment.
@nicole I was debating both approaches. I initially stared with the single Node or array. I was thinking in the long run it would be nice to handle this similar to other components with bodies (Card, Drawer) where the consumer composes it themself, that is why I took this approach.
There was a problem hiding this comment.
Would it be better then, to always tell the consumer to wrap content in a body?
There was a problem hiding this comment.
Maybe we could update to always wrapping the content in a body in the next breaking release?
There was a problem hiding this comment.
@nicolethoen that would break consumers who are already using the component.
@kmcfaul that is what I was thinking long term.
I am ok with either approach here... Like I said, I was debating which one to go with.
There was a problem hiding this comment.
I think either way will work, as long as it's clear.
There was a problem hiding this comment.
@kmcfaul what do you think?
There was a problem hiding this comment.
I think having the custom prop is good until we can refactor 👍
| @@ -5,42 +5,45 @@ cssPrefix: pf-c-accordion | |||
| propComponents: ['Accordion', 'AccordionItem', 'AccordionContent', 'AccordionToggle'] | |||
There was a problem hiding this comment.
Could you add AccordionExpandedContentBody here
…and multiple body content
What: Closes #5498