fix: zoom drag lifecycle + all-zero horizon rendering#42
Merged
Conversation
When every value in a lane is 0, extent() returns [0, 0], so max = 0 and the scale domain becomes [0, 0]. d3's linear scale with a zero- width domain returns the range midpoint for every input (the normalize(x) step divides by zero and d3 clamps to 0.5). That midpoint maps to a mid-intensity color and the loop paints a full-height band — visually indistinguishable from "weak negative data" when the intent is "no data here". Early-return after clearRect when max === 0. The lane stays transparent, showing the panel background through.
Two related bugs in the drag-to-zoom interaction: 1. mouseup was attached only to the horizon divs. Releasing over the axis, the rule line, a gap between lanes, or outside the panel never reached the handler — zoom.stop() never ran, _corner1 stayed set, and the drag continued indefinitely. Also, a data-driven re-render mid-drag (Grafana refreshes every 5-30s) destroys the horizon divs and their handlers with them. Fixed by attaching a one-shot mouseup to window on mousedown — fires no matter where the release happens, survives DOM churn. 2. The selection-rectangle SVG has position:absolute, which anchors to the nearest positioned ancestor. The scrollable container (canvasDiv) was position:static, so the overlay anchored to its grandparent instead. When scrolled, pointer() returns coords relative to canvasDiv's content origin (includes scroll offset), but the overlay positioned relative to the unscrolled grandparent — it appeared off-screen. The overlay also needs pointer-events: none so mouse events pass through to the horizons underneath. (The position:relative fix is in the consuming panel's CSS; the pointer-events fix is here.)
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #42 +/- ##
=======================================
Coverage 99.54% 99.54%
=======================================
Files 33 33
Lines 871 883 +12
Branches 193 189 -4
=======================================
+ Hits 867 879 +12
Misses 4 4
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
The window-level mouseup handler closes over the container captured at render time. If a re-render happens mid-drag (Grafana auto-refresh, or the panel's width useEffect dep firing on resize), the container gets detached via innerHTML='' but the window listener survives. On mouseup: - pointer() against the detached node computes via a zero-rect getBoundingClientRect, returning raw viewport coords - _corner1 still holds the pre-render container-relative position - stop() mixes the two coordinate systems → garbage range - onChangeTimeRange / datalink navigation fires with nonsense timestamps Two-part fix: 1. context.zoom(callback) re-registration now clears _corner1/_selection, same as disable() does. Any in-progress drag is abandoned on re-render. 2. The mouseup handler checks container.isConnected and bails to reset() if detached — belt-and-suspenders in case corner1 somehow survives. Found by bughunt-lite workflow (5-0 adversarial vote).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two fixes found while integrating cubism into a Grafana panel plugin.
Zoom drag never ends when released outside the horizon
The mouseup handler was attached to the horizon divs. Releasing the drag over the zoom overlay (a sibling element), the axis, a gap between lanes, or anywhere outside the panel never fired the handler — the selection rectangle followed the mouse forever until you happened to mouseup precisely on a horizon.
Fixed by attaching a one-shot mouseup to
windowon mousedown. Also:pointer-events: noneon the zoom overlay SVG so it doesn't swallow eventstry/finallyaround the callback invocation so a throwing callback can't leave_corner1setzoom.disable()method for consumers that reuse the context across renders and need to turn zoom off cleanlyAll-zero metrics paint solid instead of transparent
When every value in a metric is zero,
max === 0gives d3's linear scale a[0, 0]domain.scale(0)then returns the range midpoint, painting the entire lane a mid-intensity band — making zero look like constant nonzero data. Now skips rendering afterclearRectso zero lanes stay transparent.Tests
+5 tests covering both regressions (zoom state clearing, disable, try/finally, window mouseup integration, listener self-removal). 86 → 91 passing.