ColorSliderGroup - allow sending translation to the labels #782
Conversation
| * In case you would like to change the default labels (translations etc.), you can provide | ||
| * a function that received the type (one of GradientSlider.types without DEFAULT) and returns a string. | ||
| */ | ||
| getLabel: PropTypes.func, |
There was a problem hiding this comment.
Wouldn't be simpler to allow passing a map of labels
{hue: ..., lightness:..., Saturation: ..} ?
You can set here the default map in default props
in our private config you can override the defaultProps (not using ThemeManager)
There was a problem hiding this comment.
I am not sure what the best solution.
In your example you have {hue: ..., lightness:..., Saturation: ..} - lower case 'h', lower case 'l' and upper case 's' (which will cause something not to work), that is less likely to happen if you are using types (and I can't set the key to that in a simple readable way).
I started with:
labels = {};
labels[GradientSlider.types.HUE] = 'Hue';
...
But that seemed less clean.
What do you think?
There was a problem hiding this comment.
This is a very small feature and I think it's best to got with the minimum changes possible.
In my example I meant everything to be lower case, it's clearly a mistake.
I don't see any reason to add types and extra function prop just for that.
I would usually use functions for dynamic values, the title texts are static, you set them once and don't need to touch them again
There was a problem hiding this comment.
Changed to defaultProps
| @@ -62,18 +71,29 @@ export default class ColorSliderGroup extends PureBaseComponent { | |||
| _.invoke(this.props, 'onValueChange', value); | |||
| } | |||
|
|
|||
| getSlider = (type) => { | |||
There was a problem hiding this comment.
I think it's better to rename the function renderSlider like we usually do for render functions
M-i-k-e-l
deleted the
feat/color-slider-group-allow-sending-translation
branch
No description provided.