feat(ToggleGroup): support disabling all items under the parent#6700
feat(ToggleGroup): support disabling all items under the parent#6700tlabaj merged 6 commits intopatternfly:mainfrom boaz0:closes_6359
Conversation
Signed-off-by: Boaz Shuster <boaz.shuster.github@gmail.com>
Add description above disableAll
Update one of the examples to show a use case of disabelAll
Use React.Children.toArray and map instead of forEach
Change ToggleGroup to toggle group in disableAll description
| const childCompName = (child as any).type.name; | ||
| return childCompName !== ToggleGroupItem.name | ||
| ? child | ||
| : React.cloneElement(child as React.ReactElement, disableAll ? { isDisabled: true } : {}); |
There was a problem hiding this comment.
We try and avoid cloneElement when we can. Can we do this with a render prop instead?
There was a problem hiding this comment.
In the current state, I don't think so. The closest we can do is to create ToggleGroupContext which will pass this to other ToggleGroupItems through its context.
What do you think?
There was a problem hiding this comment.
Hmmm. That might be a better option @boaz0 consumers have asked we avoid cloneElement as it can be problematic.
There was a problem hiding this comment.
So you want me to do go with the context idea and revert the current changes I've made?
There was a problem hiding this comment.
I think either method works in this case because we're type checking the child before cloning
There was a problem hiding this comment.
@boaz0 I think your approach is fine for this use case.
|
Your changes have been released in:
Thanks for your contribution! 🎉 |
|
Thank you all! @nicolethoen @wise-king-sullyman @tlabaj @mcoker @kmcfaul |
What: Closes #6359