Skip to content

Commit

Permalink
address review comments / issues
Browse files Browse the repository at this point in the history
  • Loading branch information
@kurkle
kurkle committed Jun 4, 2019
1 parent e9bb15d commit a6f73d6
Show file tree
Hide file tree
Showing 2 changed files with 45 additions and 63 deletions.
86 changes: 39 additions & 47 deletions src/core/core.layouts.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,7 @@ function filterByPosition(array, position) {
}

function sortByWeight(array, reverse) {
array.forEach(function(v, i) {
v.index = i;
v.weight = v.box.weight;
return v;
});
array.sort(function(a, b) {
return array.sort(function(a, b) {
var v0 = reverse ? b : a;
var v1 = reverse ? a : b;
return v0.weight === v1.weight ?
Expand All @@ -33,9 +28,11 @@ function wrapBoxes(boxes) {
for (i = 0, ilen = (boxes || []).length; i < ilen; ++i) {
box = boxes[i];
layoutBoxes.push({
pos: box.position,
index: i,
box: box,
horizontal: box.isHorizontal()
pos: box.position,
horizontal: box.isHorizontal(),
weight: box.weight
});
}
return layoutBoxes;
Expand All @@ -56,23 +53,17 @@ function setLayoutDims(layouts, chartArea) {

function buildLayoutBoxes(boxes) {
var layoutBoxes = wrapBoxes(boxes);
var left = filterByPosition(layoutBoxes, 'left');
var right = filterByPosition(layoutBoxes, 'right');
var top = filterByPosition(layoutBoxes, 'top');
var bottom = filterByPosition(layoutBoxes, 'bottom');

// Sort boxes by weight. A higher weight is further away from the chart area
sortByWeight(left, true);
sortByWeight(right);
sortByWeight(top, true);
sortByWeight(bottom);
var left = sortByWeight(filterByPosition(layoutBoxes, 'left'), true);
var right = sortByWeight(filterByPosition(layoutBoxes, 'right'));
var top = sortByWeight(filterByPosition(layoutBoxes, 'top'), true);
var bottom = sortByWeight(filterByPosition(layoutBoxes, 'bottom'));

return {
lt: left.concat(top),
rb: right.concat(bottom),
area: filterByPosition(layoutBoxes, 'chartArea'),
v: left.concat(right),
h: top.concat(bottom)
leftAndTop: left.concat(top),
rightAndBottom: right.concat(bottom),
chartArea: filterByPosition(layoutBoxes, 'chartArea'),
vertical: left.concat(right),
horizontal: top.concat(bottom)
};
}

Expand All @@ -86,7 +77,6 @@ function updateMaxPadding(padding, box) {
}
}


function addSizeByPosition(outerDims, boxes) {
var result = extend({}, outerDims);
helpers.each(boxes, function(layout) {
Expand All @@ -95,7 +85,6 @@ function addSizeByPosition(outerDims, boxes) {
return result;
}


function getCombinedMax(maxPadding, outerDims, a, b) {
return Math.max(maxPadding[a], outerDims[a]) + Math.max(maxPadding[b], outerDims[b]);
}
Expand All @@ -109,16 +98,19 @@ 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
? {
left: Math.max(outerDims.left, maxPadding.left),
right: Math.max(outerDims.right, maxPadding.right)
}
: {
top: Math.max(outerDims.top, maxPadding.top),
bottom: Math.max(outerDims.bottom, maxPadding.bottom)
};
? marginForPositions(['left', 'right'])
: marginForPositions(['top', 'bottom']);
}

function updateChartArea(chartArea, outerDims, maxPadding) {
Expand All @@ -128,7 +120,7 @@ function updateChartArea(chartArea, outerDims, maxPadding) {
if (newWidth !== chartArea.w || newHeight !== chartArea.h) {
chartArea.w = newWidth;
chartArea.h = newHeight;
return 1;
return true;
}
}

Expand All @@ -150,7 +142,7 @@ function fitBoxes(boxes, chartArea, outerDims) {
if (updateChartArea(chartArea, outerDims, maxPadding) && refitBoxes.length) {
// Dimensions changed and there were non full width boxes before this
// -> we have to refit those
refit = 1;
refit = true;
}
if (!box.fullWidth) { // fullWidth boxes don't need to be re-fitted in any case
refitBoxes.push(layout);
Expand Down Expand Up @@ -298,8 +290,8 @@ module.exports = {
var availableWidth = width - padding.width;
var availableHeight = height - padding.height;
var boxes = buildLayoutBoxes(chart.boxes);
var verticalBoxes = boxes.v;
var horizontalBoxes = boxes.h;
var verticalBoxes = boxes.vertical;
var horizontalBoxes = boxes.horizontal;

// Essentially we now have any number of boxes on each of the 4 sides.
// Our canvas looks like the following.
Expand Down Expand Up @@ -345,16 +337,16 @@ module.exports = {

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

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

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

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

// Adjust chart area based on horizontal layouts
// Adjust chart area based on horizontal boxes
if (updateChartArea(chartArea, outerDims, maxPadding)) {
// if the area changed, re-fit vertical boxes
outerDims.left = padding.left;
Expand All @@ -363,6 +355,7 @@ module.exports = {
updateChartArea(chartArea, outerDims, maxPadding);
}

// Make sure horizontal boxes have correct width
helpers.each(horizontalBoxes, function(layout) {
if (!layout.box.fullWidth) {
layout.box.width = chartArea.w;
Expand All @@ -372,27 +365,26 @@ module.exports = {
// Adjust top/left of outerDims and boxCorner with padding if needed
adjustWithPadding(outerDims, boxCorner, maxPadding);

// Finally place the layouts to correct coordinates
// Finally place the boxes to correct coordinates
// - left / top
boxCorner = placeBoxes(boxes.lt, chartArea, outerDims, boxCorner);
boxCorner = placeBoxes(boxes.leftAndTop, chartArea, outerDims, boxCorner);

// Move to oppisite side of chart
// Move to opposite side of chart
boxCorner.left += chartArea.w;
boxCorner.top += chartArea.h;

// - right / bottom
placeBoxes(boxes.rb, chartArea, outerDims, boxCorner);
placeBoxes(boxes.rightAndBottom, chartArea, outerDims, boxCorner);

// Step 8
chart.chartArea = {
left: outerDims.left,
top: outerDims.top,
right: outerDims.left + chartArea.w,
bottom: outerDims.top + chartArea.h
};

// Step 9 - update chartArea boxes
helpers.each(boxes.area, function(layout) {
// Finally update boxes in chartArea (radial scale for example)
helpers.each(boxes.chartArea, function(layout) {
var box = layout.box;
extend(box, chart.chartArea);
box.update(chartArea.w, chartArea.h);
Expand Down
22 changes: 6 additions & 16 deletions src/core/core.scale.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -684,8 +684,7 @@ var Scale = Element.extend({
return null;
}
if (me.isHorizontal()) {
var innerWidth = me.width - (me.paddingLeft + me.paddingRight);
var tickWidth = innerWidth / Math.max((numTicks - (offset ? 0 : 1)), 1);
var tickWidth = me.width / Math.max((numTicks - (offset ? 0 : 1)), 1);
var pixel = (tickWidth * index) + me.paddingLeft;

if (offset) {
Expand All @@ -696,8 +695,7 @@ var Scale = Element.extend({
finalVal += me.isFullWidth() ? me.margins.left : 0;
return finalVal;
}
var innerHeight = me.height - (me.paddingTop + me.paddingBottom);
return me.top + (index * (innerHeight / (numTicks - 1)));
return me.top + (index * (me.height / (numTicks - 1)));
},

/**
Expand All @@ -706,15 +704,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 @@ -753,9 +745,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

0 comments on commit a6f73d6

Please sign in to comment.