From efc2b34228181a89f7103d39fbbc334cd60064e3 Mon Sep 17 00:00:00 2001 From: Brandon Pereira Date: Thu, 9 Jul 2026 12:02:55 -0600 Subject: [PATCH 01/10] chore(charts): upgrade Recharts from 2.13 to 3.x Recharts 3 reworked its internal state management, which removes the CategoricalChartState fields (chartX/chartY/activePayload) from chart event handlers and the imperative chart.setState escape hatch. Update the chart components to the new API while preserving existing behavior: - HDXMultiSeriesTimeChart: derive click/drag position from activeCoordinate and rebuild the drill-down per-series payload from graphResults, keeping zoom-brush selection and the click drill-down popover working. - DBHistogramChart: replace the chart.setState tooltip-pin hack with the controlled active/defaultIndex Tooltip props. - PropertyComparisonChart: update custom tooltip/shape typings to the renamed TooltipContentProps and select the clicked bar via activeLabel. - app.scss: hide the browser focus ring that Recharts 3's default accessibilityLayer paints when a chart is clicked, keeping charts keyboard-focusable. --- .changeset/recharts-3-upgrade.md | 10 + packages/app/package.json | 3 +- packages/app/src/HDXMultiSeriesTimeChart.tsx | 83 ++++++-- .../app/src/components/DBHistogramChart.tsx | 77 ++----- .../components/PropertyComparisonChart.tsx | 34 +-- packages/app/styles/app.scss | 13 ++ yarn.lock | 200 +++++++++++++----- 7 files changed, 270 insertions(+), 150 deletions(-) create mode 100644 .changeset/recharts-3-upgrade.md diff --git a/.changeset/recharts-3-upgrade.md b/.changeset/recharts-3-upgrade.md new file mode 100644 index 0000000000..1bc825a12c --- /dev/null +++ b/.changeset/recharts-3-upgrade.md @@ -0,0 +1,10 @@ +--- +"@hyperdx/app": minor +--- + +chore(charts): upgrade Recharts from 2.13 to 3.x. Reworks chart event handlers +to the Recharts 3 event API (zoom-brush selection, click drill-down), replaces +the histogram's imperative `chart.setState` tooltip-pin hack with the controlled +`active`/`defaultIndex` Tooltip props, updates custom tooltip/shape typings +(`TooltipContentProps`, `BarProps`), and suppresses the browser focus ring that +Recharts 3's default `accessibilityLayer` shows when a chart is clicked. diff --git a/packages/app/package.json b/packages/app/package.json index b64b682b05..ac1dfba6ec 100644 --- a/packages/app/package.json +++ b/packages/app/package.json @@ -84,10 +84,11 @@ "react-grid-layout": "^1.3.4", "react-hook-form": "^7.43.8", "react-hotkeys-hook": "^4.3.7", + "react-is": "^19.0.0", "react-json-tree": "^0.20.0", "react-markdown": "^10.1.0", "react-resizable": "^3.0.4", - "recharts": "^2.12.7", + "recharts": "^3.2.1", "rrweb": "2.0.0-alpha.8", "sass": "^1.54.8", "sql-formatter": "^15.7.2", diff --git a/packages/app/src/HDXMultiSeriesTimeChart.tsx b/packages/app/src/HDXMultiSeriesTimeChart.tsx index 5e576b7061..9012909b50 100644 --- a/packages/app/src/HDXMultiSeriesTimeChart.tsx +++ b/packages/app/src/HDXMultiSeriesTimeChart.tsx @@ -372,14 +372,18 @@ const LegendRenderer = memo<{ export const HARD_LINES_LIMIT = MAX_TIME_CHART_SERIES; const StackedBarWithOverlap = (props: BarProps) => { - const { x, y, width, height, fill } = props; + const { x, y, width, fill } = props; + // `height` may arrive as a string, so coerce it to a number before the + // arithmetic below. + const height = + typeof props.height === 'number' ? props.height : Number(props.height ?? 0); // Add a tiny bit to the height to create overlap. Otherwise there's a gap return ( 0 ? height + 0.5 : 0} + height={height > 0 ? height + 0.5 : 0} fill={fill} /> ); @@ -839,13 +843,18 @@ export const MemoChart = memo(function MemoChart({ setHighlightEnd(undefined); mouseDownPosRef.current = null; }} - onMouseDown={e => { - if (e != null && e.chartX != null && e.chartY != null) { - setHighlightStart(e.activeLabel); - mouseDownPosRef.current = e.chartX; + onMouseDown={(state, e) => { + // Record the drag start: the active bucket label and the pointer's + // plot-relative X (from the active coordinate, falling back to the + // native event's offsetX) for measuring drag distance on mouse up. + const chartX = + state?.activeCoordinate?.x ?? e?.nativeEvent?.offsetX; + if (state?.activeLabel != null && chartX != null) { + setHighlightStart(String(state.activeLabel)); + mouseDownPosRef.current = chartX; } }} - onMouseMove={e => { + onMouseMove={state => { setIsHovered(true); // Track which series' line is nearest the cursor so the lines can @@ -853,15 +862,16 @@ export const MemoChart = memo(function MemoChart({ // frame; comparing the pointer's chartY picks the nearest line. Skip // while a click-frozen tooltip is shown, matching the tooltip, and // only set state when the key changes to keep re-renders rare. + const chartY = state?.activeCoordinate?.y; const activePointYByKey = activePointYByKeyRef.current; const nextNearest = isClickActive == null && activePointYByKey.size > 1 && - e?.chartY != null + chartY != null ? findNearestSeriesKey( activePointYByKey, Array.from(activePointYByKey.keys()), - e.chartY, + chartY, NEAREST_SERIES_MAX_DISTANCE_PX, ) : undefined; @@ -869,20 +879,26 @@ export const MemoChart = memo(function MemoChart({ prev === nextNearest ? prev : nextNearest, ); - if (highlightStart != null) { - setHighlightEnd(e.activeLabel); + if (highlightStart != null && state?.activeLabel != null) { + setHighlightEnd(String(state.activeLabel)); setIsClickActive(undefined); // Clear out any click state as we're highlighting } }} - onMouseUp={e => { + onMouseUp={(state, e) => { const MIN_DRAG_DISTANCE = 20; // Minimum horizontal drag distance in pixels let dragDistance = 0; - if (mouseDownPosRef.current != null && e?.chartX != null) { - dragDistance = Math.abs(e.chartX - mouseDownPosRef.current); + const chartX = + state?.activeCoordinate?.x ?? e?.nativeEvent?.offsetX; + if (mouseDownPosRef.current != null && chartX != null) { + dragDistance = Math.abs(chartX - mouseDownPosRef.current); } - if (e?.activeLabel != null && highlightStart === e.activeLabel) { + const activeLabel = + state?.activeLabel != null + ? String(state.activeLabel) + : undefined; + if (activeLabel != null && highlightStart === activeLabel) { // If it's just a click, don't zoom setHighlightStart(undefined); setHighlightEnd(undefined); @@ -931,21 +947,42 @@ export const MemoChart = memo(function MemoChart({ } }} onClick={(state, e) => { + // Freeze a tooltip at the clicked point: take the click position + // from the active coordinate and build the per-series payload from + // the active bucket in graphResults (the popover only needs + // value/name/color/dataKey per series). + const chartX = state?.activeCoordinate?.x; + const chartY = state?.activeCoordinate?.y; if ( state != null && - state.chartX != null && - state.chartY != null && + chartX != null && + chartY != null && state.activeLabel != null && // If we didn't drag and highlight yet highlightStart == null ) { + const activeLabel = String(state.activeLabel); + const activeRow = graphResults.find( + row => String(row[timestampKey ?? 'ts_bucket']) === activeLabel, + ); + const activePayload = + activeRow != null + ? lineData + .filter(ld => activeRow[ld.dataKey] != null) + .map(ld => ({ + dataKey: ld.dataKey, + name: ld.displayName || ld.dataKey, + value: activeRow[ld.dataKey] as number, + color: ld.color, + })) + : []; setIsClickActive({ - x: state.chartX, - y: state.chartY, - activeLabel: state.activeLabel, - xPerc: state.chartX / sizeRef.current[0], - yPerc: state.chartY / sizeRef.current[1], - activePayload: state.activePayload, + x: chartX, + y: chartY, + activeLabel, + xPerc: chartX / sizeRef.current[0], + yPerc: chartY / sizeRef.current[1], + activePayload, }); // The click-frozen tooltip hides the live tooltip, so drop any // line emphasis to match. diff --git a/packages/app/src/components/DBHistogramChart.tsx b/packages/app/src/components/DBHistogramChart.tsx index c8fc48a19b..6e789a1de2 100644 --- a/packages/app/src/components/DBHistogramChart.tsx +++ b/packages/app/src/components/DBHistogramChart.tsx @@ -1,4 +1,4 @@ -import { useMemo, useRef } from 'react'; +import { useMemo, useState } from 'react'; import Link from 'next/link'; import { omit } from 'lodash'; import { useHotkeys } from 'react-hotkeys-hook'; @@ -10,7 +10,6 @@ import { XAxis, YAxis, } from 'recharts'; -import { CategoricalChartState } from 'recharts/types/chart/types'; import { BuilderChartConfigWithDateRange } from '@hyperdx/common-utils/dist/types'; import { buildMVDateRangeIndicator } from '@/ChartUtils'; @@ -43,38 +42,15 @@ function HistogramChart({ ); }, [graphResults]); - const barChartRef = useRef(null); - const activeBar = useRef(undefined); + // Index of the bar whose tooltip is "pinned" open by a click. When set, the + // tooltip is forced active on that bar via the controlled `active` + + // `defaultIndex` props below; `undefined` lets the tooltip follow hover. + const [pinnedIndex, setPinnedIndex] = useState(undefined); useHotkeys(['esc'], () => { - activeBar.current = undefined; + setPinnedIndex(undefined); }); - // Complete hack - // See: https://github.com/recharts/recharts/issues/1231#issuecomment-1237958802 - const setChartActive = (payload: { - activeCoordinate?: { x: number; y: number }; - activeLabel: any; - activePayload?: any[]; - }) => { - if (barChartRef.current == null) return; - - if (activeBar.current == null) { - // @ts-ignore - return barChartRef.current.setState({ - isTooltipActive: false, - }); - } - - // @ts-ignore - barChartRef.current.setState({ - isTooltipActive: true, - activeCoordinate: payload.activeCoordinate, - activeLabel: payload.activeLabel, - activePayload: payload.activePayload, - }); - }; - return ( { - if (activeBar.current == null) return; - - setChartActive({ - activeCoordinate: activeBar.current.activeCoordinate, - activeLabel: activeBar.current.activeLabel, - activePayload: activeBar.current.activePayload, - }); - }} - onMouseLeave={() => { - activeBar.current = undefined; - }} - onClick={click => { - activeBar.current = click; - - if (click != null) { - setChartActive({ - activeCoordinate: activeBar.current.activeCoordinate, - activeLabel: activeBar.current.activeLabel, - activePayload: activeBar.current.activePayload, - }); - } + onClick={state => { + // Toggle the pinned tooltip on the clicked bar (click the same bar + // again to unpin). + const idx = + typeof state?.activeIndex === 'number' + ? state.activeIndex + : state?.activeIndex != null + ? Number(state.activeIndex) + : undefined; + setPinnedIndex(prev => + idx == null ? undefined : prev === idx ? undefined : idx, + ); }} > } - active + // When a bar is pinned, force the tooltip active on that bar; + // otherwise let Recharts control it on hover. + {...(pinnedIndex != null + ? { active: true, defaultIndex: pinnedIndex } + : {})} /> diff --git a/packages/app/src/components/PropertyComparisonChart.tsx b/packages/app/src/components/PropertyComparisonChart.tsx index f070230815..ebc495b33c 100644 --- a/packages/app/src/components/PropertyComparisonChart.tsx +++ b/packages/app/src/components/PropertyComparisonChart.tsx @@ -1,6 +1,6 @@ import { memo, useState } from 'react'; import { withErrorBoundary } from 'react-error-boundary'; -import type { TooltipProps } from 'recharts'; +import type { TooltipContentProps as RechartsTooltipContentProps } from 'recharts'; import { Bar, BarChart, @@ -43,14 +43,12 @@ export const CHART_GAP = 16; // px; used in grid gap and layout math // Always reserved (even when pagination is hidden via visibility:hidden) so rows count is stable. export const PAGINATION_HEIGHT = 48; -type TooltipPayloadItem = { - dataKey: string; - name: string; - value: number; - payload?: Record; -}; - -type TooltipContentProps = TooltipProps & { +// The content props (payload, coordinate, active, ...) are supplied by the +// chart's Tooltip at render time, so they're optional here — the element is +// written in JSX with only the `title` prop. +type TooltipContentProps = Partial< + RechartsTooltipContentProps +> & { title?: string; }; @@ -70,11 +68,11 @@ const HDXBarChartTooltip = withErrorBoundary( {String(label).length === 0 ? Empty String : String(label)} - {(payload as TooltipPayloadItem[]) - .sort((a, b) => b.value - a.value) + {[...payload] + .sort((a, b) => Number(b.value ?? 0) - Number(a.value ?? 0)) .map(p => ( -
- {p.name}: {p.value.toFixed(2)}% +
+ {p.name}: {Number(p.value ?? 0).toFixed(2)}%
))}
@@ -146,15 +144,19 @@ export function PropertyComparisonChart({ const clipboard = useClipboard({ timeout: 2000 }); const handleChartClick = (data: any) => { - if (!data?.activePayload?.length) { + // Select the clicked bar by its label (the bar's `name`), looking the row + // up in chartData to skip the aggregated "other" bucket. + const activeLabel = data?.activeLabel; + if (activeLabel == null) { setClickedValue(null); return; } - if (data.activePayload[0]?.payload?.isOther) { + const clickedRow = chartData.find(row => row.name === activeLabel); + if (clickedRow == null || clickedRow.isOther) { setClickedValue(null); return; } - const newValue = String(data.activeLabel ?? ''); + const newValue = String(activeLabel ?? ''); // Toggle off if clicking the same bar if (newValue === clickedValue) { setClickedValue(null); diff --git a/packages/app/styles/app.scss b/packages/app/styles/app.scss index 6a1c5f89f3..268beed637 100644 --- a/packages/app/styles/app.scss +++ b/packages/app/styles/app.scss @@ -323,6 +323,19 @@ button:focus-visible { } } +// Charts are keyboard-focusable for accessibility, which paints a browser focus +// ring when the chart is clicked. Hide the visual ring on the chart wrapper, +// surface, and any focusable descendant of the surface while keeping the +// elements focusable for keyboard navigation. +.recharts-wrapper:focus, +.recharts-wrapper:focus-visible, +.recharts-surface:focus, +.recharts-surface:focus-visible, +.recharts-surface *:focus, +.recharts-surface *:focus-visible { + outline: none; +} + // Global cursor pointer for interactive elements button, a, diff --git a/yarn.lock b/yarn.lock index 036cefdf87..8d426788cf 100644 --- a/yarn.lock +++ b/yarn.lock @@ -4619,10 +4619,11 @@ __metadata: react-grid-layout: "npm:^1.3.4" react-hook-form: "npm:^7.43.8" react-hotkeys-hook: "npm:^4.3.7" + react-is: "npm:^19.0.0" react-json-tree: "npm:^0.20.0" react-markdown: "npm:^10.1.0" react-resizable: "npm:^3.0.4" - recharts: "npm:^2.12.7" + recharts: "npm:^3.2.1" rimraf: "npm:^4.4.1" rrweb: "npm:2.0.0-alpha.8" sass: "npm:^1.54.8" @@ -8327,6 +8328,28 @@ __metadata: languageName: node linkType: hard +"@reduxjs/toolkit@npm:^1.9.0 || 2.x.x": + version: 2.12.0 + resolution: "@reduxjs/toolkit@npm:2.12.0" + dependencies: + "@standard-schema/spec": "npm:^1.0.0" + "@standard-schema/utils": "npm:^0.3.0" + immer: "npm:^11.0.0" + redux: "npm:^5.0.1" + redux-thunk: "npm:^3.1.0" + reselect: "npm:^5.1.0" + peerDependencies: + react: ^16.9.0 || ^17.0.0 || ^18 || ^19 + react-redux: ^7.2.1 || ^8.1.3 || ^9.0.0 + peerDependenciesMeta: + react: + optional: true + react-redux: + optional: true + checksum: 10c0/2b85e31294a7139994fd78b08eb3ce706ce3b03b5081c13403b93ba4883a152d637171e4d9d969766238851f8915b1daa9f36b5a0a458aff0ff7600ee38795c9 + languageName: node + linkType: hard + "@rollup/plugin-commonjs@npm:~22.0.2": version: 22.0.2 resolution: "@rollup/plugin-commonjs@npm:22.0.2" @@ -8640,13 +8663,20 @@ __metadata: languageName: node linkType: hard -"@standard-schema/spec@npm:^1.1.0": +"@standard-schema/spec@npm:^1.0.0, @standard-schema/spec@npm:^1.1.0": version: 1.1.0 resolution: "@standard-schema/spec@npm:1.1.0" checksum: 10c0/d90f55acde4b2deb983529c87e8025fa693de1a5e8b49ecc6eb84d1fd96328add0e03d7d551442156c7432fd78165b2c26ff561b970a9a881f046abb78d6a526 languageName: node linkType: hard +"@standard-schema/utils@npm:^0.3.0": + version: 0.3.0 + resolution: "@standard-schema/utils@npm:0.3.0" + checksum: 10c0/6eb74cd13e52d5fc74054df51e37d947ef53f3ab9e02c085665dcca3c38c60ece8d735cebbdf18fbb13c775fbcb9becb3f53109b0e092a63f0f7389ce0993fd0 + languageName: node + linkType: hard + "@stoplight/better-ajv-errors@npm:1.0.3": version: 1.0.3 resolution: "@stoplight/better-ajv-errors@npm:1.0.3" @@ -10413,6 +10443,13 @@ __metadata: languageName: node linkType: hard +"@types/use-sync-external-store@npm:^0.0.6": + version: 0.0.6 + resolution: "@types/use-sync-external-store@npm:0.0.6" + checksum: 10c0/77c045a98f57488201f678b181cccd042279aff3da34540ad242f893acc52b358bd0a8207a321b8ac09adbcef36e3236944390e2df4fcedb556ce7bb2a88f2a8 + languageName: node + linkType: hard + "@types/uuid@npm:^8.3.4": version: 8.3.4 resolution: "@types/uuid@npm:8.3.4" @@ -13155,7 +13192,7 @@ __metadata: languageName: node linkType: hard -"clsx@npm:^2.0.0, clsx@npm:^2.1.1": +"clsx@npm:^2.1.1": version: 2.1.1 resolution: "clsx@npm:2.1.1" checksum: 10c0/c4c8eb865f8c82baab07e71bfa8897c73454881c4f99d6bc81585aecd7c441746c1399d08363dc096c550cceaf97bd4ce1e8854e1771e9998d9f94c4fe075839 @@ -14181,7 +14218,7 @@ __metadata: languageName: node linkType: hard -"decimal.js-light@npm:^2.4.1": +"decimal.js-light@npm:^2.5.1": version: 2.5.1 resolution: "decimal.js-light@npm:2.5.1" checksum: 10c0/4fd33f535aac9e5bd832796831b65d9ec7914ad129c7437b3ab991b0c2eaaa5a57e654e6174c4a17f1b3895ea366f0c1ab4955cdcdf7cfdcf3ad5a58b456c020 @@ -15127,6 +15164,20 @@ __metadata: languageName: node linkType: hard +"es-toolkit@npm:^1.39.3": + version: 1.49.0 + resolution: "es-toolkit@npm:1.49.0" + dependenciesMeta: + "@trivago/prettier-plugin-sort-imports@4.3.0": + unplugged: true + prettier-plugin-sort-re-exports@0.0.1: + unplugged: true + vitepress-plugin-sandpack@1.1.4: + unplugged: true + checksum: 10c0/ab0864bb0c5c494ed09043d53a74b58a0ab9df1b89b9b54bce72a63de2869152d7d65e489dc89ab671f0fea243728922c8a8f8c873f1b2a53350a23360b3e2df + languageName: node + linkType: hard + "esbuild@npm:^0.18.0 || ^0.19.0 || ^0.20.0 || ^0.21.0 || ^0.22.0 || ^0.23.0 || ^0.24.0 || ^0.25.0 || ^0.26.0 || ^0.27.0": version: 0.27.1 resolution: "esbuild@npm:0.27.1" @@ -16020,13 +16071,20 @@ __metadata: languageName: node linkType: hard -"eventemitter3@npm:^4.0.1, eventemitter3@npm:^4.0.7": +"eventemitter3@npm:^4.0.7": version: 4.0.7 resolution: "eventemitter3@npm:4.0.7" checksum: 10c0/5f6d97cbcbac47be798e6355e3a7639a84ee1f7d9b199a07017f1d2f1e2fe236004d14fa5dfaeba661f94ea57805385e326236a6debbc7145c8877fbc0297c6b languageName: node linkType: hard +"eventemitter3@npm:^5.0.1": + version: 5.0.4 + resolution: "eventemitter3@npm:5.0.4" + checksum: 10c0/575b8cac8d709e1473da46f8f15ef311b57ff7609445a7c71af5cd42598583eee6f098fa7a593e30f27e94b8865642baa0689e8fa97c016f742abdb3b1bf6d9a + languageName: node + linkType: hard + "events@npm:^3.2.0, events@npm:^3.3.0": version: 3.3.0 resolution: "events@npm:3.3.0" @@ -16271,13 +16329,6 @@ __metadata: languageName: node linkType: hard -"fast-equals@npm:^5.0.1": - version: 5.0.1 - resolution: "fast-equals@npm:5.0.1" - checksum: 10c0/d7077b8b681036c2840ed9860a3048e44fc268fad2b525b8f25b43458be0c8ad976152eb4b475de9617170423c5b802121ebb61ed6641c3ac035fadaf805c8c0 - languageName: node - linkType: hard - "fast-glob@npm:3.3.1": version: 3.3.1 resolution: "fast-glob@npm:3.3.1" @@ -17976,6 +18027,13 @@ __metadata: languageName: node linkType: hard +"immer@npm:^11.0.0, immer@npm:^11.1.8": + version: 11.1.9 + resolution: "immer@npm:11.1.9" + checksum: 10c0/71635a4de5cc00697687d693aaecab1f94f6fc8dc7f854fd36c3dacdc5fddbceefada83fc29115f48002a040368d5e040900e9652c2cda121bd01271a08efbe9 + languageName: node + linkType: hard + "immer@npm:^9.0.21, immer@npm:^9.0.6": version: 9.0.21 resolution: "immer@npm:9.0.21" @@ -24158,6 +24216,13 @@ __metadata: languageName: node linkType: hard +"react-is@npm:^19.0.0": + version: 19.2.7 + resolution: "react-is@npm:19.2.7" + checksum: 10c0/419fe54d5bd7fdf5414a5bb7bd9a1e0e36f9fae28ffb4cb73290fbe342bde15d8584a90d1db62547f6aa03018dce517b178a041abb522136cd4b4b51b4e94c83 + languageName: node + linkType: hard + "react-json-tree@npm:^0.20.0": version: 0.20.0 resolution: "react-json-tree@npm:0.20.0" @@ -24214,6 +24279,25 @@ __metadata: languageName: node linkType: hard +"react-redux@npm:8.x.x || 9.x.x": + version: 9.3.0 + resolution: "react-redux@npm:9.3.0" + dependencies: + "@types/use-sync-external-store": "npm:^0.0.6" + use-sync-external-store: "npm:^1.4.0" + peerDependencies: + "@types/react": ^18.2.25 || ^19 + react: ^18.0 || ^19 + redux: ^5.0.0 + peerDependenciesMeta: + "@types/react": + optional: true + redux: + optional: true + checksum: 10c0/b9f4efcfbfbc90cac9d1709ab3affb1e18a9dc9bd3cceda43bd2e1d9d2394ee0c29df36ec2202d52b566db774888b594c8c5aa86b64f27ef34fca607c687c9e3 + languageName: node + linkType: hard + "react-refresh@npm:^0.14.0": version: 0.14.2 resolution: "react-refresh@npm:0.14.2" @@ -24268,20 +24352,6 @@ __metadata: languageName: node linkType: hard -"react-smooth@npm:^4.0.0": - version: 4.0.1 - resolution: "react-smooth@npm:4.0.1" - dependencies: - fast-equals: "npm:^5.0.1" - prop-types: "npm:^15.8.1" - react-transition-group: "npm:^4.4.5" - peerDependencies: - react: ^16.8.0 || ^17.0.0 || ^18.0.0 - react-dom: ^16.8.0 || ^17.0.0 || ^18.0.0 - checksum: 10c0/5c19a2c147798c3de1329d442b1a371139c01113cc108c38c201b63502c329f943ede505c44089d26a6563eaa72a67b845d538d956f34a389b37fd3961308834 - languageName: node - linkType: hard - "react-style-singleton@npm:^2.2.2, react-style-singleton@npm:^2.2.3": version: 2.2.3 resolution: "react-style-singleton@npm:2.2.3" @@ -24298,7 +24368,7 @@ __metadata: languageName: node linkType: hard -"react-transition-group@npm:4.4.5, react-transition-group@npm:^4.4.5": +"react-transition-group@npm:4.4.5": version: 4.4.5 resolution: "react-transition-group@npm:4.4.5" dependencies: @@ -24454,31 +24524,26 @@ __metadata: languageName: node linkType: hard -"recharts-scale@npm:^0.4.4": - version: 0.4.5 - resolution: "recharts-scale@npm:0.4.5" - dependencies: - decimal.js-light: "npm:^2.4.1" - checksum: 10c0/64ce1fc4ebe62001787bf4dc4cbb779452d33831619309c71c50277c58e8968ffe98941562d9d0d5ffdb02588ebd62f4fe6548fa826110fd458db9c3cc6dadc1 - languageName: node - linkType: hard - -"recharts@npm:^2.12.7": - version: 2.13.3 - resolution: "recharts@npm:2.13.3" +"recharts@npm:^3.2.1": + version: 3.9.1 + resolution: "recharts@npm:3.9.1" dependencies: - clsx: "npm:^2.0.0" - eventemitter3: "npm:^4.0.1" - lodash: "npm:^4.17.21" - react-is: "npm:^18.3.1" - react-smooth: "npm:^4.0.0" - recharts-scale: "npm:^0.4.4" - tiny-invariant: "npm:^1.3.1" - victory-vendor: "npm:^36.6.8" + "@reduxjs/toolkit": "npm:^1.9.0 || 2.x.x" + clsx: "npm:^2.1.1" + decimal.js-light: "npm:^2.5.1" + es-toolkit: "npm:^1.39.3" + eventemitter3: "npm:^5.0.1" + immer: "npm:^11.1.8" + react-redux: "npm:8.x.x || 9.x.x" + reselect: "npm:5.2.0" + tiny-invariant: "npm:^1.3.3" + use-sync-external-store: "npm:^1.2.2" + victory-vendor: "npm:^37.0.2" peerDependencies: - react: ^16.0.0 || ^17.0.0 || ^18.0.0 - react-dom: ^16.0.0 || ^17.0.0 || ^18.0.0 - checksum: 10c0/67c516cd142da0e5968cf9b723c1bc08c2324ff4c41952d623dfce06e8c13751e1ef885b2e39aaeb5e32595566c8d91633328850adee70ad53bc8685adff456d + react: ^16.8.0 || ^17.0.0 || ^18.0.0 || ^19.0.0 + react-dom: ^16.0.0 || ^17.0.0 || ^18.0.0 || ^19.0.0 + react-is: ^16.8.0 || ^17.0.0 || ^18.0.0 || ^19.0.0 + checksum: 10c0/081106d981a250b8163633810db5b54132aba1efcf677216dd238853e78d185e6453c0c5811aec1451df1eb017aa1e9cfbe726d6d74246f8f3cda8799fd44cc9 languageName: node linkType: hard @@ -24492,6 +24557,22 @@ __metadata: languageName: node linkType: hard +"redux-thunk@npm:^3.1.0": + version: 3.1.0 + resolution: "redux-thunk@npm:3.1.0" + peerDependencies: + redux: ^5.0.0 + checksum: 10c0/21557f6a30e1b2e3e470933247e51749be7f1d5a9620069a3125778675ce4d178d84bdee3e2a0903427a5c429e3aeec6d4df57897faf93eb83455bc1ef7b66fd + languageName: node + linkType: hard + +"redux@npm:^5.0.1": + version: 5.0.1 + resolution: "redux@npm:5.0.1" + checksum: 10c0/b10c28357194f38e7d53b760ed5e64faa317cc63de1fb95bc5d9e127fab956392344368c357b8e7a9bedb0c35b111e7efa522210cfdc3b3c75e5074718e9069c + languageName: node + linkType: hard + "reflect.getprototypeof@npm:^1.0.6, reflect.getprototypeof@npm:^1.0.9": version: 1.0.10 resolution: "reflect.getprototypeof@npm:1.0.10" @@ -24716,6 +24797,13 @@ __metadata: languageName: node linkType: hard +"reselect@npm:5.2.0, reselect@npm:^5.1.0": + version: 5.2.0 + resolution: "reselect@npm:5.2.0" + checksum: 10c0/d0a8497e404b25a769fe792eacae88b9b576c30870450cd243d76ec9427d91a59c4a2cc00d824d283448b444c4c698a8f961d771c8ae39c759313afb31950e2e + languageName: node + linkType: hard + "reserved@npm:0.1.2": version: 0.1.2 resolution: "reserved@npm:0.1.2" @@ -27106,7 +27194,7 @@ __metadata: languageName: node linkType: hard -"tiny-invariant@npm:^1.3.1, tiny-invariant@npm:^1.3.3": +"tiny-invariant@npm:^1.3.3": version: 1.3.3 resolution: "tiny-invariant@npm:1.3.3" checksum: 10c0/65af4a07324b591a059b35269cd696aba21bef2107f29b9f5894d83cc143159a204b299553435b03874ebb5b94d019afa8b8eff241c8a4cfee95872c2e1c1c4a @@ -28237,7 +28325,7 @@ __metadata: languageName: node linkType: hard -"use-sync-external-store@npm:^1.2.2, use-sync-external-store@npm:^1.5.0": +"use-sync-external-store@npm:^1.2.2, use-sync-external-store@npm:^1.4.0, use-sync-external-store@npm:^1.5.0": version: 1.6.0 resolution: "use-sync-external-store@npm:1.6.0" peerDependencies: @@ -28367,9 +28455,9 @@ __metadata: languageName: node linkType: hard -"victory-vendor@npm:^36.6.8": - version: 36.6.8 - resolution: "victory-vendor@npm:36.6.8" +"victory-vendor@npm:^37.0.2": + version: 37.3.6 + resolution: "victory-vendor@npm:37.3.6" dependencies: "@types/d3-array": "npm:^3.0.3" "@types/d3-ease": "npm:^3.0.0" @@ -28385,7 +28473,7 @@ __metadata: d3-shape: "npm:^3.1.0" d3-time: "npm:^3.0.0" d3-timer: "npm:^3.0.1" - checksum: 10c0/72dd8355f706879eb6a01a8ecba7bde8d99aa3acb748111ccf498dc71a7258a03154b5c2ae72f36fb4ec775e7554fabeb3e3dd1c4c3487126b09654f7f038423 + checksum: 10c0/0a9628db30b898160409628f26d457ba15adc86472531817d73fbeb847de498d94f91dee5f4caa969195744e91be93e100eeff7a392b591ac5349343a36dec29 languageName: node linkType: hard From ce7b020e5d2ef3b909ee7a8ae0531e37ae2d93c6 Mon Sep 17 00:00:00 2001 From: Brandon Pereira Date: Thu, 9 Jul 2026 12:16:24 -0600 Subject: [PATCH 02/10] fix(charts): address recharts 3 upgrade review feedback - DBHistogramChart: lock the pinned tooltip to the clicked bar by adding trigger: 'click' alongside active/defaultIndex, so hovering another bar no longer swaps the tooltip's data and View Events link. - HDXMultiSeriesTimeChart: measure zoom drag distance consistently from the native event's offsetX on both mouse down and up, avoiding a coordinate- space mismatch with activeCoordinate that could turn a click into a zoom. - app.scss: scope the chart focus-ring suppression to the surface and its descendants, leaving the wrapper's keyboard focus ring intact (WCAG 2.4.11). - Sparkline: type the height prop as ResponsiveContainer expects. --- packages/app/src/HDXMultiSeriesTimeChart.tsx | 17 +++++++++++------ .../app/src/components/DBHistogramChart.tsx | 9 ++++++--- packages/app/src/components/Sparkline.tsx | 4 +++- packages/app/styles/app.scss | 9 ++++----- 4 files changed, 24 insertions(+), 15 deletions(-) diff --git a/packages/app/src/HDXMultiSeriesTimeChart.tsx b/packages/app/src/HDXMultiSeriesTimeChart.tsx index 9012909b50..0600d9f8e8 100644 --- a/packages/app/src/HDXMultiSeriesTimeChart.tsx +++ b/packages/app/src/HDXMultiSeriesTimeChart.tsx @@ -845,10 +845,13 @@ export const MemoChart = memo(function MemoChart({ }} onMouseDown={(state, e) => { // Record the drag start: the active bucket label and the pointer's - // plot-relative X (from the active coordinate, falling back to the - // native event's offsetX) for measuring drag distance on mouse up. - const chartX = - state?.activeCoordinate?.x ?? e?.nativeEvent?.offsetX; + // X for measuring drag distance on mouse up. Use the native event's + // offsetX (wrapper-relative) as the pixel source so mousedown and + // mouseup are measured in the same coordinate space — mixing it + // with activeCoordinate.x (plot-relative, offset by the y-axis + // width) could otherwise inflate a zero-distance click into a + // false drag. + const chartX = e?.nativeEvent?.offsetX; if (state?.activeLabel != null && chartX != null) { setHighlightStart(String(state.activeLabel)); mouseDownPosRef.current = chartX; @@ -888,8 +891,10 @@ export const MemoChart = memo(function MemoChart({ const MIN_DRAG_DISTANCE = 20; // Minimum horizontal drag distance in pixels let dragDistance = 0; - const chartX = - state?.activeCoordinate?.x ?? e?.nativeEvent?.offsetX; + // Measure against the same coordinate space recorded on mouse down + // (native offsetX) so the distance can't be skewed by the y-axis + // width. + const chartX = e?.nativeEvent?.offsetX; if (mouseDownPosRef.current != null && chartX != null) { dragDistance = Math.abs(chartX - mouseDownPosRef.current); } diff --git a/packages/app/src/components/DBHistogramChart.tsx b/packages/app/src/components/DBHistogramChart.tsx index 6e789a1de2..45ace15253 100644 --- a/packages/app/src/components/DBHistogramChart.tsx +++ b/packages/app/src/components/DBHistogramChart.tsx @@ -105,10 +105,13 @@ function HistogramChart({ content={ } - // When a bar is pinned, force the tooltip active on that bar; - // otherwise let Recharts control it on hover. + // When a bar is pinned, lock the tooltip to that bar: `trigger: + // 'click'` makes the tooltip ignore hover (which would otherwise let + // the tooltip drift to whatever bar the cursor grazes), and + // `defaultIndex` fixes it on the pinned bar. When nothing is pinned, + // Recharts controls the tooltip on hover as usual. {...(pinnedIndex != null - ? { active: true, defaultIndex: pinnedIndex } + ? { active: true, defaultIndex: pinnedIndex, trigger: 'click' } : {})} /> diff --git a/packages/app/src/components/Sparkline.tsx b/packages/app/src/components/Sparkline.tsx index 8ef8cf8317..52bc6132c5 100644 --- a/packages/app/src/components/Sparkline.tsx +++ b/packages/app/src/components/Sparkline.tsx @@ -41,7 +41,9 @@ export function Sparkline({ points: SparklinePoint[]; type: SparklineType; color: string; - height?: number | string; + // Matches ResponsiveContainer's accepted height: a pixel number or a + // percentage string (e.g. "100%"). + height?: number | `${number}%`; }) { if (points.length < 2) return null; diff --git a/packages/app/styles/app.scss b/packages/app/styles/app.scss index 268beed637..bb1f34e280 100644 --- a/packages/app/styles/app.scss +++ b/packages/app/styles/app.scss @@ -324,11 +324,10 @@ button:focus-visible { } // Charts are keyboard-focusable for accessibility, which paints a browser focus -// ring when the chart is clicked. Hide the visual ring on the chart wrapper, -// surface, and any focusable descendant of the surface while keeping the -// elements focusable for keyboard navigation. -.recharts-wrapper:focus, -.recharts-wrapper:focus-visible, +// ring when the chart is clicked. Hide the ring on the chart surface (the SVG +// that receives focus on click) and its focusable descendants, but leave the +// outer `.recharts-wrapper` untouched so keyboard users Tabbing to a chart +// still get a visible focus indicator (WCAG 2.4.11). .recharts-surface:focus, .recharts-surface:focus-visible, .recharts-surface *:focus, From a2b37e5639bed0e72f5e4a76393dfaf0884b1927 Mon Sep 17 00:00:00 2001 From: Brandon Pereira Date: Thu, 9 Jul 2026 12:43:41 -0600 Subject: [PATCH 03/10] fix(charts): address second review pass on the recharts upgrade - HDXMultiSeriesTimeChart: rebuild the click drill-down payload from the same visible series set used to draw the lines (legend selection + HARD_LINES_LIMIT), so the popover never lists deselected or over-limit series. Extracts getVisibleLineData / getSeriesDisplayName helpers reused by the lines, payload, and (indirectly) the y-axis domain. - HDXMultiSeriesTimeChart: measure zoom drag distance from activeCoordinate.x on both mouse down and up (offsetX is relative to whichever child SVG element is under the pointer, so it mismatched across bars/paths). - HDXMultiSeriesTimeChart: export a shared ActiveClickPayload type and use it for isClickActive/setIsClickActive instead of any; DBTimeChart imports it. - DBHistogramChart: ignore a non-integer activeIndex so a click resolving to no bar can't leave a stuck pin. - app.scss: suppress the chart focus ring only for pointer focus (:focus:not(:focus-visible)), preserving the keyboard focus indicator (WCAG 2.4.11). Verified in a browser that click shows no ring. - Add getVisibleLineData unit tests (selection filtering + line limit). --- packages/app/src/HDXMultiSeriesTimeChart.tsx | 122 +++++++++++------- .../__tests__/HDXMultiSeriesTimeChart.test.ts | 56 +++++++- .../app/src/components/DBHistogramChart.tsx | 19 ++- packages/app/src/components/DBTimeChart.tsx | 11 +- packages/app/styles/app.scss | 16 +-- 5 files changed, 148 insertions(+), 76 deletions(-) diff --git a/packages/app/src/HDXMultiSeriesTimeChart.tsx b/packages/app/src/HDXMultiSeriesTimeChart.tsx index 0600d9f8e8..ef2829216d 100644 --- a/packages/app/src/HDXMultiSeriesTimeChart.tsx +++ b/packages/app/src/HDXMultiSeriesTimeChart.tsx @@ -371,6 +371,49 @@ const LegendRenderer = memo<{ export const HARD_LINES_LIMIT = MAX_TIME_CHART_SERIES; +/** One series entry in a click-frozen tooltip's payload. */ +export type ActiveClickSeries = { + value?: number; + dataKey?: string; + name?: string; + /** Series color, matching the legend swatch. */ + color?: string; +}; + +/** + * State for the click-frozen ("pinned") tooltip. Produced by MemoChart's + * onClick and consumed by DBTimeChart to render the drill-down popover. + */ +export type ActiveClickPayload = { + x: number; + y: number; + activeLabel: string; + xPerc: number; + yPerc: number; + activePayload?: ActiveClickSeries[]; +}; + +/** Series label shown in the legend, tooltip, and line `name`. */ +const getSeriesDisplayName = (ld: LineData) => ld.displayName || ld.dataKey; + +/** + * The series actually drawn on the chart: the first HARD_LINES_LIMIT of + * lineData, narrowed to the legend selection when one is active. The rendered + * lines, the drill-down click payload, and the y-axis domain all derive from + * this same set so they never diverge. Exported for unit testing. + */ +export function getVisibleLineData( + lineData: LineData[], + selectedSeriesNames: Set | undefined, +): LineData[] { + const hasSelection = !!selectedSeriesNames && selectedSeriesNames.size > 0; + return lineData + .slice(0, HARD_LINES_LIMIT) + .filter( + ld => !hasSelection || selectedSeriesNames.has(getSeriesDisplayName(ld)), + ); +} + const StackedBarWithOverlap = (props: BarProps) => { const { x, y, width, fill } = props; // `height` may arrive as a string, so coerce it to a number before the @@ -491,8 +534,8 @@ export const MemoChart = memo(function MemoChart({ fitYAxisToData = false, }: { graphResults: any[]; - setIsClickActive: (v: any) => void; - isClickActive: any; + setIsClickActive: (v: ActiveClickPayload | undefined) => void; + isClickActive: ActiveClickPayload | undefined; dateRange: [Date, Date] | Readonly<[Date, Date]>; lineData: LineData[]; referenceLines?: React.ReactNode; @@ -541,33 +584,26 @@ export const MemoChart = memo(function MemoChart({ [displayType], ); - const lines = useMemo(() => { - const hasSelection = selectedSeriesNames && selectedSeriesNames.size > 0; - - const limitedGroupKeys = lineData - .map(ld => ld.dataKey) - .slice(0, HARD_LINES_LIMIT) - .filter((key, i) => { - const seriesName = lineData[i]?.displayName ?? key; - // If there's a selection, only show selected series - // If no selection, show all series - return !hasSelection || selectedSeriesNames.has(seriesName); - }); + const visibleLineData = useMemo( + () => getVisibleLineData(lineData, selectedSeriesNames), + [lineData, selectedSeriesNames], + ); + const lines = useMemo(() => { // When a series is nearest the cursor (only meaningful with more than one // line shown), thicken its line and fade the others so the eye lands on // the same series the tooltip bolds. Mirrors the legend's selected style // (thicker stroke) with a gentle fade that keeps the rest readable. const hasNearest = - limitedGroupKeys.length > 1 && + visibleLineData.length > 1 && nearestSeriesKey != null && - limitedGroupKeys.includes(nearestSeriesKey); + visibleLineData.some(ld => ld.dataKey === nearestSeriesKey); - return limitedGroupKeys.map(key => { - const lineDataIndex = lineData.findIndex(ld => ld.dataKey === key); - const color = lineData[lineDataIndex]?.color; - const strokeDasharray = lineData[lineDataIndex]?.isDashed ? '4 3' : '0'; - const seriesName = lineData[lineDataIndex]?.displayName ?? key; + return visibleLineData.map(ld => { + const key = ld.dataKey; + const color = ld.color; + const strokeDasharray = ld.isDashed ? '4 3' : '0'; + const seriesName = getSeriesDisplayName(ld); return displayType === 'stacked_bar' ? ( ); }); - }, [ - lineData, - displayType, - id, - isHovered, - selectedSeriesNames, - nearestSeriesKey, - ]); + }, [visibleLineData, displayType, id, isHovered, nearestSeriesKey]); const yAxisDomain: AxisDomain = useMemo(() => { const hasSelection = selectedSeriesNames && selectedSeriesNames.size > 0; @@ -843,15 +872,14 @@ export const MemoChart = memo(function MemoChart({ setHighlightEnd(undefined); mouseDownPosRef.current = null; }} - onMouseDown={(state, e) => { + onMouseDown={state => { // Record the drag start: the active bucket label and the pointer's - // X for measuring drag distance on mouse up. Use the native event's - // offsetX (wrapper-relative) as the pixel source so mousedown and - // mouseup are measured in the same coordinate space — mixing it - // with activeCoordinate.x (plot-relative, offset by the y-axis - // width) could otherwise inflate a zero-distance click into a - // false drag. - const chartX = e?.nativeEvent?.offsetX; + // plot-relative X for measuring drag distance on mouse up. Use the + // chart's active coordinate (not the native event's offsetX, which + // is relative to whichever child SVG element — bar rect or line + // path — is under the pointer) so mousedown and mouseup share one + // origin. + const chartX = state?.activeCoordinate?.x; if (state?.activeLabel != null && chartX != null) { setHighlightStart(String(state.activeLabel)); mouseDownPosRef.current = chartX; @@ -887,14 +915,13 @@ export const MemoChart = memo(function MemoChart({ setIsClickActive(undefined); // Clear out any click state as we're highlighting } }} - onMouseUp={(state, e) => { + onMouseUp={state => { const MIN_DRAG_DISTANCE = 20; // Minimum horizontal drag distance in pixels let dragDistance = 0; - // Measure against the same coordinate space recorded on mouse down - // (native offsetX) so the distance can't be skewed by the y-axis - // width. - const chartX = e?.nativeEvent?.offsetX; + // Measure against the same plot-relative coordinate recorded on + // mouse down so both endpoints share one origin. + const chartX = state?.activeCoordinate?.x; if (mouseDownPosRef.current != null && chartX != null) { dragDistance = Math.abs(chartX - mouseDownPosRef.current); } @@ -968,15 +995,18 @@ export const MemoChart = memo(function MemoChart({ ) { const activeLabel = String(state.activeLabel); const activeRow = graphResults.find( - row => String(row[timestampKey ?? 'ts_bucket']) === activeLabel, + row => String(row[timestampKey]) === activeLabel, ); + // Mirror the series actually drawn (legend selection + + // HARD_LINES_LIMIT) so the popover's "Filter by group" list never + // shows deselected or over-limit series. const activePayload = activeRow != null - ? lineData - .filter(ld => activeRow[ld.dataKey] != null) + ? visibleLineData + .filter(ld => typeof activeRow[ld.dataKey] === 'number') .map(ld => ({ dataKey: ld.dataKey, - name: ld.displayName || ld.dataKey, + name: getSeriesDisplayName(ld), value: activeRow[ld.dataKey] as number, color: ld.color, })) diff --git a/packages/app/src/__tests__/HDXMultiSeriesTimeChart.test.ts b/packages/app/src/__tests__/HDXMultiSeriesTimeChart.test.ts index 2cd4d74eec..8cd975b2b5 100644 --- a/packages/app/src/__tests__/HDXMultiSeriesTimeChart.test.ts +++ b/packages/app/src/__tests__/HDXMultiSeriesTimeChart.test.ts @@ -6,7 +6,12 @@ // #2362 where a semantic-hex `lineData[].color` (e.g. the output of // `getChartColorInfo()` on HyperDX) would not have a matching gradient // def after the `COLORS` palette was unified to Observable 10. -import { collectMemoChartGradientHexes } from '@/HDXMultiSeriesTimeChart'; +import type { LineData } from '@/ChartUtils'; +import { + collectMemoChartGradientHexes, + getVisibleLineData, + HARD_LINES_LIMIT, +} from '@/HDXMultiSeriesTimeChart'; import { COLORS } from '@/utils'; describe('collectMemoChartGradientHexes', () => { @@ -53,3 +58,52 @@ describe('collectMemoChartGradientHexes', () => { expect(hexes.every(h => typeof h === 'string')).toBe(true); }); }); + +// The drill-down click popover rebuilds its per-series payload from the same +// set of series that are actually drawn. These tests pin that "visible set" +// logic so the popover can never surface legend-deselected or over-limit +// series (the Recharts 2 `state.activePayload` only ever contained drawn +// series). +describe('getVisibleLineData', () => { + const makeLine = (dataKey: string, displayName?: string): LineData => ({ + dataKey, + currentPeriodKey: dataKey, + previousPeriodKey: `${dataKey}.prev`, + displayName: displayName ?? dataKey, + valueColumnName: dataKey, + color: '#abcdef', + }); + + it('returns all series (up to the limit) when there is no selection', () => { + const lineData = [makeLine('a'), makeLine('b'), makeLine('c')]; + expect(getVisibleLineData(lineData, undefined).map(l => l.dataKey)).toEqual( + ['a', 'b', 'c'], + ); + expect(getVisibleLineData(lineData, new Set()).map(l => l.dataKey)).toEqual( + ['a', 'b', 'c'], + ); + }); + + it('keeps only the selected series (matched by display name)', () => { + const lineData = [ + makeLine('a', 'Alpha'), + makeLine('b', 'Beta'), + makeLine('c', 'Gamma'), + ]; + const visible = getVisibleLineData(lineData, new Set(['Alpha', 'Gamma'])); + expect(visible.map(l => l.dataKey)).toEqual(['a', 'c']); + }); + + it('drops series beyond HARD_LINES_LIMIT', () => { + const lineData = Array.from({ length: HARD_LINES_LIMIT + 5 }, (_, i) => + makeLine(`series-${i}`), + ); + const visible = getVisibleLineData(lineData, undefined); + expect(visible).toHaveLength(HARD_LINES_LIMIT); + // A series past the limit must never appear, even if selected. + const overLimitName = `series-${HARD_LINES_LIMIT + 2}`; + expect(getVisibleLineData(lineData, new Set([overLimitName]))).toHaveLength( + 0, + ); + }); +}); diff --git a/packages/app/src/components/DBHistogramChart.tsx b/packages/app/src/components/DBHistogramChart.tsx index 45ace15253..db002ae3b6 100644 --- a/packages/app/src/components/DBHistogramChart.tsx +++ b/packages/app/src/components/DBHistogramChart.tsx @@ -60,16 +60,15 @@ function HistogramChart({ className="user-select-none cursor-crosshair" onClick={state => { // Toggle the pinned tooltip on the clicked bar (click the same bar - // again to unpin). - const idx = - typeof state?.activeIndex === 'number' - ? state.activeIndex - : state?.activeIndex != null - ? Number(state.activeIndex) - : undefined; - setPinnedIndex(prev => - idx == null ? undefined : prev === idx ? undefined : idx, - ); + // again to unpin). Coerce activeIndex to a number and ignore NaN so a + // click that resolves to no bar never leaves a stuck pin. + const raw = state?.activeIndex; + const idx = typeof raw === 'number' ? raw : Number(raw); + if (!Number.isInteger(idx)) { + setPinnedIndex(undefined); + return; + } + setPinnedIndex(prev => (prev === idx ? undefined : idx)); }} > Date: Thu, 9 Jul 2026 13:07:51 -0600 Subject: [PATCH 04/10] chore(charts): un-export internal ActiveClickSeries type Knip flagged it as an unused export; it's only referenced by the exported ActiveClickPayload, so keep it module-private. --- packages/app/src/HDXMultiSeriesTimeChart.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/app/src/HDXMultiSeriesTimeChart.tsx b/packages/app/src/HDXMultiSeriesTimeChart.tsx index ef2829216d..67a8191971 100644 --- a/packages/app/src/HDXMultiSeriesTimeChart.tsx +++ b/packages/app/src/HDXMultiSeriesTimeChart.tsx @@ -372,7 +372,7 @@ const LegendRenderer = memo<{ export const HARD_LINES_LIMIT = MAX_TIME_CHART_SERIES; /** One series entry in a click-frozen tooltip's payload. */ -export type ActiveClickSeries = { +type ActiveClickSeries = { value?: number; dataKey?: string; name?: string; From 3af3a250ce7d66235a6de7279b7c758043be173b Mon Sep 17 00:00:00 2001 From: Brandon Pereira Date: Thu, 9 Jul 2026 13:39:49 -0600 Subject: [PATCH 05/10] fix(charts): address third review pass (zoom origin, pin reset, tests, nits) - Zoom-drag distance is measured from a container-relative pointer X (clientX minus the chart container's bounding rect) captured on both mouse down and up, so it has one stable origin and is never dropped when the pointer maps to no data point. - Histogram: reset the pinned tooltip when the buckets change (a background refetch would otherwise repoint the positional pin to the wrong time range) and remount the Tooltip on pin change so defaultIndex re-seeds. Ignore a non-integer activeIndex. - Extract buildActiveClickSeries (pure) for the drill-down payload rebuild and getActiveLabel to normalize the event label; add unit tests for the payload builder and the visible-series set. - Nits: numeric ReferenceLine x on the time axis, type-guard narrowing instead of an as-number cast, type the comparison-chart click arg, drop an unreachable String(... ?? ''). --- packages/app/src/HDXMultiSeriesTimeChart.tsx | 111 ++++++++++++------ .../__tests__/HDXMultiSeriesTimeChart.test.ts | 36 ++++++ .../app/src/components/DBHistogramChart.tsx | 12 +- .../components/PropertyComparisonChart.tsx | 4 +- 4 files changed, 124 insertions(+), 39 deletions(-) diff --git a/packages/app/src/HDXMultiSeriesTimeChart.tsx b/packages/app/src/HDXMultiSeriesTimeChart.tsx index 67a8191971..f6b9f78e00 100644 --- a/packages/app/src/HDXMultiSeriesTimeChart.tsx +++ b/packages/app/src/HDXMultiSeriesTimeChart.tsx @@ -396,6 +396,37 @@ export type ActiveClickPayload = { /** Series label shown in the legend, tooltip, and line `name`. */ const getSeriesDisplayName = (ld: LineData) => ld.displayName || ld.dataKey; +/** Normalize a chart event's active label (number | string) to a string. */ +const getActiveLabel = (state?: { + activeLabel?: string | number; +}): string | undefined => + state?.activeLabel != null ? String(state.activeLabel) : undefined; + +/** + * Build the per-series payload for a click-frozen tooltip from the data row at + * the clicked bucket. Only the visible series (legend selection + + * HARD_LINES_LIMIT) with a numeric value at that bucket are included, so the + * drill-down popover mirrors exactly what is drawn. Exported for unit testing. + */ +export function buildActiveClickSeries( + visibleLineData: LineData[], + activeRow: Record | undefined, +): ActiveClickSeries[] { + if (activeRow == null) return []; + return visibleLineData.flatMap(ld => { + const value = activeRow[ld.dataKey]; + if (typeof value !== 'number') return []; + return [ + { + dataKey: ld.dataKey, + name: getSeriesDisplayName(ld), + value, + color: ld.color, + }, + ]; + }); +} + /** * The series actually drawn on the chart: the first HARD_LINES_LIMIT of * lineData, narrowed to the legend selection when one is active. The rendered @@ -705,6 +736,17 @@ export const MemoChart = memo(function MemoChart({ const sizeRef = useRef<[number, number]>([0, 0]); const [containerWidth, setContainerWidth] = useState(0); + // The chart's outer positioned container. Used to convert a pointer's + // viewport clientX into a stable container-relative X for measuring + // drag-to-zoom distance — a single origin that is always defined, unlike the + // chart's activeCoordinate (null off a data point) or a child SVG element's + // offsetX (relative to whichever bar/path is under the pointer). + const containerRef = useRef(null); + const getContainerX = useCallback((e?: { clientX?: number } | null) => { + if (e?.clientX == null || containerRef.current == null) return undefined; + return e.clientX - containerRef.current.getBoundingClientRect().left; + }, []); + // Recharts computes bar width from the smallest gap between ticks on a // numerical XAxis. With a single data point there are no gaps, so the // computed width is 0 and bars become invisible. Provide an explicit @@ -826,7 +868,10 @@ export const MemoChart = memo(function MemoChart({ }, [dateRange, granularity, dateRangeEndInclusive, displayType]); return ( -
+
{onTimeRangeSelect != null && zoomOrigin != null ? (