chore(fab): Pass icon as a prop, not a child element #159
Conversation
packages/fab/index.js
Outdated
| return; | ||
| } | ||
|
|
||
| return this.addClassesToElement('mdc-fab__icon', icon); |
There was a problem hiding this comment.
since you only use this addClassesToElement once, I think you should inline the logic:
const updatedProps = {
className: classnames('mdc-fab__icon', icon.props.className),
};
return React.cloneElement(icon, updatedProps);
packages/fab/index.js
Outdated
| </button> | ||
| ); | ||
| } | ||
|
|
||
| addClassesToElement(classes, element) { |
Codecov Report
@@ Coverage Diff @@
## master #159 +/- ##
=======================================
Coverage 95.78% 95.78%
=======================================
Files 21 21
Lines 712 712
Branches 63 64 +1
=======================================
Hits 682 682
Misses 30 30
Continue to review full report at Codecov.
|
packages/fab/index.js
Outdated
| </button> | ||
| ); | ||
| } | ||
| } | ||
|
|
||
| Fab.propTypes = { | ||
| mini: PropTypes.bool, | ||
| icon: PropTypes.element, | ||
| className: PropTypes.string, | ||
| children: PropTypes.oneOfType([ |
There was a problem hiding this comment.
remove children from proptypes. I think you're planning on using label or text props instead.
| @@ -47,14 +52,15 @@ export class Fab extends React.Component { | |||
| className={this.classes} | |||
| ref={initRipple} | |||
| {...otherProps}> | |||
| {this.addIconClassToAllChildren()} | |||
| {this.renderIcon()} | |||
There was a problem hiding this comment.
comment for line above ^ remove children from list, since you don't use it
| return React.cloneElement(item, props); | ||
| }); | ||
| }; | ||
| renderIcon() { |
There was a problem hiding this comment.
comment for lines above^
You are not using this.state.classList. Please remove that from state.
This is to prepare us for the extended FAB (#127) where we need to accept both an icon and a label as input. Both icon and label will be passed as props to the
Fabcomponent