d3.js dougnut pie chart legend toggling

I'm developing a legend toggling d3.js pie chart application using this jsFiddle as my latest version.

I am aiming to get a streamlined working example where the legend can toggle the slices, trying to deactivate all slices - resorts in a reset which reactivates all the slices. Splitting up presentation and application layer logic.

Tweening needs improvement too - as the slices pop into existence then re-tween smoothly.

How do I improve/fix the various bugs in this code base?

onLegendClick: function(dt, i){
                    //_toggle rectangle in legend

var completeData = jQuery.extend(true, [], methods.currentDataSet);

                    newDataSet = completeData;                        
                    if(methods.manipulatedData){
                        newDataSet = methods.manipulatedData;
                    }

                    d3.selectAll('rect')
                    .data([dt], function(d) { 
                        return d.data.label;
                    })
                    .style("fill-opacity", function(d, j) {
                        var isActive = Math.abs(1-d3.select(this).style("fill-opacity"));
                        if(isActive){
                            newDataSet[j].total = completeData[j].total;                                
                        }else{
                            newDataSet[j].total = 0;                            
                        }

                        return isActive;
                    });                            

                    //animate slices
                    methods.animateSlices(newDataSet);

                    //stash manipulated data
                    methods.manipulatedData = newDataSet;

                }

CodeReview is more about reviewing code than fixing various bugs, here goes my once over:

• Use a beautifier, either TidyUp in jsfiddle or jsbeautifier.org, your code is badly indented and is hard to follow because of it

• Use more meaningful names:

• onLegendClick: function(dt, i) {

• .style("fill-opacity", function(d, j)

-
Commenting is hard, only comment what is not obvious and the general purpose of code blocks

-
This is too obvious:

//animate slices
methods.animateSlices(newDataSet);

-
This is not, but there is no comment:

var isActive = Math.abs(1 - d3.select(this).style("fill-opacity"));

-
Not sure if this is a bug, or something I dont grasp, why are you referring to completeData even though you might be running of methods.manipulatedData, if this is correct then you should have a comment there

newDataSet[j].total = completeData[j].total;

-
This could use shortcut syntax:

newDataSet = completeData;
if(methods.manipulatedData) {
  newDataSet = methods.manipulatedData;
}

becomes then

newDataSet = methods.manipulatedData || completeData;

-
This could also use a short cut syntax:

if(isActive) {
  newDataSet[j].total = completeData[j].total;
} else {
  newDataSet[j].total = 0;
}

becomes then

newDataSet[j].total = isActive ? completeData[j].total : 0;

• Do not pollute the global namespace, newDataSet is used without a var declaration

All in all, I think you could have pasted more code to review, we tend to not review the code in the jsfiddle, only what you pasted. My counter proposal would look like this:

onLegendClick: function(dt, i) {

  var completeData = jQuery.extend(true, [], methods.currentDataSet),
      newDataSet = methods.manipulatedData || completeData;

  d3.selectAll('rect')
    .data([dt], function(d) {
      return d.data.label;
    })
    .style("fill-opacity", function(d, j) {
      //Witty comment on how this works
      var isActive = Math.abs(1 - d3.select(this).style("fill-opacity"));
      //Witty comment as to why I still need completeData
      newDataSet[j].total = isActive ? completeData[j].total : 0;
      return isActive;
    });

  methods.animateSlices(newDataSet);
  methods.manipulatedData = newDataSet;
}

StackExchange Code Review Q#68060, answer score: 2