From a6f73d650c3cadc5c3f0270bda31650e7ca82d63 Mon Sep 17 00:00:00 2001 From: Jukka Kurkela Date: Tue, 4 Jun 2019 09:21:22 +0300 Subject: [PATCH] address review comments / issues --- src/core/core.layouts.js | 86 ++++++++++++++++++---------------------- src/core/core.scale.js | 22 +++------- 2 files changed, 45 insertions(+), 63 deletions(-) diff --git a/src/core/core.layouts.js b/src/core/core.layouts.js index c36198698c9..ae6526e3850 100644 --- a/src/core/core.layouts.js +++ b/src/core/core.layouts.js @@ -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 ? @@ -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; @@ -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) }; } @@ -86,7 +77,6 @@ function updateMaxPadding(padding, box) { } } - function addSizeByPosition(outerDims, boxes) { var result = extend({}, outerDims); helpers.each(boxes, function(layout) { @@ -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]); } @@ -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) { @@ -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; } } @@ -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); @@ -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. @@ -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; @@ -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; @@ -372,18 +365,17 @@ 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, @@ -391,8 +383,8 @@ module.exports = { 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); diff --git a/src/core/core.scale.js b/src/core/core.scale.js index 37446d05785..b6b62f3e7b2 100644 --- a/src/core/core.scale.js +++ b/src/core/core.scale.js @@ -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) { @@ -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))); }, /** @@ -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; }, /** @@ -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;