Added caching for consistent color of values across visualization. Closes #485 #4429
Conversation
|
Jenkins standing by to test this. If you aren't a maintainer, you can ignore this comment. Someone with commit access, please review this and clear it for Jenkins to run; then say 'jenkins, test it'. |
|
|
||
| return function (value) { | ||
| return colorObj[value]; | ||
| if (!mappedColors.get(value)) | ||
| mappedColors.add(value, colorObj[value]); |
There was a problem hiding this comment.
conditional statements over two lines should include brackets: {}.
For example:
if (!mappedColors.get(value)) {
mappedColors.add(value, colorObj[value]);
}
|
@gauravsinghania In addition, in the On occasion, when I refresh the page, the colors for values change. The colors for each item remain consistent across charts in a dashboard, but the colors change for a value from say red to green. But this may be due to our current "state" issues with our visualization library. I am looking into this. I will also try and get a gif example of what I mean up here. Perhaps the default color for charts with only count values should be: |
| return color(d.label); | ||
| var label = d.label ; | ||
| if (!label) | ||
| label = this.getAttribute('data-label'); |
There was a problem hiding this comment.
Conditional statements on more than one line should include brackets.
|
@stormpython thanks for the input, I will fix the issue with the line chart and brackets. However, for the discover bar chart does a presence of color signify anything or is it okay to have black color? Also, is it only the color of the discover bar chart or some other bad effect? |
|
@stormpython colouring for line and area chart is taken care of. |
|
@gauravsinghania thanks! I will continue to review. Its important that discover bars have a color, and I discussed it with others on the team, and we think the best solution is actually to add an option to set the color explicitly. |
…bel from the element.
|
@stormpython I looked at the issue of discover bars, I felt adding color explicitly might not be the best way. So, I have added label to the elements even for a single data set (earlier label was only added when we have mutliple data sets like in a split graph etc) and thus the color gets auto picked in all cases. Because of this change some of my earlier changes are redundant. Hence, I have merged all the changes to this branch. Do share your thoughts |
|
@gauravsinghania with the new changes there is a bug when you It produces an empty chart although data is present. |
|
@stormpython thanks, will have a look. |
|
@stormpython fixed. Thanks for pointing out that flow. |
|
Ok, this is looking good to me! Passing it on to someone else to review. |
|
@stormpython thanks. |
|
Problem created by this code is it maps colors across all indexes and values alphabetically. This feature will look much better limited to the dashboard rather than spawning everywhere. Or better, by giving user the option to choose if user wants this feature, or feature across all indexes, or per dashboard. Additionally this is worsen by a a sudden remap of a value in the Legend from it's original name to 'Count' and color change to green when that value is filtered. |
Mapping of values and color. So that a given value when repeated across visualization uses the same color for a dashboard. Closes #485.