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

getDataSets does not include options #370

Closed
aguarino77 opened this issue Apr 27, 2016 · 12 comments
Closed

getDataSets does not include options #370

aguarino77 opened this issue Apr 27, 2016 · 12 comments
Labels

Comments

@aguarino77
Copy link

getDataSets is not including options in the dataset object:

    function getDataSets (labels, data, series, colors, yaxis) {
      return {
        labels: labels,
        datasets: data.map(function (item, i) {
          var dataset = angular.extend({}, colors[i], {
            label: series[i],
            data: item
          });
          if (yaxis) {
            dataset.yAxisID = yaxis[i];
          }
          return dataset;
        })
      };
    }

With the current implementation no members other than label and data will be propagated, which makes impossible to set chart properties like "fill: false".

In ChartJs 2.0 this is (line 2626)
fill: line.custom && line.custom.fill ? line.custom.fill : (this.getDataset().fill !== undefined ? this.getDataset().fill : this.chart.options.elements.line.fill),

@aguarino77
Copy link
Author

Is actually related to #309. But the proposed fix is a global settings. I would like to have a per chart/series options like initially proposed by @courchef

@jtblin
Copy link
Owner

jtblin commented Apr 29, 2016

Just use chart-options. Make sure you use the 1.0.0-alpha... series.
On 28 Apr 2016 6:57 am, "aguarino77" [email protected] wrote:

Is actually related to #309
#309. But the proposed
fix is a global settings. I would like to have a per chart/series options
like initially proposed by @courchef https://github.com/courchef


You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub
#370 (comment)

@aguarino77
Copy link
Author

passing this value to chart-options does not work. It used to work in the 0.x series
$scope.chartOptions = {datasetFill: false, animation: false, pointDot : true};

@jtblin
Copy link
Owner

jtblin commented Apr 29, 2016

Probably because the options have changed in Chart.js 2.0 e.g. http://www.chartjs.org/docs/#line-chart-chart-options

@aguarino77
Copy link
Author

If you read the chartjs docs you mention you see how the dataset object should contain the options

var data = {
    labels: ["January", "February", "March", "April", "May", "June", "July"],
    datasets: [
        {
            label: "My First dataset",
            fill: false,
            lineTension: 0.1,
            backgroundColor: "rgba(75,192,192,0.4)",
            borderColor: "rgba(75,192,192,1)",
            borderCapStyle: 'butt',
            borderDash: [],
            borderDashOffset: 0.0,
            borderJoinStyle: 'miter',
            pointBorderColor: "rgba(75,192,192,1)",
            pointBackgroundColor: "#fff",
            pointBorderWidth: 1,
            pointHoverRadius: 5,
            pointHoverBackgroundColor: "rgba(75,192,192,1)",
            pointHoverBorderColor: "rgba(220,220,220,1)",
            pointHoverBorderWidth: 2,
            pointRadius: 1,
            pointHitRadius: 10,
            data: [65, 59, 80, 81, 56, 55, 40],
        }
    ]
};

But if getDataSets is defined like this:

 function getDataSets (labels, data, series, colors, yaxis) {
      return {
        labels: labels,
        datasets: data.map(function (item, i) {
          var dataset = angular.extend({}, colors[i], {
            label: series[i],
            data: item
          });
          if (yaxis) {
            dataset.yAxisID = yaxis[i];
          }
          return dataset;
        })
      };
    }

I see no way to pass an option like "fill: false".
The function above should contain something like:

 function getDataSets (labels, data, series, colors, yaxis, options ) {
      return {
        labels: labels,
        datasets: data.map(function (item, i) {
          var dataset = angular.extend({}, colors[i], {
            label: series[i],
            data: item
          }, options);
          if (yaxis) {
            dataset.yAxisID = yaxis[i];
          }
          return dataset;
        })
      };
    }

@jtblin
Copy link
Owner

jtblin commented Apr 29, 2016

All options can be changed globally or individually. For a line, it looks like it would be elements.line.fill. See https://github.com/jtblin/angular-chart.js/blob/chartjs-2.0/examples/app.js#L42 for an example of how to set options for a specific chart.

@aguarino77
Copy link
Author

I agree that your method works globally.
But the example you cite is not appropriate, because scales is passed in the options object of the Chart line. fill must be passed in the data structure, like label. See http://www.chartjs.org/docs/#line-chart-data-structure. That's why I think dataset should be built by extending not only label, data and colors, but also more options as I suggest above. The same applies for dataset options like lineTension

@jtblin
Copy link
Owner

jtblin commented May 1, 2016

Well I see your point, but it is not correct for fill: false, this is the result of using the option I pointed above on the tick chart in the main example:

$scope.options = {
(..)
      elements: {
        line: {
          borderWidth: 0.5,
          fill: false
        },
        point: {
          radius: 0
        }
      },
(...)
    };

image

vs without this option:

image

I'll have a look at lineTension later.

@aguarino77
Copy link
Author

aguarino77 commented May 1, 2016

Ok, now I see what you mean. Referring to https://github.com/chartjs/Chart.js/blob/v2.0-dev/dist/Chart.js#L2626 you are using the second way to pass the options: they are not bound to the dataset but to the elements object of the chart options.

It might be useful to mention this in the docs, once Chartjs 2 is officially out.

Thank you!

@jtblin
Copy link
Owner

jtblin commented May 1, 2016

Yes, the only limitation is that it's per chart, not per series. I will have to add the possibility to pass the dataset directly as pointed out in #131 for that.

@courchef
Copy link

courchef commented May 1, 2016

Hi guys,

I wrote back in February that I set fill: false in global options of Chart.js, but I realized afterward it wasn't suiting my needs since I have three charts on the same page. I ended up creating two option objects in my scope, one for the main chart and one for the two secondary charts, exatcly like Jerome suggest in his previous comment.

You can view the result where the main chart, on the left, is drawn without filling. The main chart is a custom line chart that includes colored regions.
chart js

Thanks again Jerome for your work!

MDO-OTB added a commit to MDO-OTB/angular-chart.js that referenced this issue May 17, 2016
We could not set the options included in the chart dataset. We could only set the global options thanks to the directive chart-options. Therfore we could not set an option  per chart like described in Issue jtblin#370 .
I added a directive chart-dataset-overload. If it is present, createChart will merge both the dataset and the data set overload at the final step of the dataset creation.
jtblin pushed a commit that referenced this issue Jun 19, 2016
We could not set the options included in the chart dataset. We could only set the global options thanks to the directive chart-options. Therfore we could not set an option  per chart like described in Issue #370 .
I added a directive chart-dataset-overload. If it is present, createChart will merge both the dataset and the data set overload at the final step of the dataset creation.
jtblin added a commit that referenced this issue Jun 19, 2016
* rename datasetOverload to datasetOverride
* use datasetOverride for pie and doughnut charts as well
* add unit and integration tests
* add examples
* fix #370, #336, #391
jtblin added a commit that referenced this issue Jun 19, 2016
* add attribute `chart-dataset-overload`
* complete support for `dataset-override`
* rename `datasetOverload` to `datasetOverride`
* use `datasetOverride` for pie and doughnut charts as well
* add unit and integration tests
* add examples
* fix #370, #336, #391
* remove `y-axis` attribute as it can now be implemented using the `dataset-override` attribute
* compile assets and minor fixes for jshint
@jtblin
Copy link
Owner

jtblin commented Jun 19, 2016

It is now possible to override individual datasets options with #418. Published 1.0.0-alpha8 with this change.

@jtblin jtblin closed this as completed Jun 19, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants