Skip to content

Commit

Permalink
Changes to accommodate issue 4632:
Browse files Browse the repository at this point in the history
1. Unconditionally select default initial axis ticks based on SVG size.
2. Make half-stepSize adjustment to axis ticks only if multiAxis chart.
3. Unconditionally, if not user specified, snap yMin or yMax to zero
if already close.
4. 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.
5. In Scales.niceScale: Check that maxTicks is always valid (suspect
some Unit tests don't set SVG dimensions).
6. Set tickAmount explicitly in two unit tests to compensate for
variation in scaling following the above changes.
  • Loading branch information
rosco54 committed Aug 15, 2024
1 parent cad1d2f commit 1093365
Show file tree
Hide file tree
Showing 7 changed files with 75 additions and 41 deletions.
12 changes: 9 additions & 3 deletions samples/react/column/stacked-column-with-line-new.html
Original file line number Diff line number Diff line change
Expand Up @@ -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: {
Expand Down
12 changes: 9 additions & 3 deletions samples/source/column/stacked-column-with-line-new.xml
Original file line number Diff line number Diff line change
Expand Up @@ -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: {
Expand Down
12 changes: 9 additions & 3 deletions samples/vanilla-js/column/stacked-column-with-line-new.html
Original file line number Diff line number Diff line change
Expand Up @@ -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: {
Expand Down
12 changes: 9 additions & 3 deletions samples/vue/column/stacked-column-with-line-new.html
Original file line number Diff line number Diff line change
Expand Up @@ -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: {
Expand Down
64 changes: 35 additions & 29 deletions src/modules/Scales.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 =
Expand All @@ -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),
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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 &&
Expand Down
3 changes: 3 additions & 0 deletions tests/unit/exponential-data.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,9 @@ describe('Exponential values should parse', () => {
]
}
],
yaxis: {
tickAmount: 10
},
xaxis: {
type: 'datetime'
}
Expand Down
1 change: 1 addition & 0 deletions tests/unit/small-range.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down

0 comments on commit 1093365

Please sign in to comment.