Radar Chart Span Gaps

[etimberg]

Resolves #5358

To Do

• Tests
• Docs
• Code cleanup

[etimberg]

@simonbrunel I added tests and a section to the docs. I found bugs in the line element and the filler plugin when the first point in the line is skipped. This PR includes fixes for those as well

[simonbrunel]

dataset.spanGaps !== undefined ? dataset.spanGaps : me.chart.options.spanGaps, else it will ignore dataset.spanGaps: false

[simonbrunel]

model = points[i]._model;

[benmccann]

could do:

spanGaps: helpers.valueOrDefault(dataset.spanGaps, me.chart.options.spanGaps)

[benmccann]

++i is a little non-standard. The file has i++ everywhere else, which I see much more commonly

[simonbrunel]

I'm using ++i in for loops but it's an habit that comes from C/C++ which is (was?) an optimization. I'm not sure it has any incidence in JavaScript though.

[benmccann]

v8 produces the same bytecode in either case

[etimberg]

The habit for me came from my c++ days too. Will try and address the comments tomorrow

[benmccann]

there's an extra space in ++ index (also I think index++ would be more consistent)

[benmccann]

@etimberg don't forget there's a few comments on this PR

[benmccann]

@etimberg just a quick reminder about this PR

[simonbrunel]

@etimberg the CI is failing, can you have a look?

[etimberg]

@simonbrunel I fixed the linting error. Will try and investigate the test failures. They don't happen locally but I thought I'd gotten all the text off

[benmccann]

@etimberg one of the failing tests "Chart.elements.Line should be able to draw with a loop back to the beginning point when the first point is skipped" is here: https://github.com/chartjs/Chart.js/blob/master/test/specs/element.line.tests.js#L1876

With your changes there's an extra lineTo:

9: {name: "lineTo", args: [15, -10] }
10: {name: "lineTo", args: [19, -5] }
11: {name: "lineTo", args: [5, 0] }

Do you need to update element.line.tests to include an extra lineTo like this? benmccann@2454c41

Or is it a bug that there's an additional lineTo being included?

[benmccann]

@etimberg the other test that's failing are the radar image tests below. I've included these screenshots to help demonstrate how exactly it's failing

They seem to be caused by the change to:

		if (value === null || isNaN(value)) {
			return NaN; // null always in center
		}

If I change return NaN to return 0 then the tests pass

[etimberg]

Thanks for the images @benmccann. Going to look into this now.

[etimberg]

Closing since this surprisingly hard to get right. The radar chart should skip null points but doing so breaks the filler plugin.