d3 / d3-path

Serialize Canvas path commands to SVG.
https://d3js.org/d3-path
ISC License
196 stars 37 forks source link

path generating incorrect sequence in Chrome/IE/Safari, correct in Firefox #14

Closed t-morrison closed 7 years ago

t-morrison commented 7 years ago

bl.ock here: https://bl.ocks.org/moman822/7a05fb2becde5f2081e9bdb4ee5c9511

To run, check a box then click the button. Correct execution is a growing path animation.

The issue seems to be with how d3.path is putting the d attribute together in Chrome vs Firefox.

On Firefox, it is correct and the line animation comes together fine. However, in Chrome d comes out like this: "MNaN,514.8428887547777CNaN,514.8428887547777,NaN,496.99907463287065,NaN,496.99907463287065CNaN,496.99907463287065,NaN,494.1202977268...

I have console.logged the relevant parts around the issue, from line 199.

svg.selectAll('path')
        .data(datNestTemp, function(d){console.log(d); return d})
        .enter()
        .append('path')
            .attr('d',function(d){console.log(lineScale(d.values)); return lineScale(d.values)})

lineScale is straigthforward:

lineScale = d3.line()
            .curve(d3.curveCardinal())
mbostock commented 7 years ago

The issue is that you are relying on the Date constructor to parse dates, and this behavior is not consistent across browsers. Specifically, when you construct your time scale:

var timeScale = d3.scaleTime()
    .domain([new Date(d3.min(data, function(d) { return (+d.Year) + ',0,1'; })), new Date(d3.max(data, function(d) { return +d.Year + ',0,1'; }))])
    .range([0, width]);

The domain is thus equivalent to:

[new Date("2005,0,1"), new Date("2016,0,1")]

You can verify this is the first problem by using a debugger and inspecting the resulting domain, which is [Invalid Date, Invalid Date].

A second problem with this code is that you are first computing the minimum and maximum of an array of strings, rather than converting the data to dates first. (Also, d.Year is already a number here, so you don’t need to coerce it again.)

A more correct implementation is as follows:

var timeScale = d3.scaleTime()
    .domain(d3.extent(data, function(d) { return new Date(d.Year, 0, 1); }))
    .range([0, width]);

Note that this assumes that d.Year is not in the range [0, 99]; see the Date documentation for details. Alternatively, you could compute the extent of the years and then convert to dates:

var timeScale = d3.scaleTime()
    .domain(d3.extent(data, function(d) { return d.Year; })
        .map(function(year) { return new Date(year, 0, 1); }))
    .range([0, width]);

Alternatively, you could use d3.timeParse to convert a year to a date:

var timeScale = d3.scaleTime()
    .domain(d3.extent(data, function(d) { return d.Year; }).map(d3.timeParse("%Y")))
    .range([0, width]);

You have a similar problem later in your code where you say timeScale(new Date(d.Year+',1,1')).

If you have further questions, please use Stack Overflow tag d3.js to ask for help. Although I make an effort to assist everyone that asks, I am not always available to provide help promptly or directly. Stack Overflow provides a better collaborative forum for self-help: tens of thousands of D3-related questions have already been asked there, and some answered questions may be relevant to you.

Thank you for including a link to a live example that demonstrates the issue on bl.ocks.org. However, please isolate your issue and reduce the code as much as possible before asking for help. The less code you post, the easier it is for someone to debug, and the more likely you are to get a helpful response.

If you have a question about D3’s behavior and want to discuss it with other users, also consider the d3-js Google Group or joining the d3-js Slack.

Thank you! 🤗

t-morrison commented 7 years ago

Thank you!