Skip to content

Commit

Permalink
Change scale.ticks back to an array of strings (#4573)
Browse files Browse the repository at this point in the history 
Internal ticks are now stored as objects in the PRIVATE this._ticks member and must not be accessed directly from outside this class. this.ticks is around for a long time and hasn't been marked as private, so we can't change its structure without unexpected breaking changes. If you need to access the scale ticks, use scale.getTicks() instead.
  • Loading branch information
@simonbrunel
simonbrunel committed Jul 29, 2017
1 parent 56fdd7e commit 53b7a63
Show file tree
Hide file tree
Showing 5 changed files with 156 additions and 88 deletions.
2 changes: 1 addition & 1 deletion src/controllers/controller.bar.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,7 @@ module.exports = function(Chart) {
var options = scale.options;
var stackCount = me.getStackCount();
var fullSize = scale.isHorizontal() ? scale.width : scale.height;
var tickSize = fullSize / scale.ticks.length;
var tickSize = fullSize / scale.getTicks().length;
var categorySize = tickSize * options.categoryPercentage;
var fullBarSize = categorySize / stackCount;
var barSize = fullBarSize * options.barPercentage;
Expand Down
100 changes: 78 additions & 22 deletions src/core/core.scale.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,17 @@ defaults._set('scale', {
}
});

function labelsFromTicks(ticks) {
var labels = [];
var i, ilen;

for (i = 0, ilen = ticks.length; i < ilen; ++i) {
labels.push(ticks[i].label);
}

return labels;
}

module.exports = function(Chart) {

function computeTextSize(context, tick, font) {
Expand Down Expand Up @@ -103,6 +114,14 @@ module.exports = function(Chart) {
};
},

/**
* Returns the scale tick objects ({label, major})
* @since 2.7
*/
getTicks: function() {
return this._ticks;
},

// These methods are ordered by lifecyle. Utilities then follow.
// Any function defined here is inherited by all scale types.
// Any function can be extended by the scale type
Expand Down Expand Up @@ -135,6 +154,7 @@ module.exports = function(Chart) {
},
update: function(maxWidth, maxHeight, margins) {
var me = this;
var i, ilen, labels, label, ticks, tick;

// Update Lifecycle - Probably don't want to ever extend or overwrite this function ;)
me.beforeUpdate();
Expand All @@ -160,15 +180,50 @@ module.exports = function(Chart) {
me.determineDataLimits();
me.afterDataLimits();

// Ticks
// Ticks - `this.ticks` is now DEPRECATED!
// Internal ticks are now stored as objects in the PRIVATE `this._ticks` member
// and must not be accessed directly from outside this class. `this.ticks` being
// around for long time and not marked as private, we can't change its structure
// without unexpected breaking changes. If you need to access the scale ticks,
// use scale.getTicks() instead.

me.beforeBuildTicks();
me.buildTicks();

// New implementations should return an array of objects but for BACKWARD COMPAT,
// we still support no return (`this.ticks` internally set by calling this method).
ticks = me.buildTicks() || [];

me.afterBuildTicks();

me.beforeTickToLabelConversion();
me.convertTicksToLabels();

// New implementations should return the formatted tick labels but for BACKWARD
// COMPAT, we still support no return (`this.ticks` internally changed by calling
// this method and supposed to contain only string values).
labels = me.convertTicksToLabels(ticks) || me.ticks;

me.afterTickToLabelConversion();

me.ticks = labels; // BACKWARD COMPATIBILITY

// IMPORTANT: from this point, we consider that `this.ticks` will NEVER change!

// BACKWARD COMPAT: synchronize `_ticks` with labels (so potentially `this.ticks`)
for (i = 0, ilen = labels.length; i < ilen; ++i) {
label = labels[i];
tick = ticks[i];
if (!tick) {
ticks.push(tick = {
label: label,
major: false
});
} else {
tick.label = label;
}
}

me._ticks = ticks;

// Tick Rotation
me.beforeCalculateTickRotation();
me.calculateTickRotation();
Expand Down Expand Up @@ -258,6 +313,7 @@ module.exports = function(Chart) {
var me = this;
var context = me.ctx;
var tickOpts = me.options.ticks;
var labels = labelsFromTicks(me._ticks);

// Get the width of each grid by calculating the difference
// between x offsets between 0 and 1.
Expand All @@ -266,8 +322,8 @@ module.exports = function(Chart) {

var labelRotation = tickOpts.minRotation || 0;

if (me.options.display && me.isHorizontal()) {
var originalLabelWidth = helpers.longestText(context, tickFont.font, me.ticks, me.longestTextCache);
if (labels.length && me.options.display && me.isHorizontal()) {
var originalLabelWidth = helpers.longestText(context, tickFont.font, labels, me.longestTextCache);
var labelWidth = originalLabelWidth;
var cosRotation;
var sinRotation;
Expand Down Expand Up @@ -311,6 +367,8 @@ module.exports = function(Chart) {
height: 0
};

var labels = labelsFromTicks(me._ticks);

var opts = me.options;
var tickOpts = opts.ticks;
var scaleLabelOpts = opts.scaleLabel;
Expand Down Expand Up @@ -348,8 +406,8 @@ module.exports = function(Chart) {

// Don't bother fitting the ticks if we are not showing them
if (tickOpts.display && display) {
var largestTextWidth = helpers.longestText(me.ctx, tickFont.font, me.ticks, me.longestTextCache);
var tallestLabelHeightInLines = helpers.numberOfLabelLines(me.ticks);
var largestTextWidth = helpers.longestText(me.ctx, tickFont.font, labels, me.longestTextCache);
var tallestLabelHeightInLines = helpers.numberOfLabelLines(labels);
var lineSpace = tickFont.size * 0.5;
var tickPadding = me.options.ticks.padding;

Expand All @@ -369,11 +427,8 @@ module.exports = function(Chart) {
minSize.height = Math.min(me.maxHeight, minSize.height + labelHeight + tickPadding);
me.ctx.font = tickFont.font;

var firstTick = me.ticks[0];
var firstLabelWidth = computeTextSize(me.ctx, firstTick, tickFont.font);

var lastTick = me.ticks[me.ticks.length - 1];
var lastLabelWidth = computeTextSize(me.ctx, lastTick, tickFont.font);
var firstLabelWidth = computeTextSize(me.ctx, labels[0], tickFont.font);
var lastLabelWidth = computeTextSize(me.ctx, labels[labels.length - 1], tickFont.font);

// Ensure that our ticks are always inside the canvas. When rotated, ticks are right aligned which means that the right padding is dominated
// by the font height
Expand Down Expand Up @@ -471,7 +526,7 @@ module.exports = function(Chart) {
var me = this;
if (me.isHorizontal()) {
var innerWidth = me.width - (me.paddingLeft + me.paddingRight);
var tickWidth = innerWidth / Math.max((me.ticks.length - ((me.options.gridLines.offsetGridLines) ? 0 : 1)), 1);
var tickWidth = innerWidth / Math.max((me._ticks.length - ((me.options.gridLines.offsetGridLines) ? 0 : 1)), 1);
var pixel = (tickWidth * index) + me.paddingLeft;

if (includeOffset) {
Expand All @@ -483,7 +538,7 @@ module.exports = function(Chart) {
return finalVal;
}
var innerHeight = me.height - (me.paddingTop + me.paddingBottom);
return me.top + (index * (innerHeight / (me.ticks.length - 1)));
return me.top + (index * (innerHeight / (me._ticks.length - 1)));
},

// Utility for getting the pixel location of a percentage of scale
Expand Down Expand Up @@ -555,20 +610,21 @@ module.exports = function(Chart) {
var labelRotationRadians = helpers.toRadians(me.labelRotation);
var cosRotation = Math.cos(labelRotationRadians);
var longestRotatedLabel = me.longestLabelWidth * cosRotation;
var tickCount = me._ticks.length;

var itemsToDraw = [];

if (isHorizontal) {
skipRatio = false;

if ((longestRotatedLabel + optionTicks.autoSkipPadding) * me.ticks.length > (me.width - (me.paddingLeft + me.paddingRight))) {
skipRatio = 1 + Math.floor(((longestRotatedLabel + optionTicks.autoSkipPadding) * me.ticks.length) / (me.width - (me.paddingLeft + me.paddingRight)));
if ((longestRotatedLabel + optionTicks.autoSkipPadding) * tickCount > (me.width - (me.paddingLeft + me.paddingRight))) {
skipRatio = 1 + Math.floor(((longestRotatedLabel + optionTicks.autoSkipPadding) * tickCount) / (me.width - (me.paddingLeft + me.paddingRight)));
}

// if they defined a max number of optionTicks,
// increase skipRatio until that number is met
if (maxTicks && me.ticks.length > maxTicks) {
while (!skipRatio || me.ticks.length / (skipRatio || 1) > maxTicks) {
if (maxTicks && tickCount > maxTicks) {
while (!skipRatio || tickCount / (skipRatio || 1) > maxTicks) {
if (!skipRatio) {
skipRatio = 1;
}
Expand All @@ -587,17 +643,17 @@ module.exports = function(Chart) {
var yTickStart = options.position === 'bottom' ? me.top : me.bottom - tl;
var yTickEnd = options.position === 'bottom' ? me.top + tl : me.bottom;

helpers.each(me.ticks, function(tick, index) {
var label = (tick && tick.value) || tick;
helpers.each(me._ticks, function(tick, index) {
var label = tick.label;
// If the callback returned a null or undefined value, do not draw this line
if (helpers.isNullOrUndef(label)) {
return;
}

var isLastTick = me.ticks.length === index + 1;
var isLastTick = tickCount === index + 1;

// Since we always show the last tick,we need may need to hide the last shown one before
var shouldSkip = (skipRatio > 1 && index % skipRatio > 0) || (index % skipRatio === 0 && index + skipRatio >= me.ticks.length);
var shouldSkip = (skipRatio > 1 && index % skipRatio > 0) || (index % skipRatio === 0 && index + skipRatio >= tickCount);
if (shouldSkip && !isLastTick || helpers.isNullOrUndef(label)) {
return;
}
Expand Down
55 changes: 34 additions & 21 deletions src/scales/scale.time.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -312,6 +312,23 @@ function generate(min, max, minor, major, capacity, options) {
return ticks;
}

function ticksFromTimestamps(values, majorUnit) {
var ticks = [];
var i, ilen, value, major;

for (i = 0, ilen = values.length; i < ilen; ++i) {
value = values[i];
major = majorUnit ? value === +moment(value).startOf(majorUnit) : false;

ticks.push({
value: value,
major: major
});
}

return ticks;
}

module.exports = function(Chart) {

var defaultConfig = {
Expand Down Expand Up @@ -515,16 +532,17 @@ module.exports = function(Chart) {
}
}

me.ticks = ticks;
me.min = min;
me.max = max;

// PRIVATE
me._unit = unit;
me._majorUnit = majorUnit;
me._displayFormat = formats[unit];
me._majorDisplayFormat = formats[majorUnit];
me._minorFormat = formats[unit];
me._majorFormat = formats[majorUnit];
me._table = buildLookupTable(ticks, min, max, ticksOpts.mode === 'linear');

return ticksFromTimestamps(ticks, majorUnit);
},

getLabelForIndex: function(index, datasetIndex) {
Expand Down Expand Up @@ -553,31 +571,25 @@ module.exports = function(Chart) {
var options = me.options;
var time = tick.valueOf();
var majorUnit = me._majorUnit;
var majorFormat = me._majorDisplayFormat;
var majorFormat = me._majorFormat;
var majorTime = tick.clone().startOf(me._majorUnit).valueOf();
var major = majorUnit && majorFormat && time === majorTime;
var formattedTick = tick.format(major ? majorFormat : me._displayFormat);
var label = tick.format(major ? majorFormat : me._minorFormat);
var tickOpts = major ? options.ticks.major : options.ticks.minor;
var formatter = helpers.valueOrDefault(tickOpts.callback, tickOpts.userCallback);

if (formatter) {
formattedTick = formatter(formattedTick, index, ticks);
}

return {
value: formattedTick,
major: major,
time: time,
};
return formatter ? formatter(label, index, ticks) : label;
},

convertTicksToLabels: function() {
var ticks = this.ticks;
convertTicksToLabels: function(ticks) {
var labels = [];
var i, ilen;

for (i = 0, ilen = ticks.length; i < ilen; ++i) {
ticks[i] = this.tickFormatFunction(moment(ticks[i]));
labels.push(this.tickFormatFunction(moment(ticks[i].value), i, ticks));
}

return labels;
},

/**
Expand Down Expand Up @@ -610,8 +622,9 @@ module.exports = function(Chart) {
},

getPixelForTick: function(index) {
return index >= 0 && index < this.ticks.length ?
this.getPixelForOffset(this.ticks[index].time) :
var ticks = this.getTicks();
return index >= 0 && index < ticks.length ?
this.getPixelForOffset(ticks[index].value) :
null;
},

Expand Down Expand Up @@ -647,9 +660,9 @@ module.exports = function(Chart) {
getLabelCapacity: function(exampleTime) {
var me = this;

me._displayFormat = me.options.time.displayFormats.millisecond; // Pick the longest format for guestimation
me._minorFormat = me.options.time.displayFormats.millisecond; // Pick the longest format for guestimation

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

Expand Down
4 changes: 2 additions & 2 deletions test/specs/global.deprecations.tests.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -277,8 +277,8 @@ describe('Deprecations', function() {
}
});

var ticks = chart.scales.time.ticks.map(function(tick) {
return tick.value;
var ticks = chart.scales.time.getTicks().map(function(tick) {
return tick.label;
});

expect(ticks).toEqual(['8PM', '10PM']);
Expand Down

0 comments on commit 53b7a63

Please sign in to comment.