API xgrids function doesn't dinamically load the class of the new gridlines

[szmgabor]

• C3 version: 0.4.18
• D3 version: 3.5.17
• Browser: Chrome Version 61.0.3163.100 (Official Build) (64-bit)
• OS: Windows 10 64-bit

Hi, I've been using this library for a while and it's really good, congrats.
Recently I have found a problem when loading a new set of x grid lines which have different classes.
Please check out this fiddle: http://jsfiddle.net/hm5znbgt/1/

• in the first step two grid lines are loaded one having the class "active" and the other one beeing "inactive"
• then in the seconds step a new grid line is loaded having the class "inactive", but the grid is loaded with the "active" class for some reason

Thanks,
Gabor

[martingraham]

The problem, looking at the source, is that classes are only applied to new grid lines.

However, the grid lines are indexed by number, not by any data property, so the new grid line you add - "Label 2" - ends up reusing and overwriting the "Label 1" text and position. You can tell as it animates across. However it doesn't apply the new class type so it stays the original colour.

Quick fix is this:

    d3.select("#chart")
    .selectAll(".c3-xgrid-line")
    .attr("class", function (d) { return "c3-xgrid-line"+(d.class ? " "+d.class : ""); });

right after you add your new grid lines

See http://jsfiddle.net/hm5znbgt/4/

Long term solution for the c3 maintainers is to change the code in the updateGrid and redrawGrid functions

c3_chart_internal_fn.updateGrid = function (duration) {
    var $$ = this,
        main = $$.main,
        config = $$.config,
        xgridLine,
        ygridLine,
        yv;

    // hide if arc type
    $$.grid.style('visibility', $$.hasArcType() ? 'hidden' : 'visible');

    main.select('line.' + CLASS.xgridFocus).style("visibility", "hidden");
    if (config.grid_x_show) {
        $$.updateXGrid();
    }
    $$.xgridLines = main.select('.' + CLASS.xgridLines).selectAll('.' + CLASS.xgridLine).data(config.grid_x_lines);
    // exit
    $$.xgridLines.exit().transition().duration(duration).style("opacity", 0).remove();
    // enter
    xgridLine = $$.xgridLines.enter().append('g');
    
    xgridLine.append('line').style("opacity", 0);
    xgridLine.append('text').attr("text-anchor", $$.gridTextAnchor).attr("transform", config.axis_rotated ? "" : "rotate(-90)").attr('dx', $$.gridTextDx).attr('dy', -5).style("opacity", 0);
    
    // udpate
    // done in d3.transition() of the end of this function

    

    // Y-Grid
    if (config.grid_y_show) {
        $$.updateYGrid();
    }
    $$.ygridLines = main.select('.' + CLASS.ygridLines).selectAll('.' + CLASS.ygridLine).data(config.grid_y_lines);
    // enter
    ygridLine = $$.ygridLines.enter().append('g');
    ygridLine.append('line').style("opacity", 0);
    ygridLine.append('text').attr("text-anchor", $$.gridTextAnchor).attr("transform", config.axis_rotated ? "rotate(-90)" : "").attr('dx', $$.gridTextDx).attr('dy', -5).style("opacity", 0);
    // update
    yv = $$.yv.bind($$);
    $$.ygridLines.attr("class", function (d) { return CLASS.ygridLine + (d['class'] ? ' ' + d['class'] : '');  });
    $$.ygridLines.select('line').transition().duration(duration).attr("x1", config.axis_rotated ? yv : 0).attr("x2", config.axis_rotated ? yv : $$.width).attr("y1", config.axis_rotated ? 0 : yv).attr("y2", config.axis_rotated ? $$.height : yv).style("opacity", 1);
    $$.ygridLines.select('text').transition().duration(duration).attr("x", config.axis_rotated ? $$.xGridTextX.bind($$) : $$.yGridTextX.bind($$)).attr("y", yv).text(function (d) {
        return d.text;
    }).style("opacity", 1);
    // exit
    $$.ygridLines.exit().transition().duration(duration).style("opacity", 0).remove();
};
c3_chart_internal_fn.redrawGrid = function (withTransition) {
    var $$ = this,
        config = $$.config,
        xv = $$.xv.bind($$); /*,
        //lines = $$.xgridLines.select('line'),
        //texts = $$.xgridLines.select('text');
    return [(withTransition ? lines.transition() : lines).attr("x1", config.axis_rotated ? 0 : xv).attr("x2", config.axis_rotated ? $$.width : xv).attr("y1", config.axis_rotated ? xv : 0).attr("y2", config.axis_rotated ? xv : $$.height).style("opacity", 1), (withTransition ? texts.transition() : texts).attr("x", config.axis_rotated ? $$.yGridTextX.bind($$) : $$.xGridTextX.bind($$)).attr("y", xv).text(function (d) {
        return d.text;
    }).style("opacity", 1), 
           //$$.xgridLines.attr("class", function (d) { console.log ("updating grids", $$.xgridLines); return CLASS.xgridLine + (d['class'] ? ' ' + d['class'] : '');  }),
           ];
           */
    
    return [(withTransition ? $$.xgridLines.transition() : $$.xgridLines).each (function (d) {
        var elem = d3.select(this);
        elem.select('line').transition()
            .attr("x1", config.axis_rotated ? 0 : xv)
            .attr("x2", config.axis_rotated ? $$.width : xv)
            .attr("y1", config.axis_rotated ? xv : 0)
            .attr("y2", config.axis_rotated ? xv : $$.height)
            .style("opacity", 1)
        ;
        elem.select('text').transition()
            .attr("x", config.axis_rotated ? $$.yGridTextX.bind($$) : $$.xGridTextX.bind($$))
            .attr("y", xv)
            .text(d.text)
            .style("opacity", 1)
        ;
        elem.attr("class", CLASS.xgridLine + (d['class'] ? ' ' + d['class'] : ''));
    })];
};

The above worked for me but hasn't been tested except on the example above. I had to change the redraw grid code as adding the class update as an extra element in the array there seemed to cause some future transition to override the .exit() transition of old lines so they stayed around. The alternative is just to do the class update as a non-transition change in updateGrid because class attribute changes themselves aren't usefully transitionable (is that a word, it is now) anyways

Sorry is isn't done as a pull request or nuffink, but I'm no good at github :-)

[szmgabor]

Hey,

The quickfix you provided works perfectly for me. Thanks a lot.
I'm no good at github myself so I'll leave the fix to someone more capable, for now I'm fine with the quickfix.

Regards,
Gabor