fix: percentile chart tooltips and x-axis label overlap

app/src/components/AthletePerformanceCharts.tsx

@@ -4,6 +4,7 @@ import { useState, useRef } from "react";
 import type { AthleteRaceEntry } from "@/lib/types";
 import { formatTime } from "@/lib/format";
 import { DISCIPLINE_COLORS } from "@/lib/colors";
+import { cursorXToDataIndex, computeLabelStep } from "./chart-utils";
 
 const DISTANCE_COLORS: Record<string, string> = {
   "70.3": "#3b82f6",
@@ -38,6 +39,8 @@ interface Props {
 const SVG_WIDTH = 500;
 const CHART_HEIGHT = 250;
 const MARGIN = { top: 15, right: 10, bottom: 30, left: 55 };
+// Approximate width in viewBox units of an x-axis label like "Sep 2024".
+const MIN_LABEL_SPACING = 50;
 
 function StackedBarChart({ data }: { data: BarDataPoint[] }) {
   const [tooltipIdx, setTooltipIdx] = useState<number | null>(null);
@@ -48,21 +51,24 @@ function StackedBarChart({ data }: { data: BarDataPoint[] }) {
   const plotWidth = SVG_WIDTH - MARGIN.left - MARGIN.right;
 
   const yTicks = getTimeTicks(maxTotal);
-  const step = Math.max(1, Math.floor(data.length / 8));
+  const step = computeLabelStep({
+    dataLength: data.length,
+    plotWidthPx: plotWidth,
+    minLabelSpacingPx: MIN_LABEL_SPACING,
+  });
 
   function handleMouseMove(e: React.MouseEvent<SVGSVGElement>) {
     if (!svgRef.current) return;
     const rect = svgRef.current.getBoundingClientRect();
-    const x = e.clientX - rect.left;
-    const plotLeft = (MARGIN.left / SVG_WIDTH) * rect.width;
-    const plotRight = rect.width - (MARGIN.right / SVG_WIDTH) * rect.width;
-    const plotW = plotRight - plotLeft;
-    const relX = x - plotLeft;
-    if (relX < 0 || relX > plotW) {
-      setTooltipIdx(null);
-      return;
-    }
-    setTooltipIdx(Math.min(data.length - 1, Math.floor((relX / plotW) * data.length)));
+    setTooltipIdx(
+      cursorXToDataIndex({
+        clientX: e.clientX,
+        rect,
+        viewBox: { width: SVG_WIDTH, height: CHART_HEIGHT },
+        margin: MARGIN,
+        dataLength: data.length,
+      }),
+    );
   }
 
   const point = tooltipIdx !== null ? data[tooltipIdx] : null;
@@ -167,7 +173,11 @@ function PercentileLineChart({ data, color }: { data: PercentileDataPoint[]; col
 
   const innerHeight = CHART_HEIGHT - MARGIN.top - MARGIN.bottom;
   const plotWidth = SVG_WIDTH - MARGIN.left - MARGIN.right;
-  const step = Math.max(1, Math.floor(data.length / 8));
+  const step = computeLabelStep({
+    dataLength: data.length,
+    plotWidthPx: plotWidth,
+    minLabelSpacingPx: MIN_LABEL_SPACING,
+  });
 
   // Y-axis is 0-100, reversed (0 at top = best)
   const yTicks = [0, 25, 50, 75, 100];
@@ -184,16 +194,15 @@ function PercentileLineChart({ data, color }: { data: PercentileDataPoint[]; col
   function handleMouseMove(e: React.MouseEvent<SVGSVGElement>) {
     if (!svgRef.current) return;
     const rect = svgRef.current.getBoundingClientRect();
-    const x = e.clientX - rect.left;
-    const plotLeft = (MARGIN.left / SVG_WIDTH) * rect.width;
-    const plotRight = rect.width - (MARGIN.right / SVG_WIDTH) * rect.width;
-    const plotW = plotRight - plotLeft;
-    const relX = x - plotLeft;
-    if (relX < 0 || relX > plotW) {
-      setTooltipIdx(null);
-      return;
-    }
-    setTooltipIdx(Math.min(data.length - 1, Math.floor((relX / plotW) * data.length)));
+    setTooltipIdx(
+      cursorXToDataIndex({
+        clientX: e.clientX,
+        rect,
+        viewBox: { width: SVG_WIDTH, height: CHART_HEIGHT },
+        margin: MARGIN,
+        dataLength: data.length,
+      }),
+    );
   }
 
   const point = tooltipIdx !== null ? data[tooltipIdx] : null;

app/src/components/__tests__/chart-utils.test.ts

@@ -0,0 +1,111 @@
+import { describe, it, expect } from "vitest";
+import { cursorXToDataIndex, computeLabelStep } from "../chart-utils";
+
+const MARGIN = { left: 55, right: 10 };
+const VIEW_BOX = { width: 500, height: 250 };
+
+function rect(width: number, height: number) {
+  return { left: 0, width, height };
+}
+
+// The percentile chart places dot i at viewBox X = MARGIN.left + ((i + 0.5) / n) * plotWidth.
+function dotViewBoxX(i: number, n: number) {
+  const plotWidth = VIEW_BOX.width - MARGIN.left - MARGIN.right;
+  return MARGIN.left + ((i + 0.5) / n) * plotWidth;
+}
+
+// Convert a viewBox X to a client X given preserveAspectRatio="xMidYMid meet".
+function viewBoxXToClientX(vbX: number, r: { left: number; width: number; height: number }) {
+  const scale = Math.min(r.width / VIEW_BOX.width, r.height / VIEW_BOX.height);
+  const offsetX = (r.width - VIEW_BOX.width * scale) / 2;
+  return r.left + offsetX + vbX * scale;
+}
+
+describe("cursorXToDataIndex", () => {
+  it("maps cursor over each dot to the matching index when container width equals viewBox width", () => {
+    const r = rect(500, 250);
+    const n = 10;
+    for (let i = 0; i < n; i++) {
+      const clientX = viewBoxXToClientX(dotViewBoxX(i, n), r);
+      expect(
+        cursorXToDataIndex({ clientX, rect: r, viewBox: VIEW_BOX, margin: MARGIN, dataLength: n }),
+      ).toBe(i);
+    }
+  });
+
+  it("maps cursor over the rightmost dot to the last index when container is wider than viewBox", () => {
+    // Reproduces the bug: SVG is rendered with preserveAspectRatio="xMidYMid meet",
+    // so when the container is wider than the viewBox aspect ratio, the content is
+    // centered with horizontal padding. The naive mapping ignores that offset and
+    // returns a wrong (earlier) index for points near the right edge.
+    const r = rect(1000, 250);
+    const n = 10;
+    const clientX = viewBoxXToClientX(dotViewBoxX(n - 1, n), r);
+    expect(
+      cursorXToDataIndex({ clientX, rect: r, viewBox: VIEW_BOX, margin: MARGIN, dataLength: n }),
+    ).toBe(n - 1);
+  });
+
+  it("maps cursor over every dot correctly when container is wider than viewBox", () => {
+    const r = rect(1200, 250);
+    const n = 10;
+    for (let i = 0; i < n; i++) {
+      const clientX = viewBoxXToClientX(dotViewBoxX(i, n), r);
+      expect(
+        cursorXToDataIndex({ clientX, rect: r, viewBox: VIEW_BOX, margin: MARGIN, dataLength: n }),
+      ).toBe(i);
+    }
+  });
+
+  it("returns null when the cursor is outside the plot area", () => {
+    const r = rect(1000, 250);
+    // Far left of the rendered SVG box, before the content even starts.
+    expect(
+      cursorXToDataIndex({ clientX: 5, rect: r, viewBox: VIEW_BOX, margin: MARGIN, dataLength: 10 }),
+    ).toBeNull();
+    // Far right.
+    expect(
+      cursorXToDataIndex({ clientX: 995, rect: r, viewBox: VIEW_BOX, margin: MARGIN, dataLength: 10 }),
+    ).toBeNull();
+  });
+
+  it("returns null when dataLength is zero", () => {
+    const r = rect(500, 250);
+    expect(
+      cursorXToDataIndex({ clientX: 250, rect: r, viewBox: VIEW_BOX, margin: MARGIN, dataLength: 0 }),
+    ).toBeNull();
+  });
+});
+
+describe("computeLabelStep", () => {
+  // Each "Sep 2024"-style label is roughly 50px wide at the chart's font size.
+  const MIN_SPACING = 50;
+
+  it("skips labels when per-label spacing is smaller than the label width", () => {
+    // 10 labels across a 435px plot = 43.5px per label, smaller than a 50px label.
+    // The current code returns floor(10/8)=1, which is the bug — every label is
+    // drawn and they overlap. We want step >= 2 so labels do not collide.
+    const step = computeLabelStep({ dataLength: 10, plotWidthPx: 435, minLabelSpacingPx: MIN_SPACING });
+    expect(step).toBeGreaterThanOrEqual(2);
+  });
+
+  it("returns 1 when there is enough room for every label", () => {
+    // 5 labels across 500px = 100px per label, twice the label width.
+    expect(
+      computeLabelStep({ dataLength: 5, plotWidthPx: 500, minLabelSpacingPx: MIN_SPACING }),
+    ).toBe(1);
+  });
+
+  it("returns 1 for a single data point", () => {
+    expect(
+      computeLabelStep({ dataLength: 1, plotWidthPx: 500, minLabelSpacingPx: MIN_SPACING }),
+    ).toBe(1);
+  });
+
+  it("scales the step up as the data length grows", () => {
+    const step20 = computeLabelStep({ dataLength: 20, plotWidthPx: 435, minLabelSpacingPx: MIN_SPACING });
+    const step40 = computeLabelStep({ dataLength: 40, plotWidthPx: 435, minLabelSpacingPx: MIN_SPACING });
+    expect(step20).toBeGreaterThanOrEqual(3);
+    expect(step40).toBeGreaterThanOrEqual(step20);
+  });
+});

app/src/components/chart-utils.ts

@@ -0,0 +1,44 @@
+// Pure helpers for the SVG charts in AthletePerformanceCharts.
+
+// Map a cursor's clientX to the data index for a chart whose SVG uses
+// viewBox-based scaling with preserveAspectRatio="xMidYMid meet". When the
+// rendered SVG element is wider than the viewBox aspect ratio, the content is
+// drawn at uniform scale and centered horizontally, so cursor pixels do not map
+// linearly to viewBox X. This helper undoes that offset.
+export function cursorXToDataIndex(opts: {
+  clientX: number;
+  rect: { left: number; width: number; height: number };
+  viewBox: { width: number; height: number };
+  margin: { left: number; right: number };
+  dataLength: number;
+}): number | null {
+  const { clientX, rect, viewBox, margin, dataLength } = opts;
+  if (dataLength <= 0) return null;
+
+  const scale = Math.min(rect.width / viewBox.width, rect.height / viewBox.height);
+  if (scale <= 0) return null;
+  const offsetX = (rect.width - viewBox.width * scale) / 2;
+  const vbX = (clientX - rect.left - offsetX) / scale;
+
+  const plotLeft = margin.left;
+  const plotRight = viewBox.width - margin.right;
+  if (vbX < plotLeft || vbX > plotRight) return null;
+
+  const plotWidth = plotRight - plotLeft;
+  const idx = Math.floor(((vbX - plotLeft) / plotWidth) * dataLength);
+  return Math.max(0, Math.min(dataLength - 1, idx));
+}
+
+// Choose how many data labels to draw between visible ticks so adjacent labels
+// don't collide. Returns the step (1 = every label).
+export function computeLabelStep(opts: {
+  dataLength: number;
+  plotWidthPx: number;
+  minLabelSpacingPx: number;
+}): number {
+  const { dataLength, plotWidthPx, minLabelSpacingPx } = opts;
+  if (dataLength <= 1 || plotWidthPx <= 0 || minLabelSpacingPx <= 0) return 1;
+  const spacingPerLabel = plotWidthPx / dataLength;
+  if (spacingPerLabel >= minLabelSpacingPx) return 1;
+  return Math.ceil(minLabelSpacingPx / spacingPerLabel);
+}