diff --git a/.changeset/timechart-drilldown-zero-value.md b/.changeset/timechart-drilldown-zero-value.md new file mode 100644 index 0000000000..0e24af0eb0 --- /dev/null +++ b/.changeset/timechart-drilldown-zero-value.md @@ -0,0 +1,10 @@ +--- +'@hyperdx/app': patch +--- + +fix: keep the drill-down value filter when the clicked point is zero + +Clicking a time-chart point with a value of exactly 0 dropped the value filter +and searched every event in the bucket instead of the matching ones. Zero is a +real point — the chart deliberately keeps it rather than treating it as absent — +so the guard now tests for a missing value rather than a falsy one. diff --git a/packages/app/src/HDXMultiSeriesTimeChart/ChartLegend.tsx b/packages/app/src/HDXMultiSeriesTimeChart/ChartLegend.tsx new file mode 100644 index 0000000000..b7a0e4fb6d --- /dev/null +++ b/packages/app/src/HDXMultiSeriesTimeChart/ChartLegend.tsx @@ -0,0 +1,174 @@ +import { memo, useMemo, useState } from 'react'; +import cx from 'classnames'; +import { Popover } from '@mantine/core'; + +import { type LineData } from '@/ChartUtils'; +import { truncateMiddle } from '@/utils'; + +import { hasSeriesSelection } from './chartData'; +import { MAX_LEGEND_ITEMS } from './constants'; + +import styles from '@styles/HDXLineChart.module.scss'; + +function ExpandableLegendItem({ + entry, + expanded, + isSelected, + isDisabled, + onToggle, +}: { + entry: any; + expanded?: boolean; + isSelected?: boolean; + isDisabled?: boolean; + onToggle?: (isShiftKey: boolean) => void; +}) { + const [_expanded, setExpanded] = useState(false); + const isExpanded = _expanded || expanded; + + return ( + { + if (onToggle) { + onToggle(e.shiftKey); + } else { + setExpanded(v => !v); + } + }} + title={ + isSelected + ? 'Click to show all (Shift+click to deselect)' + : 'Click to show only this (Shift+click for multi-select)' + } + > +
+ + + +
+ {isExpanded || isSelected + ? entry.value + : truncateMiddle(`${entry.value}`, 35)} +
+ ); +} + +export const LegendRenderer = memo<{ + payload?: { + dataKey: string; + value: string; + color: string; + }[]; + lineDataMap: { [key: string]: LineData }; + allLineData?: LineData[]; + selectedSeries?: Set; + onToggleSeries?: (seriesName: string, isShiftKey?: boolean) => void; +}>(props => { + const { payload, lineDataMap, allLineData, selectedSeries, onToggleSeries } = + props; + + const hasSelection = hasSeriesSelection(selectedSeries); + + // Use allLineData to ensure all series are always shown in legend + const allSeriesPayload = useMemo(() => { + if (allLineData?.length) { + return allLineData.map(ld => ({ + dataKey: ld.dataKey, + value: ld.displayName || ld.dataKey, + color: ld.color, + payload: { strokeDasharray: ld.isDashed ? '4 3' : '0' }, + })); + } + return payload ?? []; + }, [allLineData, payload]); + + const sortedLegendItems = useMemo(() => { + // Order items such that current and previous period lines are consecutive + const currentPeriodKeyIndex = new Map(); + allSeriesPayload.forEach((line, index) => { + const currentPeriodKey = + lineDataMap[line.dataKey]?.currentPeriodKey || ''; + if (!currentPeriodKeyIndex.has(currentPeriodKey)) { + currentPeriodKeyIndex.set(currentPeriodKey, index); + } + }); + + // Copy before sorting: when this comes from Recharts' legend payload it is + // kept in the Immer-backed store and frozen, so an in-place sort throws. + return [...allSeriesPayload].sort((a, b) => { + const keyA = lineDataMap[a.dataKey]?.currentPeriodKey ?? ''; + const keyB = lineDataMap[b.dataKey]?.currentPeriodKey ?? ''; + + const indexA = currentPeriodKeyIndex.get(keyA) ?? 0; + const indexB = currentPeriodKeyIndex.get(keyB) ?? 0; + + return indexB - indexA || a.dataKey.localeCompare(b.dataKey); + }); + }, [allSeriesPayload, lineDataMap]); + + const shownItems = sortedLegendItems.slice(0, MAX_LEGEND_ITEMS); + const restItems = sortedLegendItems.slice(MAX_LEGEND_ITEMS); + + return ( +
+ {shownItems.map((entry, index) => { + const isSelected = !!selectedSeries?.has(entry.value); + const isDisabled = hasSelection && !isSelected; + return ( + onToggleSeries?.(entry.value, isShiftKey)} + /> + ); + })} + {restItems.length ? ( + + +
+ +{restItems.length} more +
+
+ +
+ {restItems.map((entry, index) => { + const isSelected = !!selectedSeries?.has(entry.value); + const isDisabled = hasSelection && !isSelected; + return ( + + onToggleSeries?.(entry.value, isShiftKey) + } + /> + ); + })} +
+
+
+ ) : null} +
+ ); +}); diff --git a/packages/app/src/HDXMultiSeriesTimeChart/ChartTooltipContent.tsx b/packages/app/src/HDXMultiSeriesTimeChart/ChartTooltipContent.tsx new file mode 100644 index 0000000000..bc1ef76449 --- /dev/null +++ b/packages/app/src/HDXMultiSeriesTimeChart/ChartTooltipContent.tsx @@ -0,0 +1,170 @@ +import { memo, useMemo } from 'react'; +import { withErrorBoundary } from 'react-error-boundary'; + +import { findNearestSeriesKey, type LineData } from '@/ChartUtils'; +import { + ChartTooltipContainer, + ChartTooltipHeader, + toViewportPoint, + useChartTooltipZIndex, +} from '@/components/charts/ChartTooltip'; +import type { NumberFormat } from '@/types'; + +import { + NEAREST_SERIES_MAX_DISTANCE_PX, + TOOLTIP_POINT_OFFSET_PX, +} from './constants'; +import { TooltipItem, type TooltipPayload } from './TooltipItem'; + +import styles from '@styles/HDXLineChart.module.scss'; + +type HDXLineChartTooltipProps = { + lineDataMap: { [keyName: string]: LineData }; + previousPeriodOffsetSeconds?: number; + numberFormat?: NumberFormat; + numberFormatByKey: Map; + /** Per-series active-point pixel Y, captured by the Area active dots. */ + activePointYByKeyRef: React.MutableRefObject>; + /** The chart's outer container; its viewport rect anchors this tooltip. */ + containerRef: React.MutableRefObject; +} & Record; + +/** Stable stand-in for a missing payload, so the memo below keeps its identity. */ +const EMPTY_PAYLOAD: TooltipPayload[] = []; + +/** + * The recharts `` content used for the HOVER tooltip (on the hovered + * chart and its synced followers). Clicking pins ChartSeriesTooltip instead. + * + * Because it's given `portal={document.body}`, recharts skips its own transform + * positioning, so this content self-anchors at the active point with + * `position: fixed` (container rect + `coordinate`) — matching the pinned + * tooltip's anchor, and escaping the chart's bounds so edges aren't clipped. + */ +export const HDXLineChartTooltip = withErrorBoundary( + memo((props: HDXLineChartTooltipProps) => { + const { + active, + payload, + label, + numberFormat, + numberFormatByKey, + lineDataMap, + previousPeriodOffsetSeconds, + activePointYByKeyRef, + containerRef, + } = props; + // recharts calls this with no payload when nothing is hovered, and the memo + // below runs before the `active && payload` guard. Mapping over undefined + // there throws inside the render, which withErrorBoundary latches for the + // rest of this instance's life — the chart never recovers. A shared constant + // rather than `?? []` so the memo's dependency stays referentially stable. + const typedPayload = (payload ?? EMPTY_PAYLOAD) as TooltipPayload[]; + + const tooltipZIndex = useChartTooltipZIndex(); + + const payloadByKey = useMemo( + () => new Map(typedPayload.map(p => [p.dataKey, p])), + [typedPayload], + ); + + if (active && payload && payload.length) { + // No onClose: hover renders the X hidden (kept for layout parity). + const header = ( + + ); + + // Bold the line nearest the cursor by comparing pointer Y to each series' + // active-dot Y. The dots write their positions earlier in this same render + // (Recharts draws graphical items before the tooltip), so it's current. + const pointerY: number | undefined = props.coordinate?.y; + // eslint-disable-next-line react-hooks/refs + const activePointYByKey = activePointYByKeyRef?.current ?? undefined; + const nearestSeriesKey = + typedPayload.length > 1 + ? findNearestSeriesKey( + activePointYByKey, + typedPayload.map(p => p.dataKey), + pointerY, + NEAREST_SERIES_MAX_DISTANCE_PX, + ) + : undefined; + + // Anchor at the active point (see the component docblock for why fixed). + const pointX = props.coordinate?.x; + const pointY = props.coordinate?.y; + // eslint-disable-next-line react-hooks/refs + const containerRect = containerRef?.current?.getBoundingClientRect(); + const anchor = + typeof pointX === 'number' && + typeof pointY === 'number' && + containerRect != null + ? toViewportPoint(containerRect, { x: pointX, y: pointY }) + : undefined; + const anchorStyle: React.CSSProperties = + anchor != null + ? { + position: 'fixed', + left: anchor.x, + top: anchor.y + TOOLTIP_POINT_OFFSET_PX, + transform: 'translateX(-50%)', + pointerEvents: 'none', + // z-index must live here: recharts leaves the portaled wrapper + // `position: static`, where z-index has no effect. + zIndex: tooltipZIndex, + } + : {}; + + return ( +
+ + {/* Copy before sorting: Recharts 3 freezes the payload, so an + in-place sort throws "this object has been frozen". */} + {[...payload] + .sort((a: TooltipPayload, b: TooltipPayload) => b.value - a.value) + .map((p: TooltipPayload) => { + const previousKey = lineDataMap[p.dataKey]?.previousPeriodKey; + const isPreviousPeriod = previousKey === p.dataKey; + const previousPayload = + !isPreviousPeriod && previousKey + ? payloadByKey.get(previousKey) + : undefined; + const valueColumnName = + lineDataMap[p.dataKey]?.valueColumnName ?? p.dataKey; + const numberFormatForKey = + numberFormatByKey.get(valueColumnName) ?? numberFormat; + + return ( + + ); + })} + +
+ ); + } + return null; + }), + { + onError: console.error, + fallback: ( +
+ An error occurred while rendering the tooltip. +
+ ), + }, +); diff --git a/packages/app/src/HDXMultiSeriesTimeChart.tsx b/packages/app/src/HDXMultiSeriesTimeChart/MemoChart.tsx similarity index 53% rename from packages/app/src/HDXMultiSeriesTimeChart.tsx rename to packages/app/src/HDXMultiSeriesTimeChart/MemoChart.tsx index 421e4f8351..4fd8a77376 100644 --- a/packages/app/src/HDXMultiSeriesTimeChart.tsx +++ b/packages/app/src/HDXMultiSeriesTimeChart/MemoChart.tsx @@ -7,15 +7,11 @@ import { useRef, useState, } from 'react'; -import cx from 'classnames'; -import { add, isSameSecond, sub } from 'date-fns'; -import { withErrorBoundary } from 'react-error-boundary'; import { Area, AreaChart, Bar, BarChart, - BarProps, CartesianGrid, Legend, ReferenceArea, @@ -25,405 +21,36 @@ import { XAxis, YAxis, } from 'recharts'; -import { AxisDomain } from 'recharts/types/util/types'; -import { convertGranularityToSeconds } from '@hyperdx/common-utils/dist/core/utils'; import { DisplayType } from '@hyperdx/common-utils/dist/types'; -import { Popover } from '@mantine/core'; +import { useChartSyncId } from '@/chartSync'; +import { findNearestSeriesKey, LineData } from '@/ChartUtils'; +import { ChartAnnotation } from '@/components/charts/chartAnnotations'; +import { ChartOverlayControls } from '@/components/charts/ChartOverlayControls'; +import { toViewportPoint } from '@/components/charts/ChartTooltip'; import type { NumberFormat } from '@/types'; -import { COLORS, formatNumber, truncateMiddle } from '@/utils'; +import { useFormatTime } from '@/useFormatTime'; +import { COLORS, formatNumber } from '@/utils'; import { - ChartAnnotation, - getAnnotationElements, -} from './components/charts/chartAnnotations'; -import { ChartOverlayControls } from './components/charts/ChartOverlayControls'; + type ActiveClickPayload, + buildActiveClickSeries, + getActiveLabel, + getSeriesDisplayName, + getVisibleLineData, + hasSeriesSelection, +} from './chartData'; +import { LegendRenderer } from './ChartLegend'; +import { CaptureActiveDot, StackedBarWithOverlap } from './chartPrimitives'; +import { HDXLineChartTooltip } from './ChartTooltipContent'; import { - ChartTooltipContainer, - ChartTooltipHeader, - ChartTooltipItem, - toViewportPoint, - useChartTooltipZIndex, -} from './components/charts/ChartTooltip'; -import { useChartSyncId } from './chartSync'; -import { - findNearestSeriesKey, - LineData, - MAX_TIME_CHART_SERIES, - toStartOfInterval, -} from './ChartUtils'; -import { useFormatTime } from './useFormatTime'; - -import styles from '@styles/HDXLineChart.module.scss'; - -const MAX_LEGEND_ITEMS = 4; - -// Vertical pixel distance within which a series' line counts as "near" the -// cursor for tooltip highlighting. Beyond this, no row is emphasized so the -// tooltip is not misleading when the pointer is in empty space. -const NEAREST_SERIES_MAX_DISTANCE_PX = 30; - -// Gap below the data point for the hover tooltip. Kept equal to the pinned -// tooltip's Popover `offset` so both land in the same spot. -const TOOLTIP_POINT_OFFSET_PX = 12; - -const Y_AXIS_WIDTH = 40; -const SINGLE_POINT_BAR_RIGHT_PADDING = 10; -const SINGLE_POINT_BAR_WIDTH_RATIO = 0.8; -// Top margin (px) reserved above the plot for annotation labels ("Alert"/"OK"), -// added only when a chart is showing annotations so other charts keep their -// tighter default headroom. -const ANNOTATION_LABEL_HEADROOM = 18; - -type TooltipPayload = { - dataKey: string; - name: string; - value: number; - color?: string; - stroke?: string; - strokeWidth?: number; - strokeDasharray?: string; - opacity?: number; -}; - -export const TooltipItem = memo( - ({ - p, - previous, - numberFormat, - highlighted, - dimmed, - }: { - p: TooltipPayload; - previous?: TooltipPayload; - numberFormat?: NumberFormat; - highlighted?: boolean; - dimmed?: boolean; - }) => { - return ( - - ); - }, -); - -type HDXLineChartTooltipProps = { - lineDataMap: { [keyName: string]: LineData }; - previousPeriodOffsetSeconds?: number; - numberFormat?: NumberFormat; - numberFormatByKey: Map; - /** Per-series active-point pixel Y, captured by the Area active dots. */ - activePointYByKeyRef: React.MutableRefObject>; - /** The chart's outer container; its viewport rect anchors this tooltip. */ - containerRef: React.MutableRefObject; -} & Record; - -/** - * The recharts `` content used for the HOVER tooltip (on the hovered - * chart and its synced followers). Clicking pins ChartSeriesTooltip instead. - * - * Because it's given `portal={document.body}`, recharts skips its own transform - * positioning, so this content self-anchors at the active point with - * `position: fixed` (container rect + `coordinate`) — matching the pinned - * tooltip's anchor, and escaping the chart's bounds so edges aren't clipped. - */ -const HDXLineChartTooltip = withErrorBoundary( - memo((props: HDXLineChartTooltipProps) => { - const { - active, - payload, - label, - numberFormat, - numberFormatByKey, - lineDataMap, - previousPeriodOffsetSeconds, - activePointYByKeyRef, - containerRef, - } = props; - const typedPayload = payload as TooltipPayload[]; - - const tooltipZIndex = useChartTooltipZIndex(); - - const payloadByKey = useMemo( - () => new Map(typedPayload.map(p => [p.dataKey, p])), - [typedPayload], - ); - - if (active && payload && payload.length) { - // No onClose: hover renders the X hidden (kept for layout parity). - const header = ( - - ); - - // Bold the line nearest the cursor by comparing pointer Y to each series' - // active-dot Y. The dots write their positions earlier in this same render - // (Recharts draws graphical items before the tooltip), so it's current. - const pointerY: number | undefined = props.coordinate?.y; - // eslint-disable-next-line react-hooks/refs - const activePointYByKey = activePointYByKeyRef?.current ?? undefined; - const nearestSeriesKey = - typedPayload.length > 1 - ? findNearestSeriesKey( - activePointYByKey, - typedPayload.map(p => p.dataKey), - pointerY, - NEAREST_SERIES_MAX_DISTANCE_PX, - ) - : undefined; - - // Anchor at the active point (see the component docblock for why fixed). - const pointX = props.coordinate?.x; - const pointY = props.coordinate?.y; - // eslint-disable-next-line react-hooks/refs - const containerRect = containerRef?.current?.getBoundingClientRect(); - const anchor = - typeof pointX === 'number' && - typeof pointY === 'number' && - containerRect != null - ? toViewportPoint(containerRect, { x: pointX, y: pointY }) - : undefined; - const anchorStyle: React.CSSProperties = - anchor != null - ? { - position: 'fixed', - left: anchor.x, - top: anchor.y + TOOLTIP_POINT_OFFSET_PX, - transform: 'translateX(-50%)', - pointerEvents: 'none', - // z-index must live here: recharts leaves the portaled wrapper - // `position: static`, where z-index has no effect. - zIndex: tooltipZIndex, - } - : {}; - - return ( -
- - {/* Copy before sorting: Recharts 3 freezes the payload, so an - in-place sort throws "this object has been frozen". */} - {[...payload] - .sort((a: TooltipPayload, b: TooltipPayload) => b.value - a.value) - .map((p: TooltipPayload) => { - const previousKey = lineDataMap[p.dataKey]?.previousPeriodKey; - const isPreviousPeriod = previousKey === p.dataKey; - const previousPayload = - !isPreviousPeriod && previousKey - ? payloadByKey.get(previousKey) - : undefined; - const valueColumnName = - lineDataMap[p.dataKey]?.valueColumnName ?? p.dataKey; - const numberFormatForKey = - numberFormatByKey.get(valueColumnName) ?? numberFormat; - - return ( - - ); - })} - -
- ); - } - return null; - }), - { - onError: console.error, - fallback: ( -
- An error occurred while rendering the tooltip. -
- ), - }, -); - -function ExpandableLegendItem({ - entry, - expanded, - isSelected, - isDisabled, - onToggle, -}: { - entry: any; - expanded?: boolean; - isSelected?: boolean; - isDisabled?: boolean; - onToggle?: (isShiftKey: boolean) => void; -}) { - const [_expanded, setExpanded] = useState(false); - const isExpanded = _expanded || expanded; - - return ( - { - if (onToggle) { - onToggle(e.shiftKey); - } else { - setExpanded(v => !v); - } - }} - title={ - isSelected - ? 'Click to show all (Shift+click to deselect)' - : 'Click to show only this (Shift+click for multi-select)' - } - > -
- - - -
- {isExpanded || isSelected - ? entry.value - : truncateMiddle(`${entry.value}`, 35)} -
- ); -} - -const LegendRenderer = memo<{ - payload?: { - dataKey: string; - value: string; - color: string; - }[]; - lineDataMap: { [key: string]: LineData }; - allLineData?: LineData[]; - selectedSeries?: Set; - onToggleSeries?: (seriesName: string, isShiftKey?: boolean) => void; -}>(props => { - const { payload, lineDataMap, allLineData, selectedSeries, onToggleSeries } = - props; - - const hasSelection = hasSeriesSelection(selectedSeries); - - // Use allLineData to ensure all series are always shown in legend - const allSeriesPayload = useMemo(() => { - if (allLineData?.length) { - return allLineData.map(ld => ({ - dataKey: ld.dataKey, - value: ld.displayName || ld.dataKey, - color: ld.color, - payload: { strokeDasharray: ld.isDashed ? '4 3' : '0' }, - })); - } - return payload ?? []; - }, [allLineData, payload]); - - const sortedLegendItems = useMemo(() => { - // Order items such that current and previous period lines are consecutive - const currentPeriodKeyIndex = new Map(); - allSeriesPayload.forEach((line, index) => { - const currentPeriodKey = - lineDataMap[line.dataKey]?.currentPeriodKey || ''; - if (!currentPeriodKeyIndex.has(currentPeriodKey)) { - currentPeriodKeyIndex.set(currentPeriodKey, index); - } - }); - - // Copy before sorting: when this comes from Recharts' legend payload it is - // kept in the Immer-backed store and frozen, so an in-place sort throws. - return [...allSeriesPayload].sort((a, b) => { - const keyA = lineDataMap[a.dataKey]?.currentPeriodKey ?? ''; - const keyB = lineDataMap[b.dataKey]?.currentPeriodKey ?? ''; - - const indexA = currentPeriodKeyIndex.get(keyA) ?? 0; - const indexB = currentPeriodKeyIndex.get(keyB) ?? 0; - - return indexB - indexA || a.dataKey.localeCompare(b.dataKey); - }); - }, [allSeriesPayload, lineDataMap]); - - const shownItems = sortedLegendItems.slice(0, MAX_LEGEND_ITEMS); - const restItems = sortedLegendItems.slice(MAX_LEGEND_ITEMS); - - return ( -
- {shownItems.map((entry, index) => { - const isSelected = !!selectedSeries?.has(entry.value); - const isDisabled = hasSelection && !isSelected; - return ( - onToggleSeries?.(entry.value, isShiftKey)} - /> - ); - })} - {restItems.length ? ( - - -
- +{restItems.length} more -
-
- -
- {restItems.map((entry, index) => { - const isSelected = !!selectedSeries?.has(entry.value); - const isDisabled = hasSelection && !isSelected; - return ( - - onToggleSeries?.(entry.value, isShiftKey) - } - /> - ); - })} -
-
-
- ) : null} -
- ); -}); - -export const HARD_LINES_LIMIT = MAX_TIME_CHART_SERIES; + ANNOTATION_LABEL_HEADROOM, + NEAREST_SERIES_MAX_DISTANCE_PX, + SINGLE_POINT_BAR_RIGHT_PADDING, + SINGLE_POINT_BAR_WIDTH_RATIO, + Y_AXIS_WIDTH, +} from './constants'; +import { useChartScales } from './useChartScales'; // Debounce (ms) for the chart's ResponsiveContainer resize observer. Without // it the observer fires on every frame, and a resize → re-render → resize @@ -431,188 +58,6 @@ export const HARD_LINES_LIMIT = MAX_TIME_CHART_SERIES; // editor) from ever settling. const RESPONSIVE_CONTAINER_DEBOUNCE_MS = 50; -/** One series entry in a tooltip's per-bucket payload (hover or click-frozen). */ -export type ActiveClickSeries = { - value?: number; - dataKey?: string; - name?: string; - /** Series color, matching the legend swatch. */ - color?: string; - /** Previous-period value at the same bucket, for the percent-change chip. */ - previousValue?: number; - /** Whether this series is a dashed previous-period line. */ - isPreviousPeriod?: boolean; - /** Result column the values came from, for per-column number formatting. */ - valueColumnName?: string; -}; - -/** - * State for the pinned (click-locked) tooltip. Produced by MemoChart's onClick - * and rendered by DBTimeChart via ChartSeriesTooltip. (Hover uses recharts' own - * ; recharts' is also kept for its synced cursor.) - */ -export type ActiveClickPayload = { - /** Active point in viewport coords; the Popover anchor. */ - viewportX: number; - viewportY: number; - activeLabel: string; - activePayload?: ActiveClickSeries[]; -}; - -/** 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 []; - const isPreviousPeriod = ld.previousPeriodKey === ld.dataKey; - // Pair each current-period series with its previous-period value for the - // percent-change chip. Only current-period rows carry a comparison. - const previousRaw = - !isPreviousPeriod && ld.previousPeriodKey - ? activeRow[ld.previousPeriodKey] - : undefined; - return [ - { - dataKey: ld.dataKey, - name: getSeriesDisplayName(ld), - value, - color: ld.color, - isPreviousPeriod, - valueColumnName: ld.valueColumnName, - previousValue: - typeof previousRaw === 'number' ? previousRaw : undefined, - }, - ]; - }); -} - -/** - * The series actually drawn on the chart. Without a selection, the first - * HARD_LINES_LIMIT of lineData. With a selection (legend isolate, checkbox - * filter, or table search), the selection is applied FIRST and then capped, so - * an explicitly chosen series always draws even if it ranks beyond the limit. - * Applying the cap first would slice out a chosen low-ranked series, leaving an - * empty chart while its stats still show in the legend table. The rendered - * lines and the drill-down click payload both derive from this same set so they - * never diverge. Exported for unit testing. - */ -/** - * Whether a series selection is active. The single source of truth for the - * "isolate to these series" predicate that gates line visibility, the y-axis - * domain, legend dimming, and the "Show All Series" control — so those can't - * drift out of sync. - */ -function hasSeriesSelection( - selectedSeriesNames: Set | undefined, -): selectedSeriesNames is Set { - return !!selectedSeriesNames && selectedSeriesNames.size > 0; -} - -export function getVisibleLineData( - lineData: LineData[], - selectedSeriesNames: Set | undefined, -): LineData[] { - const hasSelection = hasSeriesSelection(selectedSeriesNames); - if (hasSelection) { - return lineData - .filter(ld => selectedSeriesNames.has(getSeriesDisplayName(ld))) - .slice(0, HARD_LINES_LIMIT); - } - return lineData.slice(0, HARD_LINES_LIMIT); -} - -const StackedBarWithOverlap = (props: BarProps) => { - 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} - fill={fill} - /> - ); -}; - -type CaptureActiveDotProps = { - /** - * Called with each series' active-point pixel Y. This is a stable callback - * (not the ref itself) so Recharts, which stores this element's props in its - * Immer-backed store and freezes them, never freezes the underlying Map — - * the write happens on the ref captured in the callback's closure instead. - */ - onCapture: (dataKey: string, cy: number) => void; - cx?: number; - cy?: number; - dataKey?: string | number; - r?: number; - fill?: string; - stroke?: string; - strokeWidth?: number; -}; - -/** - * Active dot for an Area series. Records the active point's pixel Y (`cy`) - * via `onCapture`, keyed by dataKey, then draws the same dot Recharts - * renders by default. Recharts clones this element with the active-point - * props (cx, cy, dataKey, r, fill, stroke, strokeWidth) during the render - * that precedes the tooltip, so the capture is current when the tooltip reads - * it to find the series nearest the cursor. - */ -function CaptureActiveDot({ - onCapture, - cx, - cy, - dataKey, - r, - fill, - stroke, - strokeWidth, -}: CaptureActiveDotProps) { - if (dataKey != null && typeof cy === 'number' && Number.isFinite(cy)) { - // Written synchronously during render so the tooltip, which Recharts - // renders after the graphical items in the same commit, reads the - // current frame's positions rather than the previous frame's. - onCapture(String(dataKey), cy); - } - if (typeof cx !== 'number' || typeof cy !== 'number') { - return null; - } - return ( - - ); -} - /** * Compute the unique set of hexes referenced by `` defs * inside MemoChart. Exported so a unit test can pin the dedup-and-union @@ -664,7 +109,9 @@ export const MemoChart = memo(function MemoChart({ dateRangeEndInclusive = true, fitYAxisToData = false, }: { - graphResults: any[]; + // Matches what useChartScales narrows to, so the hook's stricter type is + // actually checked at this boundary rather than satisfied by `any`. + graphResults: Record[]; setIsClickActive: (v: ActiveClickPayload | undefined) => void; isClickActive: ActiveClickPayload | undefined; dateRange: [Date, Date] | Readonly<[Date, Date]>; @@ -801,64 +248,18 @@ export const MemoChart = memo(function MemoChart({ captureActivePointY, ]); - const yAxisDomain: AxisDomain = useMemo(() => { - const hasSelection = hasSeriesSelection(selectedSeriesNames); - - // Fitting the y-axis lower bound to the data only applies to line charts. - // Bar charts are always anchored at zero so the bar lengths stay - // proportional to their values. - const shouldFitYAxis = - fitYAxisToData && displayType !== DisplayType.StackedBar; - - // The data min/max is only needed to either zoom into a selection or to - // fit the lower bound to the data. When neither applies, let Recharts - // auto-calculate the upper bound while pinning the lower bound to zero. - if (!hasSelection && !shouldFitYAxis) { - return [0, 'auto']; - } - - // Calculate domain based on visible series (all series when there's no - // explicit selection). - let minValue = Infinity; - let maxValue = -Infinity; - - graphResults.forEach(dataPoint => { - lineData.forEach(ld => { - const seriesName = ld.displayName || ld.dataKey; - // Only consider visible series - if (!hasSelection || selectedSeriesNames.has(seriesName)) { - const value = dataPoint[ld.dataKey]; - if (typeof value === 'number' && !isNaN(value)) { - minValue = Math.min(minValue, value); - maxValue = Math.max(maxValue, value); - } - } - }); - }); - - // If we found valid values, return them with some padding - if (minValue !== Infinity && maxValue !== -Infinity) { - const padding = (maxValue - minValue) * 0.05; // 5% padding - // When fitting to data, allow the lower bound to follow the data - // minimum; otherwise keep it pinned at zero. The 5% padding must not - // drag the axis below zero unless the data itself is negative, so - // clamp at zero whenever the minimum is non-negative. - const lowerBound = - shouldFitYAxis && minValue < 0 - ? minValue - padding - : Math.max(0, minValue - padding); - const upperBound = maxValue + padding; - return [lowerBound, upperBound]; - } - - return ['auto', 'auto']; - }, [ + // Axis domains and annotation elements — see useChartScales. + const { yAxisDomain, xAxisDomain, annotationElements } = useChartScales({ + annotations, + dateRange, + granularity, + dateRangeEndInclusive, + displayType, + fitYAxisToData, graphResults, lineData, selectedSeriesNames, - fitYAxisToData, - displayType, - ]); + }); const [containerWidth, setContainerWidth] = useState(0); @@ -1014,41 +415,6 @@ export const MemoChart = memo(function MemoChart({ return map; }, [lineData]); - const xAxisDomain: AxisDomain = useMemo(() => { - let startTime = toStartOfInterval(dateRange[0], granularity); - let endTime = toStartOfInterval(dateRange[1], granularity); - const endTimeIsBoundaryAligned = isSameSecond(dateRange[1], endTime); - if (endTimeIsBoundaryAligned && !dateRangeEndInclusive) { - endTime = sub(endTime, { - seconds: convertGranularityToSeconds(granularity), - }); - } - - // For bar charts, extend the domain in both directions by half a granularity unit - // so that the full bar width is within the bounds of the chart - if (displayType === DisplayType.StackedBar) { - const halfGranularitySeconds = - convertGranularityToSeconds(granularity) / 2; - startTime = sub(startTime, { seconds: halfGranularitySeconds }); - endTime = add(endTime, { seconds: halfGranularitySeconds }); - } - - return [startTime.getTime() / 1000, endTime.getTime() / 1000]; - }, [dateRange, granularity, dateRangeEndInclusive, displayType]); - - // Alert/event markers as dashed lines, clamped to the chart's x-axis domain so - // an edge marker (e.g. an alert already firing at window open) stays visible - // instead of being dropped. Labels float in the reserved top headroom. - const annotationElements = useMemo(() => { - if (!annotations?.length) { - return null; - } - // xAxisDomain is a [min, max] tuple at runtime (declared as AxisDomain). - return getAnnotationElements(annotations, { - domain: xAxisDomain as [number, number], - }); - }, [annotations, xAxisDomain]); - return (
{ + return ( + + ); + }, +); diff --git a/packages/app/src/__tests__/HDXMultiSeriesTimeChart.test.ts b/packages/app/src/HDXMultiSeriesTimeChart/__tests__/HDXMultiSeriesTimeChart.test.ts similarity index 100% rename from packages/app/src/__tests__/HDXMultiSeriesTimeChart.test.ts rename to packages/app/src/HDXMultiSeriesTimeChart/__tests__/HDXMultiSeriesTimeChart.test.ts diff --git a/packages/app/src/HDXMultiSeriesTimeChart/__tests__/useChartScales.test.ts b/packages/app/src/HDXMultiSeriesTimeChart/__tests__/useChartScales.test.ts new file mode 100644 index 0000000000..6e976a133f --- /dev/null +++ b/packages/app/src/HDXMultiSeriesTimeChart/__tests__/useChartScales.test.ts @@ -0,0 +1,172 @@ +import { isValidElement } from 'react'; +import { DisplayType } from '@hyperdx/common-utils/dist/types'; +import { renderHook } from '@testing-library/react'; + +import { type LineData } from '@/ChartUtils'; +import { ChartAnnotation } from '@/components/charts/chartAnnotations'; +import { useChartScales } from '@/HDXMultiSeriesTimeChart/useChartScales'; + +/** + * The pure seam this refactor created: axis domains and annotation elements + * derived from props, with no state and no recharts tree. The branching that + * used to be buried in MemoChart — fit-to-data, legend selection, zero-anchored + * bars, the half-bucket bar padding — is what these cases pin. + */ +const series = (dataKey: string, displayName: string): LineData => ({ + dataKey, + displayName, + currentPeriodKey: dataKey, + previousPeriodKey: `${dataKey}-prev`, + valueColumnName: dataKey, + color: '#000000', +}); + +const lineData: LineData[] = [series('a', 'A'), series('b', 'B')]; + +const baseArgs = { + annotations: undefined as ChartAnnotation[] | undefined, + dateRange: [ + new Date('2026-01-01T00:00:00Z'), + new Date('2026-01-01T01:00:00Z'), + ] as readonly [Date, Date], + granularity: '1 minute', + dateRangeEndInclusive: true, + displayType: DisplayType.Line, + fitYAxisToData: false, + graphResults: [ + { a: 10, b: 20 }, + { a: 30, b: 40 }, + ] as Record[], + lineData, + selectedSeriesNames: undefined as Set | undefined, +}; + +const scales = (overrides: Partial = {}) => + renderHook(() => useChartScales({ ...baseArgs, ...overrides })).result + .current; + +describe('useChartScales y-domain', () => { + it('lets recharts auto-scale from zero with no selection and no fit', () => { + expect(scales().yAxisDomain).toEqual([0, 'auto']); + }); + + it('fits the lower bound to the data minimum, less padding', () => { + // min 10, max 40 -> 5% padding is 1.5. + expect(scales({ fitYAxisToData: true }).yAxisDomain).toEqual([8.5, 41.5]); + }); + + it('does not let the padding drag the axis below zero', () => { + // min 1, max 100 -> padding 4.95 would put the floor at -3.95, and a chart of + // durations whose axis starts below zero reads as broken. The clamp only + // lifts when the data itself goes negative. + expect( + scales({ + fitYAxisToData: true, + graphResults: [{ a: 1, b: 100 }], + }).yAxisDomain, + ).toEqual([0, 104.95]); + }); + + it('follows the data minimum when fitting and the data is negative', () => { + // min -50, max 25 -> 5% of the 75 range is 3.75, applied to both ends. + expect( + scales({ + fitYAxisToData: true, + graphResults: [ + { a: -50, b: -10 }, + { a: 0, b: 25 }, + ], + }).yAxisDomain, + ).toEqual([-53.75, 28.75]); + }); + + it('ignores deselected series when computing the range', () => { + // A alone spans 10..30, so padding is 1 and the domain is [9, 31]. With B + // included it would be [8.5, 41.5] — asserting the exact pair catches a + // wrong padding factor too, which a `toBeLessThan(40)` bound would not. + expect(scales({ selectedSeriesNames: new Set(['A']) }).yAxisDomain).toEqual( + [9, 31], + ); + }); + + it('bars stay anchored at zero even when fitting is requested', () => { + // Otherwise bar lengths stop being proportional to their values. + expect( + scales({ + fitYAxisToData: true, + displayType: DisplayType.StackedBar, + graphResults: [{ a: 100, b: 110 }], + }).yAxisDomain, + ).toEqual([0, 'auto']); + }); + + it('falls back to auto when no numeric values are present', () => { + expect( + scales({ + selectedSeriesNames: new Set(['A']), + graphResults: [{ a: null, b: 'x' }] as Record[], + }).yAxisDomain, + ).toEqual(['auto', 'auto']); + }); +}); + +describe('useChartScales x-domain', () => { + it('spans the requested range in seconds', () => { + const [start, end] = scales().xAxisDomain; + expect(end - start).toBe(3600); + }); + + it('drops the final bucket when the end is exclusive and boundary-aligned', () => { + // An exclusive end that lands exactly on a bucket boundary would otherwise + // render an extra empty bucket at the right edge. + const [, end] = scales({ dateRangeEndInclusive: false }).xAxisDomain; + const [, inclusiveEnd] = scales().xAxisDomain; + expect(inclusiveEnd - end).toBe(60); + }); + + it('pads both edges by half a bucket for bars so the full width fits', () => { + const [start, end] = scales({ + displayType: DisplayType.StackedBar, + }).xAxisDomain; + const [plainStart, plainEnd] = scales().xAxisDomain; + expect(plainStart - start).toBe(30); + expect(end - plainEnd).toBe(30); + }); +}); + +describe('useChartScales annotations', () => { + // `x` is the only thing worth asserting here: the element count is the same + // whatever time the annotation carries, so a test that only checks for a + // non-null result passes even when the time is read from the wrong field and + // every marker lands on NaN. + const markerX = (annotations: ChartAnnotation[]) => + (scales({ annotations }).annotationElements ?? []).map(el => + // A guard rather than a cast: the elements come back as ReactElement with + // unknown props, and asserting the shape would hide a rename of `x`. + isValidElement<{ x: number }>(el) ? el.props.x : undefined, + ); + + it('renders nothing when there are none', () => { + expect(scales().annotationElements).toBeNull(); + expect(scales({ annotations: [] }).annotationElements).toBeNull(); + }); + + it('places a marker at its own time, in the same unix seconds as the domain', () => { + const time = new Date('2026-01-01T00:30:00Z'); + expect(markerX([{ time, label: 'deploy' }])).toEqual([ + time.getTime() / 1000, + ]); + }); + + it('snaps a marker outside the window to the nearest edge', () => { + // An alert already firing when the window opens has a timestamp before the + // range; recharts drops such a marker outright, so it is clamped instead. + const [start, end] = scales().xAxisDomain; + expect( + markerX([ + { time: new Date('2025-12-25T00:00:00Z') }, + { time: new Date('2026-06-01T00:00:00Z') }, + ]), + ).toEqual([start, end]); + }); +}); diff --git a/packages/app/src/HDXMultiSeriesTimeChart/chartData.ts b/packages/app/src/HDXMultiSeriesTimeChart/chartData.ts new file mode 100644 index 0000000000..f2c439c259 --- /dev/null +++ b/packages/app/src/HDXMultiSeriesTimeChart/chartData.ts @@ -0,0 +1,112 @@ +import { type LineData, MAX_TIME_CHART_SERIES } from '@/ChartUtils'; + +export const HARD_LINES_LIMIT = MAX_TIME_CHART_SERIES; + +/** One series entry in a tooltip's per-bucket payload (hover or click-frozen). */ +export type ActiveClickSeries = { + value?: number; + dataKey?: string; + name?: string; + /** Series color, matching the legend swatch. */ + color?: string; + /** Previous-period value at the same bucket, for the percent-change chip. */ + previousValue?: number; + /** Whether this series is a dashed previous-period line. */ + isPreviousPeriod?: boolean; + /** Result column the values came from, for per-column number formatting. */ + valueColumnName?: string; +}; + +/** + * State for the pinned (click-locked) tooltip. Produced by MemoChart's onClick + * and rendered by DBTimeChart via ChartSeriesTooltip. (Hover uses recharts' own + * ; recharts' is also kept for its synced cursor.) + */ +export type ActiveClickPayload = { + /** Active point in viewport coords; the Popover anchor. */ + viewportX: number; + viewportY: number; + activeLabel: string; + activePayload?: ActiveClickSeries[]; +}; + +/** Series label shown in the legend, tooltip, and line `name`. */ +export const getSeriesDisplayName = (ld: LineData) => + ld.displayName || ld.dataKey; + +/** Normalize a chart event's active label (number | string) to a string. */ +export 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 []; + const isPreviousPeriod = ld.previousPeriodKey === ld.dataKey; + // Pair each current-period series with its previous-period value for the + // percent-change chip. Only current-period rows carry a comparison. + const previousRaw = + !isPreviousPeriod && ld.previousPeriodKey + ? activeRow[ld.previousPeriodKey] + : undefined; + return [ + { + dataKey: ld.dataKey, + name: getSeriesDisplayName(ld), + value, + color: ld.color, + isPreviousPeriod, + valueColumnName: ld.valueColumnName, + previousValue: + typeof previousRaw === 'number' ? previousRaw : undefined, + }, + ]; + }); +} + +/** + * Whether a series selection is active. The single source of truth for the + * "isolate to these series" predicate that gates line visibility, the y-axis + * domain, legend dimming, and the "Show All Series" control — so those can't + * drift out of sync. + */ +export function hasSeriesSelection( + selectedSeriesNames: Set | undefined, +): selectedSeriesNames is Set { + return !!selectedSeriesNames && selectedSeriesNames.size > 0; +} + +/** + * The series actually drawn on the chart. Without a selection, the first + * HARD_LINES_LIMIT of lineData. With a selection (legend isolate, checkbox + * filter, or table search), the selection is applied FIRST and then capped, so + * an explicitly chosen series always draws even if it ranks beyond the limit. + * Applying the cap first would slice out a chosen low-ranked series, leaving an + * empty chart while its stats still show in the legend table. The rendered + * lines and the drill-down click payload both derive from this same set so they + * never diverge. Exported for unit testing. + */ +export function getVisibleLineData( + lineData: LineData[], + selectedSeriesNames: Set | undefined, +): LineData[] { + const hasSelection = hasSeriesSelection(selectedSeriesNames); + if (hasSelection) { + return lineData + .filter(ld => selectedSeriesNames.has(getSeriesDisplayName(ld))) + .slice(0, HARD_LINES_LIMIT); + } + return lineData.slice(0, HARD_LINES_LIMIT); +} diff --git a/packages/app/src/HDXMultiSeriesTimeChart/chartPrimitives.tsx b/packages/app/src/HDXMultiSeriesTimeChart/chartPrimitives.tsx new file mode 100644 index 0000000000..1e49260d93 --- /dev/null +++ b/packages/app/src/HDXMultiSeriesTimeChart/chartPrimitives.tsx @@ -0,0 +1,75 @@ +import type { BarProps } from 'recharts'; + +export const StackedBarWithOverlap = (props: BarProps) => { + 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} + fill={fill} + /> + ); +}; + +type CaptureActiveDotProps = { + /** + * Called with each series' active-point pixel Y. This is a stable callback + * (not the ref itself) so Recharts, which stores this element's props in its + * Immer-backed store and freezes them, never freezes the underlying Map — + * the write happens on the ref captured in the callback's closure instead. + */ + onCapture: (dataKey: string, cy: number) => void; + cx?: number; + cy?: number; + dataKey?: string | number; + r?: number; + fill?: string; + stroke?: string; + strokeWidth?: number; +}; + +/** + * Active dot for an Area series. Records the active point's pixel Y (`cy`) + * via `onCapture`, keyed by dataKey, then draws the same dot Recharts + * renders by default. Recharts clones this element with the active-point + * props (cx, cy, dataKey, r, fill, stroke, strokeWidth) during the render + * that precedes the tooltip, so the capture is current when the tooltip reads + * it to find the series nearest the cursor. + */ +export function CaptureActiveDot({ + onCapture, + cx, + cy, + dataKey, + r, + fill, + stroke, + strokeWidth, +}: CaptureActiveDotProps) { + if (dataKey != null && typeof cy === 'number' && Number.isFinite(cy)) { + // Written synchronously during render so the tooltip, which Recharts + // renders after the graphical items in the same commit, reads the + // current frame's positions rather than the previous frame's. + onCapture(String(dataKey), cy); + } + if (typeof cx !== 'number' || typeof cy !== 'number') { + return null; + } + return ( + + ); +} diff --git a/packages/app/src/HDXMultiSeriesTimeChart/constants.ts b/packages/app/src/HDXMultiSeriesTimeChart/constants.ts new file mode 100644 index 0000000000..68a22908af --- /dev/null +++ b/packages/app/src/HDXMultiSeriesTimeChart/constants.ts @@ -0,0 +1,19 @@ +/** Layout and measurement constants shared across the chart's parts. */ +export const MAX_LEGEND_ITEMS = 4; + +// Vertical pixel distance within which a series' line counts as "near" the +// cursor for tooltip highlighting. Beyond this, no row is emphasized so the +// tooltip is not misleading when the pointer is in empty space. +export const NEAREST_SERIES_MAX_DISTANCE_PX = 30; + +// Gap below the data point for the hover tooltip. Kept equal to the pinned +// tooltip's Popover `offset` so both land in the same spot. +export const TOOLTIP_POINT_OFFSET_PX = 12; + +export const Y_AXIS_WIDTH = 40; +export const SINGLE_POINT_BAR_RIGHT_PADDING = 10; +export const SINGLE_POINT_BAR_WIDTH_RATIO = 0.8; +// Top margin (px) reserved above the plot for annotation labels ("Alert"/"OK"), +// added only when a chart is showing annotations so other charts keep their +// tighter default headroom. +export const ANNOTATION_LABEL_HEADROOM = 18; diff --git a/packages/app/src/HDXMultiSeriesTimeChart/index.ts b/packages/app/src/HDXMultiSeriesTimeChart/index.ts new file mode 100644 index 0000000000..d36c501c42 --- /dev/null +++ b/packages/app/src/HDXMultiSeriesTimeChart/index.ts @@ -0,0 +1,16 @@ +/** + * Public surface of the multi-series time chart. Split out of a single + * 1500-line module; consumers (and the tests that + * `jest.mock('@/HDXMultiSeriesTimeChart')`) import from here, so the internal + * file layout stays free to change. + */ +export { + type ActiveClickPayload, + type ActiveClickSeries, + buildActiveClickSeries, + getVisibleLineData, + HARD_LINES_LIMIT, +} from './chartData'; +export { TOOLTIP_POINT_OFFSET_PX } from './constants'; +export { collectMemoChartGradientHexes, MemoChart } from './MemoChart'; +export { TooltipItem } from './TooltipItem'; diff --git a/packages/app/src/HDXMultiSeriesTimeChart/useChartScales.ts b/packages/app/src/HDXMultiSeriesTimeChart/useChartScales.ts new file mode 100644 index 0000000000..1a76b324c0 --- /dev/null +++ b/packages/app/src/HDXMultiSeriesTimeChart/useChartScales.ts @@ -0,0 +1,144 @@ +import { useMemo } from 'react'; +import { add, isSameSecond, sub } from 'date-fns'; +import { AxisDomain } from 'recharts/types/util/types'; +import { convertGranularityToSeconds } from '@hyperdx/common-utils/dist/core/utils'; +import { DisplayType } from '@hyperdx/common-utils/dist/types'; + +import { type LineData, toStartOfInterval } from '@/ChartUtils'; +import { + ChartAnnotation, + getAnnotationElements, +} from '@/components/charts/chartAnnotations'; + +import { hasSeriesSelection } from './chartData'; + +type UseChartScalesArgs = { + annotations: ChartAnnotation[] | undefined; + dateRange: readonly [Date, Date]; + granularity: string; + dateRangeEndInclusive: boolean; + displayType: DisplayType; + fitYAxisToData: boolean | undefined; + graphResults: Record[]; + lineData: LineData[]; + selectedSeriesNames: Set | undefined; +}; + +/** + * Derive the chart's axis domains and the annotation elements that hang off the + * x-domain. + * + * Extracted from MemoChart because it is pure derivation from props — no state, + * no event handlers, no recharts tree — and the y-domain rules (fit-to-data, + * legend selection, zero-anchored bars) are easier to follow on their own. + */ +export function useChartScales({ + annotations, + dateRange, + granularity, + dateRangeEndInclusive, + displayType, + fitYAxisToData, + graphResults, + lineData, + selectedSeriesNames, +}: UseChartScalesArgs) { + const yAxisDomain: AxisDomain = useMemo(() => { + const hasSelection = hasSeriesSelection(selectedSeriesNames); + + // Fitting the y-axis lower bound to the data only applies to line charts. + // Bar charts are always anchored at zero so the bar lengths stay + // proportional to their values. + const shouldFitYAxis = + fitYAxisToData && displayType !== DisplayType.StackedBar; + + // The domain follows the visible series only. With no selection and no fit, + // let Recharts auto-scale, which pins the lower bound to 0. + if (!hasSelection && !shouldFitYAxis) { + return [0, 'auto']; + } + + // Calculate domain based on visible series (all series when there's no + // explicit selection). + let minValue = Infinity; + let maxValue = -Infinity; + + graphResults.forEach(dataPoint => { + lineData.forEach(ld => { + const seriesName = ld.displayName || ld.dataKey; + // Only consider visible series + if (!hasSelection || selectedSeriesNames.has(seriesName)) { + const value = dataPoint[ld.dataKey]; + if (typeof value === 'number' && !isNaN(value)) { + minValue = Math.min(minValue, value); + maxValue = Math.max(maxValue, value); + } + } + }); + }); + + // If we found valid values, return them with some padding + if (minValue !== Infinity && maxValue !== -Infinity) { + const padding = (maxValue - minValue) * 0.05; // 5% padding + // When fitting to data, allow the lower bound to follow the data + // minimum; otherwise keep it pinned at zero. The 5% padding must not + // drag the axis below zero unless the data itself is negative, so + // clamp at zero whenever the minimum is non-negative. + const lowerBound = + shouldFitYAxis && minValue < 0 + ? minValue - padding + : Math.max(0, minValue - padding); + const upperBound = maxValue + padding; + return [lowerBound, upperBound]; + } + + return ['auto', 'auto']; + }, [ + graphResults, + lineData, + selectedSeriesNames, + fitYAxisToData, + displayType, + ]); + + // Typed as the tuple it actually returns rather than the wider AxisDomain, so + // the annotation elements below can read [min, max] without asserting. Still + // assignable to XAxis's `domain`. + const xAxisDomain: [number, number] = useMemo(() => { + let startTime = toStartOfInterval(dateRange[0], granularity); + let endTime = toStartOfInterval(dateRange[1], granularity); + const endTimeIsBoundaryAligned = isSameSecond(dateRange[1], endTime); + if (endTimeIsBoundaryAligned && !dateRangeEndInclusive) { + endTime = sub(endTime, { + seconds: convertGranularityToSeconds(granularity), + }); + } + + // For bar charts, extend the domain in both directions by half a granularity unit + // so that the full bar width is within the bounds of the chart + if (displayType === DisplayType.StackedBar) { + const halfGranularitySeconds = + convertGranularityToSeconds(granularity) / 2; + startTime = sub(startTime, { seconds: halfGranularitySeconds }); + endTime = add(endTime, { seconds: halfGranularitySeconds }); + } + + return [startTime.getTime() / 1000, endTime.getTime() / 1000]; + }, [dateRange, granularity, dateRangeEndInclusive, displayType]); + + // Alert/event markers as dashed lines, clamped to the chart's x-axis domain so + // an edge marker (e.g. an alert already firing at window open) stays visible + // instead of being dropped. Labels float in the reserved top headroom. + const annotationElements = useMemo(() => { + if (!annotations?.length) { + return null; + } + return getAnnotationElements(annotations, { domain: xAxisDomain }); + }, [annotations, xAxisDomain]); + + return { + yAxisDomain, + xAxisDomain, + annotationElements, + }; +} diff --git a/packages/app/src/components/DBTimeChart/ChartTooltipOverlay.tsx b/packages/app/src/components/DBTimeChart/ChartTooltipOverlay.tsx new file mode 100644 index 0000000000..c48162269a --- /dev/null +++ b/packages/app/src/components/DBTimeChart/ChartTooltipOverlay.tsx @@ -0,0 +1,157 @@ +import { useEffect, useRef } from 'react'; +import { NumberFormat } from '@hyperdx/common-utils/dist/types'; +import { Popover, Portal } from '@mantine/core'; + +import { ChartSeriesTooltip } from '@/components/charts/ChartSeriesTooltip'; +import { useChartTooltipZIndex } from '@/components/charts/ChartTooltip'; +import { + type ActiveClickPayload, + TOOLTIP_POINT_OFFSET_PX, +} from '@/HDXMultiSeriesTimeChart'; + +// The interactive PINNED tooltip, rendered over the chart in a body-portaled +// Mantine Popover anchored at the clicked point. Hover uses the recharts tooltip +// in MemoChart instead; this is only for the click-locked state. +export function ChartTooltipOverlay({ + payload, + buildSearchUrl, + onDismiss, + onFocusSeries, + onShowAllSeries, + fallbackNumberFormat, + numberFormatByKey, + previousPeriodOffsetSeconds, +}: { + payload: ActiveClickPayload | undefined; + buildSearchUrl: (key?: string, value?: number) => string | null; + onDismiss: () => void; + /** Focus a series by its raw series key (dataKey) and display name. */ + onFocusSeries: (payload: { dataKey?: string; name: string }) => void; + /** Clear an active series focus; undefined when nothing is focused. */ + onShowAllSeries?: () => void; + fallbackNumberFormat?: NumberFormat; + /** Per-value-column formats, keyed by result column name. */ + numberFormatByKey: Map; + previousPeriodOffsetSeconds?: number; +}) { + const isOpen = + payload != null && + payload.activePayload != null && + payload.activePayload.length > 0; + + const popoverZIndex = useChartTooltipZIndex({ pinned: true }); + + const dropdownRef = useRef(null); + + // The pinned tooltip anchors at `position: fixed` viewport coords captured + // once at click time. When a surrounding scroll container scrolls, the chart + // moves but the fixed tooltip stays glued to the viewport, detaching from its + // data point (Mantine's closeOnClickOutside/closeOnEscape don't fire on + // scroll). Dismiss on scroll instead so it never floats away — but ignore + // scrolls originating inside the tooltip's own scrollable series list, or a + // long tooltip couldn't be scrolled without instantly closing. + useEffect(() => { + if (!isOpen) return; + const handleScroll = (e: Event) => { + const target = e.target as Node | null; + if (target != null && dropdownRef.current?.contains(target)) { + return; + } + onDismiss(); + }; + window.addEventListener('scroll', handleScroll, { + capture: true, + passive: true, + }); + return () => { + window.removeEventListener('scroll', handleScroll, { capture: true }); + }; + }, [isOpen, onDismiss]); + + // Dismiss on outside click. Mantine's closeOnClickOutside misses it because + // the chart's recharts onClick calls stopPropagation (see + // HDXMultiSeriesTimeChart handleClick); a capture-phase listener sees the + // click regardless, ignoring clicks inside the tooltip's own dropdown. + useEffect(() => { + if (!isOpen) return; + const handleMouseDown = (e: MouseEvent) => { + const target = e.target; + if (target instanceof Node && dropdownRef.current?.contains(target)) { + return; + } + onDismiss(); + }; + document.addEventListener('mousedown', handleMouseDown, true); + return () => { + document.removeEventListener('mousedown', handleMouseDown, true); + }; + }, [isOpen, onDismiss]); + + if (!isOpen) { + return null; + } + + return ( + // Portal to body so the `position: fixed` anchor resolves against the + // viewport: dashboard tiles use CSS transforms, and a transformed ancestor + // would otherwise make `fixed` resolve against it and throw the tooltip off. + + { + if (!opened) { + onDismiss(); + } + }} + closeOnClickOutside + closeOnEscape + trapFocus={false} + withinPortal + position="bottom" + // Same gap the hover tooltip uses, so the two states don't sit at + // different distances from the point. + offset={TOOLTIP_POINT_OFFSET_PX} + middlewares={{ flip: true, shift: true }} + returnFocus={false} + zIndex={popoverZIndex} + > + + {/* 1x1 anchor at the clicked data point. */} +
+ + + + + + + ); +} diff --git a/packages/app/src/components/DBTimeChart.tsx b/packages/app/src/components/DBTimeChart/DBTimeChart.tsx similarity index 50% rename from packages/app/src/components/DBTimeChart.tsx rename to packages/app/src/components/DBTimeChart/DBTimeChart.tsx index 8d1b99ccb4..63b6fef370 100644 --- a/packages/app/src/components/DBTimeChart.tsx +++ b/packages/app/src/components/DBTimeChart/DBTimeChart.tsx @@ -1,35 +1,15 @@ -import React, { - memo, - useCallback, - useEffect, - useId, - useMemo, - useRef, - useState, -} from 'react'; -import { add, differenceInSeconds } from 'date-fns'; -import { - convertGranularityToSeconds, - getAlignedDateRange, -} from '@hyperdx/common-utils/dist/core/utils'; -import { - isBuilderChartConfig, - isPromqlChartConfig, - isRawSqlChartConfig, -} from '@hyperdx/common-utils/dist/guards'; +import React, { memo, useCallback, useEffect, useMemo, useState } from 'react'; +import { differenceInSeconds } from 'date-fns'; +import { getAlignedDateRange } from '@hyperdx/common-utils/dist/core/utils'; +import { isBuilderChartConfig } from '@hyperdx/common-utils/dist/guards'; import { BuilderChartConfigWithDateRange, ChartConfigWithDateRange, DisplayType, } from '@hyperdx/common-utils/dist/types'; -import { Popover, Portal } from '@mantine/core'; -import { IconChartBar, IconChartLine } from '@tabler/icons-react'; import api from '@/api'; import { - AGG_FNS, - buildEventsSearchUrl, - ChartKeyJoiner, convertToTimeChartConfig, formatResponseForTimeChart, getPreviousDateRange, @@ -37,240 +17,23 @@ import { useTimeChartSettings, } from '@/ChartUtils'; import { ChartAnnotation } from '@/components/charts/chartAnnotations'; -import { ChartSeriesTooltip } from '@/components/charts/ChartSeriesTooltip'; -import { useChartTooltipZIndex } from '@/components/charts/ChartTooltip'; +import ChartContainer from '@/components/charts/ChartContainer'; +import ChartErrorState, { + ChartErrorStateVariant, +} from '@/components/charts/ChartErrorState'; import { type ActiveClickPayload, MemoChart } from '@/HDXMultiSeriesTimeChart'; import { useQueriedChartConfig } from '@/hooks/useChartConfig'; import { useMVOptimizationExplanation } from '@/hooks/useMVOptimizationExplanation'; import { useChartNumberFormats, useSource } from '@/source'; -import type { NumberFormat } from '@/types'; - -import ChartContainer from './charts/ChartContainer'; -import ChartErrorState, { - ChartErrorStateVariant, -} from './charts/ChartErrorState'; -import DateRangeIndicator from './charts/DateRangeIndicator'; -import DisplaySwitcher from './charts/DisplaySwitcher'; -import MVOptimizationIndicator from './MaterializedViews/MVOptimizationIndicator'; - -/** A single group column / value pair decoded from a chart series key. */ -export type SeriesGroupFilter = { column: string; value: string }; - -// Only one pinned tooltip at a time across all charts. Module-level (not -// context) because charts can be scattered with no common provider, and their -// onClick stopPropagation hides cross-chart clicks from Mantine's click-outside. -const pinnedTooltipRegistry = new Map void>(); - -function broadcastTooltipPinned(activeId: string) { - pinnedTooltipRegistry.forEach((dismiss, id) => { - if (id !== activeId) { - dismiss(); - } - }); -} - -// Registers this chart's dismiss handler and returns a callback to close every -// other chart's pinned tooltip (call it when pinning this one). -function useCrossChartPinDismiss(onDismiss: () => void): () => void { - const id = useId(); - // Keep the latest onDismiss without re-subscribing each render. - const onDismissRef = useRef(onDismiss); - useEffect(() => { - onDismissRef.current = onDismiss; - }, [onDismiss]); - useEffect(() => { - pinnedTooltipRegistry.set(id, () => onDismissRef.current()); - return () => { - pinnedTooltipRegistry.delete(id); - }; - }, [id]); - - return useCallback(() => broadcastTooltipPinned(id), [id]); -} - -// Decode a Recharts series key (e.g. "count · error · api") into the -// underlying group-column filters. This is the same decode `buildSearchUrl` -// uses to build a drill-down URL, extracted so the focus callback can hand the -// caller structured filters (rather than a display string) to apply to a -// sibling results list. -export function decodeSeriesGroupFilters({ - seriesKey, - groupColumns, - isSingleValueColumn, -}: { - seriesKey: string | undefined; - groupColumns: string[]; - isSingleValueColumn: boolean | undefined; -}): SeriesGroupFilter[] { - const seriesKeys = seriesKey?.split(ChartKeyJoiner); - const groupFilters: SeriesGroupFilter[] = []; - - if (seriesKeys?.length && groupColumns?.length) { - // When the series has multiple value columns, the key is prefixed with the - // value column name (e.g. "count · error"), so the group values start at - // index 1. (The "no group columns" case the original inline code also - // guarded is impossible here — this block only runs when groupColumns is - // non-empty.) - const startsWithValueColumn = !(isSingleValueColumn ?? true); - const groupValues = startsWithValueColumn - ? seriesKeys.slice(1) - : seriesKeys; - - groupValues.forEach((value, index) => { - if (groupColumns[index] != null) { - groupFilters.push({ column: groupColumns[index], value }); - } - }); - } - - return groupFilters; -} - -// The interactive PINNED tooltip, rendered over the chart in a body-portaled -// Mantine Popover anchored at the clicked point. Hover uses the recharts tooltip -// in MemoChart instead; this is only for the click-locked state. -function ChartTooltipOverlay({ - payload, - buildSearchUrl, - onDismiss, - onFocusSeries, - onShowAllSeries, - fallbackNumberFormat, - numberFormatByKey, - previousPeriodOffsetSeconds, -}: { - payload: ActiveClickPayload | undefined; - buildSearchUrl: (key?: string, value?: number) => string | null; - onDismiss: () => void; - /** Focus a series by its raw series key (dataKey) and display name. */ - onFocusSeries: (payload: { dataKey?: string; name: string }) => void; - /** Clear an active series focus; undefined when nothing is focused. */ - onShowAllSeries?: () => void; - fallbackNumberFormat?: NumberFormat; - /** Per-value-column formats, keyed by result column name. */ - numberFormatByKey: Map; - previousPeriodOffsetSeconds?: number; -}) { - const isOpen = - payload != null && - payload.activePayload != null && - payload.activePayload.length > 0; - - const popoverZIndex = useChartTooltipZIndex({ pinned: true }); - - const dropdownRef = useRef(null); - - // The pinned tooltip anchors at `position: fixed` viewport coords captured - // once at click time. When a surrounding scroll container scrolls, the chart - // moves but the fixed tooltip stays glued to the viewport, detaching from its - // data point (Mantine's closeOnClickOutside/closeOnEscape don't fire on - // scroll). Dismiss on scroll instead so it never floats away — but ignore - // scrolls originating inside the tooltip's own scrollable series list, or a - // long tooltip couldn't be scrolled without instantly closing. - useEffect(() => { - if (!isOpen) return; - const handleScroll = (e: Event) => { - const target = e.target as Node | null; - if (target != null && dropdownRef.current?.contains(target)) { - return; - } - onDismiss(); - }; - window.addEventListener('scroll', handleScroll, { - capture: true, - passive: true, - }); - return () => { - window.removeEventListener('scroll', handleScroll, { capture: true }); - }; - }, [isOpen, onDismiss]); - - // Dismiss on outside click. Mantine's closeOnClickOutside misses it because - // the chart's recharts onClick calls stopPropagation (see - // HDXMultiSeriesTimeChart handleClick); a capture-phase listener sees the - // click regardless, ignoring clicks inside the tooltip's own dropdown. - useEffect(() => { - if (!isOpen) return; - const handleMouseDown = (e: MouseEvent) => { - const target = e.target; - if (target instanceof Node && dropdownRef.current?.contains(target)) { - return; - } - onDismiss(); - }; - document.addEventListener('mousedown', handleMouseDown, true); - return () => { - document.removeEventListener('mousedown', handleMouseDown, true); - }; - }, [isOpen, onDismiss]); - - if (!isOpen) { - return null; - } - - return ( - // Portal to body so the `position: fixed` anchor resolves against the - // viewport: dashboard tiles use CSS transforms, and a transformed ancestor - // would otherwise make `fixed` resolve against it and throw the tooltip off. - - { - if (!opened) { - onDismiss(); - } - }} - closeOnClickOutside - closeOnEscape - trapFocus={false} - withinPortal - position="bottom" - offset={12} - middlewares={{ flip: true, shift: true }} - returnFocus={false} - zIndex={popoverZIndex} - > - - {/* 1x1 anchor at the clicked data point. */} -
- - - - - - - ); -} +import { ChartTooltipOverlay } from './ChartTooltipOverlay'; +import { useCrossChartPinDismiss } from './crossChartPin'; +import { + buildSeriesSearchUrl, + decodeSeriesGroupFilters, + type SeriesGroupFilter, +} from './searchUrl'; +import { useChartToolbarItems } from './useChartToolbarItems'; type DBTimeChartComponentProps = { config: ChartConfigWithDateRange; @@ -553,7 +316,9 @@ function DBTimeChartComponent({ ActiveClickPayload | undefined >(undefined); - const dismissPinned = useCallback(() => setActiveClickPayload(undefined), []); + const dismissPinned = useCallback(() => { + setActiveClickPayload(undefined); + }, []); const notifyTooltipPinned = useCrossChartPinDismiss(dismissPinned); // Pin the tooltip on click. Not gated on `source`: source-less charts still @@ -580,104 +345,18 @@ function DBTimeChartComponent({ }, [activeClickPayload]); const buildSearchUrl = useCallback( - (seriesKey?: string, seriesValue?: number) => { - // Raw SQL charts are not supported for drill-down as we don't know the source which is being used. - if ( - clickedActiveLabelDate == null || - source == null || - isRawSqlChartConfig(config) || - isPromqlChartConfig(config) - ) { - return null; - } - - // Parse the series key to extract group values - const seriesKeys = seriesKey?.split(ChartKeyJoiner); - const groupFilters = decodeSeriesGroupFilters({ + (seriesKey?: string, seriesValue?: number) => + buildSeriesSearchUrl({ seriesKey, - groupColumns, - isSingleValueColumn, - }); - - // Build value range filter for Y-axis if provided - let valueRangeFilter: - | { - expression: string; - value: number; - } - | undefined; - - if ( - seriesValue && - Array.isArray(config.select) && - config.select.length > 0 - ) { - // Determine which value column to filter on - let valueExpression: string | undefined; - - if ((isSingleValueColumn ?? true) && config.select.length === 1) { - const firstSelect = config.select[0]; - const aggFn = - typeof firstSelect === 'string' ? undefined : firstSelect.aggFn; - // Only add value range filter if the aggregation is attributable - const isAttributable = - AGG_FNS.find(fn => fn.value === aggFn)?.isAttributable !== false; - - if (isAttributable) { - valueExpression = - typeof firstSelect === 'string' - ? firstSelect - : firstSelect.valueExpression; - } - } else if (seriesKeys?.length && (valueColumns?.length ?? 0) > 0) { - const firstPart = seriesKeys[0]; - const valueColumnIndex = valueColumns?.findIndex( - col => col === firstPart, - ); - - if ( - valueColumnIndex != null && - valueColumnIndex >= 0 && - valueColumnIndex < config.select.length - ) { - const selectItem = config.select[valueColumnIndex]; - const aggFn = - typeof selectItem === 'string' ? undefined : selectItem.aggFn; - // Only add value range filter if the aggregation is attributable - const isAttributable = - AGG_FNS.find(fn => fn.value === aggFn)?.isAttributable !== false; - - if (isAttributable) { - valueExpression = - typeof selectItem === 'string' - ? selectItem - : selectItem.valueExpression; - } - } - } - - if (valueExpression) { - valueRangeFilter = { - expression: valueExpression, - value: seriesValue, - }; - } - } - - // Calculate time range from clicked date and granularity - const from = clickedActiveLabelDate; - const to = add(clickedActiveLabelDate, { - seconds: convertGranularityToSeconds(granularity), - }); - - return buildEventsSearchUrl({ + seriesValue, + clickedActiveLabelDate, source, config, - dateRange: [from, to], - groupFilters, - valueRangeFilter, - }); - }, + granularity, + groupColumns, + valueColumns, + isSingleValueColumn, + }), [ clickedActiveLabelDate, config, @@ -712,91 +391,20 @@ function DBTimeChartComponent({ [onFocusSeries, groupColumns, isSingleValueColumn, handleToggleSeries], ); - const toolbarItemsMemo = useMemo(() => { - const allToolbarItems = []; - - if (toolbarPrefix && toolbarPrefix.length > 0) { - allToolbarItems.push(...toolbarPrefix); - } - - if (source && showMVOptimizationIndicator && builderQueriedConfig) { - allToolbarItems.push( - , - ); - } - - const mvDateRange = mvOptimizationData?.optimizedConfig?.dateRange; - const isAlignedToChartGranularity = - queriedConfig.alignDateRangeToGranularity !== false; - - if ( - showDateRangeIndicator && - (mvDateRange || isAlignedToChartGranularity) - ) { - const mvGranularity = isAlignedToChartGranularity - ? undefined - : mvOptimizationData?.explanations.find(e => e.success)?.mvConfig - .minGranularity; - - allToolbarItems.push( - , - ); - } - - if (showDisplaySwitcher) { - allToolbarItems.push( - , - }, - { - value: DisplayType.StackedBar, - label: config.compareToPreviousPeriod - ? 'Bar Chart Unavailable When Comparing to Previous Period' - : 'Display as Bar Chart', - icon: , - disabled: config.compareToPreviousPeriod, - }, - ]} - />, - ); - } - - if (toolbarSuffix && toolbarSuffix.length > 0) { - allToolbarItems.push(...toolbarSuffix); - } - - return allToolbarItems; - }, [ + const toolbarItemsMemo = useChartToolbarItems({ builderQueriedConfig, config, displayType, handleSetDisplayType, + mvOptimizationData, + queriedConfig, + showDateRangeIndicator, showDisplaySwitcher, + showMVOptimizationIndicator, source, toolbarPrefix, toolbarSuffix, - showMVOptimizationIndicator, - showDateRangeIndicator, - mvOptimizationData, - queriedConfig, - ]); + }); return ( diff --git a/packages/app/src/components/__tests__/DBTimeChart.test.tsx b/packages/app/src/components/DBTimeChart/__tests__/DBTimeChart.test.tsx similarity index 98% rename from packages/app/src/components/__tests__/DBTimeChart.test.tsx rename to packages/app/src/components/DBTimeChart/__tests__/DBTimeChart.test.tsx index 1e86e1e60d..055849c387 100644 --- a/packages/app/src/components/__tests__/DBTimeChart.test.tsx +++ b/packages/app/src/components/DBTimeChart/__tests__/DBTimeChart.test.tsx @@ -39,11 +39,11 @@ jest.mock('@/source', () => ({ .mockReturnValue({ formatByColumn: new Map(), chartFormat: undefined }), })); -jest.mock('../MaterializedViews/MVOptimizationIndicator', () => +jest.mock('@/components/MaterializedViews/MVOptimizationIndicator', () => jest.fn(() => null), ); -jest.mock('../charts/DateRangeIndicator', () => jest.fn(() => null)); +jest.mock('@/components/charts/DateRangeIndicator', () => jest.fn(() => null)); describe('DBTimeChart', () => { const mockUseQueriedChartConfig = useQueriedChartConfig as jest.Mock; diff --git a/packages/app/src/components/DBTimeChart/__tests__/crossChartPin.test.tsx b/packages/app/src/components/DBTimeChart/__tests__/crossChartPin.test.tsx new file mode 100644 index 0000000000..03273a7396 --- /dev/null +++ b/packages/app/src/components/DBTimeChart/__tests__/crossChartPin.test.tsx @@ -0,0 +1,69 @@ +import { renderHook } from '@testing-library/react'; + +import { useCrossChartPinDismiss } from '@/components/DBTimeChart/crossChartPin'; + +/** + * The registry backing this hook is module-level and survives renders, unmounts + * and every test in this file's worker — deliberately, because charts can be + * scattered with no common provider. That makes leaks between consumers a real + * failure mode rather than a theoretical one, so the cases below cover both + * directions: who gets dismissed, and who stops being reachable. + */ +describe('useCrossChartPinDismiss', () => { + it('dismisses the other chart but not the one doing the pinning', () => { + const dismissA = jest.fn(); + const dismissB = jest.fn(); + + const a = renderHook(() => useCrossChartPinDismiss(dismissA)); + renderHook(() => useCrossChartPinDismiss(dismissB)); + + a.result.current(); + + expect(dismissB).toHaveBeenCalledTimes(1); + expect(dismissA).not.toHaveBeenCalled(); + }); + + it('stops calling a consumer once it unmounts', () => { + const dismissA = jest.fn(); + const dismissB = jest.fn(); + + const a = renderHook(() => useCrossChartPinDismiss(dismissA)); + const b = renderHook(() => useCrossChartPinDismiss(dismissB)); + + b.unmount(); + a.result.current(); + + // A stale entry here would call into an unmounted component's setState on + // every pin, for the life of the page. + expect(dismissB).not.toHaveBeenCalled(); + }); + + it('calls the latest callback, not the one from the first render', () => { + const first = jest.fn(); + const second = jest.fn(); + + const a = renderHook(() => useCrossChartPinDismiss(jest.fn())); + const b = renderHook(({ cb }) => useCrossChartPinDismiss(cb), { + initialProps: { cb: first }, + }); + + b.rerender({ cb: second }); + a.result.current(); + + expect(second).toHaveBeenCalledTimes(1); + expect(first).not.toHaveBeenCalled(); + }); + + it('dismisses every other consumer, not just one', () => { + const dismissA = jest.fn(); + const others = [jest.fn(), jest.fn(), jest.fn()]; + + const a = renderHook(() => useCrossChartPinDismiss(dismissA)); + others.forEach(cb => renderHook(() => useCrossChartPinDismiss(cb))); + + a.result.current(); + + others.forEach(cb => expect(cb).toHaveBeenCalledTimes(1)); + expect(dismissA).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/app/src/components/DBTimeChart/__tests__/searchUrl.test.ts b/packages/app/src/components/DBTimeChart/__tests__/searchUrl.test.ts new file mode 100644 index 0000000000..04a9355f13 --- /dev/null +++ b/packages/app/src/components/DBTimeChart/__tests__/searchUrl.test.ts @@ -0,0 +1,213 @@ +import { + type ChartConfigWithDateRange, + SourceKind, + TSource, +} from '@hyperdx/common-utils/dist/types'; + +import { buildSeriesSearchUrl } from '@/components/DBTimeChart'; + +// The URL string itself is ChartUtils' concern and already covered there. What +// matters here is the branching this function does before delegating — which was +// previously locked inside a useCallback and untestable. +type SearchUrlArgs = { + dateRange: [Date, Date]; + groupFilters: { column: string; value: string }[]; + valueRangeFilter?: { expression: string; value: number }; +}; + +const mockBuildEventsSearchUrl = jest.fn( + () => '/search?mocked=1', +); +jest.mock('@/ChartUtils', () => ({ + ...jest.requireActual('@/ChartUtils'), + buildEventsSearchUrl: (arg: SearchUrlArgs) => mockBuildEventsSearchUrl(arg), +})); + +/** Args the function handed to buildEventsSearchUrl on its first call. */ +const delegatedArgs = () => mockBuildEventsSearchUrl.mock.calls[0][0]; + +const source = { + id: 'src-1', + kind: SourceKind.Log, + name: 'logs', + connection: 'conn-1', + from: { databaseName: 'default', tableName: 'otel_logs' }, + timestampValueExpression: 'Timestamp', +} as unknown as TSource; + +const clicked = new Date('2026-01-01T00:00:00Z'); + +const args = { + clickedActiveLabelDate: clicked as Date | undefined, + source: source as TSource | undefined, + granularity: '1 minute', + groupColumns: [] as string[], + valueColumns: undefined as string[] | undefined, + isSingleValueColumn: true as boolean | undefined, +}; + +const configWith = ( + select: { aggFn: string; valueExpression: string }[], +): ChartConfigWithDateRange => + ({ + connection: 'conn-1', + from: { databaseName: 'default', tableName: 'otel_logs' }, + timestampValueExpression: 'Timestamp', + where: '', + select, + dateRange: [clicked, clicked], + }) as ChartConfigWithDateRange; + +const rawSqlConfig = { + connection: 'conn-1', + configType: 'sql', + sqlTemplate: 'SELECT 1', + dateRange: [clicked, clicked], +} satisfies Partial as ChartConfigWithDateRange; + +const promqlConfig = { + connection: 'conn-1', + configType: 'promql', + promqlExpression: 'up', + dateRange: [clicked, clicked], +} satisfies Partial as ChartConfigWithDateRange; + +beforeEach(() => jest.clearAllMocks()); + +describe('buildSeriesSearchUrl', () => { + describe('returns null when drill-down cannot be supported', () => { + it('with no clicked date', () => { + expect( + buildSeriesSearchUrl({ + ...args, + clickedActiveLabelDate: undefined, + config: configWith([{ aggFn: 'avg', valueExpression: 'Duration' }]), + }), + ).toBeNull(); + expect(mockBuildEventsSearchUrl).not.toHaveBeenCalled(); + }); + + it('with no resolved source', () => { + expect( + buildSeriesSearchUrl({ + ...args, + source: undefined, + config: configWith([{ aggFn: 'avg', valueExpression: 'Duration' }]), + }), + ).toBeNull(); + }); + + it('for a raw SQL chart', () => { + // Raw SQL doesn't resolve to a single source, so there is nothing to search. + expect( + buildSeriesSearchUrl({ + ...args, + config: rawSqlConfig, + }), + ).toBeNull(); + }); + + it('for a PromQL chart', () => { + // Same reason as raw SQL, and covered separately because the two share one + // condition — without this, dropping the isPromqlChartConfig arm stays green. + expect( + buildSeriesSearchUrl({ + ...args, + config: promqlConfig, + }), + ).toBeNull(); + }); + }); + + it('ranges from the clicked bucket to one granularity later', () => { + buildSeriesSearchUrl({ + ...args, + config: configWith([{ aggFn: 'avg', valueExpression: 'Duration' }]), + }); + const { dateRange } = delegatedArgs(); + expect(dateRange[0]).toEqual(clicked); + expect(dateRange[1].getTime() - clicked.getTime()).toBe(60_000); + }); + + describe('value-range filter', () => { + it('is added for an attributable aggregation', () => { + buildSeriesSearchUrl({ + ...args, + seriesValue: 250, + config: configWith([{ aggFn: 'max', valueExpression: 'Duration' }]), + }); + const { valueRangeFilter } = delegatedArgs(); + expect(valueRangeFilter).toEqual({ + expression: 'Duration', + value: 250, + }); + }); + + it('is omitted for a non-attributable aggregation', () => { + // A `count`/`sum` point is not attributable to any single event's value, so + // filtering on it would return rows that never contributed to the point. + buildSeriesSearchUrl({ + ...args, + seriesValue: 250, + config: configWith([{ aggFn: 'count', valueExpression: 'Duration' }]), + }); + const { valueRangeFilter } = delegatedArgs(); + expect(valueRangeFilter).toBeUndefined(); + }); + + it('is omitted when no series value was clicked', () => { + buildSeriesSearchUrl({ + ...args, + config: configWith([{ aggFn: 'max', valueExpression: 'Duration' }]), + }); + const { valueRangeFilter } = delegatedArgs(); + expect(valueRangeFilter).toBeUndefined(); + }); + + it('is added for a clicked value of zero', () => { + // Zero is a real point — buildActiveClickSeries keeps it rather than + // dropping it — so a truthiness guard here would silently widen the + // drill-down to every event in the bucket. + buildSeriesSearchUrl({ + ...args, + seriesValue: 0, + config: configWith([{ aggFn: 'max', valueExpression: 'Duration' }]), + }); + const { valueRangeFilter } = delegatedArgs(); + expect(valueRangeFilter).toEqual({ + expression: 'Duration', + value: 0, + }); + }); + + it('resolves the value column by series-key prefix on a multi-value chart', () => { + // With more than one value column the series key is prefixed with the + // column name, so the filter must follow that prefix to the right select + // item rather than defaulting to select[0]. + buildSeriesSearchUrl({ + ...args, + seriesKey: 'p95', + seriesValue: 900, + isSingleValueColumn: false, + valueColumns: ['count', 'p95'], + config: configWith([ + { aggFn: 'count', valueExpression: 'Body' }, + { aggFn: 'p95', valueExpression: 'Duration' }, + ]), + }); + const { valueRangeFilter } = delegatedArgs(); + expect(valueRangeFilter).toEqual({ expression: 'Duration', value: 900 }); + }); + }); + + it('passes decoded group filters through', () => { + buildSeriesSearchUrl({ + ...args, + seriesKey: 'api', + groupColumns: ['ServiceName'], + config: configWith([{ aggFn: 'avg', valueExpression: 'Duration' }]), + }); + const { groupFilters } = delegatedArgs(); + expect(groupFilters).toEqual([{ column: 'ServiceName', value: 'api' }]); + }); +}); diff --git a/packages/app/src/components/DBTimeChart/crossChartPin.ts b/packages/app/src/components/DBTimeChart/crossChartPin.ts new file mode 100644 index 0000000000..1fda2b8a08 --- /dev/null +++ b/packages/app/src/components/DBTimeChart/crossChartPin.ts @@ -0,0 +1,34 @@ +import { useCallback, useEffect, useId, useRef } from 'react'; + +// Only one pinned tooltip at a time across all charts. Module-level (not +// context) because charts can be scattered with no common provider, and their +// onClick stopPropagation hides cross-chart clicks from Mantine's click-outside. +const pinnedTooltipRegistry = new Map void>(); + +function broadcastTooltipPinned(activeId: string) { + pinnedTooltipRegistry.forEach((dismiss, id) => { + if (id !== activeId) { + dismiss(); + } + }); +} + +// Registers this chart's dismiss handler and returns a callback to close every +// other chart's pinned tooltip (call it when pinning this one). +export function useCrossChartPinDismiss(onDismiss: () => void): () => void { + const id = useId(); + // Keep the latest onDismiss without re-subscribing each render. + const onDismissRef = useRef(onDismiss); + useEffect(() => { + onDismissRef.current = onDismiss; + }, [onDismiss]); + + useEffect(() => { + pinnedTooltipRegistry.set(id, () => onDismissRef.current()); + return () => { + pinnedTooltipRegistry.delete(id); + }; + }, [id]); + + return useCallback(() => broadcastTooltipPinned(id), [id]); +} diff --git a/packages/app/src/components/DBTimeChart/index.ts b/packages/app/src/components/DBTimeChart/index.ts new file mode 100644 index 0000000000..45a76286ed --- /dev/null +++ b/packages/app/src/components/DBTimeChart/index.ts @@ -0,0 +1,11 @@ +/** + * Public surface of the time-chart tile. Split out of a single 1000-line module; + * consumers (and the test files that `jest.mock('@/components/DBTimeChart')`) + * import from here, so the internal file layout stays free to change. + */ +export { DBTimeChart } from './DBTimeChart'; +export { + buildSeriesSearchUrl, + decodeSeriesGroupFilters, + type SeriesGroupFilter, +} from './searchUrl'; diff --git a/packages/app/src/components/DBTimeChart/searchUrl.ts b/packages/app/src/components/DBTimeChart/searchUrl.ts new file mode 100644 index 0000000000..8badb11e6e --- /dev/null +++ b/packages/app/src/components/DBTimeChart/searchUrl.ts @@ -0,0 +1,184 @@ +import { add } from 'date-fns'; +import { convertGranularityToSeconds } from '@hyperdx/common-utils/dist/core/utils'; +import { + isPromqlChartConfig, + isRawSqlChartConfig, +} from '@hyperdx/common-utils/dist/guards'; +import { + ChartConfigWithDateRange, + TSource, +} from '@hyperdx/common-utils/dist/types'; + +import { AGG_FNS, buildEventsSearchUrl, ChartKeyJoiner } from '@/ChartUtils'; + +export type SeriesGroupFilter = { column: string; value: string }; + +// Decode a Recharts series key (e.g. "count · error · api") into the +// underlying group-column filters. This is the same decode buildSeriesSearchUrl +// uses, extracted so the focus callback can hand the caller structured filters +// (rather than a display string) to apply to a sibling results list. +export function decodeSeriesGroupFilters({ + seriesKey, + groupColumns, + isSingleValueColumn, +}: { + seriesKey: string | undefined; + groupColumns: string[]; + isSingleValueColumn: boolean | undefined; +}): SeriesGroupFilter[] { + const seriesKeys = seriesKey?.split(ChartKeyJoiner); + const groupFilters: SeriesGroupFilter[] = []; + + if (seriesKeys?.length && groupColumns?.length) { + // When the series has multiple value columns, the key is prefixed with the + // value column name (e.g. "count · error"), so the group values start at + // index 1. (The "no group columns" case the original inline code also + // guarded is impossible here — this block only runs when groupColumns is + // non-empty.) + const startsWithValueColumn = !(isSingleValueColumn ?? true); + const groupValues = startsWithValueColumn + ? seriesKeys.slice(1) + : seriesKeys; + + groupValues.forEach((value, index) => { + if (groupColumns[index] != null) { + groupFilters.push({ column: groupColumns[index], value }); + } + }); + } + + return groupFilters; +} + +/** + * Build the drill-down search URL for a clicked point, or null when the chart + * cannot support it: raw SQL and PromQL charts don't resolve to a single source, + * and a click with no resolved date has no range to filter on. + * + * Pure, and extracted from a useCallback in the chart so the branching here can + * be read and tested without a render — in particular which value column a + * series key maps to, and whether that column's aggregation is attributable to + * individual events at all (a non-attributable agg must not produce a value + * filter, or the drill-down returns rows that never contributed to the point). + */ +export function buildSeriesSearchUrl({ + seriesKey, + seriesValue, + clickedActiveLabelDate, + source, + config, + granularity, + groupColumns, + valueColumns, + isSingleValueColumn, +}: { + seriesKey?: string; + seriesValue?: number; + clickedActiveLabelDate: Date | undefined; + source: TSource | undefined; + config: ChartConfigWithDateRange; + granularity: string; + groupColumns: string[]; + valueColumns: string[] | undefined; + isSingleValueColumn: boolean | undefined; +}): string | null { + // Raw SQL charts are not supported for drill-down as we don't know the source which is being used. + if ( + clickedActiveLabelDate == null || + source == null || + isRawSqlChartConfig(config) || + isPromqlChartConfig(config) + ) { + return null; + } + + // Parse the series key to extract group values + const seriesKeys = seriesKey?.split(ChartKeyJoiner); + const groupFilters = decodeSeriesGroupFilters({ + seriesKey, + groupColumns, + isSingleValueColumn, + }); + + // Build value range filter for Y-axis if provided + let valueRangeFilter: + | { + expression: string; + value: number; + } + | undefined; + + // `!= null`, not truthiness: a clicked value of exactly 0 is a real point — + // buildActiveClickSeries preserves zeroes — and skipping the filter for it + // drills down to every event in the bucket instead of the matching ones. + if ( + seriesValue != null && + Array.isArray(config.select) && + config.select.length > 0 + ) { + // Determine which value column to filter on + let valueExpression: string | undefined; + + if ((isSingleValueColumn ?? true) && config.select.length === 1) { + const firstSelect = config.select[0]; + const aggFn = + typeof firstSelect === 'string' ? undefined : firstSelect.aggFn; + // Only add value range filter if the aggregation is attributable + const isAttributable = + AGG_FNS.find(fn => fn.value === aggFn)?.isAttributable !== false; + + if (isAttributable) { + valueExpression = + typeof firstSelect === 'string' + ? firstSelect + : firstSelect.valueExpression; + } + } else if (seriesKeys?.length && (valueColumns?.length ?? 0) > 0) { + const firstPart = seriesKeys[0]; + const valueColumnIndex = valueColumns?.findIndex( + col => col === firstPart, + ); + + if ( + valueColumnIndex != null && + valueColumnIndex >= 0 && + valueColumnIndex < config.select.length + ) { + const selectItem = config.select[valueColumnIndex]; + const aggFn = + typeof selectItem === 'string' ? undefined : selectItem.aggFn; + // Only add value range filter if the aggregation is attributable + const isAttributable = + AGG_FNS.find(fn => fn.value === aggFn)?.isAttributable !== false; + + if (isAttributable) { + valueExpression = + typeof selectItem === 'string' + ? selectItem + : selectItem.valueExpression; + } + } + } + + if (valueExpression) { + valueRangeFilter = { + expression: valueExpression, + value: seriesValue, + }; + } + } + + // Calculate time range from clicked date and granularity + const from = clickedActiveLabelDate; + const to = add(clickedActiveLabelDate, { + seconds: convertGranularityToSeconds(granularity), + }); + + return buildEventsSearchUrl({ + source, + config, + dateRange: [from, to], + groupFilters, + valueRangeFilter, + }); +} diff --git a/packages/app/src/components/DBTimeChart/useChartToolbarItems.tsx b/packages/app/src/components/DBTimeChart/useChartToolbarItems.tsx new file mode 100644 index 0000000000..f17580645c --- /dev/null +++ b/packages/app/src/components/DBTimeChart/useChartToolbarItems.tsx @@ -0,0 +1,139 @@ +import React, { useMemo } from 'react'; +import { + type BuilderChartConfigWithDateRange, + type ChartConfigWithDateRange, + DisplayType, + type TSource, +} from '@hyperdx/common-utils/dist/types'; +import { IconChartBar, IconChartLine } from '@tabler/icons-react'; + +import DateRangeIndicator from '@/components/charts/DateRangeIndicator'; +import DisplaySwitcher from '@/components/charts/DisplaySwitcher'; +import MVOptimizationIndicator from '@/components/MaterializedViews/MVOptimizationIndicator'; +import { useMVOptimizationExplanation } from '@/hooks/useMVOptimizationExplanation'; + +type UseChartToolbarItemsArgs = { + builderQueriedConfig: BuilderChartConfigWithDateRange | undefined; + config: ChartConfigWithDateRange; + displayType: DisplayType | undefined; + handleSetDisplayType: (displayType: DisplayType) => void; + // Derived from the hook rather than hand-copied, so a change to its shape is + // a type error here instead of a field that quietly stops being read. + mvOptimizationData: ReturnType['data']; + queriedConfig: ChartConfigWithDateRange; + showDateRangeIndicator: boolean; + showDisplaySwitcher: boolean; + showMVOptimizationIndicator: boolean; + source: TSource | undefined; + toolbarPrefix: React.ReactNode[] | undefined; + toolbarSuffix: React.ReactNode[] | undefined; +}; + +/** + * Assemble the chart's toolbar: caller-supplied prefix/suffix items plus the + * indicators the chart owns (materialized-view optimization, effective date + * range) and the display-type switcher. + * + * Extracted from DBTimeChart because it is a long, purely presentational list + * build with no bearing on the chart's data or interaction state. + */ +export function useChartToolbarItems({ + builderQueriedConfig, + config, + displayType, + handleSetDisplayType, + mvOptimizationData, + queriedConfig, + showDateRangeIndicator, + showDisplaySwitcher, + showMVOptimizationIndicator, + source, + toolbarPrefix, + toolbarSuffix, +}: UseChartToolbarItemsArgs) { + return useMemo(() => { + const allToolbarItems = []; + + if (toolbarPrefix && toolbarPrefix.length > 0) { + allToolbarItems.push(...toolbarPrefix); + } + + if (source && showMVOptimizationIndicator && builderQueriedConfig) { + allToolbarItems.push( + , + ); + } + + const mvDateRange = mvOptimizationData?.optimizedConfig?.dateRange; + const isAlignedToChartGranularity = + queriedConfig.alignDateRangeToGranularity !== false; + + if ( + showDateRangeIndicator && + (mvDateRange || isAlignedToChartGranularity) + ) { + const mvGranularity = isAlignedToChartGranularity + ? undefined + : mvOptimizationData?.explanations.find(e => e.success)?.mvConfig + .minGranularity; + + allToolbarItems.push( + , + ); + } + + if (showDisplaySwitcher) { + allToolbarItems.push( + , + }, + { + value: DisplayType.StackedBar, + label: config.compareToPreviousPeriod + ? 'Bar Chart Unavailable When Comparing to Previous Period' + : 'Display as Bar Chart', + icon: , + disabled: config.compareToPreviousPeriod, + }, + ]} + />, + ); + } + + if (toolbarSuffix && toolbarSuffix.length > 0) { + allToolbarItems.push(...toolbarSuffix); + } + + return allToolbarItems; + }, [ + builderQueriedConfig, + config, + displayType, + handleSetDisplayType, + showDisplaySwitcher, + source, + toolbarPrefix, + toolbarSuffix, + showMVOptimizationIndicator, + showDateRangeIndicator, + mvOptimizationData, + queriedConfig, + ]); +}