Better number -> string callback for the radial linear scale

[etimberg]

Fixes #2602

Image Before Change

Image After Change

[etimberg]

@simonbrunel I'm not sure what the best way to handle this is. Right now, the radial linear config has a copy of the exact same function as the linear scale config.

It would almost be nice if we wrote this using ES6 imports because then the config could go off and live in another file and be imported in 2 places but ultimately exist only once in the build. That's probably for later though

[simonbrunel]

I'm not sure neither but it's similar to controller.doughnut.js and controler.polarArea.js. Found these ones some time ago (when trying to make Code Climate happier) and I was thinking to move it in code.legend.js (I still have a local branch with that changes).

For your case, a solution could be to move that code in core.scale.js:

Chart.Scale.ticks = {
    generators: {
        linear: function() {
           // your linear code
        },
        values: {
            // the default one
        }
    }
}

and in scale.linear.js and scale.radialLinear.js:

ticks: {
    callback: Chart.Scale.ticks.generators.linear
}

Then we can add more tick generation methods, maybe :)

It could be also in core.ticks.js (via Chart.ticks = { ... }) or even in scale.linearbase.js.

Just draft ideas though, not sure that the best way to solve that.

[etimberg]

@simonbrunel I moved the common stuff into Chart.Ticks.formatters. if we like where this is going, I can try and factor out the generator functions into Chart.Ticks.generators. This would probably eliminate much of the need for the scale.linearBase file.

[simonbrunel]

I think we can save some bits and a function call: callback: Chart.Ticks.formatters.values

[simonbrunel]

Same here: callback: Chart.Ticks.formatters.linear

[simonbrunel]

callback: Chart.Ticks.formatters.logarithmic

[simonbrunel]

Same

[simonbrunel]

It looks really great 👍 Just a few minor optimizations but I like how it helps to refactor code. Doing this for generators seems a good idea.

[etimberg]

Great, I'll make the changes and move the generators in as well

[etimberg]

I've moved the linear and logarithmic generators into Chart.Ticks.generators. Now I'm wishing we had written the category scale to work as a numeric scale that simply displayed values. It would give a lot more code re-use

[etimberg]

@simonbrunel do you think we need to move any more stuff into the Chart.Ticks.generators namespace? if not, I think this is good to merge.

[simonbrunel]

do we really need to keep these properties?

[etimberg]

I know that the zoom plugin uses them. I don't know if anything else does

[simonbrunel]

What the reason to move min and max into a new dataRange object?

[etimberg]

mostly so that I was able to only pass the dataRange to the generator rather than passing the entire scale object

[simonbrunel]

hum, so it could break plugins that modify min and max?

Would not it be simpler to keep min/max (no dataRange) and create the object when calling the generator:

var ticks = me.ticks = Chart.Ticks.generators.linear(numericGeneratorOptions, {
    min: me.min,
    max: me.max
});

[etimberg]

that would also work. I can refactor this to use that if you prefer