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

dc.js should use a namespace when subscribing to events #842

Open
JackStouffer opened this issue Jan 20, 2015 · 8 comments
Open

dc.js should use a namespace when subscribing to events #842

JackStouffer opened this issue Jan 20, 2015 · 8 comments
Milestone

Comments

@JackStouffer
Copy link

When creating two charts where one is focused on the other with either rangeChart or focusChart, adding an on filtered event to the rangeChart overrides the behavior. For example, these are charts from the stock demo:

moveChart
        .renderArea(true)
        .width(990)
        .height(200)
        .transitionDuration(1000)
        .margins({top: 30, right: 50, bottom: 25, left: 40})
        .dimension(moveMonths)
        .mouseZoomable(true)
        // Specify a range chart to link the brush extent of the range with the zoom focue of the current chart.
        .rangeChart(volumeChart)
        .x(d3.time.scale().domain([new Date(1985, 0, 1), new Date(2012, 11, 31)]))
        .round(d3.time.month.round)
        .xUnits(d3.time.months)
        .elasticY(true)
        .renderHorizontalGridLines(true)
        .legend(dc.legend().x(800).y(10).itemHeight(13).gap(5))
        .brushOn(false)
        // Add the base layer of the stack with group. The second parameter specifies a series name for use in the
        // legend
        // The `.valueAccessor` will be used for the base layer
        .group(indexAvgByMonthGroup, 'Monthly Index Average')
        .valueAccessor(function (d) {
            return d.value.avg;
        })
        // stack additional layers with `.stack`. The first paramenter is a new group.
        // The second parameter is the series name. The third is a value accessor.
        .stack(monthlyMoveGroup, 'Monthly Index Move', function (d) {
            return d.value;
        })
        // title can be called by any stack layer.
        .title(function (d) {
            var value = d.value.avg ? d.value.avg : d.value;
            if (isNaN(value)) {
                value = 0;
            }
            return dateFormat(d.key) + '\n' + numberFormat(value);
        });

    volumeChart.width(990)
        .height(40)
        .margins({top: 0, right: 50, bottom: 20, left: 40})
        .dimension(moveMonths)
        .group(volumeByMonthGroup)
        .centerBar(true)
        .gap(1)
        .x(d3.time.scale().domain([new Date(1985, 0, 1), new Date(2012, 11, 31)]))
        .round(d3.time.month.round)
        .alwaysUseRounding(true)
        .xUnits(d3.time.months);

however, if you add

 volumeChart.width(990)
        .height(40)
        .margins({top: 0, right: 50, bottom: 20, left: 40})
        .dimension(moveMonths)
        .group(volumeByMonthGroup)
        .centerBar(true)
        .gap(1)
        .x(d3.time.scale().domain([new Date(1985, 0, 1), new Date(2012, 11, 31)]))
        .round(d3.time.month.round)
        .alwaysUseRounding(true)
        .xUnits(d3.time.months)
        .on("filtered", function (chart) { doSomething(); });

to the end of the volumeChart, moveChart will only show the selected values, but it no longer updates its x domain.

@gordonwoodhull
Copy link
Contributor

You can work around this by putting your event in a namespace, e.g. on("filtered.foo", ...).

Probably dc.js should namespace its own use of events, but it has only recently been ported to d3.dispatch which supports namespaces.

@JackStouffer
Copy link
Author

I was able to work around it by copying the interal focusChart method into my event, like so

                   .on("filtered", function (cart) {
                       doSomething():

                       if (!chart.filter()) {
                            dc.events.trigger(function () {
                                moveChart.x().domain(moveChart.xOriginalDomain());
                            });
                        } else if (!rangesEqual(chart.filter(), moveChart.filter())) {
                            dc.events.trigger(function () {
                                moveChart.focus(chart.filter());
                            });
                        }
                    });

But I believe it's a bug when a user can unknowingly override internal library behavior.

@gordonwoodhull
Copy link
Contributor

Does the namespacing not work? Seems a lot more elegant to me.

@JackStouffer
Copy link
Author

Oh it does, I was just posting how I originally fixed it.

@gordonwoodhull
Copy link
Contributor

Cool. Yes, it probably makes sense for dc.js to always subscribe and unsubscribe using a namespace like .dcjs - I'll retitle this.

@gordonwoodhull gordonwoodhull added this to the v2.1 milestone Jan 20, 2015
@gordonwoodhull gordonwoodhull changed the title Custom On Filtered Event Overrides rangeChart/focusChart Behavior dc.js should use a namespace when subscribing to events Jan 20, 2015
@gordonwoodhull
Copy link
Contributor

I guess the only breakage here would be if people are intentionally overriding or disabling internal usage of events.

@JackStouffer
Copy link
Author

Yeah, but if people are overriding internal events for functionality, they should expect them to change or break on version changes. Especially since version 2 is still in beta.

@gordonwoodhull
Copy link
Contributor

Beta actually means interface freeze for us, but it would be easy to put this in 2.1.

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

No branches or pull requests

2 participants