Skip to content

Commit

Permalink
Removal of Scales.autoScaleY() and all calls to it:
Browse files Browse the repository at this point in the history
Options.chart.zoom.autoScaleYaxis had no effect, now fixed:
Possibly since release 3.46.0 as a result of changes related to
brush charts, Y axes are always scaled when zoomed regardless of
autoScaleYaxis, either in the options.chart.zoom or options.chart.brush.
The correct behaviour has been restored but is controlled within
Range.getMinYMaxY().

A note for debugging:
This routine is called numerous times during chart preparation,
including before all charts are fully populated with state relating to
the brush option. The crucial pass through this function is, I believe,
as a result of the event trigger to update the target chart with zoom
information, when it is then possible to determine the required
brushSource state.

Scales.autoScaleY() and calls to it have been removed completely as
they are no longer needed, and was duplicating the role of
Range.getMinYMaxY().

This change exposed the disableDefault in passive event listener common
issue, which was resolved by setting "passive: false" in svg.js.
  • Loading branch information
rosco54 committed Feb 21, 2024
1 parent 80eba01 commit 6a300a5
Show file tree
Hide file tree
Showing 5 changed files with 22 additions and 133 deletions.
34 changes: 21 additions & 13 deletions src/modules/Range.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,19 +44,27 @@ class Range {
seriesX = [...new Set([].concat(...gl.seriesX.slice(startingIndex, endingIndex)))]
firstXIndex = 0
lastXIndex = seriesX.length - 1
if (cnf.xaxis.min) {
for (
firstXIndex = 0;
firstXIndex < lastXIndex && seriesX[firstXIndex] <= cnf.xaxis.min;
firstXIndex++
) {}
}
if (cnf.xaxis.max) {
for (
;
lastXIndex > firstXIndex && seriesX[lastXIndex] >= cnf.xaxis.max;
lastXIndex--
) {}
// Eventually brushSource will be set if the current chart is a target.
// That is, after the appropriate event causes us to update.
let brush = gl.brushSource?.w.config.chart.brush
if ((cnf.chart.zoom.enabled && cnf.chart.zoom.autoScaleYaxis)
|| (brush?.enabled && brush?.autoScaleYaxis)
) {
// Scale the Y axis to the min..max within the zoomed X axis domain.
if (cnf.xaxis.min) {
for (
firstXIndex = 0;
firstXIndex < lastXIndex && seriesX[firstXIndex] <= cnf.xaxis.min;
firstXIndex++
) {}
}
if (cnf.xaxis.max) {
for (
;
lastXIndex > firstXIndex && seriesX[lastXIndex] >= cnf.xaxis.max;
lastXIndex--
) {}
}
}
}

Expand Down
99 changes: 0 additions & 99 deletions src/modules/Scales.js
Original file line number Diff line number Diff line change
Expand Up @@ -711,103 +711,4 @@ export default class Scales {
})
})
}

// experimental feature which scales the y-axis to a min/max based on x-axis range
autoScaleY(ctx, yaxis, e) {
if (!ctx) {
ctx = this
}

const w = ctx.w

if (w.globals.isMultipleYAxis || w.globals.collapsedSeries.length) {
// The autoScale option for multiple y-axis is turned off as it leads to buggy behavior.
// Also, when a series is collapsed, it results in incorrect behavior. Hence turned it off for that too - fixes apexcharts.js#795
console.warn('autoScaleYaxis not supported in a multi-yaxis chart.')
return yaxis
}

const seriesX = w.globals.seriesX[0]

let isStacked = w.config.chart.stacked

yaxis.forEach((yaxe, yi) => {
let firstXIndex = 0

for (let xi = 0; xi < seriesX.length; xi++) {
if (seriesX[xi] >= e.xaxis.min) {
firstXIndex = xi
break
}
}

let initialMin = w.globals.minYArr[yi]
let initialMax = w.globals.maxYArr[yi]
let min, max

let stackedSer = w.globals.stackedSeriesTotals

w.globals.series.forEach((serie, sI) => {
let firstValue = serie[firstXIndex]

if (isStacked) {
firstValue = stackedSer[firstXIndex]
min = max = firstValue

stackedSer.forEach((y, yI) => {
if (seriesX[yI] <= e.xaxis.max && seriesX[yI] >= e.xaxis.min) {
if (y > max && y !== null) max = y
if (serie[yI] < min && serie[yI] !== null) min = serie[yI]
}
})
} else {
min = max = firstValue

serie.forEach((y, yI) => {
if (seriesX[yI] <= e.xaxis.max && seriesX[yI] >= e.xaxis.min) {
let valMin = y
let valMax = y
w.globals.series.forEach((wS, wSI) => {
if (y !== null) {
valMin = Math.min(wS[yI], valMin)
valMax = Math.max(wS[yI], valMax)
}
})
if (valMax > max && valMax !== null) max = valMax
if (valMin < min && valMin !== null) min = valMin
}
})
}

if (min === undefined && max === undefined) {
min = initialMin
max = initialMax
}
min *= min < 0 ? 1.1 : 0.9
max *= max < 0 ? 0.9 : 1.1

if (min === 0 && max === 0) {
min = -1
max = 1
}

if (max < 0 && max < initialMax) {
max = initialMax
}
if (min < 0 && min > initialMin) {
min = initialMin
}

if (yaxis.length > 1) {
yaxis[sI].min = yaxe.min === undefined ? min : yaxe.min
yaxis[sI].max = yaxe.max === undefined ? max : yaxe.max
} else {
yaxis[0].min = yaxe.min === undefined ? min : yaxe.min
yaxis[0].max = yaxe.max === undefined ? max : yaxe.max
}
})
})

return yaxis
}
}
6 changes: 0 additions & 6 deletions src/modules/Toolbar.js
Original file line number Diff line number Diff line change
Expand Up @@ -402,12 +402,6 @@ export default class Toolbar {
}

let yaxis = Utils.clone(w.globals.initialConfig.yaxis)
if (w.config.chart.zoom.autoScaleYaxis) {
const scale = new Scales(this.ctx)
yaxis = scale.autoScaleY(this.ctx, yaxis, {
xaxis,
})
}

if (!w.config.chart.group) {
// if chart in a group, prevent yaxis update here
Expand Down
14 changes: 0 additions & 14 deletions src/modules/ZoomPanSelection.js
Original file line number Diff line number Diff line change
Expand Up @@ -605,13 +605,6 @@ export default class ZoomPanSelection extends Toolbar {
})
}

if (w.config.chart.zoom.autoScaleYaxis) {
const scale = new Scales(me.ctx)
yaxis = scale.autoScaleY(me.ctx, yaxis, {
xaxis,
})
}

if (toolbar) {
let beforeZoomRange = toolbar.getBeforeZoomRange(xaxis, yaxis)
if (beforeZoomRange) {
Expand Down Expand Up @@ -764,13 +757,6 @@ export default class ZoomPanSelection extends Toolbar {
max: xHighestValue,
}

if (w.config.chart.zoom.autoScaleYaxis) {
const scale = new Scales(this.ctx)
yaxis = scale.autoScaleY(this.ctx, yaxis, {
xaxis,
})
}

let options = {
xaxis: {
min: xLowestValue,
Expand Down
2 changes: 1 addition & 1 deletion src/svgjs/svg.js
Original file line number Diff line number Diff line change
Expand Up @@ -2229,7 +2229,7 @@
SVG.listeners[index][ev][ns][listener._svgjsListenerId] = l

// add listener
node.addEventListener(ev, l, options || { passive: true })
node.addEventListener(ev, l, options || { passive: false })
}

// Add event unbinder in the SVG namespace
Expand Down

0 comments on commit 6a300a5

Please sign in to comment.