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 overlapping auto-generated ticks on time scale #6115

Merged
merged 4 commits into from
May 9, 2019
Merged

Fix overlapping auto-generated ticks on time scale #6115

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
fb4c42d
Fix overlapping auto-generated ticks on time scale
kurkle Mar 6, 2019
6691449
Use margins instead of assuming padding of 1 label
kurkle Mar 6, 2019
870265a
update comment
kurkle Mar 6, 2019
8ccc738
Update tests
kurkle Apr 29, 2019
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
9 changes: 8 additions & 1 deletion src/scales/scale.time.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -776,13 +776,20 @@ module.exports = Scale.extend({
var me = this;
var timeOpts = me.options.time;
var displayFormats = timeOpts.displayFormats;
var margins = me.margins;

// pick the longest format (milliseconds) for guestimation
var format = displayFormats[timeOpts.unit] || displayFormats.millisecond;

var exampleLabel = me.tickFormatFunction(exampleTime, 0, [], format);
var tickLabelWidth = me.getLabelWidth(exampleLabel);
var innerWidth = me.isHorizontal() ? me.width : me.height;

// Using margins instead of padding because padding is not calculated
// at this point (buildTicks). Margins are provided from previous calculation
// in layout steps 5/6
var innerWidth = me.isHorizontal()
benmccann marked this conversation as resolved.
Show resolved Hide resolved
? me.width - (margins.left + margins.right)
: me.height - (margins.top + margins.bottom);
var capacity = Math.floor(innerWidth / tickLabelWidth);

return capacity > 0 ? capacity : 1;
Expand Down
86 changes: 43 additions & 43 deletions test/specs/scale.time.tests.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -433,28 +433,28 @@ describe('Time scale tests', function() {
});

it('should use the min option when less than first label for building ticks', function() {
config.time.min = '2014-12-29T04:00:00';
config.ticks.min = '2014-12-29T04:00:00';

var scale = createScale(mockData, config);
expect(scale.ticks[0]).toEqual('Jan 1');
});

it('should use the min option when greater than first label for building ticks', function() {
config.time.min = '2015-01-02T04:00:00';
config.ticks.min = '2015-01-02T04:00:00';

var scale = createScale(mockData, config);
expect(scale.ticks[0]).toEqual('Jan 2');
});

it('should use the max option when greater than last label for building ticks', function() {
config.time.max = '2015-01-05T06:00:00';
config.ticks.max = '2015-01-05T06:00:00';

var scale = createScale(mockData, config);
expect(scale.ticks[scale.ticks.length - 1]).toEqual('Jan 3');
});

it('should use the max option when less than last label for building ticks', function() {
config.time.max = '2015-01-02T23:00:00';
config.ticks.max = '2015-01-02T23:00:00';

var scale = createScale(mockData, config);
expect(scale.ticks[scale.ticks.length - 1]).toEqual('Jan 2');
Expand Down Expand Up @@ -669,8 +669,8 @@ describe('Time scale tests', function() {

expect(scale._ticks.map(function(tick) {
return tick.major;
})).toEqual([true, false, false, false, false, false, true]);
expect(scale.ticks).toEqual(['<8:00:00 pm>', '<8:00:10 pm>', '<8:00:20 pm>', '<8:00:30 pm>', '<8:00:40 pm>', '<8:00:50 pm>', '<8:01:00 pm>']);
})).toEqual([true, false, false, false, true]);
expect(scale.ticks).toEqual(['<8:00:00 pm>', '<8:00:15 pm>', '<8:00:30 pm>', '<8:00:45 pm>', '<8:01:00 pm>']);
});

it('should update ticks.callback correctly', function() {
Expand All @@ -681,7 +681,7 @@ describe('Time scale tests', function() {
return '{' + value + '}';
};
chart.update();
expect(scale.ticks).toEqual(['{8:00:00 pm}', '{8:00:10 pm}', '{8:00:20 pm}', '{8:00:30 pm}', '{8:00:40 pm}', '{8:00:50 pm}', '{8:01:00 pm}']);
expect(scale.ticks).toEqual(['{8:00:00 pm}', '{8:00:15 pm}', '{8:00:30 pm}', '{8:00:45 pm}', '{8:01:00 pm}']);
});
});

Expand All @@ -708,7 +708,7 @@ describe('Time scale tests', function() {
major: {
enabled: true,
callback: function(value) {
return '[' + value + ']';
return '[[' + value + ']]';
}
},
minor: {
Expand All @@ -729,8 +729,8 @@ describe('Time scale tests', function() {

expect(scale._ticks.map(function(tick) {
return tick.major;
})).toEqual([true, false, false, false, false, false, true]);
expect(scale.ticks).toEqual(['[8:00 pm]', '(8:00:10 pm)', '(8:00:20 pm)', '(8:00:30 pm)', '(8:00:40 pm)', '(8:00:50 pm)', '[8:01 pm]']);
})).toEqual([true, false, false, false, true]);
expect(scale.ticks).toEqual(['[[8:00 pm]]', '(8:00:15 pm)', '(8:00:30 pm)', '(8:00:45 pm)', '[[8:01 pm]]']);
});

it('should only use ticks.minor callback if ticks.major.enabled is false', function() {
Expand All @@ -739,7 +739,7 @@ describe('Time scale tests', function() {

chart.options.scales.xAxes[0].ticks.major.enabled = false;
chart.update();
expect(scale.ticks).toEqual(['(8:00:00 pm)', '(8:00:10 pm)', '(8:00:20 pm)', '(8:00:30 pm)', '(8:00:40 pm)', '(8:00:50 pm)', '(8:01:00 pm)']);
expect(scale.ticks).toEqual(['(8:00:00 pm)', '(8:00:15 pm)', '(8:00:30 pm)', '(8:00:45 pm)', '(8:01:00 pm)']);
});

it('should use ticks.callback if ticks.major.callback is omitted', function() {
Expand All @@ -748,7 +748,7 @@ describe('Time scale tests', function() {

chart.options.scales.xAxes[0].ticks.major.callback = undefined;
chart.update();
expect(scale.ticks).toEqual(['<8:00 pm>', '(8:00:10 pm)', '(8:00:20 pm)', '(8:00:30 pm)', '(8:00:40 pm)', '(8:00:50 pm)', '<8:01 pm>']);
expect(scale.ticks).toEqual(['<8:00 pm>', '(8:00:15 pm)', '(8:00:30 pm)', '(8:00:45 pm)', '<8:01 pm>']);
});

it('should use ticks.callback if ticks.minor.callback is omitted', function() {
Expand All @@ -757,7 +757,7 @@ describe('Time scale tests', function() {

chart.options.scales.xAxes[0].ticks.minor.callback = undefined;
chart.update();
expect(scale.ticks).toEqual(['[8:00 pm]', '<8:00:10 pm>', '<8:00:20 pm>', '<8:00:30 pm>', '<8:00:40 pm>', '<8:00:50 pm>', '[8:01 pm]']);
expect(scale.ticks).toEqual(['[[8:00 pm]]', '<8:00:15 pm>', '<8:00:30 pm>', '<8:00:45 pm>', '[[8:01 pm]]']);
});
});

Expand Down Expand Up @@ -853,7 +853,7 @@ describe('Time scale tests', function() {
scales: {
xAxes: [{
type: 'time',
time: {
ticks: {
min: moment().subtract(1, 'months'),
max: moment(),
}
Expand Down Expand Up @@ -910,8 +910,8 @@ describe('Time scale tests', function() {
var scale = chart.scales.x;
var options = chart.options.scales.xAxes[0];

options.time.min = '2012';
options.time.max = '2051';
options.ticks.min = '2012';
options.ticks.max = '2051';
chart.update();

expect(scale.min).toEqual(+moment('2012', 'YYYY'));
Expand All @@ -924,8 +924,8 @@ describe('Time scale tests', function() {
var scale = chart.scales.x;
var options = chart.options.scales.xAxes[0];

options.time.min = '2017';
options.time.max = '2042';
options.ticks.min = '2017';
options.ticks.max = '2042';
chart.update();

expect(scale.min).toEqual(+moment('2017', 'YYYY'));
Expand Down Expand Up @@ -1004,8 +1004,8 @@ describe('Time scale tests', function() {
var scale = chart.scales.x;
var options = chart.options.scales.xAxes[0];

options.time.min = '2012';
options.time.max = '2051';
options.ticks.min = '2012';
options.ticks.max = '2051';
chart.update();

expect(scale.min).toEqual(+moment('2012', 'YYYY'));
Expand All @@ -1018,8 +1018,8 @@ describe('Time scale tests', function() {
var scale = chart.scales.x;
var options = chart.options.scales.xAxes[0];

options.time.min = '2017';
options.time.max = '2043';
options.ticks.min = '2017';
options.ticks.max = '2043';
chart.update();

expect(scale.min).toEqual(+moment('2017', 'YYYY'));
Expand Down Expand Up @@ -1103,7 +1103,7 @@ describe('Time scale tests', function() {
var scale = chart.scales.x;
var options = chart.options.scales.xAxes[0];

options.time.min = '2012';
options.ticks.min = '2012';
chart.update();

var start = scale.left;
Expand All @@ -1117,7 +1117,7 @@ describe('Time scale tests', function() {
var scale = chart.scales.x;
var options = chart.options.scales.xAxes[0];

options.time.max = '2050';
options.ticks.max = '2050';
chart.update();

var start = scale.left;
Expand All @@ -1131,8 +1131,8 @@ describe('Time scale tests', function() {
var scale = chart.scales.x;
var options = chart.options.scales.xAxes[0];

options.time.min = '2012';
options.time.max = '2050';
options.ticks.min = '2012';
options.ticks.max = '2050';
chart.update();

var start = scale.left;
Expand Down Expand Up @@ -1187,8 +1187,8 @@ describe('Time scale tests', function() {
var scale = chart.scales.x;
var options = chart.options.scales.xAxes[0];

options.time.min = '2012';
options.time.max = '2050';
options.ticks.min = '2012';
options.ticks.max = '2050';
chart.update();

var start = scale.left;
Expand Down Expand Up @@ -1280,7 +1280,7 @@ describe('Time scale tests', function() {
});
});

describe('when time.min and/or time.max are defined', function() {
describe('when ticks.min and/or ticks.max are defined', function() {
['auto', 'data', 'labels'].forEach(function(source) {
['data', 'ticks'].forEach(function(bounds) {
describe('and ticks.source is "' + source + '" and bounds "' + bounds + '"', function() {
Expand Down Expand Up @@ -1320,8 +1320,8 @@ describe('Time scale tests', function() {
var min = '02/19 07:00';
var max = '02/24 08:00';

options.time.min = min;
options.time.max = max;
options.ticks.min = min;
options.ticks.max = max;
chart.update();

expect(scale.min).toEqual(+moment(min, 'MM/DD HH:mm'));
Expand All @@ -1340,8 +1340,8 @@ describe('Time scale tests', function() {
var min = '02/21 07:00';
var max = '02/22 20:00';

options.time.min = min;
options.time.max = max;
options.ticks.min = min;
options.ticks.max = max;
chart.update();

expect(scale.min).toEqual(+moment(min, 'MM/DD HH:mm'));
Expand Down Expand Up @@ -1414,8 +1414,8 @@ describe('Time scale tests', function() {
var scale = chart.scales.x;
var options = chart.options.scales.xAxes[0];

options.time.min = '2012';
options.time.max = '2051';
options.ticks.min = '2012';
options.ticks.max = '2051';
chart.update();

expect(scale.getPixelForValue('2012')).toBeCloseToPixel(scale.left);
Expand All @@ -1427,8 +1427,8 @@ describe('Time scale tests', function() {
var scale = chart.scales.x;
var options = chart.options.scales.xAxes[0];

options.time.min = '2012';
options.time.max = '2051';
options.ticks.min = '2012';
options.ticks.max = '2051';
options.offset = true;
chart.update();

Expand Down Expand Up @@ -1541,7 +1541,7 @@ describe('Time scale tests', function() {
var scale = chart.scales.x;
var options = chart.options.scales.xAxes[0];

options.time.min = '2012';
options.ticks.min = '2012';
chart.update();

var start = scale.left;
Expand All @@ -1556,7 +1556,7 @@ describe('Time scale tests', function() {
var scale = chart.scales.x;
var options = chart.options.scales.xAxes[0];

options.time.max = '2050';
options.ticks.max = '2050';
chart.update();

var start = scale.left;
Expand All @@ -1571,8 +1571,8 @@ describe('Time scale tests', function() {
var scale = chart.scales.x;
var options = chart.options.scales.xAxes[0];

options.time.min = '2012';
options.time.max = '2050';
options.ticks.min = '2012';
options.ticks.max = '2050';
chart.update();

var start = scale.left;
Expand Down Expand Up @@ -1629,8 +1629,8 @@ describe('Time scale tests', function() {
var scale = chart.scales.x;
var options = chart.options.scales.xAxes[0];

options.time.min = '2012';
options.time.max = '2050';
options.ticks.min = '2012';
options.ticks.max = '2050';
chart.update();

var start = scale.left;
Expand Down