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.

Sign up for GitHub

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

Jump to bottom

Fix unit determination when autoSkip is enabled #6583

Merged
merged 1 commit into from Oct 24, 2019
Merged

Fix unit determination when autoSkip is enabled #6583

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
19 changes: 12 additions & 7 deletions src/scales/scale.time.js
View file
Open in desktop
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
View file
Open in desktop
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