Skip to content

Commit b816d2f

Browse files
AsturaPhoenixAsturaPhoenix
authored
Handling zoom edge cases (#690) (#708)
* Check whether scale bounds need to be updated before getInitialScaleBounds Scale bounds may be stale if data/bounds are updated just before getInitialScaleBounds. Among other things, this could cause isZoomedOrPanned to be false after resetZoom. This does not address all cases where data/bounds are updated while the chart is zoomed or panned, though resetZoom now corrects this discrepancy. * Respect minRange in zoom edge cases 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.
1 parent a3cbb60 commit b816d2f

4 files changed

Lines changed: 168 additions & 19 deletions

File tree

‎src/core.js‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -199,6 +199,7 @@ export function pan(chart, delta, enabledScales, transition = 'none') {
199199

200200
export function getInitialScaleBounds(chart) {
201201
const state = getState(chart);
202+
storeOriginalScaleLimits(chart, state);
202203
const scaleBounds = {};
203204
for (const scaleId of Object.keys(chart.scales)) {
204205
const {min, max} = state.originalScaleLimits[scaleId] || {min: {}, max: {}};

‎src/scale.types.js‎

Lines changed: 11 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -47,24 +47,17 @@ export function updateRange(scale, {min, max}, limits, zoom = false) {
4747
const minLimit = getLimit(state, scale, scaleLimits, 'min', -Infinity);
4848
const maxLimit = getLimit(state, scale, scaleLimits, 'max', Infinity);
4949

50-
const cmin = Math.max(min, minLimit);
51-
const cmax = Math.min(max, maxLimit);
52-
const range = zoom ? Math.max(cmax - cmin, minRange) : scale.max - scale.min;
53-
if (cmax - cmin !== range) {
54-
if (minLimit > cmax - range) {
55-
min = cmin;
56-
max = cmin + range;
57-
} else if (maxLimit < cmin + range) {
58-
max = cmax;
59-
min = cmax - range;
60-
} else {
61-
const offset = (range - cmax + cmin) / 2;
62-
min = cmin - offset;
63-
max = cmax + offset;
64-
}
65-
} else {
66-
min = cmin;
67-
max = cmax;
50+
const range = zoom ? Math.max(max - min, minRange) : scale.max - scale.min;
51+
const offset = (range - max + min) / 2;
52+
min -= offset;
53+
max += offset;
54+
55+
if (min < minLimit) {
56+
min = minLimit;
57+
max = Math.min(minLimit + range, maxLimit);
58+
} else if (max > maxLimit) {
59+
max = maxLimit;
60+
min = Math.max(maxLimit - range, minLimit);
6861
}
6962
scaleOpts.min = min;
7063
scaleOpts.max = max;

‎test/specs/api.spec.js‎

Lines changed: 155 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,107 @@ describe('api', function() {
9191
expect(chart.scales.y.min).toBe(0);
9292
expect(chart.scales.y.max).toBe(100);
9393
});
94+
95+
it('should honor fitted scale updates on reset', function() {
96+
const chart = window.acquireChart({
97+
type: 'scatter',
98+
data: {
99+
datasets: [
100+
{
101+
data: [
102+
{x: 0, y: 0},
103+
{x: 100, y: 100}
104+
]
105+
}
106+
]
107+
}
108+
});
109+
110+
chart.zoom(1.5);
111+
chart.data.datasets[0].data[0].x = -100;
112+
chart.update();
113+
chart.resetZoom();
114+
expect(chart.scales.x.min).toBe(-100);
115+
expect(chart.scales.x.max).toBe(100);
116+
});
117+
118+
it('should no-op with fully constrained limits', function() {
119+
const chart = window.acquireChart({
120+
type: 'scatter',
121+
options: {
122+
scales: {
123+
x: {
124+
min: 0,
125+
max: 100
126+
},
127+
y: {
128+
min: 0,
129+
max: 100
130+
}
131+
},
132+
plugins: {
133+
zoom: {
134+
limits: {
135+
x: {
136+
min: 0,
137+
max: 100,
138+
minRange: 100
139+
}
140+
}
141+
}
142+
}
143+
}
144+
});
145+
146+
chart.zoom(1.5);
147+
expect(chart.scales.x.min).toBe(0);
148+
expect(chart.scales.x.max).toBe(100);
149+
});
150+
151+
it('should honor zoom changes against a limit', function() {
152+
const chart = window.acquireChart({
153+
type: 'scatter',
154+
options: {
155+
scales: {
156+
x: {
157+
min: 0,
158+
max: 100
159+
},
160+
y: {
161+
min: 0,
162+
max: 100
163+
}
164+
},
165+
plugins: {
166+
zoom: {
167+
limits: {
168+
x: {
169+
min: 0,
170+
max: 100
171+
}
172+
}
173+
}
174+
}
175+
}
176+
});
177+
chart.zoom({
178+
x: 1.99,
179+
focalPoint: {
180+
x: chart.scales.x.getPixelForValue(0)
181+
}
182+
});
183+
expect(chart.scales.x.min).toBe(0);
184+
expect(chart.scales.x.max).toBe(1);
185+
186+
chart.zoom({
187+
x: -98,
188+
focalPoint: {
189+
x: chart.scales.x.getPixelForValue(1)
190+
}
191+
});
192+
expect(chart.scales.x.min).toBe(0);
193+
expect(chart.scales.x.max).toBe(100);
194+
});
94195
});
95196

96197
describe('getInitialScaleBounds', function() {
@@ -123,6 +224,35 @@ describe('api', function() {
123224
expect(chart.getInitialScaleBounds().y.min).toBe(0);
124225
expect(chart.getInitialScaleBounds().y.max).toBe(100);
125226
});
227+
228+
it('should provide updated scale bounds upon data update', function() {
229+
const chart = window.acquireChart({
230+
type: 'scatter',
231+
data: {
232+
datasets: [
233+
{
234+
data: [
235+
{x: 0, y: 0},
236+
{x: 100, y: 100}
237+
]
238+
}
239+
]
240+
}
241+
});
242+
243+
expect(chart.getInitialScaleBounds().x.min).toBe(0);
244+
expect(chart.getInitialScaleBounds().x.max).toBe(100);
245+
expect(chart.getInitialScaleBounds().y.min).toBe(0);
246+
expect(chart.getInitialScaleBounds().y.max).toBe(100);
247+
248+
chart.data.datasets[0].data[0].x = -100;
249+
chart.update();
250+
251+
expect(chart.getInitialScaleBounds().x.min).toBe(-100);
252+
expect(chart.getInitialScaleBounds().x.max).toBe(100);
253+
expect(chart.getInitialScaleBounds().y.min).toBe(0);
254+
expect(chart.getInitialScaleBounds().y.max).toBe(100);
255+
});
126256
});
127257

128258
describe('isZoomedOrPanned', function() {
@@ -186,5 +316,30 @@ describe('api', function() {
186316
chart.resetZoom();
187317
expect(chart.isZoomedOrPanned()).toBe(false);
188318
});
319+
320+
it('should work with updated data and fitted scales', function() {
321+
const chart = window.acquireChart({
322+
type: 'scatter',
323+
data: {
324+
datasets: [
325+
{
326+
data: [
327+
{x: 0, y: 0},
328+
{x: 100, y: 100}
329+
]
330+
}
331+
]
332+
}
333+
});
334+
335+
// This sequence of operations captures a previous bug.
336+
chart.zoom(1.5);
337+
chart.data.datasets[0].data[0].x = -100;
338+
chart.update();
339+
chart.resetZoom();
340+
expect(chart.scales.x.min).toBe(-100);
341+
expect(chart.scales.x.max).toBe(100);
342+
expect(chart.isZoomedOrPanned()).toBe(false);
343+
});
189344
});
190345
});

‎test/specs/zoom.spec.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -933,7 +933,7 @@ describe('zoom', function() {
933933
expect(chart.scales.y.min).toBe(3);
934934
expect(chart.scales.y.max).toBe(5);
935935
chart.zoom(0.9);
936-
expect(chart.scales.y.min).toBe(2);
936+
expect(chart.scales.y.min).toBe(1);
937937
expect(chart.scales.y.max).toBe(5);
938938
chart.zoom(0.9);
939939
expect(chart.scales.y.min).toBe(1);

0 commit comments

Comments
 (0)