-
Notifications
You must be signed in to change notification settings - Fork 722
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Ability to add different colors for bars in the same dataset #179
base: master
Are you sure you want to change the base?
Conversation
This is a good idea, we should look to try and get this merged at some point! |
Is this something that will still be merged? I would like to use frappe, but will have to go a different route if this PR isn't merged, as my use case involves different colored bars. |
Heya @stephlane544 sorry this took a while to review. There are a few problems with this feature for which I think merging this won't be a good idea.
Let me know your thoughts. Would love some inputs from @himynameisdave as well |
After quite a while seeing some discussion around this PR was a nice surprise! In response to @scmmishra comments:
... in my case a different color for a sample that didn't pass validation was highly demanded by the end users. They work at a high pace, having seconds to notice the result. We experimented with different types of visualization and that approach was picked as optimal.
Fully agree, I didn't use legend for this case and it's pretty much a logical consequence of the approach...
Well, I think that the main role of a chart library is to provide multiple possibilities. Different domains have their sometimes very specific needs.
Markers were less verbose in my case. Btw, in a different project I use Highcharts bar chart to display the general share of top 5 customers, each one has it's own color (which is picked from their graphic identity). It's much more appealing than a standard single-color chart and my users really like it. They don't need to read the labels to notice in a flash, that this week X overtook Z. |
@scmmishra My use case needs the different color bars as well, because it's one data set, but showing different levels of something, and using the corresponding colors for the levels is used all throughout our platform, so for UI, it's actually better for us to use these colors so that the users can easily see what the data is just by the colors. In addition, it's a small enough chart, that we don't have any labels or markers on the x-axis and having the colors in addition to the custom tooltips will help users see what's going on better. Additionally, my company does customer validation and customer confirmation testing, and this has all been verified through those processes and the chart is in use in other areas of the platform, so it's not going to change. I also agree with @reksc above when they said "the main role of a chart library is to provide multiple possibilities. Different domains have their sometimes very specific needs." |
@stephlane544 @reksc I see merit in the points you have given, I think we can go ahead and merge this to the library.
This point holds true! I shall review this PR, see if we can make anything better, and then we can go ahead and merge this. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @reksc, for some weird reason, the default color for the charts is now black.
Can you look at this bug?
So in case no color is provided by default, the charts render black. Also, can you rebase your branch with the master, that'll resolve any conflicts and will keep the git history clean.
P.S. I have a testbed, you can use this to debug the issue.
This would be really helpful; I want to update my datasets while keeping the colors consistent per dataset. |
Hi @scmmishra, I finally found some time to upgrade the PR. I think I addressed all the issues mentioned above. The bar chart should fall back gently to the default colors set, if none are provided. I also refactored the ternary statements spaghetti to a |
Is there any update on this? This feature would be really helpful. |
Hey guys! Let's push this one! |
I would love to see this also! |
This would save me so much head ache right now 🚀 |
Colouring different bars is very useful if each bar compares either the same aspect of different entities of different aspects of the same entity: By the way: No legend is required in both cases. |
Explanation About What Code Achieves:
This change to the BarChart adds support for different bar colors within the same dataset. I had a particular use-case when the chart is dynamically updated with incoming data and depending on the value it should add a green/red bar respectively.
Now the library supports array values inside
colors
option and will check for an array undercolors
attribute for each dataset when usingchart.update()
method.Some of the competitive libraries have this covered, but they miss the things I like about Frappe Charts.
Screenshots/GIFs:
Steps To Test:
colors
option (each root-level index represents a dataset, in our case there is a single dataset only).TODOs: