palantir / plottable

:bar_chart: A library of modular chart components built on D3
http://plottablejs.org/
MIT License
2.97k stars 223 forks source link

Ability to specify tick generation algorithm on Axis #1058

Closed jtlan closed 10 years ago

jtlan commented 10 years ago

Currently, the tick marks on Axis.Numeric are generated as follows:

    public _getTickValues(): any[] {
      return this._scale.ticks();
    }

However, the user sometimes has specific tick values they want shown (ex: 0 and 100%). To cover cases like that, it would help if the user could specify the ticks directly, or a tick generation algorithm.

endrjuskr commented 10 years ago

I would add method customTickValues(generator: (s: Scale) => any[]), which will ensure that we are not using default tick values generator from d3.Scale. In _getTickValues() I would add check if we specify custom generator and if so invoke it.

On the other hand we could change _getTickValues to setter and getter, where setter takes generator and getter invokes provided generator (custom or default). The problem in that solution is confusing naming, because user may expect to provided already an array instead of generator.

teamdandelion commented 10 years ago

sidebar - i am amused that we still have the "axis of praxis" label. it is a throwback from a day when plottable was less professional :)

jtlan commented 10 years ago

Something to think about: Does it make sense to have this ability on the Scale instead of the Axis? That would scale better to cases where an Axis and Gridlines are used together.

jtlan commented 10 years ago

Possibly related: #1102.

endrjuskr commented 10 years ago

I agree with moving it into Scale. In constructor we can pass generator export interface TickGenerator { (s: D3.Scale.QuantitativeScale, count: number) : number[] }

and ticks we would check if we specify custom generator and call it if so.

jtlan commented 10 years ago

Also, fixing this should also fix #1011.

endrjuskr commented 10 years ago

Probably after generation call check for count of returned ticks is required.

endrjuskr commented 10 years ago

Here is a little different approach.

We set default generator to: _tickGenerationPolicy: TickGenerationPolicy = function (s: D3.Scale.QuantitativeScale, count: number) { return s.ticks(count); };

And expose getter and setter. In 'ticks' method we just call this generator and don't check ticks count.

jtlan commented 10 years ago

We could also choose not to expose numTicks() as an API point, and instead have the user set the generator if they want to change the number of ticks:

var myNumTicks = 15;
scale.ticksGenerator((s: Plottable.Abstract.QuantitativeScale) => return s.ticks(myNumTicks));

This is clunkier, but has the advantage of simplifying the type signature of the generator.

endrjuskr commented 10 years ago

I think we can leave numTicks() and still simplify signature, but it will also be a breaking change in API, because of removing param in ticks. That would change default generator to: _tickGenerationPolicy: TickGenerationPolicy = function (s: D3.Scale.QuantitativeScale, count: number) { return s.ticks(this.numTicks()); }; This solution gives a chance to easily change scale without creating a new algorithm, which might be pretty common scenario.

jtlan commented 10 years ago

That would change default generator to: _tickGenerationPolicy: TickGenerationPolicy = function (s: D3.Scale.QuantitativeScale, count: number) { return s.ticks(this.numTicks()); }; This solution gives a chance to easily change scale without creating a new algorithm, which might be pretty common scenario.

Sounds reasonable. I'd call it a TickGenerator rather than a TickGenerationPolicy, and pass in the Plottable scale instead of the D3 scale.