Skip to content
This repository has been archived by the owner on Mar 4, 2020. It is now read-only.

fix(Flex): handle cases of falsy values provided as children #890

Merged
merged 9 commits into from
Feb 12, 2019
Merged

fix(Flex): handle cases of falsy values provided as children #890

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
de5656f
add null checks
kuzhelov-ms Feb 12, 2019
6192676
address flex gap issue
kuzhelov-ms Feb 12, 2019
408e36c
improve null detection logic
kuzhelov-ms Feb 12, 2019
9f54e63
Merge branch 'master' into fix/flex
kuzhelov Feb 12, 2019
1f6d91f
update changelog
kuzhelov-ms Feb 12, 2019
0aaa6a5
upd changelog
kuzhelov-ms Feb 12, 2019
5a3bd2a
fix changelog
kuzhelov-ms Feb 12, 2019
47b787b
upd
kuzhelov-ms Feb 12, 2019
5d0cd5c
improve expressiveness of filtering expression
kuzhelov-ms Feb 12, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,12 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm

## [Unreleased]

### Features
- Export `arrow-up`,`arrow-down` and `chat` SVG icon @VyshnaviDasari ([#873](https://github.com/stardust-ui/react/pull/873))

### Fixes
- Properly handle falsy values provided as `Flex` and `Flex.Item` children @kuzhelov ([#890](https://github.com/stardust-ui/react/pull/890))

<!--------------------------------[ v0.21.0 ]------------------------------- -->
## [v0.21.0](https://github.com/stardust-ui/react/tree/v0.21.0) (2019-02-12)
[Compare changes](https://github.com/stardust-ui/react/compare/v0.20.0...v0.21.0)
Expand Down Expand Up @@ -83,7 +89,6 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
- Add sample screener tests with steps for `Dropdown` @silviuavram ([#797](https://github.com/stardust-ui/react/pull/797))
- Add shorthand support for `triggerButton` in `Dropdown` @silviuavram ([#815](https://github.com/stardust-ui/react/pull/815))
- Add toggle functionality in the `Popoup` even if the `trigger` is not button @kolaps33 ([#758](https://github.com/stardust-ui/react/pull/758))
- Export `arrow-up`,`arrow-down` and `chat` SVG icon @VyshnaviDasari ([#873](https://github.com/stardust-ui/react/pull/873))

### Fixes
- Handle `onClick` and `onFocus` on ListItems correctly @layershifter ([#779](https://github.com/stardust-ui/react/pull/779))
Expand Down
33 changes: 20 additions & 13 deletions packages/react/src/components/Flex/Flex.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,20 +90,27 @@ class Flex extends UIComponent<ReactProps<FlexProps>> {
renderChildren = (gapClasses: string) => {
const { column, gap, children } = this.props

return React.Children.map(children, (child: React.ReactElement<any>, index) => {
const childElement =
child.type && ((child.type as any) as typeof FlexItem).__isFlexItem
? React.cloneElement(child, {
flexDirection: column ? 'column' : 'row',
})
: child

const renderGap = index !== 0
let isFirstElement = true
return React.Children.map(children, (child: any) => {
const isFlexItemElement: boolean = _.get(child, 'type.__isFlexItem')
const maybeChildElement = isFlexItemElement
? React.cloneElement(child, {
flexDirection: column ? 'column' : 'row',
})
: child

const renderGap = !isFirstElement
if (maybeChildElement) {
isFirstElement = false
}

return (
<>
{renderGap && gap && <Flex.Gap className={cx(`${Flex.className}__gap`, gapClasses)} />}
{childElement}
</>
maybeChildElement && (
Copy link
Member

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

Copy link
Member

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

Copy link
Contributor Author

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

<>
{renderGap && gap && <Flex.Gap className={cx(`${Flex.className}__gap`, gapClasses)} />}
{maybeChildElement}
</>
)
)
})
}
Expand Down
5 changes: 4 additions & 1 deletion packages/react/src/components/Flex/FlexItem.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import * as React from 'react'
import * as PropTypes from 'prop-types'
import cx from 'classnames'
import * as _ from 'lodash'
import { UIComponent, commonPropTypes } from '../../lib'
import { mergeStyles } from '../../lib/mergeThemes'
import { Extendable } from '../../types'
Expand Down Expand Up @@ -78,7 +79,9 @@ class FlexItem extends UIComponent<Extendable<FlexItemProps>> {
})
}

return applyStyles(React.Children.only(children), styles, classes)
return _.isNil(children)
? children
: applyStyles(React.Children.only(children), styles, classes)
}
}

Expand Down