dc-js / dc.js

Multi-Dimensional charting built to work natively with crossfilter rendered with d3.js
Apache License 2.0
7.42k stars 1.81k forks source link

way to identify chart type #904

Open gordonwoodhull opened 9 years ago

gordonwoodhull commented 9 years ago

it's been requested a couple of times recently, here:

http://stackoverflow.com/questions/29409674/determine-identify-dc-js-chart-type

and on gitter.

The workaround (as suggested by @mtraynham) is to DIY:

var chart = dc.rowChart("#history");
chart.chartType = 'row';
chart.chartType; // "row"

But we might as well do this in the library, if there's no reason not to.

mtraynham commented 9 years ago

A proposal for how this works:

We should only provide getter access to this property. It probably should not be set up like a chainable getter/setter, and instead I'm inclined to say it's just an exposed property on the chart.

Since everything inherits from the base-mixin, we can add the functionality there.

Consider the following solution: https://github.com/dc-js/dc.js/blob/develop/src/base-mixin.js#L7

dc.baseMixin = function (_chart, _chartType) {
    Object.defineProperty(_chart, 'chartType', {
          value: _chartType,
          enumerable: true
    }); 
    ...
    return _chart;
};

https://github.com/dc-js/dc.js/blob/develop/src/heatmap.js#L46

var _chart = dc.colorMixin(dc.marginMixin(dc.baseMixin({}, 'heatmap')));

Usage:

var chart = dc.heatmap('#chart1');
console.log(chart.chartType); // 'heatmap'
chart.chartType = 'bar';
console.log(chart.chartType); // 'heatmap'

Defaults of Object.defineProperty are:

{
    writable: false,
    configurable: false,
    enumerable: false
}

It wouldn't be bad to find this when enumerating the properties of the chart. It would be wrong to be able to assign the property.

gordonwoodhull commented 9 years ago

That sounds reasonable.

Initially I thought that people might derive their own charts from dc.js charts, but that is already illegal because charts have different contructors from mixins. I guess the only way to find out if people need to identify their charts differently is to implement it as you suggest, and see if people complain.

Obviously dc.js has never had a mission of protecting users from themselves, of locking things down that people might break. It is sometimes helpful to override methods that weren't meant to be overridden, as a workaround or to implement unsupported behavior. But I don't see that applying here.

mtraynham commented 9 years ago

Interesting thought. It's very plausible to derive their own charts, but I myself have never encountered the case and more often altered the existing charts for features. It may not be totally illegal, as there is nothing stopping a user from doing:

var baseBar = dc.barChart;
dc.barChart = function (parent, chartGroup) {
// dc.extendedBarChart = function (parent, chartGroup) {
    var chart = baseBar(parent, chartGroup);
    // overrides?
    return chart;
}