`enter` event being erroneously triggered?

[samselikoff]

I'm extending a chart:

    d3.chart('BarChart').extend('AccountingBarChart', {
      initialize: function() {
        // this.dataLabels is a mixin
        this.dataLabels.layer('labels').on('enter', function() {
          this.text('$' + this.text());
        });
      }
    });

then rendering it multiple times

    setInterval(function() {
      barData.forEach(function(d) {
        d.value = d.value + 1;
      });
      barChart.draw(barData);
    }, 2000);

The second time the chart gets rendered, it triggers the enter event and tries to execute the above code, even though there's no entering data. Any ideas?

[jugglinmike]

Hi @samselikoff,

It looks like you've identified a leak in our "event" abstraction. Because the
lifecycle events are driven by d3 selections, d3.chart emits all lifecycle
events consistently with every call to Layer#draw. The catch is that the
events are triggered with an empty selection as their context, so your example
should behave as intended (only setting the text of elements as they first
enter the chart), even if the handler functions themselves are invoked multiple
times. I don't have your complete source, so can you verify that for me?

The fact that layers still behave as expected is probably why this detail has
gone unreported for so long. I'd encourage you to only place logic inside the
layer event handlers that is directly related to modifying the data-driven
elements (i.e. somehow involved with setting content/attributes). That said, I
recognize that this behavior is somewhat surprising, and d3.chart should avoid
it. I'll include a patch for this in v0.2.

[samselikoff]

I see. My example actually does break, since calling this.text() fails.

I'm definitely open to better ways of accomplishing the above. Where would you suggest the hypothetical d3.chart consumer put his code, to format the data labels?

[jugglinmike]

Your invocation of this.text() is in the right place--it sounds like this problem may be more serious than I first thought. I'm not sure what you mean when you say that "calling this.text() fails", though--could you share the complete source, maybe on JSFiddle?

[samselikoff]

Is d3.chart on a CDN?

On Fri, Jan 10, 2014 at 10:43 AM, jugglinmike notifications@github.comwrote:

Your invocation of this.text() is in the right place--it sounds like this
problem may be more serious than I first thought. I'm not sure what you
mean when you say that "calling this.text() fails", though--could you
share the complete source, maybe on JSFiddle?

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/69#issuecomment-32037698
.

[jugglinmike]

No CDN, but you can use the source served by GitHub for demo purposes. That might not be necessary, though. As written, all the labels would receive the same text content. If instead you wanted to prepend a dollar sign character to each label individually, this would do the trick:

  this.dataLabels.layer('labels').on('enter', function() {
-   this.text('$' + this.text());
+  this.text(function() {
+    return '$' + d3.select(this).text();
+  });
 });

[samselikoff]

This works, but I guess it feels odd that the enter handler will be triggered each time. So basically, it may just be an empty selection? Why trigger at all?

[jugglinmike]

You're definitely right about the event being extraneous (we're including a fix for this in version 0.2); in the mean time, I wanted to help out with the example code you shared.

[samselikoff]

Perfect - thanks!

[jugglinmike]

Sure thing :)

In case you're curious, here's the commit that fixes the behavior internally: b505e01

It's actually a very simple fix, but @iros and I had to step through a bunch of sloppy tests that either shared state or expected empty events. In other words, your report also helped us improve the test suite!

[samselikoff]

awesome! Thanks again for all the great work :)