Skip to content

Commit 2102d7b

Browse files
authored
fix(dashboards): Remove waiting for animation on chart zoom (#93909)
This is causing zooming issues where the finish handler isn't triggering and flushing the queued up time range changes (i.e. updating the URL params so the remaining charts and page filters can update). We've decided to immediately trigger the state changes because we know the chart zoom callback consistently triggers
1 parent 40294b9 commit 2102d7b

File tree

2 files changed

+52
-76
lines changed

2 files changed

+52
-76
lines changed

static/app/components/charts/chartZoom.tsx

Lines changed: 28 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -154,38 +154,36 @@ class ChartZoom extends Component<Props> {
154154
end: endFormatted,
155155
});
156156

157-
this.zooming = () => {
158-
if (usePageDate && router) {
159-
const newQuery = {
160-
...router.location.query,
161-
pageStart: start ? getUtcDateString(start) : undefined,
162-
pageEnd: end ? getUtcDateString(end) : undefined,
163-
pageStatsPeriod: period ?? undefined,
164-
};
165-
166-
// Only push new location if query params has changed because this will cause a heavy re-render
167-
if (qs.stringify(newQuery) !== qs.stringify(router.location.query)) {
168-
router.push({
169-
pathname: router.location.pathname,
170-
query: newQuery,
171-
});
172-
}
173-
} else {
174-
updateDateTime(
175-
{
176-
period,
177-
start: startFormatted
178-
? getUtcToLocalDateObject(startFormatted)
179-
: startFormatted,
180-
end: endFormatted ? getUtcToLocalDateObject(endFormatted) : endFormatted,
181-
},
182-
router,
183-
{save: saveOnZoom}
184-
);
157+
if (usePageDate && router) {
158+
const newQuery = {
159+
...router.location.query,
160+
pageStart: start ? getUtcDateString(start) : undefined,
161+
pageEnd: end ? getUtcDateString(end) : undefined,
162+
pageStatsPeriod: period ?? undefined,
163+
};
164+
165+
// Only push new location if query params has changed because this will cause a heavy re-render
166+
if (qs.stringify(newQuery) !== qs.stringify(router.location.query)) {
167+
router.push({
168+
pathname: router.location.pathname,
169+
query: newQuery,
170+
});
185171
}
172+
} else {
173+
updateDateTime(
174+
{
175+
period,
176+
start: startFormatted
177+
? getUtcToLocalDateObject(startFormatted)
178+
: startFormatted,
179+
end: endFormatted ? getUtcToLocalDateObject(endFormatted) : endFormatted,
180+
},
181+
router,
182+
{save: saveOnZoom}
183+
);
184+
}
186185

187-
this.saveCurrentPeriod({period, start, end});
188-
};
186+
this.saveCurrentPeriod({period, start, end});
189187
};
190188

191189
/**
@@ -290,17 +288,8 @@ class ChartZoom extends Component<Props> {
290288

291289
/**
292290
* Chart event when *any* rendering+animation finishes
293-
*
294-
* `this.zooming` acts as a callback function so that
295-
* we can let the native zoom animation on the chart complete
296-
* before we update URL state and re-render
297291
*/
298292
handleChartFinished = (_props: any, chart: any) => {
299-
if (typeof this.zooming === 'function') {
300-
this.zooming();
301-
this.zooming = null;
302-
}
303-
304293
// This attempts to activate the area zoom toolbox feature
305294
const zoom = chart._componentsViews?.find((c: any) => c._features?.dataZoom);
306295
if (zoom && !zoom._features.dataZoom._isZoomActive) {

static/app/components/charts/useChartZoom.tsx

Lines changed: 24 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -133,12 +133,6 @@ export function useChartZoom({
133133
const navigate = useNavigate();
134134
const router = useRouter();
135135

136-
/**
137-
* Used to store the date update function so that we can call it after the chart
138-
* animation is complete
139-
*/
140-
const zooming = useRef<(() => void) | null>(null);
141-
142136
/**
143137
* Sets the new period due to a zoom related action
144138
*
@@ -165,34 +159,32 @@ export function useChartZoom({
165159
end: getQueryTime(newPeriod.end),
166160
});
167161

168-
zooming.current = () => {
169-
if (usePageDate) {
170-
const newQuery = {
171-
...location.query,
172-
pageStart: startFormatted,
173-
pageEnd: endFormatted,
174-
pageStatsPeriod: newPeriod.period ?? undefined,
175-
};
162+
if (usePageDate) {
163+
const newQuery = {
164+
...location.query,
165+
pageStart: startFormatted,
166+
pageEnd: endFormatted,
167+
pageStatsPeriod: newPeriod.period ?? undefined,
168+
};
176169

177-
// Only push new location if query params has changed because this will cause a heavy re-render
178-
if (qs.stringify(newQuery) !== qs.stringify(location.query)) {
179-
navigate({
180-
pathname: location.pathname,
181-
query: newQuery,
182-
});
183-
}
184-
} else {
185-
updateDateTime(
186-
{
187-
period: newPeriod.period,
188-
start: startFormatted,
189-
end: endFormatted,
190-
},
191-
router,
192-
{save: saveOnZoom}
193-
);
170+
// Only push new location if query params has changed because this will cause a heavy re-render
171+
if (qs.stringify(newQuery) !== qs.stringify(location.query)) {
172+
navigate({
173+
pathname: location.pathname,
174+
query: newQuery,
175+
});
194176
}
195-
};
177+
} else {
178+
updateDateTime(
179+
{
180+
period: newPeriod.period,
181+
start: startFormatted,
182+
end: endFormatted,
183+
},
184+
router,
185+
{save: saveOnZoom}
186+
);
187+
}
196188
},
197189
[onZoom, navigate, location, router, saveOnZoom, usePageDate]
198190
);
@@ -231,11 +223,6 @@ export function useChartZoom({
231223
* before we update URL state and re-render
232224
*/
233225
const handleChartFinished = useCallback<EChartFinishedHandler>((_props, chart) => {
234-
if (typeof zooming.current === 'function') {
235-
zooming.current();
236-
zooming.current = null;
237-
}
238-
239226
// This attempts to activate the area zoom toolbox feature
240227
const zoom = (chart as any)._componentsViews?.find((c: any) => c._features?.dataZoom);
241228
if (zoom && !zoom._features.dataZoom._isZoomActive) {

0 commit comments

Comments
 (0)