Skip to content

Commit

Permalink
get rid of margins and fullWidth
Browse files Browse the repository at this point in the history
  • Loading branch information
@kurkle
kurkle committed Jun 4, 2019
1 parent a6f73d6 commit 771a435
Show file tree
Hide file tree
Showing 7 changed files with 21 additions and 74 deletions.
34 changes: 6 additions & 28 deletions src/core/core.layouts.js
View file
Expand Up @@ -98,21 +98,6 @@ function adjustWithPadding(outerDims, boxCorner, maxPadding) {
});
}


function getMargin(horizontal, outerDims, maxPadding) {
function marginForPositions(positions) {
var margin = {};
positions.forEach(function(pos) {
margin[pos] = Math.max(outerDims[pos], maxPadding[pos]);
});
return margin;
}

return horizontal
? marginForPositions(['left', 'right'])
: marginForPositions(['top', 'bottom']);
}

function updateChartArea(chartArea, outerDims, maxPadding) {
var newWidth = chartArea.outerWidth - getCombinedMax(maxPadding, outerDims, 'left', 'right');
var newHeight = chartArea.outerHeight - getCombinedMax(maxPadding, outerDims, 'top', 'bottom');
Expand All @@ -133,11 +118,7 @@ function fitBoxes(boxes, chartArea, outerDims) {
layout = boxes[i];
box = layout.box;

box.update(
layout.width || chartArea.w,
layout.height || chartArea.h,
getMargin(layout.horizontal, outerDims, maxPadding)
);
box.update(layout.width || chartArea.w, layout.height || chartArea.h);
updateMaxPadding(maxPadding, box);
if (updateChartArea(chartArea, outerDims, maxPadding) && refitBoxes.length) {
// Dimensions changed and there were non full width boxes before this
Expand All @@ -154,7 +135,6 @@ function fitBoxes(boxes, chartArea, outerDims) {
}

function placeBoxes(boxes, chartArea, outerDims, boxCorner) {
var userPadding = chartArea.padding;
var top = boxCorner.top;
var left = boxCorner.left;
var i, ilen, layout, box;
Expand All @@ -163,8 +143,8 @@ function placeBoxes(boxes, chartArea, outerDims, boxCorner) {
layout = boxes[i];
box = layout.box;
if (layout.horizontal) {
box.left = box.fullWidth ? userPadding.left : outerDims.left;
box.right = box.fullWidth ? chartArea.outerWidth - userPadding.right : outerDims.left + chartArea.w;
box.left = outerDims.left;
box.right = outerDims.left + chartArea.w;
box.top = top;
box.bottom = top + box.height;
top = box.bottom;
Expand Down Expand Up @@ -337,13 +317,13 @@ module.exports = {

setLayoutDims(verticalBoxes.concat(horizontalBoxes), chartArea);

// First fit vertical boxes, starting from padding and no margins
// First fit vertical boxes
outerDims = fitBoxes(verticalBoxes, chartArea, padding);

// Adjust chart area based on vertical boxes.
updateChartArea(chartArea, outerDims, maxPadding);

// Fit horizontal boxes, providing vertical box widths as margins
// Then fit horizontal boxes
outerDims = fitBoxes(horizontalBoxes, chartArea, outerDims);

// Adjust chart area based on horizontal boxes
Expand All @@ -357,9 +337,7 @@ module.exports = {

// Make sure horizontal boxes have correct width
helpers.each(horizontalBoxes, function(layout) {
if (!layout.box.fullWidth) {
layout.box.width = chartArea.w;
}
layout.box.width = chartArea.w;
});

// Adjust top/left of outerDims and boxCorner with padding if needed
Expand Down
33 changes: 4 additions & 29 deletions src/core/core.scale.js
View file
Expand Up @@ -240,7 +240,7 @@ var Scale = Element.extend({
helpers.callback(this.options.beforeUpdate, [this]);
},

update: function(maxWidth, maxHeight, margins) {
update: function(maxWidth, maxHeight) {
var me = this;
var i, ilen, labels, label, ticks, tick;

Expand All @@ -250,12 +250,6 @@ var Scale = Element.extend({
// Absorb the master measurements
me.maxWidth = maxWidth;
me.maxHeight = maxHeight;
me.margins = helpers.extend({
left: 0,
right: 0,
top: 0,
bottom: 0
}, margins);

me._maxLabelLines = 0;
me.longestLabelWidth = 0;
Expand Down Expand Up @@ -476,8 +470,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 @@ -552,26 +545,10 @@ var Scale = Element.extend({
}
}

me.handleMargins();

me.width = minSize.width;
me.height = minSize.height;
},

/**
* Handle margins and padding interactions
* @private
*/
handleMargins: function() {
var me = this;
if (me.margins) {
me.paddingLeft = Math.max(me.paddingLeft, me.margins.left);
me.paddingTop = Math.max(me.paddingTop, me.margins.top);
me.paddingRight = Math.max(me.paddingRight, me.margins.right);
me.paddingBottom = Math.max(me.paddingBottom, me.margins.bottom);
}
},

afterFit: function() {
helpers.callback(this.options.afterFit, [this]);
},
Expand Down Expand Up @@ -685,15 +662,13 @@ var Scale = Element.extend({
}
if (me.isHorizontal()) {
var tickWidth = me.width / Math.max((numTicks - (offset ? 0 : 1)), 1);
var pixel = (tickWidth * index) + me.paddingLeft;
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;
}
return me.top + (index * (me.height / (numTicks - 1)));
},
Expand Down
3 changes: 1 addition & 2 deletions src/plugins/plugin.legend.js
View file
Expand Up @@ -130,7 +130,7 @@ var Legend = Element.extend({
// Any function can be extended by the legend type

beforeUpdate: noop,
update: function(maxWidth, maxHeight, margins) {
update: function(maxWidth, maxHeight) {
var me = this;

// Update Lifecycle - Probably don't want to ever extend or overwrite this function ;)
Expand All @@ -139,7 +139,6 @@ var Legend = Element.extend({
// Absorb the master measurements
me.maxWidth = maxWidth;
me.maxHeight = maxHeight;
me.margins = margins;

// Dimensions
me.beforeSetDimensions();
Expand Down
3 changes: 1 addition & 2 deletions src/plugins/plugin.title.js
View file
Expand Up @@ -34,7 +34,7 @@ var Title = Element.extend({
// These methods are ordered by lifecycle. Utilities then follow.

beforeUpdate: noop,
update: function(maxWidth, maxHeight, margins) {
update: function(maxWidth, maxHeight) {
var me = this;

// Update Lifecycle - Probably don't want to ever extend or overwrite this function ;)
Expand All @@ -43,7 +43,6 @@ var Title = Element.extend({
// Absorb the master measurements
me.maxWidth = maxWidth;
me.maxHeight = maxHeight;
me.margins = margins;

// Dimensions
me.beforeSetDimensions();
Expand Down
4 changes: 0 additions & 4 deletions src/scales/scale.time.js
View file
Expand Up @@ -786,10 +786,6 @@ module.exports = Scale.extend({
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 / size.w : me.height / size.h);

if (me.options.offset) {
Expand Down
6 changes: 3 additions & 3 deletions test/specs/core.layouts.tests.js
View file
Expand Up @@ -172,7 +172,7 @@ describe('Chart.layouts', function() {
expect(chart.scales.yScale.paddingLeft).toBeCloseToPixel(0);
expect(chart.scales.yScale.paddingTop).toBeCloseToPixel(7);
expect(chart.scales.yScale.paddingRight).toBeCloseToPixel(0);
expect(chart.scales.yScale.paddingBottom).toBeCloseToPixel(30);
expect(chart.scales.yScale.paddingBottom).toBeCloseToPixel(7);
});

it('should fit scales that overlap the chart area', function() {
Expand Down Expand Up @@ -306,8 +306,8 @@ describe('Chart.layouts', function() {
expect(chart.scales.xScale1.top).toBeCloseToPixel(484);

expect(chart.scales.xScale2.bottom).toBeCloseToPixel(62);
expect(chart.scales.xScale2.left).toBeCloseToPixel(0);
expect(chart.scales.xScale2.right).toBeCloseToPixel(512);
expect(chart.scales.xScale2.left).toBeCloseToPixel(39);
expect(chart.scales.xScale2.right).toBeCloseToPixel(496);
expect(chart.scales.xScale2.top).toBeCloseToPixel(32);

// Is yScale at the right spot
Expand Down
12 changes: 6 additions & 6 deletions test/specs/scale.linear.tests.js
View file
Expand Up @@ -910,13 +910,13 @@ describe('Linear Scale', function() {
var yScale = chart.scales.yScale0;
expect(xScale.paddingTop).toBeCloseToPixel(0);
expect(xScale.paddingBottom).toBeCloseToPixel(0);
expect(xScale.paddingLeft).toEqual(yScale.width);
expect(xScale.paddingLeft).toBeCloseToPixel(12);
expect(xScale.paddingRight).toBeCloseToPixel(13.5);
expect(xScale.width).toBeCloseToPixel(468 - 6); // minus lineSpace
expect(xScale.height).toBeCloseToPixel(30);

expect(yScale.paddingTop).toBeCloseToPixel(32);
expect(yScale.paddingBottom).toBeCloseToPixel(30);
expect(yScale.paddingTop).toBeCloseToPixel(7);
expect(yScale.paddingBottom).toBeCloseToPixel(7);
expect(yScale.paddingLeft).toBeCloseToPixel(0);
expect(yScale.paddingRight).toBeCloseToPixel(0);
expect(yScale.width).toBeCloseToPixel(30 + 6); // plus lineSpace
Expand All @@ -929,13 +929,13 @@ describe('Linear Scale', function() {

expect(xScale.paddingTop).toBeCloseToPixel(0);
expect(xScale.paddingBottom).toBeCloseToPixel(0);
expect(xScale.paddingLeft).toEqual(yScale.width);
expect(xScale.paddingLeft).toBeCloseToPixel(12);
expect(xScale.paddingRight).toBeCloseToPixel(13.5);
expect(xScale.width).toBeCloseToPixel(440);
expect(xScale.height).toBeCloseToPixel(53);

expect(yScale.paddingTop).toBeCloseToPixel(32);
expect(yScale.paddingBottom).toBeCloseToPixel(53);
expect(yScale.paddingTop).toBeCloseToPixel(7);
expect(yScale.paddingBottom).toBeCloseToPixel(7);
expect(yScale.paddingLeft).toBeCloseToPixel(0);
expect(yScale.paddingRight).toBeCloseToPixel(0);
expect(yScale.width).toBeCloseToPixel(58);
Expand Down

0 comments on commit 771a435

Please sign in to comment.