ColorPicker => MultiColorPicker #625
Conversation
|
Behaviour looks good, 💃 from me subject to @dmt0 's code review |
| ColorPicker.propTypes = { | ||
| onColorChange: PropTypes.func.isRequired, | ||
| selectedColor: PropTypes.string, | ||
| selectedColor: PropTypes.any, |
There was a problem hiding this comment.
There's a handy construct for this:
PropTypes.oneOfType([PropTypes.string, PropTypes.number])
| } | ||
|
|
||
| setColor(color) { | ||
| this.props.updatePlot(color); |
There was a problem hiding this comment.
why not just pass updatePlot as a prop?
There was a problem hiding this comment.
It is passed, from the connect wrapper, I just prefer having a separate function,
one setColor, one setColors, that's all
| {label: _('Multiple'), value: 'multiple'}, | ||
| ]; | ||
| const selectedConstantColorOption = this.props.parentState | ||
| ? this.props.parentState.selectedConstantColorOption |
There was a problem hiding this comment.
parentState only has one variable, why not just have parentSelectedConstantColorOptions?
There was a problem hiding this comment.
I'm using parent state to pass MarkerColor state, it has more than one variable.
There was a problem hiding this comment.
I'm not seeing other variables
No description provided.