From eb302dad64a6cf017ca65f77dce1d33b85315e62 Mon Sep 17 00:00:00 2001
From: Josh Kelley <joshkel@gmail.com>
Date: Thu, 14 Nov 2024 13:58:46 -0500
Subject: [PATCH] Fix "original" scale limits with nonlinear pan (#774)
* Add some JSDoc types
These currently raise spurious errors, due to limitations in Chart.js types.
* Fix handling of "original" with panning nonlinear scales
Nonlinear scales failed to resolve "original" within panNumericalScale.
To fix this:
* Resolving "original" within panNumericalScale seemed redundant, when
updateRange was already doing it, so I instead moved the logic into
updateRange and control it with a new special value for the `zoom`
parameter. (Hopefully the use of a special string like this is
acceptable as long as JSDoc calls it out - if I should use another
approach, please let me know.)
* This change put updateRange over the configured ESLint complexity
limit, so I extracted a new getScaleLimits function to reduce
complexity.
Add unit tests, both for this bug and for the preexisting but untested
"original" feature.
* Upgrade Chart.js to fix type errors
---
package-lock.json | 16 +++++-----
package.json | 2 +-
src/scale.types.js | 53 ++++++++++++++++++++++++++-----
test/specs/pan.spec.js | 71 ++++++++++++++++++++++++++++++++++++++++++
4 files changed, 126 insertions(+), 16 deletions(-)
diff --git a/package-lock.json b/package-lock.json
index aae87f18..7f218473 100644
--- a/package-lock.json
+++ b/package-lock.json
@@ -19,7 +19,7 @@
"@typescript-eslint/eslint-plugin": "^5.4.0",
"@typescript-eslint/parser": "^5.4.0",
"babel-loader": "^8.3.0",
- "chart.js": "^4.2.1",
+ "chart.js": "^4.3.2",
"chartjs-adapter-date-fns": "^2.0.1",
"chartjs-test-utils": "^0.3.0",
"concurrently": "^6.0.2",
@@ -5428,15 +5428,15 @@
}
},
"node_modules/chart.js": {
- "version": "4.2.1",
- "resolved": "https://registry.npmjs.org/chart.js/-/chart.js-4.2.1.tgz",
- "integrity": "sha512-6YbpQ0nt3NovAgOzbkSSeeAQu/3za1319dPUQTXn9WcOpywM8rGKxJHrhS8V8xEkAlk8YhEfjbuAPfUyp6jIsw==",
+ "version": "4.3.2",
+ "resolved": "https://registry.npmjs.org/chart.js/-/chart.js-4.3.2.tgz",
+ "integrity": "sha512-pvQNyFOY1QmbmIr8oDORL16/FFivfxj8V26VFpFilMo4cNvkV5WXLJetDio365pd9gKUHGdirUTbqJfw8tr+Dg==",
"dev": true,
"dependencies": {
"@kurkle/color": "^0.3.0"
},
"engines": {
- "pnpm": "^7.0.0"
+ "pnpm": ">=7"
}
},
"node_modules/chartjs-adapter-date-fns": {
@@ -23314,9 +23314,9 @@
"dev": true
},
"chart.js": {
- "version": "4.2.1",
- "resolved": "https://registry.npmjs.org/chart.js/-/chart.js-4.2.1.tgz",
- "integrity": "sha512-6YbpQ0nt3NovAgOzbkSSeeAQu/3za1319dPUQTXn9WcOpywM8rGKxJHrhS8V8xEkAlk8YhEfjbuAPfUyp6jIsw==",
+ "version": "4.3.2",
+ "resolved": "https://registry.npmjs.org/chart.js/-/chart.js-4.3.2.tgz",
+ "integrity": "sha512-pvQNyFOY1QmbmIr8oDORL16/FFivfxj8V26VFpFilMo4cNvkV5WXLJetDio365pd9gKUHGdirUTbqJfw8tr+Dg==",
"dev": true,
"requires": {
"@kurkle/color": "^0.3.0"
diff --git a/package.json b/package.json
index ed3c91e8..f55c0ef5 100644
--- a/package.json
+++ b/package.json
@@ -40,7 +40,7 @@
"@typescript-eslint/eslint-plugin": "^5.4.0",
"@typescript-eslint/parser": "^5.4.0",
"babel-loader": "^8.3.0",
- "chart.js": "^4.2.1",
+ "chart.js": "^4.3.2",
"chartjs-adapter-date-fns": "^2.0.1",
"chartjs-test-utils": "^0.3.0",
"concurrently": "^6.0.2",
diff --git a/src/scale.types.js b/src/scale.types.js
index 0d83ebf6..a4699d06 100644
--- a/src/scale.types.js
+++ b/src/scale.types.js
@@ -1,6 +1,20 @@
import {valueOrDefault} from 'chart.js/helpers';
import {getState} from './state';
+/**
+ * @typedef {import('chart.js').Point} Point
+ * @typedef {import('chart.js').Scale} Scale
+ * @typedef {import('../types/options').LimitOptions} LimitOptions
+ * @typedef {{min: number, max: number}} ScaleRange
+ * @typedef {import('../types/options').ScaleLimits} ScaleLimits
+ */
+
+/**
+ * @param {Scale} scale
+ * @param {number} zoom
+ * @param {Point} center
+ * @returns {ScaleRange}
+ */
function zoomDelta(scale, zoom, center) {
const range = scale.max - scale.min;
const newRange = range * (zoom - 1);
@@ -20,6 +34,15 @@ function zoomDelta(scale, zoom, center) {
};
}
+/**
+ * @param {Scale} scale
+ * @param {LimitOptions|undefined} limits
+ * @returns {ScaleLimits}
+ */
+function getScaleLimits(scale, limits) {
+ return limits && (limits[scale.id] || limits[scale.axis]) || {};
+}
+
function getLimit(state, scale, scaleLimits, prop, fallback) {
let limit = scaleLimits[prop];
if (limit === 'original') {
@@ -29,6 +52,12 @@ function getLimit(state, scale, scaleLimits, prop, fallback) {
return valueOrDefault(limit, fallback);
}
+/**
+ * @param {Scale} scale
+ * @param {number} pixel0
+ * @param {number} pixel1
+ * @returns {ScaleRange}
+ */
function getRange(scale, pixel0, pixel1) {
const v0 = scale.getValueForPixel(pixel0);
const v1 = scale.getValueForPixel(pixel1);
@@ -38,15 +67,27 @@ function getRange(scale, pixel0, pixel1) {
};
}
+/**
+ * @param {Scale} scale
+ * @param {ScaleRange} minMax
+ * @param {LimitOptions} [limits]
+ * @param {boolean|'pan'} [zoom]
+ * @returns {boolean}
+ */
export function updateRange(scale, {min, max}, limits, zoom = false) {
const state = getState(scale.chart);
- const {id, axis, options: scaleOpts} = scale;
+ const {options: scaleOpts} = scale;
- const scaleLimits = limits && (limits[id] || limits[axis]) || {};
+ const scaleLimits = getScaleLimits(scale, limits);
const {minRange = 0} = scaleLimits;
const minLimit = getLimit(state, scale, scaleLimits, 'min', -Infinity);
const maxLimit = getLimit(state, scale, scaleLimits, 'max', Infinity);
+ if (zoom === 'pan' && (min < minLimit || max > maxLimit)) {
+ // At limit: No change but return true to indicate no need to store the delta.
+ return true;
+ }
+
const range = zoom ? Math.max(max - min, minRange) : scale.max - scale.min;
const offset = (range - max + min) / 2;
min -= offset;
@@ -139,20 +180,18 @@ const OFFSETS = {
year: 182 * 24 * 60 * 60 * 1000 // 182 d
};
-function panNumericalScale(scale, delta, limits, canZoom = false) {
+function panNumericalScale(scale, delta, limits, pan = false) {
const {min: prevStart, max: prevEnd, options} = scale;
const round = options.time && options.time.round;
const offset = OFFSETS[round] || 0;
const newMin = scale.getValueForPixel(scale.getPixelForValue(prevStart + offset) - delta);
const newMax = scale.getValueForPixel(scale.getPixelForValue(prevEnd + offset) - delta);
- const {min: minLimit = -Infinity, max: maxLimit = Infinity} = canZoom && limits && limits[scale.axis] || {};
- if (isNaN(newMin) || isNaN(newMax) || newMin < minLimit || newMax > maxLimit) {
- // At limit: No change but return true to indicate no need to store the delta.
+ if (isNaN(newMin) || isNaN(newMax)) {
// NaN can happen for 0-dimension scales (either because they were configured
// with min === max or because the chart has 0 plottable area).
return true;
}
- return updateRange(scale, {min: newMin, max: newMax}, limits, canZoom);
+ return updateRange(scale, {min: newMin, max: newMax}, limits, pan ? 'pan' : false);
}
function panNonLinearScale(scale, delta, limits) {
diff --git a/test/specs/pan.spec.js b/test/specs/pan.spec.js
index 77725754..072e757b 100644
--- a/test/specs/pan.spec.js
+++ b/test/specs/pan.spec.js
@@ -124,6 +124,77 @@ describe('pan', function() {
expect(scale.options.min).toBe(2);
expect(scale.options.max).toBe(2);
});
+
+ it('should respect original limits', function() {
+ const chart = window.acquireChart({
+ type: 'line',
+ data,
+ options: {
+ plugins: {
+ zoom: {
+ pan: {
+ enabled: true,
+ mode: 'x',
+ },
+ limits: {
+ x: {
+ min: 'original',
+ max: 'original',
+ }
+ },
+ }
+ },
+ scales: {
+ x: {
+ min: 1,
+ max: 2
+ }
+ }
+ }
+ });
+ const scale = chart.scales.x;
+ expect(scale.min).toBe(1);
+ expect(scale.max).toBe(2);
+ chart.pan(100);
+ expect(scale.min).toBe(1);
+ expect(scale.max).toBe(2);
+ });
+
+ it('should respect original limits for nonlinear scales', function() {
+ const chart = window.acquireChart({
+ type: 'line',
+ data,
+ options: {
+ plugins: {
+ zoom: {
+ pan: {
+ enabled: true,
+ mode: 'x',
+ },
+ limits: {
+ x: {
+ min: 'original',
+ max: 'original',
+ }
+ },
+ }
+ },
+ scales: {
+ x: {
+ type: 'logarithmic',
+ min: 1,
+ max: 10
+ }
+ }
+ }
+ });
+ const scale = chart.scales.x;
+ expect(scale.min).toBe(1);
+ expect(scale.max).toBe(10);
+ chart.pan(100);
+ expect(scale.min).toBe(1);
+ expect(scale.max).toBe(10);
+ });
});
describe('events', function() {