Skip to content
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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

reksc
Copy link

@reksc reksc commented May 21, 2018

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 under colors attribute for each dataset when using chart.update() method.

Some of the competitive libraries have this covered, but they miss the things I like about Frappe Charts.

Screenshots/GIFs:

frappe-charts-different-bar-colors

Steps To Test:
  1. Define a chart and pass an array of colors under the first index of colors option (each root-level index represents a dataset, in our case there is a single dataset only).
testChart = new Chart("#chart-test", {
    title: "Bar Chart with custom colors per each bar",
    data: {
        labels: ["Sample 1", "Sample 2", "Sample 3", "Sample 4"],
        datasets: [{
            "values": [500.412, 882.709, 421.700, 671.034],
        }],
        yRegions: [{
            label: "Max", start: 500, end: 1000,
            options: { labelPos: 'right' }
        }]
    },
    type: 'bar',
    height: 330,
    colors: [['#4dcd32', '#4dcd32', '#dd0453', '#4dcd32']],
});
  1. If needed, update the chart with an additional bar & color:
testChart.update({
    labels: ["Sample 1", "Sample 2", "Sample 3", "Sample 4", "Sample 5"],
    datasets: [
        {
            "values": [500.412, 882.709, 421.700, 671.034, 303.3],
            "colors": ['#4dcd32', '#4dcd32', '#dd0453', '#4dcd32', '#dd0453'], // we're in a context of a particular dataset
        }
    ],
});
TODOs:
  • None

@coveralls
Copy link

coveralls commented May 21, 2018

Pull Request Test Coverage Report for Build 65

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 38.158%

Totals Coverage Status
Change from base Build 63: 0.0%
Covered Lines: 29
Relevant Lines: 62

💛 - Coveralls

@himynameisdave
Copy link
Contributor

This is a good idea, we should look to try and get this merged at some point!

@stephanie-cherba
Copy link

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.

@scmmishra
Copy link
Contributor

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.

  1. Using multiple colours to represent the same dataset is bad from a UX point of view, the purity of data is paramount in visualization
  2. This cannot be effectively represented in the legend as well, not at least in this PR
  3. This will probably work fine only when we have a single dataset, multiple dataset with multiple colours will only confuse users.
  4. I could see a reason you use this, perhaps to show data that's below a critical level, but we have markers for the same purpose.

Let me know your thoughts. Would love some inputs from @himynameisdave as well

@reksc
Copy link
Author

reksc commented Sep 16, 2020

After quite a while seeing some discussion around this PR was a nice surprise!

In response to @scmmishra comments:

  1. Using multiple colours to represent the same dataset is bad from a UX point of view, the purity of data is paramount in visualization

... 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.

  1. This cannot be effectively represented in the legend as well, not at least in this PR

Fully agree, I didn't use legend for this case and it's pretty much a logical consequence of the approach...

  1. This will probably work fine only when we have a single dataset, multiple dataset with multiple colours will only confuse users.

Well, I think that the main role of a chart library is to provide multiple possibilities. Different domains have their sometimes very specific needs.

  1. I could see a reason you use this, perhaps to show data that's below a critical level, but we have markers for the same purpose.

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.

@stephanie-cherba
Copy link

@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."

@scmmishra
Copy link
Contributor

scmmishra commented Sep 18, 2020

@stephlane544 @reksc I see merit in the points you have given, I think we can go ahead and merge this to the library.

the main role of a chart library is to provide multiple possibilities.

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.

Copy link
Contributor

@scmmishra scmmishra left a 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?

image

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.

src/js/objects/ChartComponents.js Outdated Show resolved Hide resolved
@switz
Copy link

switz commented Dec 16, 2020

This would be really helpful; I want to update my datasets while keeping the colors consistent per dataset.

@reksc
Copy link
Author

reksc commented Jan 11, 2021

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 resolveBarColor helper function in utils/colors.js. Happy New Year :).

@reksc reksc requested a review from scmmishra January 11, 2021 12:40
@ashitrath
Copy link

Is there any update on this? This feature would be really helpful.

@Rushan4eg
Copy link

Hey guys! Let's push this one!

@dellow
Copy link

dellow commented Aug 13, 2021

I would love to see this also!

@alexciesielski
Copy link

alexciesielski commented Sep 29, 2021

This would save me so much head ache right now 🚀

@OVillani
Copy link

Colouring different bars is very useful if each bar compares either the same aspect of different entities of different aspects of the same entity:
Case 1: Lets say there are benchmarks to display for different departments/classes/groups. Of course each bar or column will be named at the x-axis, but it is very user friendly to use different colours. So each group/class/department can bee recognized very easily by its colour - especially if there is not only one but several charts representing different aspects for the same departments, classes, ...
Case 2: Lets say one wants to show different KPIs all using the same scale (e.g. percentages, marks, points etc) for the same entity. Each KPI can be shown in a different colour - despite the fact the name will be shown at the base of the bar too. This increases the recognizability for each KPI - again, especially when several charts are using the same colour coding.

By the way: No legend is required in both cases.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.