fix: percentile chart tooltips and x-axis label overlap#199
Merged
Conversation
The SVG charts use viewBox 500x250 with default preserveAspectRatio "meet", so on containers wider than 500px the content is drawn at scale 1 and centered. The previous cursor-to-index math ignored that offset, so hovering over the rightmost dot mapped to an earlier index. The label-step formula also returned 1 for any chart with up to 15 races, causing labels like "Sep 2024" to collide. Extract cursorXToDataIndex and computeLabelStep into a pure module with unit tests that reproduce both bugs, then wire them into both the stacked bar and percentile line charts. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Sep 2024) overlapped one another whenever an athlete had roughly 10 races, because the previous step formula returned1for any chart with ≤15 points.Root cause
The SVG uses
viewBox="0 0 500 250"withclassName="w-full"and a fixedstyle={{ height: 250 }}. With the defaultpreserveAspectRatio="xMidYMid meet", when the container is wider than 500px the content is drawn at scale 1 and centered, so the rendered chart only occupies the middle 500px of the SVG element. The oldhandleMouseMovetreated the entire rendered SVG width as if it were the chart content, so the cursor was mapped a few cells short of where it actually was.Fix
app/src/components/chart-utils.tswith two pure helpers:cursorXToDataIndex— undoes the preserveAspectRatio offset and maps client X back into viewBox coordinates.computeLabelStep— derives the tick step from actual per-label spacing vs. minimum label width (~50px forSep 2024).StackedBarChartandPercentileLineChartinAthletePerformanceCharts.tsxuse the helpers.chart-utils.test.tswith 9 vitest cases. Five of them failed against the original logic — including the exact "rightmost dot returns index 6 in a 1000px container" case from the bug report — and all 31 tests pass after the fix.Test plan
npm test(31 passed)npx tsc --noEmitnpm run lint/athlete/jed-radbone--au-m(10 races, 1120px-wide SVG): every dot returns its own tooltip, including the rightmost (2026 IRONMAN 70.3 Da Nang); only 5 of 10 date labels are drawn, no overlap.🤖 Generated with Claude Code