From 9c907b22bf144ef697b96edb4add9796d5b0e022 Mon Sep 17 00:00:00 2001 From: rcourtman Date: Wed, 15 Apr 2026 11:39:06 +0100 Subject: [PATCH] Fix dashboard trends tooltip portal rendering --- .../subsystems/frontend-primitives.md | 7 ++ .../shared/InteractiveSparkline.tsx | 95 +++++++++---------- .../SharedPrimitives.guardrails.test.ts | 8 +- .../__tests__/InteractiveSparkline.test.tsx | 11 +++ .../shared/interactiveSparklineModel.ts | 23 +---- .../shared/useInteractiveSparklineState.ts | 4 - 6 files changed, 67 insertions(+), 81 deletions(-) diff --git a/docs/release-control/v6/internal/subsystems/frontend-primitives.md b/docs/release-control/v6/internal/subsystems/frontend-primitives.md index 7999df152..18f217dcf 100644 --- a/docs/release-control/v6/internal/subsystems/frontend-primitives.md +++ b/docs/release-control/v6/internal/subsystems/frontend-primitives.md @@ -944,6 +944,13 @@ and `frontend-modern/src/components/shared/interactiveSparklineModel.ts` owns sparkline downsampling, gap segmentation, axis-tick math, and hover-selection policy. Future sparkline work should extend those owners instead of pushing canvas scheduling or chart-shape math back into the shared component shell. +That same sparkline boundary now also owns floating tooltip shell routing: +local hover tooltips must derive viewport anchor coordinates from the shared +runtime/model path and render through +`frontend-modern/src/components/shared/TooltipPortal.tsx`, not as HTML +`foreignObject` shells inside the `preserveAspectRatio="none"` chart SVG where +cross-browser scaling can stretch the tooltip surface or drop its semantic +shell styling. That same shared sparkline boundary now also owns active-series isolation metadata. The shell may expose `data-active-series-display` and `data-rendered-series-count` for proof and inspection, but only the shared diff --git a/frontend-modern/src/components/shared/InteractiveSparkline.tsx b/frontend-modern/src/components/shared/InteractiveSparkline.tsx index e5a54051c..b5ad8a0bb 100644 --- a/frontend-modern/src/components/shared/InteractiveSparkline.tsx +++ b/frontend-modern/src/components/shared/InteractiveSparkline.tsx @@ -1,9 +1,9 @@ import { Component, For, Show } from 'solid-js'; import { formatInteractiveSparklineHoverTime, - type InteractiveSparklineHoverState, type InteractiveSparklineProps, } from './interactiveSparklineModel'; +import { TooltipPortal } from './TooltipPortal'; import { useInteractiveSparklineState } from './useInteractiveSparklineState'; export type { @@ -27,11 +27,6 @@ const sparklineYAxisFontSize = (size?: InteractiveSparklineProps['size']) => const sparklineXAxisFontSize = (size?: InteractiveSparklineProps['size']) => size === 'lg' ? '12' : '10'; -const tooltipWidth = (hover: InteractiveSparklineHoverState) => (hover.focusedTooltip ? 112 : 138); - -const tooltipHeight = (hover: InteractiveSparklineHoverState) => - 22 + hover.values.length * 16 + (hover.totalValues > hover.values.length ? 14 : 0); - export const InteractiveSparkline: Component = (props) => { let chartSurfaceRef: Element | undefined; let canvasRef: HTMLCanvasElement | undefined; @@ -187,52 +182,6 @@ export const InteractiveSparkline: Component = (props preserveAspectRatio="none" aria-hidden="true" > - - {(hover) => ( - -
-
- {formatInteractiveSparklineHoverTime(hover().timestamp)} -
- - {(entry) => ( -
- - {entry.name} - - {sparkline.formatValue(entry.value)} - -
- )} - - hover().values.length}> -
- +{hover().totalValues - hover().values.length} more series -
- -
- - )} -
= (props
+ + {(hover) => ( + +
+
+ {formatInteractiveSparklineHoverTime(hover().timestamp)} +
+ + {(entry) => ( +
+ + {entry.name} + + {sparkline.formatValue(entry.value)} + +
+ )} + + hover().values.length}> +
+ +{hover().totalValues - hover().values.length} more series +
+ +
+ + )} + ); }; diff --git a/frontend-modern/src/components/shared/SharedPrimitives.guardrails.test.ts b/frontend-modern/src/components/shared/SharedPrimitives.guardrails.test.ts index 1e30bcce8..411085ab9 100644 --- a/frontend-modern/src/components/shared/SharedPrimitives.guardrails.test.ts +++ b/frontend-modern/src/components/shared/SharedPrimitives.guardrails.test.ts @@ -244,9 +244,8 @@ describe('shared primitive guardrails', () => { expect(tooltipPortalSource).toContain('text-base-content'); expect(tooltipPortalSource).toContain('border-border'); expect(tooltipPortalSource).not.toContain("'background-color': 'rgb(15, 23, 42)'"); - expect(interactiveSparklineSource).toContain('bg-surface'); + expect(interactiveSparklineSource).toContain('TooltipPortal'); expect(interactiveSparklineSource).toContain('text-base-content'); - expect(interactiveSparklineSource).toContain('border-border'); expect(interactiveSparklineSource).not.toContain("'background-color': 'rgb(15, 23, 42)'"); }); @@ -780,6 +779,7 @@ describe('shared primitive guardrails', () => { expect(interactiveSparklineSource).toContain('data-active-series-display'); expect(interactiveSparklineSource).toContain('data-active-hover-cursor-x'); expect(interactiveSparklineSource).toContain('data-sparkline-tooltip="true"'); + expect(interactiveSparklineSource).toContain('TooltipPortal'); expect(interactiveSparklineSource).toContain('data-sparkline-y-axis="true"'); expect(interactiveSparklineSource).toContain('data-sparkline-x-axis="true"'); expect(interactiveSparklineSource).toContain('axisPositionPercent(tick.y, sparkline.vbH)'); @@ -816,10 +816,10 @@ describe('shared primitive guardrails', () => { expect(interactiveSparklineModelSource).toContain('buildInteractiveSparklineChartData'); expect(interactiveSparklineModelSource).toContain('computeInteractiveSparklineHoverState'); expect(interactiveSparklineModelSource).toContain('getInteractiveSparklineCursorXForTimestamp'); + expect(interactiveSparklineModelSource).toContain('const tooltipX = chartRect.left + mouseX;'); expect(interactiveSparklineModelSource).toContain( - 'let tooltipY = (mouseY / chartRect.height) * vbH - 6;', + 'const tooltipY = chartRect.top + mouseY - 6;', ); - expect(interactiveSparklineModelSource).not.toContain('let tooltipY = chartRect.top - 6;'); expect(interactiveSparklineModelSource).toContain('downsampleLTTB'); expect(interactiveSparklineModelSource).toContain('findNearestMetricPoint'); }); diff --git a/frontend-modern/src/components/shared/__tests__/InteractiveSparkline.test.tsx b/frontend-modern/src/components/shared/__tests__/InteractiveSparkline.test.tsx index 491b2e4d1..d8aa77126 100644 --- a/frontend-modern/src/components/shared/__tests__/InteractiveSparkline.test.tsx +++ b/frontend-modern/src/components/shared/__tests__/InteractiveSparkline.test.tsx @@ -9,9 +9,16 @@ import { buildInteractiveSparklineSynchronizedReadout } from '@/components/share describe('InteractiveSparkline hover behavior', () => { afterEach(() => { vi.useRealTimers(); + vi.restoreAllMocks(); cleanup(); }); + const mockImmediateRaf = () => + vi.spyOn(window, 'requestAnimationFrame').mockImplementation((callback: FrameRequestCallback) => { + callback(0); + return 1; + }); + it('keeps the sparkline on shell, runtime, and model owners', () => { expect(interactiveSparklineSource).toContain('useInteractiveSparklineState'); expect(interactiveSparklineSource).not.toContain('style={{'); @@ -60,6 +67,7 @@ describe('InteractiveSparkline hover behavior', () => { it('shows a vertical dashed hover line and a tooltip', async () => { vi.useFakeTimers(); vi.setSystemTime(new Date('2024-01-01T12:00:00Z')); + mockImmediateRaf(); const now = Date.now(); const { container } = render(() => ( @@ -311,6 +319,7 @@ describe('InteractiveSparkline hover behavior', () => { it('limits tooltip rows and shows the "+N more series" affordance', async () => { vi.useFakeTimers(); vi.setSystemTime(new Date('2024-01-01T12:00:00Z')); + mockImmediateRaf(); const now = Date.now(); const makeSeries = (i: number, value: number) => ({ @@ -358,6 +367,7 @@ describe('InteractiveSparkline hover behavior', () => { it('clamps tooltip position so it stays in the viewport', async () => { vi.useFakeTimers(); vi.setSystemTime(new Date('2024-01-01T12:00:00Z')); + mockImmediateRaf(); const now = Date.now(); const { container } = render(() => ( @@ -414,6 +424,7 @@ describe('InteractiveSparkline hover behavior', () => { it('anchors the tooltip to the pointer instead of the chart top edge', async () => { vi.useFakeTimers(); vi.setSystemTime(new Date('2024-01-01T12:00:00Z')); + mockImmediateRaf(); const now = Date.now(); const { container } = render(() => ( diff --git a/frontend-modern/src/components/shared/interactiveSparklineModel.ts b/frontend-modern/src/components/shared/interactiveSparklineModel.ts index 8062d7022..2d3e1da45 100644 --- a/frontend-modern/src/components/shared/interactiveSparklineModel.ts +++ b/frontend-modern/src/components/shared/interactiveSparklineModel.ts @@ -596,8 +596,6 @@ export const computeInteractiveSparklineHoverState = ({ sortTooltipByValue, highlightNearestSeriesOnHover, lockedSeriesIndex, - tooltipPadding, - tooltipEstimatedWidth, }: { chartData: InteractiveSparklineChartData; chartRect: DOMRect; @@ -610,8 +608,6 @@ export const computeInteractiveSparklineHoverState = ({ sortTooltipByValue?: boolean; highlightNearestSeriesOnHover?: boolean; lockedSeriesIndex: number | null; - tooltipPadding: number; - tooltipEstimatedWidth: number; }): InteractiveSparklineHoverState | null => { if (chartData.validSeries.length === 0 || chartData.rangeMs <= 0 || chartRect.width <= 0) { return null; @@ -706,23 +702,8 @@ export const computeInteractiveSparklineHoverState = ({ } const totalValues = focusedTooltip ? tooltipValues.length : values.length; - let tooltipX = chartX; - let tooltipY = (mouseY / chartRect.height) * vbH - 6; - - const shownRows = tooltipValues.length; - const tooltipWidth = - (focusedTooltip ? tooltipEstimatedWidth * 0.78 : tooltipEstimatedWidth) * - (vbW / Math.max(chartRect.width, 1)); - const tooltipHeight = - (22 + shownRows * 16 + (totalValues > shownRows ? 14 : 0)) * - (vbH / Math.max(chartRect.height, 1)); - const minTooltipX = tooltipPadding + tooltipWidth / 2; - const maxTooltipX = Math.max(minTooltipX, vbW - tooltipPadding - tooltipWidth / 2); - tooltipX = clampInteractiveSparklineValue(tooltipX, minTooltipX, maxTooltipX); - - const minTooltipY = tooltipHeight + tooltipPadding; - const maxTooltipY = Math.max(minTooltipY, vbH - tooltipPadding); - tooltipY = clampInteractiveSparklineValue(tooltipY, minTooltipY, maxTooltipY); + const tooltipX = chartRect.left + mouseX; + const tooltipY = chartRect.top + mouseY - 6; return { x: chartX, diff --git a/frontend-modern/src/components/shared/useInteractiveSparklineState.ts b/frontend-modern/src/components/shared/useInteractiveSparklineState.ts index 4516f4361..3aed3db38 100644 --- a/frontend-modern/src/components/shared/useInteractiveSparklineState.ts +++ b/frontend-modern/src/components/shared/useInteractiveSparklineState.ts @@ -32,8 +32,6 @@ export function useInteractiveSparklineState( const vbH = 100; const vbW = 200; const xAxisBandPx = 16; - const tooltipPadding = 8; - const tooltipEstimatedWidth = 190; const maxRows = () => props.maxTooltipRows ?? 6; const yMode = () => props.yMode ?? 'percent'; const activeSeriesDisplay = () => props.activeSeriesDisplay ?? 'emphasize'; @@ -78,8 +76,6 @@ export function useInteractiveSparklineState( sortTooltipByValue: props.sortTooltipByValue, highlightNearestSeriesOnHover: props.highlightNearestSeriesOnHover, lockedSeriesIndex: props.highlightNearestSeriesOnHover ? lockedSeriesIndex() : null, - tooltipPadding, - tooltipEstimatedWidth, }); setHoveredState(computed); if (!props.onHoverSyncChange || !props.hoverSourceKey) {