Skip to content

fix(dashboards): Stop chart drag-to-zoom selecting the wrong time range#119013

Merged
gggritso merged 1 commit into
masterfrom
georgegritsouk/dain-1753-chart-band-selector-selects-the-wrong-time-range
Jul 6, 2026
Merged

fix(dashboards): Stop chart drag-to-zoom selecting the wrong time range#119013
gggritso merged 1 commit into
masterfrom
georgegritsouk/dain-1753-chart-band-selector-selects-the-wrong-time-range

Conversation

@gggritso

@gggritso gggritso commented Jul 3, 2026

Copy link
Copy Markdown
Member

Dragging to zoom on a dashboard time-series widget could snap the global time filter to the wrong (collapsed) range — e.g. a ~2h selection landing on ~1 minute.

Two related bugs in Dashboards time series chart navigation

  1. Dragging to zoom creates weird opaque rectangles
  2. Drag-to-zoom seems to zoom to incorrect range if many zooms are done without reloading

It looks like the root cause is apache/echarts#21661, in which re-mounting a chart reinitiates the zoom toolbox, which weirdly re-fires old zoom events? Truly bizarre. To guard against this, allowing notMerge=false makes sure that ECharts doesn't do a full re-init of the toolbox, it just updates it if it changed (it didn't), which prevents this weird re-initialization loop.

I worked through this with Claude for a while, and I'm convinced that the solution works. Since notMerge is able to cause broken UI, I removed that prop completely.

This is the same root cause as LOGS-901.

Fixes DAIN-1753

Investigation notes

Verified live with temporary console instrumentation on the repro dashboard (single time-series widget)

Repro: drag-zoom (~2h) → dashboard Cancel → drag-zoom again → second selection collapses to a wrong, tiny range and the selection rectangle looks more opaque.

Two theories were disproven by the logs, not assumed:

  • Orphaned ECharts "cover" SVGs stacking (the sibling LOGS-901 hypothesis): the brush controller's tracked covers always equalled its mounted covers (0/0 or 1/1) — no orphans.
  • Timezone offset: would corrupt the first drag and shift by whole hours, not collapse the second drag to ~1 min.

What the logs actually showed: a single drag fired handleDataZoom 3–6 times, each with a different from: toolbox-feature_NNN uid climbing by +26 and a drifting range (10 → 126 → 126 → 126 → 12 min); the last write won and it was the stale collapsed one.

  • Not leaked listeners: echarts-for-react unbindEvents()/bindEvents() on every update, and the events had different from/ranges (a single event to N stale listeners would be identical N times). These are N genuinely distinct dispatches.
  • The +26 uid jump = the toolbox feature being rebuilt from scratch each time = full notMerge re-init.
  • Shape is a feedback loop: drag → setPeriod → URL/page-filter change → widget re-render → setOption(notMerge:true) full re-init → toolbox dataZoom re-emits a dataZoomsetPeriod → … until it settles on a drifted/collapsed range.

Fix confirmation: with notMerge={false}, each drag fires handleDataZoom exactly once, the from uid stays stable, and both drags land the correct ~2h range (117 / 119 min).

Upstream: apache/echarts#21661 (open, "pending") is the same regression; #21644/#21669 are related. No merged upstream fix exists, so this is a client-side mitigation.

Blast radius: dashboards, insights, and all explore time-series charts now merge (they shared the latent bug; metrics/spans were already on notMerge={false}).

Dragging to zoom on a dashboard time-series widget could update the global
time filter to the wrong (collapsed) range. The widget inherited BaseChart's
notMerge=true default, so every data refresh did a full ECharts re-init that
re-created the toolbox dataZoom "select" component. On ECharts 6.1 that rebuild
re-emits a stale `dataZoom` event, so a single drag cascaded into repeated
refetches that settled on a wrong, collapsed range (see apache/echarts#21661).

Force merge mode and remove the notMerge prop entirely so it can't be re-enabled:
the existing replaceMerge already fully replaces the volatile series/axes, which
was the only reason notMerge was ever reached for. Update the explore callers
(metrics, spans, logs) that passed notMerge accordingly.

Fixes DAIN-1753

Co-Authored-By: Claude <noreply@anthropic.com>
@linear-code

linear-code Bot commented Jul 3, 2026

Copy link
Copy Markdown

DAIN-1753

@github-actions github-actions Bot added the Scope: Frontend Automatically applied to PRs that change frontend components label Jul 3, 2026
@github-actions

github-actions Bot commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

Story previews

Preview the stories changed in this PR on the Vercel deployment:

Preview deployment: https://sentry-le5uzmcc4.sentry.dev

@gggritso

gggritso commented Jul 3, 2026

Copy link
Copy Markdown
Member Author

@cursor review

@cursor cursor Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 8047a99. Configure here.

@gggritso gggritso requested a review from JoshuaKGoldberg July 3, 2026 19:34
@gggritso gggritso marked this pull request as ready for review July 3, 2026 19:34
@gggritso gggritso requested review from a team as code owners July 3, 2026 19:34

@nsdeschenes nsdeschenes left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

echarts *shakes fist*

@gggritso gggritso merged commit 5c1f318 into master Jul 6, 2026
83 checks passed
@gggritso gggritso deleted the georgegritsouk/dain-1753-chart-band-selector-selects-the-wrong-time-range branch July 6, 2026 15:06
JoshuaKGoldberg added a commit that referenced this pull request Jul 9, 2026
Dragging to zoom on issue details could rebuild the hidden ECharts
dataZoom toolbox and replay stale zoom events, similar to the dashboards
fix in #119013.

This moves the ECharts merge settings into shared chart zoom config and
uses it from both ChartZoom implementations. It also keeps the inside
dataZoom model for synced release-bubble charts while disabling the
hidden toolbox when zoom navigation is off.

Adds a small hook test for rerenders and disabled zoom behavior.

---------

Co-authored-by: Codex <noreply@openai.com>
Co-authored-by: Josh Goldberg ✨ <github@joshuakgoldberg.com>
Co-authored-by: Claude <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Scope: Frontend Automatically applied to PRs that change frontend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants