fix(Flex): handle cases of falsy values provided as children #890
Conversation
kuzhelov
requested review from
dvdzkwsk,
layershifter,
levithomason,
miroslavstastny and
mnajdova
as code owners
| let isFirstElement = true | ||
| return React.Children.map(children, (child: any) => { | ||
| const maybeChildElement = | ||
| child && child.type && ((child.type as any) as typeof FlexItem).__isFlexItem |
There was a problem hiding this comment.
addressed failures when child is null - happens when primitive values (other than string) are passed
There was a problem hiding this comment.
const isFlexItem = child && child.type && ((child.type as any) as typeof FlexItem).__isFlexItem
// or
const isFlexItem: boolean = _.get(child, 'type.__isFlexItem')
Codecov Report
@@ Coverage Diff @@
## master #890 +/- ##
=======================================
Coverage 92.99% 92.99%
=======================================
Files 21 21
Lines 728 728
Branches 69 69
=======================================
Hits 677 677
Misses 51 51Continue to review full report at Codecov.
|
| {renderGap && gap && <Flex.Gap className={cx(`${Flex.className}__gap`, gapClasses)} />} | ||
| {childElement} | ||
| </> | ||
| maybeChildElement && ( |
There was a problem hiding this comment.
Do we need a condition there?
Without it we will get:
<>
{null}
{null}
</>But it will not affect the markup
There was a problem hiding this comment.
Nevermind,
If children is null or undefined, this method will return null or undefined rather than an array.
https://reactjs.org/docs/react-api.html#reactchildrenmap
There was a problem hiding this comment.
yes - but, essentially, we won't like to draw anything if falsy value is provided - and this is exactly the effect logic provides. Thus I would rather leave it the way it is now - as a benefit this won't introduce any additional React elements to output elements tree
kuzhelov
changed the title
| @@ -78,7 +78,7 @@ class FlexItem extends UIComponent<Extendable<FlexItemProps>> { | |||
| }) | |||
| } | |||
|
|
|||
| return applyStyles(React.Children.only(children), styles, classes) | |||
| return children ? applyStyles(React.Children.only(children), styles, classes) : children | |||
There was a problem hiding this comment.
| return children ? applyStyles(React.Children.only(children), styles, classes) : children | |
| return _.isNil(children) ? null : applyStyles(React.Children.only(children), styles, classes) |
May be this will be more clear?
No description provided.