-
Notifications
You must be signed in to change notification settings - Fork 54
fix(Flex): handle cases of falsy values provided as children #890
Conversation
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
addressed failures when child
is null
- happens when primitive values (other than string
) are passed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
const isFlexItem = child && child.type && ((child.type as any) as typeof FlexItem).__isFlexItem
// or
const isFlexItem: boolean = _.get(child, 'type.__isFlexItem')
Codecov Report
Continue 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
@@ -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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.