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

Refactor core.layouts #6304

Merged
merged 1 commit into from Jun 19, 2019
Merged

Refactor core.layouts #6304

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
454 changes: 217 additions & 237 deletions src/core/core.layouts.js
View file
Open in desktop

Large diffs are not rendered by default.

43 changes: 17 additions & 26 deletions src/core/core.scale.js
View file
Open in desktop
Expand Up @@ -476,8 +476,7 @@ var Scale = Element.extend({

// Width
if (isHorizontal) {
// subtract the margins to line up with the chartArea if we are a full width scale
minSize.width = me.isFullWidth() ? me.maxWidth - me.margins.left - me.margins.right : me.maxWidth;
minSize.width = me.maxWidth;
} else if (display) {
minSize.width = getTickMarkLength(gridLineOpts) + getScaleLabelHeight(scaleLabelOpts);
}
Expand Down Expand Up @@ -565,10 +564,10 @@ var Scale = Element.extend({
handleMargins: function() {
var me = this;
if (me.margins) {
me.paddingLeft = Math.max(me.paddingLeft - me.margins.left, 0);
me.paddingTop = Math.max(me.paddingTop - me.margins.top, 0);
me.paddingRight = Math.max(me.paddingRight - me.margins.right, 0);
me.paddingBottom = Math.max(me.paddingBottom - me.margins.bottom, 0);
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);
}
},

Expand Down Expand Up @@ -679,21 +678,21 @@ var Scale = Element.extend({
getPixelForTick: function(index) {
var me = this;
var offset = me.options.offset;
var numTicks = me._ticks.length;
if (index < 0 || index > numTicks - 1) {
return null;
}
if (me.isHorizontal()) {
var innerWidth = me.width - (me.paddingLeft + me.paddingRight);
var tickWidth = innerWidth / Math.max((me._ticks.length - (offset ? 0 : 1)), 1);
var pixel = (tickWidth * index) + me.paddingLeft;
var tickWidth = me.width / Math.max((numTicks - (offset ? 0 : 1)), 1);
var pixel = (tickWidth * index);

if (offset) {
pixel += tickWidth / 2;
}

var finalVal = me.left + pixel;
finalVal += me.isFullWidth() ? me.margins.left : 0;
return finalVal;
return me.left + pixel;
}
var innerHeight = me.height - (me.paddingTop + me.paddingBottom);
return me.top + (index * (innerHeight / (me._ticks.length - 1)));
return me.top + (index * (me.height / (numTicks - 1)));
},

/**
Expand All @@ -702,15 +701,9 @@ var Scale = Element.extend({
*/
getPixelForDecimal: function(decimal) {
var me = this;
if (me.isHorizontal()) {
var innerWidth = me.width - (me.paddingLeft + me.paddingRight);
var valueOffset = (innerWidth * decimal) + me.paddingLeft;

var finalVal = me.left + valueOffset;
finalVal += me.isFullWidth() ? me.margins.left : 0;
return finalVal;
}
return me.top + (decimal * me.height);
return me.isHorizontal()
? me.left + decimal * me.width
: me.top + decimal * me.height;
},

/**
Expand Down Expand Up @@ -749,9 +742,7 @@ var Scale = Element.extend({
var ticksLength = me._tickSize() * (tickCount - 1);

// Axis length
var axisLength = isHorizontal
? me.width - (me.paddingLeft + me.paddingRight)
: me.height - (me.paddingTop + me.PaddingBottom);
var axisLength = isHorizontal ? me.width : me.height;

var result = [];
var i, tick;
Expand Down
6 changes: 5 additions & 1 deletion src/scales/scale.category.js
View file
Open in desktop
Expand Up @@ -101,7 +101,11 @@ module.exports = Scale.extend({
},

getPixelForTick: function(index) {
return this.getPixelForValue(this.ticks[index], index + this.minIndex);
var ticks = this.ticks;
if (index < 0 || index > ticks.length - 1) {
return null;
}
return this.getPixelForValue(ticks[index], index + this.minIndex);
},

getValueForPixel: function(pixel) {
Expand Down
6 changes: 5 additions & 1 deletion src/scales/scale.linear.js
View file
Open in desktop
Expand Up @@ -184,7 +184,11 @@ module.exports = LinearScaleBase.extend({
},

getPixelForTick: function(index) {
return this.getPixelForValue(this.ticksAsNumbers[index]);
var ticks = this.ticksAsNumbers;
if (index < 0 || index > ticks.length - 1) {
return null;
}
return this.getPixelForValue(ticks[index]);
}
});

Expand Down
6 changes: 5 additions & 1 deletion src/scales/scale.logarithmic.js
View file
Open in desktop
Expand Up @@ -247,7 +247,11 @@ module.exports = Scale.extend({
},

getPixelForTick: function(index) {
return this.getPixelForValue(this.tickValues[index]);
var ticks = this.tickValues;
if (index < 0 || index > ticks.length - 1) {
return null;
}
return this.getPixelForValue(ticks[index]);
},

/**
Expand Down
11 changes: 2 additions & 9 deletions src/scales/scale.time.js
View file
Open in desktop
Expand Up @@ -774,7 +774,7 @@ module.exports = Scale.extend({
var me = this;
var ticksOpts = me.options.ticks;
var tickLabelWidth = me.ctx.measureText(label).width;
var angle = helpers.toRadians(ticksOpts.maxRotation);
benmccann marked this conversation as resolved.
Show resolved Hide resolved
var angle = helpers.toRadians(me.isHorizontal() ? ticksOpts.maxRotation : ticksOpts.minRotation);
Copy link
Contributor

@nagix nagix Jun 6, 2019 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maxRotation defaults to 50, but vertical axis ticks are only rotated to minRotation

Is this a correct assumption? Document says maxRotation and minRotation are only applicable to horizontal scales.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Example: https://codepen.io/kurkle/pen/rggrEG

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it is not completely supported yet. Below is the result with position: 'right'. Need to adjust paddingTop and paddingBottom at the end of scale.fit?
Screen Shot 2019-06-06 at 5 25 28 PM

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, currently scale.labelRotation is set only for horizontal scales in calculateTickRotation.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed, but I'd leave those to another PR.

var cosRotation = Math.cos(angle);
var sinRotation = Math.sin(angle);
var tickFontSize = valueOrDefault(ticksOpts.fontSize, defaults.global.defaultFontSize);
Expand All @@ -800,19 +800,12 @@ 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, ticksFromTimestamps(me, [exampleTime], me._majorUnit), format);
var size = me._getLabelSize(exampleLabel);

// 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 capacity = Math.floor(me.isHorizontal()
? (me.width - margins.left - margins.right) / size.w
: (me.height - margins.top - margins.bottom) / size.h);
var capacity = Math.floor(me.isHorizontal() ? me.width / size.w : me.height / size.h);

if (me.options.offset) {
capacity--;
Expand Down