PXD-1368 fix(localconf): vis colors in localconf.js #338
Conversation
| stroke={localTheme['barGraph.lineColor']} | ||
| fill={localTheme['barGraph.lineColor']} | ||
| stroke='#666666' | ||
| fill='#666666' |
There was a problem hiding this comment.
is there a way to make these configurable, like pass a prop to the chart?
There was a problem hiding this comment.
I think for now we only have 1 style (with white background), it is not necessary to make text colors configurable...
There was a problem hiding this comment.
true, but if we want to use this chart anywhere else in the future we'd want it to be a prop not hardcoded --> moving to separate ticket to customize all styles [PXD-1521]
src/localconf.js
Outdated
| 'pieChartTwoColor.pie1Color': '#3283c8', | ||
| 'pieChartTwoColor.pie2Color': '#e7e7e7', | ||
| tableBarColor: '#7d7474', | ||
| const visColor = { |
There was a problem hiding this comment.
Can we rename visColor, categoryColor9, and categoryColor2? I feel like they could be more descriptive like colorsForCharts or something
There was a problem hiding this comment.
Sure~ How about visColor->colorsForCharts, categoryColor9 -> category9Colors, categoryColor2 -> category2Colors?
There was a problem hiding this comment.
colorsForCharts is good, but I think the other two are still not clear in terms of what they contain, can they be something like barChartColors and pieChartColors
3b38ca6 to
3a5ed1e
Compare
3a5ed1e to
606b3ca
Compare
|
Committed by mistake, sorry about that, just remove the useless file, please check again~ |
Change visualization colors in localconf.js: since only charts are using those colors, let helper access the colors, and remove all
localThemeprops from previous chart-related components.