Skip to content

Commit

Permalink
Fix "original" scale limits with nonlinear pan (#774)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
joshkel authored Nov 14, 2024
1 parent c1985b1 commit eb302da
Show file tree
Hide file tree
Showing 4 changed files with 126 additions and 16 deletions.
16 changes: 8 additions & 8 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
53 changes: 46 additions & 7 deletions src/scale.types.js
Original file line number Diff line number Diff line change
@@ -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);
Expand All @@ -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') {
Expand All @@ -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);
Expand All @@ -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;
Expand Down Expand Up @@ -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) {
Expand Down
71 changes: 71 additions & 0 deletions test/specs/pan.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down

0 comments on commit eb302da

Please sign in to comment.