Skip to content

Commit

Permalink
Respect minRange in zoom edge cases
Browse files Browse the repository at this point in the history
Addresses at least two edge cases where minRange produced unintuitive effects:

 * When zooming in and the zoom range was close to both the min and max limits, minRange adjustment would produce jitter violating the max.
 * When zooming out against a limit, range was truncated such that a zoomout from the opposite side of the chart could no-op.

However, this may have an undesirable implication for category axis zooming since zooming out against a limit can now alternate between zooming out by 1 or 2 categories whereas before it would tend to truncate to 1.
  • Loading branch information
AsturaPhoenix committed Nov 3, 2022
1 parent 8974f29 commit fa1d55e
Show file tree
Hide file tree
Showing 3 changed files with 90 additions and 19 deletions.
29 changes: 11 additions & 18 deletions src/scale.types.js
Original file line number Diff line number Diff line change
Expand Up @@ -47,24 +47,17 @@ export function updateRange(scale, {min, max}, limits, zoom = false) {
const minLimit = getLimit(state, scale, scaleLimits, 'min', -Infinity);
const maxLimit = getLimit(state, scale, scaleLimits, 'max', Infinity);

const cmin = Math.max(min, minLimit);
const cmax = Math.min(max, maxLimit);
const range = zoom ? Math.max(cmax - cmin, minRange) : scale.max - scale.min;
if (cmax - cmin !== range) {
if (minLimit > cmax - range) {
min = cmin;
max = cmin + range;
} else if (maxLimit < cmin + range) {
max = cmax;
min = cmax - range;
} else {
const offset = (range - cmax + cmin) / 2;
min = cmin - offset;
max = cmax + offset;
}
} else {
min = cmin;
max = cmax;
const range = zoom ? Math.max(max - min, minRange) : scale.max - scale.min;
const offset = (range - max + min) / 2;
min -= offset;
max += offset;

if (min < minLimit) {
min = minLimit;
max = Math.min(minLimit + range, maxLimit);
} else if (max > maxLimit) {
max = maxLimit;
min = Math.max(maxLimit - range, minLimit);
}
scaleOpts.min = min;
scaleOpts.max = max;
Expand Down
78 changes: 78 additions & 0 deletions test/specs/api.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,84 @@ describe('api', function() {
expect(chart.scales.x.min).toBe(-100);
expect(chart.scales.x.max).toBe(100);
});

it('should no-op with fully constrained limits', function() {
const chart = window.acquireChart({
type: 'scatter',
options: {
scales: {
x: {
min: 0,
max: 100
},
y: {
min: 0,
max: 100
}
},
plugins: {
zoom: {
limits: {
x: {
min: 0,
max: 100,
minRange: 100
}
}
}
}
}
});

chart.zoom(1.5);
expect(chart.scales.x.min).toBe(0);
expect(chart.scales.x.max).toBe(100);
});

it('should honor zoom changes against a limit', function() {
const chart = window.acquireChart({
type: 'scatter',
options: {
scales: {
x: {
min: 0,
max: 100
},
y: {
min: 0,
max: 100
}
},
plugins: {
zoom: {
limits: {
x: {
min: 0,
max: 100
}
}
}
}
}
});
chart.zoom({
x: 1.99,
focalPoint: {
x: chart.scales.x.getPixelForValue(0)
}
});
expect(chart.scales.x.min).toBe(0);
expect(chart.scales.x.max).toBe(1);

chart.zoom({
x: -98,
focalPoint: {
x: chart.scales.x.getPixelForValue(1)
}
});
expect(chart.scales.x.min).toBe(0);
expect(chart.scales.x.max).toBe(100);
});
});

describe('getInitialScaleBounds', function() {
Expand Down
2 changes: 1 addition & 1 deletion test/specs/zoom.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -933,7 +933,7 @@ describe('zoom', function() {
expect(chart.scales.y.min).toBe(3);
expect(chart.scales.y.max).toBe(5);
chart.zoom(0.9);
expect(chart.scales.y.min).toBe(2);
expect(chart.scales.y.min).toBe(1);
expect(chart.scales.y.max).toBe(5);
chart.zoom(0.9);
expect(chart.scales.y.min).toBe(1);
Expand Down

0 comments on commit fa1d55e

Please sign in to comment.