Fixes Bug 776 - Provide API to remove renderlets

[mtraynham]

Fixes Bug #776

This change adds the following methods:

• .addRenderlet
• .removeRenderlet
• .renderlets

This deprecates:

• .renderlet (in favor of .addRenderlet, which it calls behind the scenes using dc.utils.uniqueId as the renderlet key)

If you are opposed, I'm fine with removing the .renderlets getter function. I just thought it might be useful to trigger the renderlets at a time other than render and redraw, or specifically call a single renderlet.

Edit Oop didn't see the other pull request #779 open :P I would still say we need to keep backwards compatability for .renderlet and this uses d3.dispatch as @gordonwoodhull had suggested.

[gordonwoodhull]

I meant to suggest that we don't need any API for this at all: we can use the existing d3.dispatch instance we use for other events and we can use chart.on(), which covers both adding and removing.

Note that your getter only really allows dispatching (d3.dispatch doesn't give access to the actual listeners), so it might be better to offer trigger for all events rather than having anything specialized for renderlets.

[mtraynham]

Oh gotcha. I will change it to that instead. Do you want me to leave the renderlet function for compatibility sake?

[gordonwoodhull]

Yeah, kudos for figuring out a way to make it backward compatible! I love the idea of deprecating methods wherever we can. Maybe we could stick in a disableable console.log when we do that? I don't know what the best practices are.

[mtraynham]

I think that's a great idea! Maybe a dc.util.deprecate function that logs a templated message with the caller's function name.

[mtraynham]

Alrighty, added a deprecate function to dc.logger, which just wraps the function and dumps it to the console only the first time. It uses dc's already built in logger (which I didn't even know was there till now).

[gordonwoodhull]

@mtraynham, even though this is almost certainly backward compatible, I'd rather merge it to develop in case there is something we missed. Am I being paranoid?

[mtraynham]

Heh :) Well I fixed it because it was marked as v2.0, but I have no issue with v2.1. There isn't a 3.x branch, but if there was, I would have removed the renderlet method entirely in favor of on.

Up to you @gordonwoodhull :P

[gordonwoodhull]

I see. I'll try to clarify the milestones, since my thinking has changed about what 2.0 means.

At first I thought it was a nice round number and it should be as perfect as it can be, but since then I've come to value stability, and think only bug fixes should go in there.

[gordonwoodhull]

Concluded that we should set the bar where the interface can be shown to be backward-compatible, and I am quite sure that this is. Merging for beta.2

[gordonwoodhull]

Looking at it more closely, renderlets don't offer anything you can't get from postRender or postRedraw. I think they only exist because those didn't used to use d3.dispatch either, so you couldn't add more than one.

[mtraynham]

Yeah, I noticed that too. The _activateRenderlets function calls them before postRender and postRedraw, but only after the transition completes.

It looks like there are quite a few places where that d3.dispatch _listener object is used. It would be nice to consolidate a lot of it, and spec out a proper timeline for triggering events to the _listener. For example, it seems unnecessary to have a specific function for zooming:

    _chart._invokeZoomedListener = function () {
        _listeners.zoomed(_chart);
    };

When you could just call _listeners.zoomed(_chart);

Also, almost all the events take the chart as the only argument. If you just changed the on function to bind the listener it would simplify things even more:

    _chart.on = function (event, listener) {
        if (listener) {
            listener = listener.bind(this, _chart);
        }
        _listeners.on(event, listener);
        return _chart;
    };

Then you could just call _listeners.zoomed();

[jardelmv]

How can I replace this line:
javascript mychart.renderlet(function(_chart){ _chart.selectAll("rect.bar").on("click", function(d){timeline(1,d.x);});});
to this suggestion:
javascript chart.on("renderlet.<renderletKey>", renderletFunction)
to avoid the warning "chart.renderlet has been deprecated. Please use chart.on("renderlet.", renderletFunction)"?

[RickWalker]

I think you can just do:

mychart.on("renderlet.barclicked", function(_chart){ 
    _chart.selectAll("rect.bar").on("click", function(d){
        timeline(1,d.x);
    });
});

where barclicked is anything you want to use to identify this function.

[jardelmv]

It works! I did not know what I should use in "renderletKey". Thanks.