From 346e9c943bf87ba95f38211650afe09074489fb8 Mon Sep 17 00:00:00 2001 From: Ross Johnson <159597299+rosco54@users.noreply.github.com> Date: Thu, 15 Aug 2024 12:42:42 +1000 Subject: [PATCH] Changes to accommodate issue 4632: 1. Unconditionally select default initial axis ticks based on SVG size. 2. Unconditionally, if not user specified, snap yMin or yMax to zero if already close. 3. Re-order yaxes in one sample chart to prioritize stacked column visual resolution. Add comment to draw attention to this feature of multi axis charts. 4. In Scales.niceScale: Check that maxTicks is always valid (suspect some Unit tests don't set SVG dimensions). 5. Set tickAmount explicitly in two unit tests to compensate for variation in scaling following the above changes. --- .../column/stacked-column-with-line-new.html | 12 +++- .../column/stacked-column-with-line-new.xml | 12 +++- .../column/stacked-column-with-line-new.html | 12 +++- .../column/stacked-column-with-line-new.html | 12 +++- src/modules/Scales.js | 64 ++++++++++--------- tests/unit/exponential-data.spec.js | 3 + tests/unit/small-range.spec.js | 1 + 7 files changed, 75 insertions(+), 41 deletions(-) diff --git a/samples/react/column/stacked-column-with-line-new.html b/samples/react/column/stacked-column-with-line-new.html index 933697c2e..91175c51f 100644 --- a/samples/react/column/stacked-column-with-line-new.html +++ b/samples/react/column/stacked-column-with-line-new.html @@ -295,13 +295,19 @@ } } }, + // In order to align the ticks of multiple yaxes with the charts horizontal grid + // lines, the scaling of the first [unhidden] yaxis determines the number + // of grid lines and hence the number of ticks for all subsequent yaxes, unless + // they specify a tickAmount. + // We want the stacked bars to have the best possible resolution + // visually so they have been listed first. yaxis: [ { - seriesName: 'Line', - opposite: true + seriesName: ['Bar1','Bar2','Bar3','bar4'] }, { - seriesName: ['Bar1','Bar2','Bar3','bar4'] + seriesName: 'Line', + opposite: true } ], legend: { diff --git a/samples/source/column/stacked-column-with-line-new.xml b/samples/source/column/stacked-column-with-line-new.xml index 9a91aa143..1896eb36e 100644 --- a/samples/source/column/stacked-column-with-line-new.xml +++ b/samples/source/column/stacked-column-with-line-new.xml @@ -76,13 +76,19 @@ xaxis: { } } }, +// In order to align the ticks of multiple yaxes with the charts horizontal grid +// lines, the scaling of the first [unhidden] yaxis determines the number +// of grid lines and hence the number of ticks for all subsequent yaxes, unless +// they specify a tickAmount. +// We want the stacked bars to have the best possible resolution +// visually so they have been listed first. yaxis: [ { - seriesName: 'Line', - opposite: true + seriesName: ['Bar1','Bar2','Bar3','bar4'] }, { - seriesName: ['Bar1','Bar2','Bar3','bar4'] + seriesName: 'Line', + opposite: true } ], legend: { diff --git a/samples/vanilla-js/column/stacked-column-with-line-new.html b/samples/vanilla-js/column/stacked-column-with-line-new.html index 1907f5d91..4ccda7e11 100644 --- a/samples/vanilla-js/column/stacked-column-with-line-new.html +++ b/samples/vanilla-js/column/stacked-column-with-line-new.html @@ -278,13 +278,19 @@ } } }, + // In order to align the ticks of multiple yaxes with the charts horizontal grid + // lines, the scaling of the first [unhidden] yaxis determines the number + // of grid lines and hence the number of ticks for all subsequent yaxes, unless + // they specify a tickAmount. + // We want the stacked bars to have the best possible resolution + // visually so they have been listed first. yaxis: [ { - seriesName: 'Line', - opposite: true + seriesName: ['Bar1','Bar2','Bar3','bar4'] }, { - seriesName: ['Bar1','Bar2','Bar3','bar4'] + seriesName: 'Line', + opposite: true } ], legend: { diff --git a/samples/vue/column/stacked-column-with-line-new.html b/samples/vue/column/stacked-column-with-line-new.html index 7d48f9f17..3b6e591b3 100644 --- a/samples/vue/column/stacked-column-with-line-new.html +++ b/samples/vue/column/stacked-column-with-line-new.html @@ -298,13 +298,19 @@ } } }, + // In order to align the ticks of multiple yaxes with the charts horizontal grid + // lines, the scaling of the first [unhidden] yaxis determines the number + // of grid lines and hence the number of ticks for all subsequent yaxes, unless + // they specify a tickAmount. + // We want the stacked bars to have the best possible resolution + // visually so they have been listed first. yaxis: [ { - seriesName: 'Line', - opposite: true + seriesName: ['Bar1','Bar2','Bar3','bar4'] }, { - seriesName: ['Bar1','Bar2','Bar3','bar4'] + seriesName: 'Line', + opposite: true } ], legend: { diff --git a/src/modules/Scales.js b/src/modules/Scales.js index ed691916b..4e978bfe5 100644 --- a/src/modules/Scales.js +++ b/src/modules/Scales.js @@ -29,6 +29,9 @@ export default class Scales { axisCnf = w.config.yaxis[index] maxTicks = Math.max((gl.svgHeight - 100) / 15, 2) } + if (!Utils.isNumber(maxTicks)) { + maxTicks = 10 + } gotMin = axisCnf.min !== undefined && axisCnf.min !== null gotMax = axisCnf.max !== undefined && axisCnf.min !== null let gotStepSize = @@ -37,8 +40,6 @@ export default class Scales { axisCnf.tickAmount !== undefined && axisCnf.tickAmount !== null let ticks = gotTickAmount ? axisCnf.tickAmount - : !axisCnf.forceNiceScale - ? 10 : gl.niceScaleDefaultTicks[ Math.min( Math.round(maxTicks / 2), @@ -100,19 +101,17 @@ export default class Scales { // Determine Range let range = Math.abs(yMax - yMin) - if (axisCnf.forceNiceScale) { - // Snap min or max to zero if close - let proximityRatio = 0.15 - if (!gotMin && yMin > 0 && yMin / range < proximityRatio) { - yMin = 0 - gotMin = true - } - if (!gotMax && yMax < 0 && -yMax / range < proximityRatio) { - yMax = 0 - gotMax = true - } - range = Math.abs(yMax - yMin) + // Snap min or max to zero if close + let proximityRatio = 0.15 + if (!gotMin && yMin > 0 && yMin / range < proximityRatio) { + yMin = 0 + gotMin = true + } + if (!gotMax && yMax < 0 && -yMax / range < proximityRatio) { + yMax = 0 + gotMax = true } + range = Math.abs(yMax - yMin) // Calculate a pretty step value based on ticks @@ -231,18 +230,25 @@ export default class Scales { } else { // Snap range to ticks if (!gotMin && !gotMax) { - if (gotTickAmount) { - // Allow a half-stepSize shift if series doesn't cross the X axis - // to ensure graph doesn't clip. Not if it does cross, in order - // to keep the 0 aligned with a grid line in multi axis charts. - let shift = stepSize / (yMax - yMin > yMax ? 1 : 2) - let tMin = shift * Math.floor(yMin / shift) - if (Math.abs(tMin - yMin) <= shift / 2) { - yMin = tMin - yMax = yMin + stepSize * tiks - } else { - yMax = shift * Math.ceil(yMax / shift) - yMin = yMax - stepSize * tiks + if (gl.isMultipleYAxis && gotTickAmount) { + // Ensure graph doesn't clip. + let tMin = stepSize * Math.floor(yMin / stepSize) + let tMax = tMin + stepSize * tiks + if (tMax < yMax) { + stepSize *= 2 + } + yMin = tMin + tMax = yMax + yMax = yMin + stepSize * tiks + // Snap min or max to zero if possible + range = Math.abs(yMax - yMin) + if (yMin > 0 && yMin < Math.abs(tMax - yMax)) { + yMin = 0 + yMax = stepSize * tiks + } + if (yMax < 0 && -yMax < Math.abs(tMin - yMin)) { + yMax = 0 + yMin = -stepSize * tiks } } else { yMin = stepSize * Math.floor(yMin / stepSize) @@ -297,9 +303,9 @@ export default class Scales { } // Prune tiks down to range if series is all integers. Since tiks > range, - // range is very low (< 10 or so). Skip this step if tickAmount is true - // because, either the user set it, or the chart is multiscale and this - // axis is not determining the number of grid lines. + // range is very low (< 10 or so). Skip this step if gotTickAmount is true + // because either the user set tickAmount or the chart is multiscale and + // this axis is not determining the number of grid lines. if ( !gotTickAmount && axisCnf.forceNiceScale && diff --git a/tests/unit/exponential-data.spec.js b/tests/unit/exponential-data.spec.js index 1be0b8f39..a54ee0856 100644 --- a/tests/unit/exponential-data.spec.js +++ b/tests/unit/exponential-data.spec.js @@ -72,6 +72,9 @@ describe('Exponential values should parse', () => { ] } ], + yaxis: { + tickAmount: 10 + }, xaxis: { type: 'datetime' } diff --git a/tests/unit/small-range.spec.js b/tests/unit/small-range.spec.js index b89b05ce2..9f6b2c136 100644 --- a/tests/unit/small-range.spec.js +++ b/tests/unit/small-range.spec.js @@ -139,6 +139,7 @@ describe('yaxis scale to ignore duplication if fractions are present in series', ], }, yaxis: { + tickAmount: 8, labels: { formatter: (val) => { return val.toFixed(2)