Auto skip last

[ghost]

We are currently using the category scale to show dates in a combo bar/line chart and would rather not have a large tick gap with inconsistent spacing between ticks.

Default functionality:
https://codepen.io/anon/pen/GEEqpr

AutoSkipLast disabled:
https://codepen.io/anon/pen/pwwbex

[etimberg]

I agree that this definitely fixes the issue you are seeing. Not sure if we want to stick with a new boolean option or have some other kind of more generic option. @simonbrunel thoughts?

[simonbrunel]

If that option only applies to the auto skipper and there is no use cases where we want to preserve other tick (e.g. autoSkipBeforeLast), then I'm fine with that option. I can't find a more generic name, @etimberg do you have any suggestion?

Actually, the only thing I'm thinking is if we are going to have more and more options to control the auto skipper, then maybe we should start to gather them under the autoSkip option:

autoSkip: false;   // disabled
autoSkip: true;    // enabled with default options
autoSkip: {        // enabled with custom options
    keepLast: true,
    padding: ...   // deprecates autoSkipPadding
}

An idea that could make this feature more generic: instead of autoSkipLast, we could use a callback: autoSkipInclude/autoSkip.include (or a better name) that will force the auto skipper to keep specific ticks:

include: function(tick, index, count) {
    return index === count-1;
}

I don't know if that would be useful, neither if that's the best function signature.

Any thoughts?

[etimberg]

@simonbrunel I like the idea of nesting these options underneath the autoSkip setting.

I like the idea of a function that can override the default decision. Maybe we'd also want to provide a function that can do the measuring as well (right now we just use helpers.longestText). I'm not 100% on that function signature as well, but that's an implementation detail that could be worked out.

I wonder if we could refactor the autoSkip logic out into an internal plugin. #4117 refactored grid line drawing to a plugin so it should be possible.

[simonbrunel]

I'm going to rewrite the helpers.longestText to be more generic and also handle more text formats. I'm not sure we should expose an option to override the text length, but I'm not familiar enough with scales and the auto skipper, so maybe I missed some use cases.

Could be a good improvement to export the auto skip logic in a separate plugin, I don't know if that's easily and cleanly possible.

[ghost]

I like the idea of moving autoskip into an internal plugin. I'll give it a shot.

[etimberg]

Agreed about the use case of changing the lengths. It was just a thought I had.

[ghost]

How about something like this:

'use strict';

module.exports = function(Chart) {
	var helpers = Chart.helpers;

	return {
		id: 'tickAutoSkip',

		beforeDraw: function(chart, options) {
			helpers.each(chart.scales, function(scale) {
				var scaleOptions = scale.options;

				if (scaleOptions.display && scale.isHorizontal()) {
					scale.tickSkip = [1, 3, 5, 7];
				}
			});
		},
	};
};

Then in core.scale.js:

helpers.each(me.ticks, function(tick, index) {
	// Don't draw skipped ticks
	if (me.tickSkip && me.tickSkip.indexOf(index) !== -1) {
		return;
	}

[etimberg]

Not sure about

if (me.tickSkip && me.tickSkip.indexOf(index) !== -1) {
  return;
}

if tickSkip contains a lot of indexes, this becomes O(n^2). I'd prefer a map or set.

[simonbrunel]

Agree but also I'm not sure about a plugin accessing (private) members of scales. I think scales should provide all options that allows the user to fully customize ticks (e.g. a filter callback, etc.). Then the auto skip plugin would simply override these options.

Could require some substantial changes, so maybe we should first implement the feature of this PR directly in scale and work on the plugin refactor in another PR?

[ghost]

Doesn't seem like this is needed anymore with the changes to master.