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

AutoSkip in update #6355

Merged
merged 5 commits into from Sep 7, 2019
Merged

AutoSkip in update #6355

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
50be2ce
AutoSkip in update
benmccann Jun 24, 2019
ecc654a
Address review comments
benmccann Aug 12, 2019
1dec31d
Add v3 to TODO
benmccann Aug 13, 2019
e5c8150
Address review comments
benmccann Aug 16, 2019
557b9ca
Remove unrelated code cleanup
benmccann Aug 22, 2019
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
99 changes: 46 additions & 53 deletions src/core/core.scale.js
View file
Open in desktop
Expand Up @@ -250,9 +250,18 @@ var Scale = Element.extend({
helpers.callback(this.options.beforeUpdate, [this]);
},

/**
* @param {number} maxWidth - the max width in pixels
* @param {number} maxHeight - the max height in pixels
* @param {object} margins - the space between the edge of the other scales and edge of the chart
* This space comes from two sources:
* - padding - space that's required to show the labels at the edges of the scale
benmccann marked this conversation as resolved.
Show resolved Hide resolved
* - thickness of scales or legends in another orientation
*/
update: function(maxWidth, maxHeight, margins) {
var me = this;
var i, ilen, labels, label, ticks, tick;
var tickOpts = me.options.ticks;
var i, ilen, labels, ticks;

// Update Lifecycle - Probably don't want to ever extend or overwrite this function ;)
me.beforeUpdate();
Expand All @@ -271,7 +280,6 @@ var Scale = Element.extend({
me._maxLabelLines = 0;
me.longestLabelWidth = 0;
me.longestTextCache = me.longestTextCache || {};
me._ticksToDraw = null;
me._gridLineItems = null;
me._labelItems = null;

Expand All @@ -296,10 +304,22 @@ var Scale = Element.extend({

// 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() || [];
ticks = me.buildTicks();

// Allow modification of ticks in callback.
ticks = me.afterBuildTicks(ticks) || ticks;
if (ticks) {
simonbrunel marked this conversation as resolved.
Show resolved Hide resolved
ticks = me.afterBuildTicks(ticks);
} else {
// Support old implementations (that modified `this.ticks` directly in buildTicks)
me.ticks = me.afterBuildTicks(me.ticks);
benmccann marked this conversation as resolved.
Show resolved Hide resolved
ticks = [];
for (i = 0, ilen = me.ticks.length; i < ilen; ++i) {
ticks.push({
value: me.ticks[i],
major: false
});
}
}

me.beforeTickToLabelConversion();

Expand All @@ -312,20 +332,11 @@ var Scale = Element.extend({

me.ticks = labels; // BACKWARD COMPATIBILITY

// IMPORTANT: from this point, we consider that `this.ticks` will NEVER change!
// IMPORTANT: below 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;
}
ticks[i].label = labels[i];
}

me._ticks = ticks;
Expand All @@ -344,9 +355,12 @@ var Scale = Element.extend({
me.beforeFit();
me.fit();
me.afterFit();
//
// Auto-skip
me._ticksToDraw = tickOpts.display && tickOpts.autoSkip ? me._autoSkip(me.getTicks()) : ticks;
benmccann marked this conversation as resolved.
Show resolved Hide resolved

me.afterUpdate();

// TODO(v3): remove minSize as a public property and return value from all layout boxes. It is unused
benmccann marked this conversation as resolved.
Show resolved Hide resolved
return me.minSize;

},
Expand Down Expand Up @@ -424,14 +438,7 @@ var Scale = Element.extend({
},
buildTicks: helpers.noop,
afterBuildTicks: function(ticks) {
var me = this;
// ticks is empty for old axis implementations here
if (helpers.isArray(ticks) && ticks.length) {
return helpers.callback(me.options.afterBuildTicks, [me, ticks]);
}
// Support old implementations (that modified `this.ticks` directly in buildTicks)
me.ticks = helpers.callback(me.options.afterBuildTicks, [me, me.ticks]) || me.ticks;
return ticks;
return helpers.callback(this.options.afterBuildTicks, [this, ticks]) || ticks;
},

beforeTickToLabelConversion: function() {
Expand Down Expand Up @@ -508,6 +515,7 @@ var Scale = Element.extend({
height: 0
};

var chart = me.chart;
var opts = me.options;
var tickOpts = opts.ticks;
var scaleLabelOpts = opts.scaleLabel;
Expand Down Expand Up @@ -593,8 +601,13 @@ var Scale = Element.extend({

me.handleMargins();

me.width = minSize.width;
me.height = minSize.height;
if (isHorizontal) {
simonbrunel marked this conversation as resolved.
Show resolved Hide resolved
me.width = me._length = chart.width - me.margins.left - me.margins.right;
simonbrunel marked this conversation as resolved.
Show resolved Hide resolved
me.height = minSize.height;
} else {
me.width = minSize.width;
me.height = me._length = chart.height - me.margins.top - me.margins.bottom;
}
},

/**
Expand All @@ -603,12 +616,10 @@ var Scale = Element.extend({
*/
handleMargins: function() {
var me = this;
if (me.margins) {
me.margins.left = Math.max(me.paddingLeft, me.margins.left);
me.margins.top = Math.max(me.paddingTop, me.margins.top);
me.margins.right = Math.max(me.paddingRight, me.margins.right);
me.margins.bottom = Math.max(me.paddingBottom, me.margins.bottom);
}
me.margins.left = Math.max(me.paddingLeft, me.margins.left);
me.margins.top = Math.max(me.paddingTop, me.margins.top);
me.margins.right = Math.max(me.paddingRight, me.margins.right);
me.margins.bottom = Math.max(me.paddingBottom, me.margins.bottom);
},

afterFit: function() {
Expand Down Expand Up @@ -870,25 +881,6 @@ var Scale = Element.extend({
return false;
},

_getTicksToDraw: function() {
var me = this;
var optionTicks = me.options.ticks;
var ticks = me._ticksToDraw;

if (ticks) {
return ticks;
}

ticks = me.getTicks();

if (optionTicks.display && optionTicks.autoSkip) {
ticks = me._autoSkip(ticks);
}

me._ticksToDraw = ticks;
return ticks;
},

/**
* @private
*/
Expand All @@ -900,8 +892,9 @@ var Scale = Element.extend({
var position = options.position;
var offsetGridLines = gridLines.offsetGridLines;
var isHorizontal = me.isHorizontal();
var ticks = me._getTicksToDraw();
var ticks = me._ticksToDraw;
var ticksLength = ticks.length + (offsetGridLines ? 1 : 0);

var tl = getTickMarkLength(gridLines);
var items = [];
var axisWidth = gridLines.drawBorder ? valueAtIndexOrDefault(gridLines.lineWidth, 0, 0) : 0;
Expand Down Expand Up @@ -1008,7 +1001,7 @@ var Scale = Element.extend({
var position = options.position;
var isMirrored = optionTicks.mirror;
var isHorizontal = me.isHorizontal();
var ticks = me._getTicksToDraw();
var ticks = me._ticksToDraw;
var fonts = parseTickFontOptions(optionTicks);
var tickPadding = optionTicks.padding;
var tl = getTickMarkLength(options.gridLines);
Expand Down
3 changes: 3 additions & 0 deletions test/specs/core.scale.tests.js
View file
Open in desktop
Expand Up @@ -495,6 +495,9 @@ describe('Core.scale', function() {
it('should default to one layer for custom scales', function() {
var customScale = Chart.Scale.extend({
draw: function() {},
buildTicks: function() {
benmccann marked this conversation as resolved.
Show resolved Hide resolved
this.ticks = ['tick'];
},
convertTicksToLabels: function() {
return ['tick'];
}
Expand Down