fix(chart): explicit loading and empty states with safe scaling - #1793
Open
bilhokista wants to merge 2 commits into
Open
fix(chart): explicit loading and empty states with safe scaling#1793bilhokista wants to merge 2 commits into
bilhokista wants to merge 2 commits into
Conversation
|
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
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.
Closes #1578.
The bugs
Math.max()of an empty list is-Infinity, and that value flowed straight through the component:With no data the y-axis rendered the literal text
-Infinity,-Infinity,0over a full set of grid lines — a plot that reads as "the values are zero" rather than "we have no values". Three related failures came with it:0, so every bar height became(0 / 0) * 100→NaN%.Math.min(index, series[0]?.data.length - 1)evaluatesundefined - 1→NaN, which then poisoned every downstream index lookup.0,floor(1/2) = 0, and1 - 1 = 0— the same label three times, presented as though it were a range.There was also no loading state, and
role="img"carried onlyaria-label={title}, so the data itself was unreachable to a screen reader (and the label wasundefinedwhen no title was passed).The change
Confined to
interactive-chart.tsx.skeleton.tsxandempty-state.tsxalready existed and are used as-is — no changes needed there.Three pure helpers are exported so the edge cases can be tested directly rather than through the DOM:
computeMaxValue(series)returnsnumber | null, wherenullmeans "nothing to scale against". It filters non-finite values and treats a max of0as unscalable, so neither-InfinitynorNaNcan reach a style attribute.pickAxisLabels(points)returns 0–3 labels de-duplicated by position.describeSeries(series, format)builds the plain-language summary: point count, span, range, and latest value, with separate wording for a single point so it does not imply a range.Behaviour added:
isLoadingprop) renders skeletons sized to the real layout, witharia-busyand arole="status"announcement.EmptyState, withemptyTitle/emptyDescriptionoverridable.sr-onlytext and is folded into the plot'saria-label.One deliberate design decision worth flagging: the empty state is keyed on the whole
seriesprop, not on the visible subset. If it were keyed onmaxValue, hiding the last series via the legend would replace the card with an empty state — taking away the very legend needed to turn it back on. When data exists but everything is hidden, the legend stays and an inlinerole="status"message explains why the plot is gone. That case is tested.Tests
New
interactive-chart.test.tsx, 21 cases: 6 oncomputeMaxValue(including the empty, all-zero, and non-finite traps), 4 onpickAxisLabels, 3 ondescribeSeries, and 8 rendering cases covering loading, empty, all-zero, custom wording, single-point, the a11y summary, and the hidden-legend case. The empty and single-point tests assert directly that noNaNorInfinityreaches a rendered height.Honest note on verification: I could not run the frontend Vitest suite locally (no full workspace install). Both files were parsed with the TypeScript compiler API, and the three exported helpers were transpiled and executed standalone against 20 assertions — all passed, including a baseline check that
Math.max(...[])really is-Infinity, which is the root of the original bug. The eight rendering cases need CI.🤖 Generated with Claude Code
https://claude.ai/code/session_01CrfEY1tvXrbeMDAUzxfuk7