From 52569854dcf15d4a53d279e068d206073f08e9d5 Mon Sep 17 00:00:00 2001 From: Tom Alexander Date: Wed, 12 Aug 2026 16:47:31 -0400 Subject: [PATCH] feat(charts): name the service on release marker hover Colour was the only thing tying a marker to a service, and that lookup breaks down exactly when it matters. The legend caps at 4 entries, so on a chart with more series than that a marker can be tinted to match a line whose legend entry is hidden behind "+N more" - there is nothing on screen to resolve the colour against. Collapsed clusters were worse still: "6 releases" named nobody at all. Hovering a marker now lists every release in its cluster with the service that shipped it, its version, and the time. Implementation notes: - The hit targets live in a ZIndexLayer above every other chart layer. Without that the series areas (zIndex 100) and the marker lines (400) receive the pointer first. The lines and labels are now pointer-transparent for the same reason; they are decoration and the hit layer owns the interaction. - Targets are confined to the label headroom rather than the full plot height. Covering the plot made the series tooltip fire alongside this one, and the label is the natural thing to aim at anyway. Events are never stopped, so drag-to-zoom still works underneath. - One target per cluster, spanning it, so the muted lines inside a cluster are covered by their anchor rather than intercepting it. - The tooltip is portaled and fixed-positioned, matching the series tooltip: a dashboard tile clips its overflow, so an absolutely positioned tooltip is cut off at the tile edge. Its anchor is measured from the hit band in the event handler, so nothing reads a ref during render. Layout is shared between the rendered lines and the hit layer via `layoutAnnotations`, so the hover bands can't drift from the markers they belong to. --- .changeset/release-marker-hover-tooltip.md | 8 ++ packages/app/src/HDXMultiSeriesTimeChart.tsx | 35 ++++++ .../components/charts/AnnotationHitLayer.tsx | 104 ++++++++++++++++++ .../components/charts/AnnotationTooltip.tsx | 80 ++++++++++++++ .../__tests__/chartAnnotations.test.tsx | 60 ++++++++++ .../components/charts/chartAnnotations.tsx | 40 ++++++- .../e2e/features/release-markers.spec.ts | 25 +++++ .../tests/e2e/page-objects/DashboardPage.ts | 10 ++ 8 files changed, 356 insertions(+), 6 deletions(-) create mode 100644 .changeset/release-marker-hover-tooltip.md create mode 100644 packages/app/src/components/charts/AnnotationHitLayer.tsx create mode 100644 packages/app/src/components/charts/AnnotationTooltip.tsx diff --git a/.changeset/release-marker-hover-tooltip.md b/.changeset/release-marker-hover-tooltip.md new file mode 100644 index 0000000000..6567c44199 --- /dev/null +++ b/.changeset/release-marker-hover-tooltip.md @@ -0,0 +1,8 @@ +--- +'@hyperdx/app': minor +--- + +Hovering a release marker now lists every release in its cluster with the +service that shipped it, its version, and the time. Colour alone could not +identify a service once a chart had more series than the legend shows, and a +collapsed "N releases" cluster named none of them. diff --git a/packages/app/src/HDXMultiSeriesTimeChart.tsx b/packages/app/src/HDXMultiSeriesTimeChart.tsx index d829541f5a..922f07ccc9 100644 --- a/packages/app/src/HDXMultiSeriesTimeChart.tsx +++ b/packages/app/src/HDXMultiSeriesTimeChart.tsx @@ -17,6 +17,7 @@ import { BarChart, BarProps, CartesianGrid, + Customized, Legend, ReferenceArea, ReferenceLine, @@ -33,9 +34,15 @@ import { Popover } from '@mantine/core'; import type { NumberFormat } from '@/types'; import { COLORS, formatNumber, truncateMiddle } from '@/utils'; +import { + AnnotationHitLayer, + type HoveredAnnotation, +} from './components/charts/AnnotationHitLayer'; +import { AnnotationTooltip } from './components/charts/AnnotationTooltip'; import { ChartAnnotation, getAnnotationElements, + layoutAnnotations, resolveAnnotationSeries, } from './components/charts/chartAnnotations'; import { ChartOverlayControls } from './components/charts/ChartOverlayControls'; @@ -1435,6 +1442,21 @@ export const MemoChart = memo(function MemoChart({ ); }, [annotations, lineData]); + // Same geometry the hit layer positions against, so the hover bands can't + // drift from the lines they belong to. + const laidOutAnnotations = useMemo(() => { + if (!coloredAnnotations?.length) { + return null; + } + return layoutAnnotations(coloredAnnotations, { + domain: xAxisDomain, + plotWidth: Math.max(0, containerWidth - Y_AXIS_WIDTH), + }); + }, [coloredAnnotations, xAxisDomain, containerWidth]); + + const [hoveredAnnotation, setHoveredAnnotation] = + useState(null); + const annotationElements = useMemo(() => { if (!coloredAnnotations?.length) { return null; @@ -1469,6 +1491,9 @@ export const MemoChart = memo(function MemoChart({ style={{ position: 'relative', width: '100%', height: '100%' }} > {nearestSeriesStyle} + {hoveredAnnotation != null && ( + + )} + } + /> + )} {highlightStart && highlightEnd ? ( ` so it can read the + * real plot geometry (`usePlotArea`, `useXAxisScale`) instead of re-deriving it + * from the container width. Handlers deliberately do not stop propagation: + * events still bubble to the chart wrapper, which is what keeps drag-to-zoom + * and the series tooltip working underneath the bands. + */ +export function AnnotationHitLayer({ + annotations, + onHover, +}: { + annotations: PositionedAnnotation[]; + onHover: (hovered: HoveredAnnotation | null) => void; +}) { + const plot = usePlotArea(); + const scale = useXAxisScale(); + + if (plot == null || scale == null) { + return null; + } + + // One target per cluster, not per line. Anchors carry `members`; the muted + // lines inside a cluster are already covered by their anchor's band, and + // giving them their own rects would just have them intercept each other. + const anchors = annotations.filter(annotation => annotation.members != null); + + return ( + // Above every other chart layer, otherwise the series areas (zIndex 100) + // and the marker lines (400) receive the pointer instead of these bands. + + + {anchors.map((annotation, i) => { + const members = annotation.members ?? [annotation]; + // Span the whole cluster so hovering any of its lines — including the + // muted ones, which carry no label — surfaces the same tooltip. + const xs = members + .map(member => scale(member.x)) + .filter((px): px is number => px != null && Number.isFinite(px)); + if (xs.length === 0) { + return null; + } + const left = Math.min(...xs); + const right = Math.max(...xs); + + const width = Math.max(right - left, MIN_HIT_WIDTH_PX); + const x = left - (width - (right - left)) / 2; + + return ( + { + // Measured here rather than passed down from the chart + // container: reading the DOM in an event handler is fine, + // whereas reading a ref during render is not. + const band = event.currentTarget.getBoundingClientRect(); + onHover({ + annotation, + point: { x: band.left + band.width / 2, y: band.bottom }, + }); + }} + onMouseLeave={() => onHover(null)} + /> + ); + })} + + + ); +} diff --git a/packages/app/src/components/charts/AnnotationTooltip.tsx b/packages/app/src/components/charts/AnnotationTooltip.tsx new file mode 100644 index 0000000000..21c858b37a --- /dev/null +++ b/packages/app/src/components/charts/AnnotationTooltip.tsx @@ -0,0 +1,80 @@ +import { createPortal } from 'react-dom'; +import { Group, Stack, Text } from '@mantine/core'; + +import type { HoveredAnnotation } from './AnnotationHitLayer'; +import { + ChartTooltipContainer, + ChartTooltipHeader, + useChartTooltipZIndex, +} from './ChartTooltip'; + +/** Gap below the marker so the tooltip does not sit on top of its own label. */ +const TOOLTIP_OFFSET_PX = 6; + +/** + * Hover tooltip for a release marker: one row per release in the hovered + * cluster, each naming the service it belongs to. + * + * The service name is the whole point. On the chart the only carrier of that is + * the marker's colour, which stops being resolvable once the legend overflows + * past `MAX_LEGEND_ITEMS` and the matching entry hides behind "+N more". + * + * Portaled to the body and positioned fixed, matching the series tooltip: a + * dashboard tile clips its overflow, so an absolutely-positioned tooltip gets + * cut off at the tile edge. `hovered.point` is already in viewport pixels, + * measured from the hit band when the pointer entered it. + */ +export function AnnotationTooltip({ hovered }: { hovered: HoveredAnnotation }) { + const zIndex = useChartTooltipZIndex(); + const members = hovered.annotation.members ?? [hovered.annotation]; + + if (typeof document === 'undefined') { + return null; + } + + return createPortal( +
+ } + > + + {members.map((member, i) => ( + +
+ {member.group != null && ( + + {member.group} + + )} + + {member.label} + + + ))} + + +
, + document.body, + ); +} diff --git a/packages/app/src/components/charts/__tests__/chartAnnotations.test.tsx b/packages/app/src/components/charts/__tests__/chartAnnotations.test.tsx index af5ff334d2..01c5210c10 100644 --- a/packages/app/src/components/charts/__tests__/chartAnnotations.test.tsx +++ b/packages/app/src/components/charts/__tests__/chartAnnotations.test.tsx @@ -5,6 +5,7 @@ import { type ChartAnnotation, getAnnotationElements, labelSeparationPx, + layoutAnnotations, MAX_ANNOTATION_MARKERS, mergeAnnotations, resolveAnnotationSeries, @@ -428,3 +429,62 @@ describe('resolveAnnotationSeries', () => { expect(input.color).toBe('#fallback'); }); }); + +describe('layoutAnnotations', () => { + const collapsing = { + domain: [0, 1000] as [number, number], + plotWidth: 1000, + }; + const release = (seconds: number, label: string, group: string) => ({ + time: seconds * 1000, + label, + group, + kind: 'release', + groupNoun: 'releases', + }); + + // The hover tooltip reads `members` so a cluster shown as "N releases" can + // still name every service inside it — which colour alone cannot do once the + // legend overflows. + it('gives a collapsed cluster the members it swallowed', () => { + const laidOut = layoutAnnotations( + [ + release(0, '1.0.0', 'checkout'), + release(10, '2.0.0', 'payments'), + release(600, '3.0.0', 'search'), + ], + collapsing, + ); + + const anchors = laidOut.filter(a => a.members != null); + expect(anchors).toHaveLength(2); + expect(anchors[0].label).toBe('2 releases'); + expect(anchors[0].members?.map(m => m.group)).toEqual([ + 'checkout', + 'payments', + ]); + // Members keep their own labels, so the tooltip shows real versions. + expect(anchors[0].members?.map(m => m.label)).toEqual(['1.0.0', '2.0.0']); + expect(anchors[1].members?.map(m => m.group)).toEqual(['search']); + }); + + it('gives a lone marker itself as its only member', () => { + const [only] = layoutAnnotations( + [release(500, '1.0.0', 'checkout')], + collapsing, + ); + + expect(only.members?.map(m => m.label)).toEqual(['1.0.0']); + }); + + // Before the plot is measured there is no collapsing, but hover still needs + // members to render a row. + it('populates members even when the geometry is unknown', () => { + const laidOut = layoutAnnotations( + [release(0, '1.0.0', 'checkout'), release(10, '2.0.0', 'payments')], + { domain: collapsing.domain }, + ); + + expect(laidOut.every(a => a.members?.length === 1)).toBe(true); + }); +}); diff --git a/packages/app/src/components/charts/chartAnnotations.tsx b/packages/app/src/components/charts/chartAnnotations.tsx index 934de4c8e8..67edbf9165 100644 --- a/packages/app/src/components/charts/chartAnnotations.tsx +++ b/packages/app/src/components/charts/chartAnnotations.tsx @@ -73,10 +73,16 @@ const STROKE_OPACITY = 0.9; const MUTED_STROKE_OPACITY = 0.35; /** A marker resolved to its clamped x position, in unix seconds. */ -type PositionedAnnotation = ChartAnnotation & { +export type PositionedAnnotation = ChartAnnotation & { x: number; /** Label suppressed — a neighbour carries the group label for this cluster. */ muted?: boolean; + /** + * Every marker in this one's cluster, itself included, in time order. Only + * set on the labelled anchor. Hover surfaces read this so a cluster shown as + * "3 releases" can name all three. + */ + members?: PositionedAnnotation[]; }; /** @@ -213,8 +219,9 @@ function collapseLabels( } const size = end - start; + const members = sorted.slice(start, end); if (size === 1) { - collapsed.push(anchor); + collapsed.push({ ...anchor, members }); } else { // A cluster covering several series can't wear one of their colors // without claiming the others' events as that series'. Go neutral so @@ -227,6 +234,7 @@ function collapseLabels( ...anchor, label: `${size} ${noun}`, color: spansSeries ? MIXED_SERIES_COLOR : anchor.color, + members, }); } for (let i = start + 1; i < end; i++) { @@ -256,10 +264,10 @@ function collapseLabels( * Generic over source — feature hooks map their events to `ChartAnnotation[]`. * Capped at `MAX_ANNOTATION_MARKERS` to protect against pathological inputs. */ -export function getAnnotationElements( +export function layoutAnnotations( annotations: ChartAnnotation[], opts: { domain: [number, number]; plotWidth?: number }, -): ReactElement[] { +): PositionedAnnotation[] { const [minX, maxX] = opts.domain; const positioned = positionAnnotations(annotations, [minX, maxX]); @@ -272,9 +280,24 @@ export function getAnnotationElements( const laidOut = plotWidth > 0 && spanSeconds > 0 ? collapseLabels(positioned, plotWidth / spanSeconds) - : positioned; + : positioned.map(annotation => ({ + ...annotation, + members: [annotation], + })); + + return laidOut.slice(0, MAX_ANNOTATION_MARKERS); +} - return laidOut.slice(0, MAX_ANNOTATION_MARKERS).map((annotation, i) => { +/** + * Renders the laid-out markers as dashed vertical reference lines. Takes the + * output of `layoutAnnotations` so the hover hit layer can position itself + * against exactly the same geometry. + */ +export function getAnnotationElements( + annotations: ChartAnnotation[], + opts: { domain: [number, number]; plotWidth?: number }, +): ReactElement[] { + return layoutAnnotations(annotations, opts).map((annotation, i) => { const color = annotation.color ?? 'var(--color-border)'; return ( ) : undefined diff --git a/packages/app/tests/e2e/features/release-markers.spec.ts b/packages/app/tests/e2e/features/release-markers.spec.ts index 8962793e84..01e1450fe7 100644 --- a/packages/app/tests/e2e/features/release-markers.spec.ts +++ b/packages/app/tests/e2e/features/release-markers.spec.ts @@ -144,6 +144,31 @@ test.describe('Release markers', { tag: ['@full-stack', '@dashboard'] }, () => { await expect(page).not.toHaveURL(/releaseMarkers=true/); }); + // Colour alone can't identify a service once the legend overflows its 4-item + // cap and the matching entry hides behind "+N more". Hover has to name the + // service outright. + test('names the service on hover', async ({ page }) => { + await seedReleases(); + + await dashboardPage.createNewDashboard(); + await dashboardPage.addTileWithSource( + 'Release marker hover', + DEFAULT_LOGS_SOURCE_NAME, + ); + await dashboardPage.setGlobalFilter(`ServiceName:"${RELEASE_SERVICE}"`); + await dashboardPage.toggleReleaseAnnotations(); + + await expect(dashboardPage.getAnnotationMarkers()).toHaveCount(2); + + const targets = dashboardPage.getAnnotationHitTargets(); + await expect(targets).toHaveCount(2); + await targets.first().hover(); + + // Service name and version together, so no colour lookup is needed. + await expect(page.getByText(RELEASE_SERVICE).last()).toBeVisible(); + await expect(page.getByText(OLD_VERSION).last()).toBeVisible(); + }); + // A marker only aids correlation if the reader can tie it to something on // the chart. An aggregate line over several services can't do that, so // rather than draw a wall of markers naming services with no visible line, diff --git a/packages/app/tests/e2e/page-objects/DashboardPage.ts b/packages/app/tests/e2e/page-objects/DashboardPage.ts index c5f7182ee6..31284350ee 100644 --- a/packages/app/tests/e2e/page-objects/DashboardPage.ts +++ b/packages/app/tests/e2e/page-objects/DashboardPage.ts @@ -1432,6 +1432,16 @@ export class DashboardPage { return this.getTile(tileIndex).locator('.recharts-reference-line'); } + /** + * Transparent hover targets over the annotation markers, one per cluster. + * Hovering one opens a tooltip naming the services that released. + */ + getAnnotationHitTargets(tileIndex = 0): Locator { + return this.getTile(tileIndex).locator( + '.recharts-annotation-hit-layer rect', + ); + } + /** * Text labels for the annotation markers. Recharts hoists reference-line * labels into a separate z-index layer, so they are NOT descendants of the