Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve label capacity calculation in time scale #6297

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
83 changes: 53 additions & 30 deletions src/scales/scale.time.js
Original file line number Diff line number Diff line change
Expand Up @@ -272,23 +272,50 @@ function determineStepSize(min, max, unit, capacity) {
return factor;
}

function determineMajorUnit(unit) {
benmccann marked this conversation as resolved.
Show resolved Hide resolved
for (var i = UNITS.indexOf(unit) + 1, ilen = UNITS.length; i < ilen; ++i) {
if (INTERVALS[UNITS[i]].common) {
return UNITS[i];
}
}
}

/**
* Figures out what unit results in an appropriate number of auto-generated ticks
*/
function determineUnitForAutoTicks(minUnit, min, max, capacity) {
function determineUnitsForAutoTicks(scale, min, max) {
var timeOpts = scale.options.time;
var minor = timeOpts.unit;
var range = max - min;
var ilen = UNITS.length;
var i, interval, factor;

for (i = UNITS.indexOf(minUnit); i < ilen - 1; ++i) {
interval = INTERVALS[UNITS[i]];
factor = interval.steps ? interval.steps[interval.steps.length - 1] : MAX_INTEGER;

if (interval.common && Math.ceil((max - min) / (factor * interval.size)) <= capacity) {
return UNITS[i];
var i, major, interval, steps, factor, capacity;

if (minor) {
major = determineMajorUnit(minor);
benmccann marked this conversation as resolved.
Show resolved Hide resolved
capacity = scale.getLabelCapacity(max, minor, major);
} else {
for (i = UNITS.indexOf(timeOpts.minUnit); i < ilen; ++i) {
minor = UNITS[i];
major = determineMajorUnit(minor);
interval = INTERVALS[minor];

if (interval.common) {
steps = interval.steps;
factor = steps ? steps[steps.length - 1] : MAX_INTEGER;
capacity = scale.getLabelCapacity(max, minor, major);

if (Math.ceil(range / (factor * interval.size)) <= capacity) {
break;
}
}
}
}

return UNITS[ilen - 1];
return {
minor: minor,
major: major,
capacity: capacity
};
}

/**
Expand All @@ -308,26 +335,19 @@ function determineUnitForFormatting(scale, ticks, minUnit, min, max) {
return UNITS[minUnit ? UNITS.indexOf(minUnit) : 0];
}

function determineMajorUnit(unit) {
for (var i = UNITS.indexOf(unit) + 1, ilen = UNITS.length; i < ilen; ++i) {
if (INTERVALS[UNITS[i]].common) {
return UNITS[i];
}
}
}

/**
* Generates a maximum of `capacity` timestamps between min and max, rounded to the
* `minor` unit, aligned on the `major` unit and using the given scale time `options`.
* Important: this method can return ticks outside the min and max range, it's the
* responsibility of the calling code to clamp values if needed.
*/
function generate(scale, min, max, capacity) {
function generate(scale, min, max) {
var adapter = scale._adapter;
var options = scale.options;
var timeOpts = options.time;
var minor = timeOpts.unit || determineUnitForAutoTicks(timeOpts.minUnit, min, max, capacity);
var major = determineMajorUnit(minor);
var units = determineUnitsForAutoTicks(scale, min, max);
var minor = units.minor;
var major = units.major;
var stepSize = valueOrDefault(timeOpts.stepSize, timeOpts.unitStepSize);
var weekday = minor === 'week' ? timeOpts.isoWeekday : false;
var majorTicksEnabled = options.ticks.major.enabled;
Expand All @@ -338,7 +358,7 @@ function generate(scale, min, max, capacity) {
var time;

if (!stepSize) {
stepSize = determineStepSize(min, max, minor, capacity);
stepSize = determineStepSize(min, max, minor, units.capacity);
}

// For 'week' unit, handle the first day of week option
Expand Down Expand Up @@ -623,7 +643,7 @@ module.exports = Scale.extend({
break;
case 'auto':
default:
timestamps = generate(me, min, max, me.getLabelCapacity(min), options);
timestamps = generate(me, min, max);
}

if (options.bounds === 'ticks' && timestamps.length) {
Expand Down Expand Up @@ -800,21 +820,24 @@ module.exports = Scale.extend({
/**
* @private
*/
getLabelCapacity: function(exampleTime) {
getLabelCapacity: function(exampleTime, minor, major) {
var me = this;
var timeOpts = me.options.time;
var options = me.options;
var timeOpts = options.time;
var displayFormats = timeOpts.displayFormats;

// pick the longest format (milliseconds) for guestimation
var format = displayFormats[timeOpts.unit] || displayFormats.millisecond;
var exampleLabel = me.tickFormatFunction(exampleTime, 0, ticksFromTimestamps(me, [exampleTime], me._majorUnit), format);
var format = displayFormats[minor || timeOpts.unit] || displayFormats.millisecond;
var exampleLabel = me.tickFormatFunction(exampleTime, 0, [], format);
var size = me._getLabelSize(exampleLabel);
var capacity = Math.floor(me.isHorizontal() ? me.width / size.w : me.height / size.h);

if (me.options.offset) {
if (options.offset) {
capacity--;
}

if (major && options.ticks.major.enabled) {
capacity = Math.min(capacity, me.getLabelCapacity(exampleTime, major));
}

return capacity > 0 ? capacity : 1;
}
});
Expand Down
10 changes: 5 additions & 5 deletions test/specs/scale.time.tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ describe('Time scale tests', function() {
};

var scaleOptions = Chart.scaleService.getScaleDefaults('time');
var scale = createScale(mockData, scaleOptions, {width: 1000, height: 200});
var scale = createScale(mockData, scaleOptions, {width: 500, height: 200});
var ticks = getTicksLabels(scale);

// `bounds === 'data'`: first and last ticks removed since outside the data range
Expand All @@ -135,7 +135,7 @@ describe('Time scale tests', function() {
var mockData = {
labels: [newDateFromRef(0), newDateFromRef(1), newDateFromRef(2), newDateFromRef(4), newDateFromRef(6), newDateFromRef(7), newDateFromRef(9)], // days
};
var scale = createScale(mockData, Chart.scaleService.getScaleDefaults('time'), {width: 1000, height: 200});
var scale = createScale(mockData, Chart.scaleService.getScaleDefaults('time'), {width: 500, height: 200});
var ticks = getTicksLabels(scale);

// `bounds === 'data'`: first and last ticks removed since outside the data range
Expand Down Expand Up @@ -181,7 +181,7 @@ describe('Time scale tests', function() {
}],
}
}
}, {canvas: {width: 800, height: 200}});
}, {canvas: {width: 400, height: 200}});

var xScale = chart.scales.xScale0;
var ticks = getTicksLabels(xScale);
Expand Down Expand Up @@ -229,7 +229,7 @@ describe('Time scale tests', function() {
}],
}
}
}, {canvas: {width: 800, height: 200}});
}, {canvas: {width: 400, height: 200}});

var tScale = chart.scales.tScale0;
var ticks = getTicksLabels(tScale);
Expand Down Expand Up @@ -595,7 +595,7 @@ describe('Time scale tests', function() {
}],
}
}
}, {canvas: {width: 800, height: 200}});
}, {canvas: {width: 300, height: 200}});

this.scale = this.chart.scales.xScale0;
});
Expand Down