Skip to content

Commit 76c13e1

Browse files
skvalejbadan
skvale
authored and
jbadan
committed
fix: Forms;SideNav;TreeView: Make React.Children null safe (#535)
1 parent 85d8139 commit 76c13e1

File tree

11 files changed

+203
-90
lines changed

11 files changed

+203
-90
lines changed

‎src/Forms/FormRadioGroup.js‎

Lines changed: 7 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -15,17 +15,13 @@ class FormRadioGroup extends Component {
1515
return (
1616
<div
1717
{...props}>
18-
{React.Children.map(children, child => {
19-
if (React.isValidElement(child)) {
20-
return React.cloneElement(child, {
21-
disabled: child.props.disabled || disabled,
22-
inline: child.props.inline || inline,
23-
name: child.props.name || this.groupId,
24-
onChange: child.props.onChange || onChange
25-
});
26-
} else {
27-
return child;
28-
}
18+
{React.Children.toArray(children).map(child => {
19+
return React.cloneElement(child, {
20+
disabled: child.props.disabled || disabled,
21+
inline: child.props.inline || inline,
22+
name: child.props.name || this.groupId,
23+
onChange: child.props.onChange || onChange
24+
});
2925
})}
3026
</div>
3127
);

‎src/SideNavigation/SideNav.js‎

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -40,15 +40,11 @@ class SideNav extends Component {
4040

4141
return (
4242
<nav {...rest} className={sideNavClasses}>
43-
{React.Children.map(children, (child) => {
44-
if (React.isValidElement(child)) {
45-
return React.cloneElement(child, {
46-
onItemSelect: this.handleSelect,
47-
selectedId: this.state.selectedId
48-
});
49-
} else {
50-
return child;
51-
}
43+
{React.Children.toArray(children).map(child => {
44+
return React.cloneElement(child, {
45+
onItemSelect: this.handleSelect,
46+
selectedId: this.state.selectedId
47+
});
5248
})}
5349
</nav>
5450
);

‎src/SideNavigation/_SideNavList.js‎

Lines changed: 7 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -30,17 +30,13 @@ class SideNavList extends React.Component {
3030
aria-expanded={hasParent && open}
3131
aria-hidden={hasParent && !open}
3232
className={sideNavListClasses}>
33-
{React.Children.map(children, (child) => {
34-
if (React.isValidElement(child)) {
35-
return React.cloneElement(child, {
36-
isSubItem: hasParent,
37-
onItemSelect: onItemSelect,
38-
selected: selectedId === child.props.id,
39-
selectedId: selectedId
40-
});
41-
} else {
42-
return child;
43-
}
33+
{React.Children.toArray(children).map(child => {
34+
return React.cloneElement(child, {
35+
isSubItem: hasParent,
36+
onItemSelect: onItemSelect,
37+
selected: selectedId === child.props.id,
38+
selectedId: selectedId
39+
});
4440
})}
4541
</ul>
4642
);

‎src/SideNavigation/_SideNavListItem.js‎

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -38,11 +38,8 @@ class SideNavListItem extends React.Component {
3838
);
3939
};
4040

41-
let hasChild = false;
42-
React.Children.forEach(children, (child) => {
43-
if (React.isValidElement(child) && child.type === SideNavList) {
44-
hasChild = true;
45-
}
41+
const hasChild = React.Children.toArray(children).some(child => {
42+
return child.type === SideNavList;
4643
});
4744

4845
const renderLink = () => {
@@ -72,8 +69,8 @@ class SideNavListItem extends React.Component {
7269
className='fd-side-nav__item'
7370
key={id}>
7471
{url && renderLink()}
75-
{React.Children.map(children, (child) => {
76-
if (React.isValidElement(child) && child.type !== SideNavList) {
72+
{React.Children.toArray(children).map(child => {
73+
if (child.type !== SideNavList) {
7774
return React.cloneElement(child, {
7875
children: (<React.Fragment>
7976
{glyph ? (
@@ -92,7 +89,7 @@ class SideNavListItem extends React.Component {
9289
}
9390
}
9491
});
95-
} else if (React.isValidElement(child) && child.type === SideNavList) {
92+
} else if (child.type === SideNavList) {
9693
return React.cloneElement(child, {
9794
hasParent: true,
9895
onItemSelect: onItemSelect,

‎src/TreeView/TreeRow.test.js‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -183,6 +183,42 @@ describe('<TreeView.Row />', () => {
183183
</TreeView>
184184
);
185185

186+
const falseyCondition = false;
187+
const conditionalTreeView = (
188+
<TreeView>
189+
<TreeView.Head>
190+
<TreeView.Col>Column Header 1</TreeView.Col>
191+
{falseyCondition && <TreeView.Col>Column Header 2</TreeView.Col>}
192+
<TreeView.Col>Column Header 3</TreeView.Col>
193+
<TreeView.Col>Column Header 4</TreeView.Col>
194+
</TreeView.Head>
195+
<TreeView.Tree>
196+
<TreeView.Item>
197+
<TreeView.Row>
198+
<TreeView.Col>First Level</TreeView.Col>
199+
</TreeView.Row>
200+
<TreeView.Branch>
201+
<TreeView.Item>
202+
<TreeView.Row>
203+
{falseyCondition && (
204+
<TreeView.Col>
205+
<a href='http://me.com'>First Level</a>
206+
</TreeView.Col>
207+
)}
208+
<TreeView.Col>Second level</TreeView.Col>
209+
<TreeView.Col />
210+
<TreeView.Col />
211+
</TreeView.Row>
212+
{falseyCondition && <TreeView.Branch />}
213+
</TreeView.Item>
214+
{falseyCondition && <TreeView.Item />}
215+
</TreeView.Branch>
216+
</TreeView.Item>
217+
{falseyCondition && <TreeView.Item />}
218+
</TreeView.Tree>
219+
</TreeView>
220+
);
221+
186222
test('create tree component', () => {
187223
// multi-level tree
188224
let component = renderer.create(multiLevelTreeView);
@@ -193,6 +229,11 @@ describe('<TreeView.Row />', () => {
193229
component = renderer.create(richTreeView);
194230
tree = component.toJSON();
195231
expect(tree).toMatchSnapshot();
232+
233+
// conditional tree
234+
component = renderer.create(conditionalTreeView);
235+
tree = component.toJSON();
236+
expect(tree).toMatchSnapshot();
196237
});
197238

198239
test('open all tree from header', () => {

‎src/TreeView/TreeView.js‎

Lines changed: 22 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -117,30 +117,28 @@ class TreeView extends Component {
117117

118118
return (
119119
<div {...rest}>
120-
{
121-
React.Children.map(children, (child) => {
122-
const isTreeHead = child.type && child.type.displayName === 'TreeView.Head';
123-
const isTree = child.type && child.type.displayName === 'TreeView.Tree';
124-
125-
if (isTreeHead) {
126-
// Pass expand all callbacks to TreeHead
127-
return React.cloneElement(child, {
128-
onExpandAll: this.toggleExpandAll,
129-
isExpanded: isExpandAll
130-
});
131-
}
132-
133-
if (isTree) {
134-
// Pass expand callbacks to Tree
135-
return React.cloneElement(child, {
136-
expandData,
137-
onExpandClick: this.toggleExpand
138-
});
139-
}
140-
141-
return child;
142-
})
143-
}
120+
{React.Children.toArray(children).map(child => {
121+
const isTreeHead = child.type && child.type.displayName === 'TreeView.Head';
122+
const isTree = child.type && child.type.displayName === 'TreeView.Tree';
123+
124+
if (isTreeHead) {
125+
// Pass expand all callbacks to TreeHead
126+
return React.cloneElement(child, {
127+
onExpandAll: this.toggleExpandAll,
128+
isExpanded: isExpandAll
129+
});
130+
}
131+
132+
if (isTree) {
133+
// Pass expand callbacks to Tree
134+
return React.cloneElement(child, {
135+
expandData,
136+
onExpandClick: this.toggleExpand
137+
});
138+
}
139+
140+
return child;
141+
})}
144142
</div>
145143
);
146144
}

‎src/TreeView/_BaseTree.js‎

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -26,15 +26,13 @@ class BaseTree extends Component {
2626
aria-hidden={level > 0 && !isExpanded}
2727
className={className}
2828
role={level === 0 ? 'tree' : 'group'}>
29-
{
30-
React.Children.map(children, (child) => {
31-
return React.cloneElement(child, {
32-
expandData,
33-
level,
34-
onExpandClick
35-
});
36-
})
37-
}
29+
{React.Children.toArray(children).map(child => {
30+
return React.cloneElement(child, {
31+
expandData,
32+
level,
33+
onExpandClick
34+
});
35+
})}
3836
</ul>
3937
);
4038
}

‎src/TreeView/_TreeHead.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ class TreeHead extends Component {
2323
<div {...rest} className={headerClassName}>
2424
<div className='fd-tree__row fd-tree__row--header'>
2525
{
26-
React.Children.map(children, (child, index) => {
26+
React.Children.toArray(children).map((child, index) => {
2727
const isFirstTreeCol = index === 0 && child.type && child.type.displayName === 'TreeView.Col';
2828

2929
// Add control class to first TreeCol element

‎src/TreeView/_TreeItem.js‎

Lines changed: 12 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -31,33 +31,29 @@ class TreeItem extends Component {
3131
const isExpanded = isExpandedProp || !!expandData[this.rowId];
3232

3333
// Render child TreeBranch with correct props
34-
const childBranch = React.Children.map(children, (child) => {
35-
const isTreeBranch = child.type && child.type.displayName === 'TreeView.Branch';
36-
37-
return isTreeBranch ?
38-
React.cloneElement(child, {
34+
const childBranch = React.Children.toArray(children)
35+
.filter(child => child.type && child.type.displayName === 'TreeView.Branch')
36+
.map(child => {
37+
return React.cloneElement(child, {
3938
expandData,
4039
onExpandClick,
4140
isExpanded,
4241
// Increment child branch level
4342
level: level + 1
44-
}) :
45-
null;
46-
});
43+
});
44+
});
4745

4846
// Render child TreeRow with correct props
49-
const childRow = React.Children.map(children, (child) => {
50-
const isTreeRow = child.type && child.type.displayName === 'TreeView.Row';
51-
52-
return isTreeRow ?
53-
React.cloneElement(child, {
47+
const childRow = React.Children.toArray(children)
48+
.filter(child => child.type && child.type.displayName === 'TreeView.Row')
49+
.map(child => {
50+
return React.cloneElement(child, {
5451
isExpanded,
5552
onExpandClick: () => onExpandClick(this.rowId),
5653
isParent: !!childBranch[0],
5754
rowId: this.rowId
58-
}) :
59-
null;
60-
});
55+
});
56+
});
6157

6258
return (
6359
<li

‎src/TreeView/_TreeRow.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ class TreeRow extends Component {
1414
} = this.props;
1515

1616
// Render child TreeCols
17-
const cells = React.Children.map(children, (child, index) => {
17+
const cells = React.Children.toArray(children).map((child, index) => {
1818
const isTreeCol = child.type && child.type.displayName === 'TreeView.Col';
1919
const isFirstTreeCol = index === 0 && isTreeCol;
2020

0 commit comments

Comments
 (0)