diff --git a/.changeset/heatmap-select-persist.md b/.changeset/heatmap-select-persist.md new file mode 100644 index 0000000000..bf29e58b2c --- /dev/null +++ b/.changeset/heatmap-select-persist.md @@ -0,0 +1,5 @@ +--- +"@hyperdx/app": patch +--- + +fix: Persist heatmap drag-select rectangle on Event Deltas and Search heatmaps so the dashed selection stays visible after mouseup diff --git a/packages/app/src/components/DBHeatmapChart.test.tsx b/packages/app/src/components/DBHeatmapChart.test.tsx new file mode 100644 index 0000000000..4bf406c9cd --- /dev/null +++ b/packages/app/src/components/DBHeatmapChart.test.tsx @@ -0,0 +1,148 @@ +import type uPlot from 'uplot'; + +import { applySelectionToChart, SelectionBounds } from './DBHeatmapChart'; + +type StubU = { + scales: { + x?: { min?: number; max?: number }; + y?: { min?: number; max?: number }; + }; + valToPos: jest.Mock; + setSelect: jest.Mock< + void, + [{ left: number; top: number; width: number; height: number }, boolean] + >; +}; + +function makeStubU(opts?: { + yScale?: { min?: number; max?: number }; + xScale?: { min?: number; max?: number }; + valToPos?: (val: number, axis: 'x' | 'y') => number; +}): StubU { + const yScale = opts?.yScale === undefined ? { min: 0, max: 10 } : opts.yScale; + const xScale = + opts?.xScale === undefined ? { min: 0, max: 1_000_000 } : opts.xScale; + // Default: identity passthrough so callers can verify exact arguments. + const valToPos = jest.fn( + opts?.valToPos ?? ((val: number, _axis: 'x' | 'y') => val), + ); + const setSelect = jest.fn(); + return { + scales: { x: xScale, y: yScale }, + valToPos, + setSelect, + }; +} + +describe('applySelectionToChart', () => { + it('clears the selection when bounds is null', () => { + const u = makeStubU(); + applySelectionToChart(u as unknown as uPlot, null, 'linear'); + expect(u.setSelect).toHaveBeenCalledTimes(1); + expect(u.setSelect).toHaveBeenCalledWith( + { left: 0, top: 0, width: 0, height: 0 }, + false, + ); + expect(u.valToPos).not.toHaveBeenCalled(); + }); + + it('clears the selection when bounds is undefined', () => { + const u = makeStubU(); + applySelectionToChart(u as unknown as uPlot, undefined, 'linear'); + expect(u.setSelect).toHaveBeenCalledWith( + { left: 0, top: 0, width: 0, height: 0 }, + false, + ); + }); + + it('returns early without calling setSelect when y scale is unpopulated', () => { + const u = makeStubU({ yScale: { min: undefined, max: undefined } }); + const bounds: SelectionBounds = { xMin: 0, xMax: 1, yMin: 1, yMax: 5 }; + applySelectionToChart(u as unknown as uPlot, bounds, 'linear'); + expect(u.setSelect).not.toHaveBeenCalled(); + }); + + it('converts xMin/xMax from seconds to ms before calling valToPos', () => { + const u = makeStubU(); + const bounds: SelectionBounds = { xMin: 5, xMax: 10, yMin: 1, yMax: 9 }; + applySelectionToChart(u as unknown as uPlot, bounds, 'linear'); + expect(u.valToPos).toHaveBeenCalledWith(5_000, 'x'); + expect(u.valToPos).toHaveBeenCalledWith(10_000, 'x'); + }); + + it('passes raw y values through under linear scale', () => { + const u = makeStubU({ yScale: { min: 0, max: 100 } }); + const bounds: SelectionBounds = { xMin: 0, xMax: 1, yMin: 5, yMax: 50 }; + applySelectionToChart(u as unknown as uPlot, bounds, 'linear'); + // y values must NOT be log-transformed under linear scale. + expect(u.valToPos).toHaveBeenCalledWith(50, 'y'); + expect(u.valToPos).toHaveBeenCalledWith(5, 'y'); + }); + + it('applies Math.log to y values under log scale', () => { + const u = makeStubU({ yScale: { min: 0, max: 100 } }); + const bounds: SelectionBounds = { xMin: 0, xMax: 1, yMin: 1, yMax: Math.E }; + applySelectionToChart(u as unknown as uPlot, bounds, 'log'); + // Math.log(1) === 0, Math.log(Math.E) === 1 + expect(u.valToPos).toHaveBeenCalledWith(0, 'y'); + expect(u.valToPos).toHaveBeenCalledWith(1, 'y'); + }); + + it('clamps yMin === 0 to the chart-floor (yScaleMin) under linear scale', () => { + const u = makeStubU({ yScale: { min: 2, max: 100 } }); + const bounds: SelectionBounds = { xMin: 0, xMax: 1, yMin: 0, yMax: 50 }; + applySelectionToChart(u as unknown as uPlot, bounds, 'linear'); + // yMin=0 should clamp UP to yScale.min (2), not pass through. + expect(u.valToPos).toHaveBeenCalledWith(2, 'y'); + expect(u.valToPos).not.toHaveBeenCalledWith(0, 'y'); + }); + + it('clamps yMin === 0 to the chart-floor (yScaleMin) under log scale', () => { + const u = makeStubU({ yScale: { min: -1, max: 5 } }); + const bounds: SelectionBounds = { xMin: 0, xMax: 1, yMin: 0, yMax: Math.E }; + applySelectionToChart(u as unknown as uPlot, bounds, 'log'); + // yMin=0 (or any non-positive) bypasses Math.log and clamps to yScale.min. + expect(u.valToPos).toHaveBeenCalledWith(-1, 'y'); + }); + + it('clamps yMax above the chart ceiling to yScaleMax under linear', () => { + const u = makeStubU({ yScale: { min: 0, max: 10 } }); + const bounds: SelectionBounds = { xMin: 0, xMax: 1, yMin: 1, yMax: 999 }; + applySelectionToChart(u as unknown as uPlot, bounds, 'linear'); + expect(u.valToPos).toHaveBeenCalledWith(10, 'y'); + }); + + it('handles reversed xMin > xMax via Math.min/max for left/right', () => { + const u = makeStubU(); + const bounds: SelectionBounds = { xMin: 10, xMax: 5, yMin: 1, yMax: 9 }; + applySelectionToChart(u as unknown as uPlot, bounds, 'linear'); + // valToPos identity: xMin*1000=10000, xMax*1000=5000 + // left should be Math.min(10000, 5000) = 5000, width = 10000-5000 = 5000 + const call = u.setSelect.mock.calls[0][0]; + expect(call.left).toBe(5_000); + expect(call.width).toBe(5_000); + }); + + it('produces non-negative height when yLowPx < yHighPx', () => { + // valToPos returns inverse: smaller y-data = larger pixel-y (uPlot semantics). + const u = makeStubU({ + yScale: { min: 0, max: 10 }, + // Force yLowPx < yHighPx by inverting the mapping + valToPos: (val, axis) => (axis === 'y' ? 100 - val : val), + }); + const bounds: SelectionBounds = { xMin: 0, xMax: 1, yMin: 2, yMax: 8 }; + applySelectionToChart(u as unknown as uPlot, bounds, 'linear'); + const call = u.setSelect.mock.calls[0][0]; + // yHighPx = 100-8 = 92, yLowPx = 100-2 = 98, height = max(0, 98-92) = 6 + expect(call.top).toBe(92); + expect(call.height).toBe(6); + }); + + it('passes fireHook=false to setSelect to avoid re-entering the setSelect hook', () => { + const u = makeStubU(); + const bounds: SelectionBounds = { xMin: 0, xMax: 1, yMin: 1, yMax: 9 }; + applySelectionToChart(u as unknown as uPlot, bounds, 'linear'); + const fireHook = u.setSelect.mock.calls[0][1]; + expect(fireHook).toBe(false); + }); +}); diff --git a/packages/app/src/components/DBHeatmapChart.tsx b/packages/app/src/components/DBHeatmapChart.tsx index e69e5ed673..4deac54ea1 100644 --- a/packages/app/src/components/DBHeatmapChart.tsx +++ b/packages/app/src/components/DBHeatmapChart.tsx @@ -35,6 +35,85 @@ import { SQLPreview } from './ChartSQLPreview'; type Mode2DataArray = [number[], number[], number[]]; +/** + * Drag-select bounds in data space: x in seconds (URL convention), y in + * the y-axis's natural unit (NOT log-space; callers pass the actual value + * users would expect to see, e.g. ms latency). yMin may be 0 when the + * selection touched the bottom bucket; the renderer clamps to the chart's + * visible y-axis floor. + */ +export type SelectionBounds = { + xMin: number; + xMax: number; + yMin: number; + yMax: number; +}; + +/** + * Reapply a persisted selection rectangle to a uPlot instance. Called on + * chart create and whenever the bounds prop changes; uPlot's `u.select` + * is owned by the chart, so it gets wiped on any chart recreation. We use + * the URL-backed bounds as the source of truth and mirror them onto the + * chart imperatively. fireHook=false avoids re-entering the setSelect hook. + */ +export function applySelectionToChart( + u: uPlot, + bounds: SelectionBounds | null | undefined, + scaleType: HeatmapScaleType, +) { + // The clear path runs BEFORE the scale-not-populated guard below so a + // null bounds always clears the rectangle, even on first paint when + // u.scales.y is not yet populated. Keep this ordering when refactoring; + // swapping it would suppress clears while scales are loading. + if (bounds == null) { + u.setSelect({ left: 0, top: 0, width: 0, height: 0 }, false); + return; + } + + const { xMin, xMax, yMin, yMax } = bounds; + + // x is in seconds in the URL; uPlot's x-axis is configured ms:1 so + // values are stored as ms. + const xMinPx = u.valToPos(xMin * 1000, 'x'); + const xMaxPx = u.valToPos(xMax * 1000, 'x'); + + const yScaleMin = u.scales.y?.min; + const yScaleMax = u.scales.y?.max; + if (yScaleMin == null || yScaleMax == null) { + return; + } + + // For log scale, y-axis values are stored in log-space. Convert and + // clamp to the visible axis: yMin may be 0 (bottom-bucket adjustment in + // HeatmapContainer's onFilter wrapper); yMax may exceed the axis. + let yLowPlot: number; + let yHighPlot: number; + if (scaleType === 'log') { + yHighPlot = yMax > 0 ? Math.min(Math.log(yMax), yScaleMax) : yScaleMax; + yLowPlot = yMin > 0 ? Math.max(Math.log(yMin), yScaleMin) : yScaleMin; + } else { + yHighPlot = Math.min(yMax, yScaleMax); + yLowPlot = Math.max(yMin, yScaleMin); + } + + // uPlot's y-axis: high data values map to small pixel y (top of chart). + const yHighPx = u.valToPos(yHighPlot, 'y'); + const yLowPx = u.valToPos(yLowPlot, 'y'); + + const left = Math.min(xMinPx, xMaxPx); + const right = Math.max(xMinPx, xMaxPx); + + u.setSelect( + { + left, + top: yHighPx, + width: right - left, + height: Math.max(0, yLowPx - yHighPx), + }, + false, + ); +} + // From: https://github.com/leeoniya/uPlot/blob/a4edb297a9b80baf781f4d05a40fb52fae737bff/demos/latency-heatmap.html#L436 function heatmapPaths(opts: { disp: { fill: { lookup: string[]; values: any } }; @@ -534,6 +613,7 @@ function HeatmapContainer({ enabled = true, onFilter, onClearFilter, + selectionBounds, title, toolbarPrefix, toolbarSuffix, @@ -544,6 +624,14 @@ function HeatmapContainer({ enabled?: boolean; onFilter?: (xMin: number, xMax: number, yMin: number, yMax: number) => void; onClearFilter?: () => void; + /** + * The currently-applied drag-select bounds. When provided, the heatmap + * draws the dashed selection rectangle and reapplies it after any uPlot + * recreation (theme switch, prop change, resize) so the user always sees + * which slice they filtered. Caller owns the URL/query-state plumbing; + * this is purely a visual mirror of that state. + */ + selectionBounds?: SelectionBounds | null; title?: React.ReactNode; toolbarPrefix?: React.ReactNode[]; toolbarSuffix?: React.ReactNode[]; @@ -600,39 +688,54 @@ function HeatmapContainer({ enabled: !!minMaxData && bucketConfig != null && max > effectiveMin, }); - const generatedTsBuckets = timeBucketByGranularity( - dateRange[0], - dateRange[1], - granularity, + // Memoize so timeBucketByGranularity's fresh Date[] doesn't defeat + // the heatmapData memoization downstream. dateRange itself may be a + // fresh array each render, so depend on primitive ms + granularity. + const fromMs = dateRange[0]?.getTime() ?? 0; + const toMs = dateRange[1]?.getTime() ?? 0; + const generatedTsBuckets = useMemo( + () => timeBucketByGranularity(dateRange[0], dateRange[1], granularity), + // eslint-disable-next-line react-hooks/exhaustive-deps + [fromMs, toMs, granularity], ); - const timestampColumn = inferTimestampColumn(data?.meta ?? []); - - // Compute the y-axis value for a given bucket index. - // For log scale we store values in log space so that bins are uniformly - // spaced on the linear uPlot y-axis. The heatmapPaths renderer assumes - // uniform increments (yBinIncr = ys[1] - ys[0]) to compute tile height; - // with actual log-spaced values the first increment is tiny relative to the - // full range and tiles render at ~0px height (invisible). - const bucketToYValue = (j: number) => { - if (scaleType === 'log' && effectiveMin > 0 && max > effectiveMin) { - // Return the natural-log of the actual bucket boundary so that the - // y-values are uniformly spaced. Tick labels are exponentiated back - // via the tickFormatter below. - const actualValue = - effectiveMin * Math.pow(max / effectiveMin, j / nBuckets); - return Math.log(actualValue); + // Stable [time, bucket, count] arrays let uplot-react skip its setData + // path when only URL-state (xMin/xMax/yMin/yMax) changed. Pairs with the + // selectionBounds prop on Heatmap below: the prop reapplies u.select on + // any chart recreation, this memo prevents the recreation in the + // common case. + const heatmapData = useMemo(() => { + const time: number[] = []; + const bucket: number[] = []; + const count: number[] = []; + + // timestampColumn is derived from data.meta, so data covers the dep. + const timestampColumn = inferTimestampColumn(data?.meta ?? []); + + if (data == null || timestampColumn == null) { + return [time, bucket, count]; } - // Linear: min + j * step - return effectiveMin + j * ((max - effectiveMin) / nBuckets); - }; - const time: number[] = []; // x values - const bucket: number[] = []; // y value series 1 - const count: number[] = []; // y value series 2 - if (data != null && timestampColumn != null) { - let dataIndex = 0; + // Compute the y-axis value for a given bucket index. + // For log scale we store values in log space so that bins are uniformly + // spaced on the linear uPlot y-axis. The heatmapPaths renderer assumes + // uniform increments (yBinIncr = ys[1] - ys[0]) to compute tile height; + // with actual log-spaced values the first increment is tiny relative to + // the full range and tiles render at ~0px height (invisible). + const bucketToYValue = (j: number) => { + if (scaleType === 'log' && effectiveMin > 0 && max > effectiveMin) { + // Return the natural-log of the actual bucket boundary so that the + // y-values are uniformly spaced. Tick labels are exponentiated back + // via the tickFormatter below. + const actualValue = + effectiveMin * Math.pow(max / effectiveMin, j / nBuckets); + return Math.log(actualValue); + } + // Linear: min + j * step + return effectiveMin + j * ((max - effectiveMin) / nBuckets); + }; + let dataIndex = 0; for (let i = 0; i < generatedTsBuckets.length; i++) { const generatedTs = generatedTsBuckets[i].getTime(); @@ -657,7 +760,11 @@ function HeatmapContainer({ } } } - } + + return [time, bucket, count]; + }, [data, generatedTsBuckets, scaleType, effectiveMin, max, nBuckets]); + + const time = heatmapData[0]; const toolbarItemsMemo = useMemo(() => { const allToolbarItems: React.ReactNode[] = []; @@ -743,7 +850,7 @@ function HeatmapContainer({ ) : ( )} @@ -875,6 +983,7 @@ function Heatmap({ onClearFilter, scaleType = 'linear', palette, + selectionBounds, }: { data: Mode2DataArray; numberFormat?: NumberFormat; @@ -882,6 +991,7 @@ function Heatmap({ onClearFilter?: () => void; scaleType?: HeatmapScaleType; palette: string[]; + selectionBounds?: SelectionBounds | null; }) { const [highlightedPoint, setHighlightedPoint] = useState< | { @@ -919,13 +1029,59 @@ function Heatmap({ // persisted u.select rectangle (which is owned by uPlot, not React). const uplotRef = useRef(null); + // Hold selectionBounds and scaleType in refs so the uPlot `ready` hook + // (captured inside the options useMemo via closure) always sees the + // latest values without needing them in the memo's dep array. Mutating + // a ref doesn't trigger re-renders, so the options identity stays + // stable across selection/scale changes. + const selectionBoundsRef = useRef(selectionBounds); + const scaleTypeRef = useRef(scaleType); + // Runs every commit; mirrors latest props into refs (no deps array on + // purpose). + useEffect(() => { + selectionBoundsRef.current = selectionBounds; + scaleTypeRef.current = scaleType; + }); + + const { ref, width, height } = useElementSize(); + + // Reapply the URL-backed selection whenever the bounds prop changes + // (e.g. a fresh drag-select arrives via the round-trip through the + // parent's URL state, or the parent clears the filter), or when the + // container resizes (uPlot calls setSize, not a full recreate, so the + // pixel-space u.select would otherwise be stale). This complements the + // uPlot `ready` hook below: that handles initial chart creation (and + // any recreation), this handles bounds and size changes against an + // existing chart. + useEffect(() => { + if (uplotRef.current) { + applySelectionToChart(uplotRef.current, selectionBounds, scaleType); + } + }, [selectionBounds, scaleType, width, height]); + // Timestamp of the most recent drag-end. Guards the container's onClick // handler from clearing the selection when the synthetic click event // that fires on mouseup-after-drag arrives. const justDraggedAtRef = useRef(0); - const { ref, width, height } = useElementSize(); - + // Stabilize on numberFormat content, not reference. Callers (e.g. + // DBSearchHeatmapChart) build a fresh `numberFormat` object on every + // render; depending on its identity would rebuild tickFormatter, then + // the options memo, then uplot-react would see new top-level keys via + // optionsUpdateState and treat the change as 'create', destroying the + // chart and wiping u.select. Hashing the contents lets the memo skip + // when the actual format is unchanged. (HDX-4147) + // + // Relies on NumberFormat being JSON-serializable: today it is plain + // config (string + number fields), so JSON.stringify is a faithful + // fingerprint. If the type ever grows a function-valued field + // (e.g. a custom `formatter` callback), switch to a shallow-equal + // helper keyed on the known fields, because functions stringify to + // undefined and would silently skip rebuilds. + const numberFormatKey = useMemo( + () => (numberFormat ? JSON.stringify(numberFormat) : ''), + [numberFormat], + ); const tickFormatter = useCallback( (value: number) => { // y-values are stored in log space for log scale; exponentiate back @@ -951,7 +1107,8 @@ function Heatmap({ compactDisplay: 'short', }).format(actualValue); }, - [numberFormat, scaleType], + // eslint-disable-next-line react-hooks/exhaustive-deps + [numberFormatKey, scaleType], ); const options: uPlot.Options = useMemo(() => { @@ -1048,7 +1205,7 @@ function Heatmap({ }, plugins: [ // legendAsTooltipPlugin() - // eslint-disable-next-line react-hooks/refs -- mouseInsideRef is read at event time, not during render + highlightDataPlugin({ proximity: 20, onPointHighlight: ({ @@ -1080,6 +1237,17 @@ function Heatmap({ }), { hooks: { + // Fires once after uPlot finishes initial layout/draw. At + // onCreate time scales aren't reliably populated for mode-2 + // facet data, so reapplying the URL-backed selection from + // here ensures valToPos has the bounds it needs. (HDX-4147) + ready: u => { + applySelectionToChart( + u, + selectionBoundsRef.current, + scaleTypeRef.current, + ); + }, setSelect: u => { // Ignore zero-size selections (e.g. single-click) if (u.select.width <= 0 || u.select.height <= 0) { diff --git a/packages/app/src/components/Search/DBSearchHeatmapChart.tsx b/packages/app/src/components/Search/DBSearchHeatmapChart.tsx index e618526abb..e6d2ea5813 100644 --- a/packages/app/src/components/Search/DBSearchHeatmapChart.tsx +++ b/packages/app/src/components/Search/DBSearchHeatmapChart.tsx @@ -26,6 +26,7 @@ import DBHeatmapChart, { darkPalette, type HeatmapScaleType, lightPalette, + type SelectionBounds, toHeatmapChartConfig, } from '../DBHeatmapChart'; @@ -93,6 +94,28 @@ export function DBSearchHeatmapChart({ prevDateRangeRef.current = key; }, [fromMs, toMs, setFields]); + // Mirror the URL's selection coordinates onto the heatmap so the dashed + // rectangle stays in sync after any uPlot recreation. Memoize on the + // primitive coords so the prop's identity is stable when the URL hasn't + // changed; otherwise the downstream useEffect would re-fire on every + // render. (HDX-4147) + const selectionBounds: SelectionBounds | null = useMemo(() => { + if ( + fields.xMin == null || + fields.xMax == null || + fields.yMin == null || + fields.yMax == null + ) { + return null; + } + return { + xMin: fields.xMin, + xMax: fields.xMax, + yMin: fields.yMin, + yMax: fields.yMax, + }; + }, [fields.xMin, fields.xMax, fields.yMin, fields.yMax]); + return ( { setFields({ xMin, xMax, yMin, yMax }); }}