Skip to content

Commit

Permalink
Fix unit determination when autoSkip is enabled (#6583)
Browse files Browse the repository at this point in the history
  • Loading branch information
benmccann authored and etimberg committed Oct 24, 2019
1 parent 6c9f202 commit f606c23
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 11 deletions.
19 changes: 12 additions & 7 deletions src/scales/scale.time.js
Expand Up @@ -257,7 +257,7 @@ function determineUnitForAutoTicks(minUnit, min, max, capacity) {

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

if (interval.common && Math.ceil((max - min) / (factor * interval.size)) <= capacity) {
return UNITS[i];
Expand All @@ -270,12 +270,12 @@ function determineUnitForAutoTicks(minUnit, min, max, capacity) {
/**
* Figures out what unit to format a set of ticks with
*/
function determineUnitForFormatting(scale, ticks, minUnit, min, max) {
function determineUnitForFormatting(scale, numTicks, minUnit, min, max) {
var i, unit;

for (i = UNITS.length - 1; i >= UNITS.indexOf(minUnit); i--) {
unit = UNITS[i];
if (INTERVALS[unit].common && scale._adapter.diff(max, min, unit) >= ticks.length - 1) {
if (INTERVALS[unit].common && scale._adapter.diff(max, min, unit) >= numTicks - 1) {
return unit;
}
}
Expand Down Expand Up @@ -562,11 +562,12 @@ module.exports = Scale.extend({
var min = me.min;
var max = me.max;
var options = me.options;
var tickOpts = options.ticks;
var timeOpts = options.time;
var timestamps = me._timestamps;
var ticks = [];
var capacity = me.getLabelCapacity(min);
var source = options.ticks.source;
var source = tickOpts.source;
var distribution = options.distribution;
var i, ilen, timestamp;

Expand Down Expand Up @@ -599,13 +600,17 @@ module.exports = Scale.extend({
me.max = max;

// PRIVATE
me._unit = timeOpts.unit || determineUnitForFormatting(me, ticks, timeOpts.minUnit, me.min, me.max);
me._majorUnit = !options.ticks.major.enabled || me._unit === 'year' ? undefined
// determineUnitForFormatting relies on the number of ticks so we don't use it when
// autoSkip is enabled because we don't yet know what the final number of ticks will be
me._unit = timeOpts.unit || (tickOpts.autoSkip
? determineUnitForAutoTicks(timeOpts.minUnit, me.min, me.max, capacity)
: determineUnitForFormatting(me, ticks.length, timeOpts.minUnit, me.min, me.max));
me._majorUnit = !tickOpts.major.enabled || me._unit === 'year' ? undefined
: determineMajorUnit(me._unit);
me._table = buildLookupTable(me._timestamps.data, min, max, distribution);
me._offsets = computeOffsets(me._table, ticks, min, max, options);

if (options.ticks.reverse) {
if (tickOpts.reverse) {
ticks.reverse();
}

Expand Down
8 changes: 4 additions & 4 deletions test/specs/scale.time.tests.js
Expand Up @@ -128,7 +128,7 @@ describe('Time scale tests', function() {
var ticks = getTicksLabels(scale);

// `bounds === 'data'`: first and last ticks removed since outside the data range
expect(ticks).toEqual(['Jan 2', 'Jan 3', 'Jan 4', 'Jan 5', 'Jan 6', 'Jan 7', 'Jan 8', 'Jan 9', 'Jan 10']);
expect(ticks.length).toEqual(217);
});

it('should accept labels as date objects', function() {
Expand All @@ -139,7 +139,7 @@ describe('Time scale tests', function() {
var ticks = getTicksLabels(scale);

// `bounds === 'data'`: first and last ticks removed since outside the data range
expect(ticks).toEqual(['Jan 2', 'Jan 3', 'Jan 4', 'Jan 5', 'Jan 6', 'Jan 7', 'Jan 8', 'Jan 9', 'Jan 10']);
expect(ticks.length).toEqual(217);
});

it('should accept data as xy points', function() {
Expand Down Expand Up @@ -187,7 +187,7 @@ describe('Time scale tests', function() {
var ticks = getTicksLabels(xScale);

// `bounds === 'data'`: first and last ticks removed since outside the data range
expect(ticks).toEqual(['Jan 2', 'Jan 3', 'Jan 4', 'Jan 5', 'Jan 6', 'Jan 7', 'Jan 8', 'Jan 9', 'Jan 10']);
expect(ticks.length).toEqual(217);
});

it('should accept data as ty points', function() {
Expand Down Expand Up @@ -235,7 +235,7 @@ describe('Time scale tests', function() {
var ticks = getTicksLabels(tScale);

// `bounds === 'data'`: first and last ticks removed since outside the data range
expect(ticks).toEqual(['Jan 2', 'Jan 3', 'Jan 4', 'Jan 5', 'Jan 6', 'Jan 7', 'Jan 8', 'Jan 9', 'Jan 10']);
expect(ticks.length).toEqual(217);
});
});

Expand Down

0 comments on commit f606c23

Please sign in to comment.