Use abs() when comparing for spanGaps

[luke-heberling]

Not sure if this is intended but I've been using data in descending order by X axis value, to share underlying data structures with components that need it that way. It seems to work fine, except that spanGaps breaks because the "gap" is negative and always compares less-than the threshold.

This fixes that issue, but of course isn't appropriate if ascending order is required -- I may have missed something but I didn't see a specification about that in the docs.

[kurkle]

Makes sense. If all tests pass, I think this is ok. Can you add a regression test?

[luke-heberling]

Thanks! I added tests, needed more than just a regression test for this because I didn't find any existing tests for using a numeric value for spanGaps.

[kurkle]

I suspect the data is sorted when parsed and the order does not really matter. Can you verify that? If I'm right, your actual use case must be more complex. Maybe using some optimizations to skip the sorting?

[kurkle]

Do these lines affect the tests in any way? These should be the defaults.

[luke-heberling]

No, they don't. I added them when what I really needed was the linear scale, and forgot to remove them. I went ahead and pushed an update.

[kurkle]

Thanks! I added tests, needed more than just a regression test for this because I didn't find any existing tests for using a numeric value for spanGaps.

I think there are some fixtures.

[luke-heberling]

I suspect the data is sorted when parsed and the order does not really matter. Can you verify that? If I'm right, your actual use case must be more complex. Maybe using some optimizations to skip the sorting?

I don't think so... I'm definitely not using "parsing: false" which if I understand right is what would skip that.
However, now that you put it that way... if the data is expected to be sorted before the spanGaps calc sees it then there's something else going on. The test appears to show that the data is NOT sorted, note how the index for "stop" changes based on the order.

[kurkle]

I suspect the data is sorted when parsed and the order does not really matter. Can you verify that? If I'm right, your actual use case must be more complex. Maybe using some optimizations to skip the sorting?

I don't think so... I'm definitely not using "parsing: false" which if I understand right is what would skip that.
However, now that you put it that way... if the data is expected to be sorted before the spanGaps calc sees it then there's something else going on. The test appears to show that the data is NOT sorted, note how the index for "stop" changes based on the order.

I think I was just wrong :)

[luke-heberling]

Here's a repro using the CDN chart.js, the only difference is the order of the data.

Working spangaps: https://jsfiddle.net/82zcyh9n/13/
broken spangaps: https://jsfiddle.net/82zcyh9n/12/

[luke-heberling]

I think the fact that the data is unsorted makes sense, if the line controller ignores the order and draws the points in whatever order provided (meaning it may be possible to backtrack on the x axis). If that's the case, then spanGaps is arguably not working correctly except when "parsing: false" is provided to signal that the data is already in final order? I'm really outta my depth on this code though, so take with a grain of salt :)

[luke-heberling]

I think the fact that the data is unsorted makes sense, if the line controller ignores the order and draws the points in whatever order provided (meaning it may be possible to backtrack on the x axis). If that's the case, then spanGaps is arguably not working correctly except when "parsing: false" is provided to signal that the data is already in final order?

This does appear to be true, check this one out with out-of-order points:
no spangaps: https://jsfiddle.net/kne0oa7y/
yes spangaps: https://jsfiddle.net/3vLzwhu9/

It seems like spanGaps doesn't use sorted data, even when it might be expected (because parsing: false" is not provided).
The change to use abs() might still be a good step forward, which would meet reasonable expectations for reversed data.