Repository navigation
feat(ai): theme charts, and describe them to a screen reader - #10
Conversation
0494cb2 to
d8a70d6
Compare
054561a to
e02a66a
Compare
Adds AITheme — a ThemeExtension a host registers on ThemeData.extensions — with ChartThemeData as its first component theme, plus a ChartTheme InheritedTheme for subtree overrides and a `theme` argument on both chart widgets. Thirteen nullable fields cover the palette, the plot height, the scatter and bubble radii, the histogram's bucket count, the axis, pie-label and title styles, the grid line colour and the heatmap's three ramp stops. An unset field derives from the ambient ColorScheme, so overriding the palette leaves everything else following the app in both brightnesses. There was nothing to hang a chart theme on: no ThemeExtension and no InheritedWidget existed here outside 2.3's translations scope. The shape mirrors stream_core_flutter's StreamTheme (<Component>ThemeData plus an InheritedTheme, defaults resolved in the widget's build) so a host of both SDKs finds what it expects, but AITheme carries no brightness, colour scheme or typography of its own — every widget here already resolves from the ambient ColorScheme, and a second brightness would be a second source of truth able to disagree with Theme.of. copyWith/merge/lerp/== are hand-written: one component theme does not justify a codegen dependency in a package whose premise is a short dependency list. In lerp, a field set on only one side swaps at t == 0.5 instead of interpolating. MaterialApp animates its ThemeData, so lerp runs on every light/dark switch, and null here means "derive from the theme" rather than "transparent" or "zero" — Color.lerp(null, c, t) would fade a grid line through transparency mid-transition and lerpDouble(null, 220, t) would grow a chart up from zero height. Palettes of different lengths interpolate by wrapping the shorter one, the way it is cycled at paint time, so no series loses its colour partway through. Neither AIMarkdownBody nor StreamingMessageView gained a parameter, so the fence widget cache's key is untouched: a chart resolves the theme from its own BuildContext and the inherited-dependency mechanism marks the cached element dirty regardless of widget identity. ai_markdown_body_test asserts it — one identical widget instance, two palettes. A chartTheme constructor argument left out of that global key would instead let two differently-themed bodies share one palette. Charts also had no Semantics at all, fl_chart painting to a canvas, so a ChartView was an empty box to a screen reader. Each now carries one node summarising the kind, title, axes, series, counts and value range, with a sentence per USpecKind. ChartSemantics.fromSpec gathers the facts and formats the numbers for speech; AITranslations.chartSemanticsLabel composes the sentence, so it is translatable like every other string — one method rather than a dozen phrase-sized ones, because a sentence's word order varies far more between languages than a tooltip's does. The node excludes its subtree: the tick labels and a heatmap's row, column and legend labels are real Text widgets, and walking them yields a run of bare numbers with nothing saying which axis they belong to. This is also the first code here to read USpec.xLabel and USpec.yLabel, which the parsers had been filling in for nobody. Two visible changes. Pie slice labels pick black or white per slice instead of always white, which disappears on any light slice a host palette might carry; under the default palette that flips every label dark. And USpecParser leaves an unnamed series' name empty instead of substituting 'Series' or 'Pie' — 2.3 deferred those here, since a parser has no BuildContext — with the name resolved at render time from AITranslations.unnamedChartSeries. The 'Pie' variant is dropped rather than translated: it reached the screen nowhere. All ten committed CI goldens are unchanged, defaults having been preserved verbatim. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Two defects in the chart accessibility summary, both reaching the screen reader as the only description a chart has — its Semantics node excludes the painted axis labels, so there is nothing else to fall back on. The trailing-zero strip in `_formatValue` ran on the whole formatted number instead of the fraction, so anything rounding to `x.00` kept a dangling point: 99.999 became '100.', read aloud as "one hundred point". Values below 0.005 collapsed to '0.' at both ends of the range, losing the range entirely. `ChartSemantics.fromSpec` also flattened every series, while `ChartView` plots only the first for a pie and a histogram — so a two-series pie showing four slices announced eight, and computed the largest slice's share against a total it never draws. `USpecParser` accepts a multi-series pie from model output, so nothing unusual was needed to hit it. Deriving the described series list up front also removes an unguarded `reduce` over an empty first series, which threw `StateError` inside `build` and took the whole `AIMarkdownBody` down with it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
e02a66a to
a543fb4
Compare
Adds the coverage the theming work was missing, each case checked by
breaking the code it guards and watching it fail:
- No test set a dark Brightness, so `_kDarkScale` and the three ramp
fallbacks never ran. Delete the brightness check and the suite stayed
green while every dark-mode heatmap inverted the intensity it encodes.
- The ramp test matched with `contains` over an unordered bag of fills,
so its own `reason` strings ("the minimum cell sits at the low stop")
were unchecked and a reversed ramp passed. Now positional, and the mid
stop is asserted too.
- `copyWith`/`merge`/`==`/`hashCode` are four hand-written 13-line
tables, spot-checked on two fields. A loop over all thirteen asserts
each one round-trips and that nothing else moves, which is what catches
a crossed line such as `scatterRadius: bubbleMinRadius ?? ...`.
- `ChartTheme.wrap` had no coverage at all, though carrying the scope
across a route is the reason the class is an InheritedTheme. Mirrors
the AITranslationsScope route test.
Doc corrections:
- `USeries.name` said "shown in legends". This package draws no legend,
and the field is now `''` for an unnamed series, which the doc never
mentioned.
- The README described `ChartView` as "line, bar, or pie ... powered by
fl_chart" (it handles eight kinds, and the heatmap is a widget grid),
listed two parser formats of seven, and said unset theme fields derive
from the ambient ColorScheme — five of them fall back to constants.
- The README's Dutch `chartSemanticsLabel` example labelled every chart
a bar chart; it now switches on `chart.kind` and handles `isEmpty`.
- The example's pie comment pointed at an amber slice the four-slice
chart never reached, and the ROADMAP claimed the example showed the
unnamed-row fallback while every series was named. Both are now true.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The eight chart goldens all render the default theme, so they show that defaults didn't move and nothing else — a feature whose whole point is visual had no visual evidence of what it does, which is why running update_goldens produced no new files: there were no new cases to render. Four cases covering the two things a host actually changes: - `chart_view_themed_bar` — a host palette and grid colour reaching the paint. - `chart_view_themed_heatmap` — a host three-stop ramp. - `chart_view_dark_line` — the default line chart under a dark theme. - `chart_view_dark_heatmap` — the ramp inversion `_kDarkScale` exists for, which until now had no coverage of any kind and which the luminance test can only describe. Scope, noted at the group: CI goldens obscure text with an opaque paint, so these pin fills, grid lines and the ramp. Label colours — including the per-slice pie contrast — are invisible to every golden here and stay covered by the widget tests. The committed `goldens/ci/` PNGs have to be rendered on Linux, so these four fail locally until the update_goldens workflow generates them. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The ChartThemeData docs argued their design at length where a caller only needs to know what a field does and what it falls back to. Every field doc is now one or two lines, and the class doc keeps the three things a host acts on — null means derive, how the three levels compose, copyWith can't unset — while the essay on why layout isn't themeable is down to the one consequence a host can see. Rationale comments survive only where they stop a wrong change: _swap's reason not to interpolate a null, _lerpPalette's wrapping, and AITheme's hand-written == that ThemeData compares with mapEquals. The rest of the argument lives in the ROADMAP entry, which is where it belongs. Three docs were also wrong, not just long: - `ChartView.semanticsLabel` said `''` adds "no semantics at all". It removes this widget's node and so puts the excluded subtree labels back in reach — the opposite of hiding the chart. - `ChartSemantics.seriesNames` claimed axis order; it is series order. - `rowCount`/`columnCount` read as heatmap-only but are populated for every kind, which a translator reading them could not have known. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Before the specifics: most of what I found is the same kind of thing. The description, CHANGELOG and doc comments are unusually thorough, but several claims describe what the code was meant to do rather than what it does — the contrast argument, "flips every default label dark", and a -0 bug filed under 🐞 Fixed. Each was reasoned about carefully and none was run. Worth re-checking the rest with that in mind.
On the three comments in chartSemanticsLabel / ChartSemantics: I don't think they're three separate bugs so much as one gap — the sentence was designed carefully but never listened to across the eight USpecKinds. I'd treat it as one task and walk all eight while you're in there. I also spotted smaller things I'm not filing separately (the README's sample translation reads categoryCount for histograms, and single-item charts say "1 slices").
Detail is inline.
| /// | ||
| /// Material's own threshold, so a slice that reads as a dark surface gets | ||
| /// light text and vice versa. | ||
| static Color _onSlice(Color slice) => |
There was a problem hiding this comment.
This picks the wrong colour on half the default palette.
estimateBrightnessForColor compares its kThreshold = 0.15 to (L + 0.05)² rather than to the luminance itself, so the real switchover sits around L = 0.34 — much higher than the constant suggests. Only green and teal in kDefaultChartSeriesColors actually flip.
Numbers below are from Flutter's own Color.computeLuminance, and the black87 column is composited first — Colors.black87 is 0xDD000000, so it lands at about 87% opacity over the slice rather than being pure black. Checking these against #000000 will give you different figures.
| slice | luminance | _onSlice picks |
contrast of label vs. slice | if it picked black87 |
|---|---|---|---|---|
#4A90D9 blue |
0.264 | white | 3.34 | 5.59 |
#E67E22 orange |
0.319 | white | 2.85 | 6.46 |
#2ECC71 green |
0.450 | black87 | 8.53 | — |
#E74C3C red |
0.225 | white | 3.82 | 4.97 |
#9B59B6 purple |
0.175 | white | 4.67 | 4.10 |
#1ABC9C teal |
0.386 | black87 | 7.56 | — |
pieLabelStyle resolves to 11px semibold, which is small text by WCAG's definition — large text starts at 18pt, or 14pt bold — so the bar is 4.5:1. Blue, orange and red all fail it, and this change leaves them exactly as they were. The example app's coral is the same story (3.05, where black would give 6.11).
Two knock-ons. The evidence in the description points the other way from the code: it says dark text on the blue slice wins 6.2:1 against white's 3.4:1, and those ratios are roughly right, but _onSlice(#4A90D9) returns white — so the code is taking the 3.4. And "under the default palette this flips every label dark" appears in the body, the CHANGELOG and the ROADMAP, though only two of six do.
To head off the obvious reply: the goldens can't catch this, for the reason the PR itself gives — alchemist blocks fl_chart's canvas-painted slice titles. So the widget test is the only cover, and the one at chart_view_test.dart:549 uses navy (L 0.016) and yellow (L 0.928) — far enough apart that any threshold passes, and neither in the default palette. Running it against kDefaultChartSeriesColors would have caught this.
Simplest fix is probably to compute both ratios and keep the better one, rather than leaning on a threshold whose own comment in Flutter admits it "biases more towards using light text than WCAG20 recommends.".
| ..add(_range(chart)); | ||
| case USpecKind.heatmap: | ||
| parts | ||
| ..add('${chart.rowCount} rows by ${chart.columnCount} columns') |
There was a problem hiding this comment.
Heatmaps don't say what they're showing.
The label is "Heatmap, 2 rows by 4 columns, values 2 to 14", and with the painted labels excluded, that's all there is. A user learns there are two rows but never that they're Mon and Tue.
The justification — the summary already carries the counts and axis names — holds for the fl_chart kinds, but a heatmap usually has no axis names unless the data set xLabel/yLabel, and the thing that identifies a row is the series name, which line 217 skips specifically for heatmaps. You can hear it in the example app: its heatmap is unreadable.
chart.seriesNames is already populated, so row names are about a line; column labels would need a new field on ChartSemantics.
To be clear, I'm not reopening excludeSemantics: true — I think that call is right. It's precisely because the subtree is excluded that the sentence has to carry everything, which is what makes this one matter.
| parts | ||
| ..add('${chart.pointCount} points') | ||
| ..add(_range(chart)); | ||
| if (chart.sizeMin case final min?) parts.add('sizes $min to ${chart.sizeMax}'); |
There was a problem hiding this comment.
This clause describes an encoding that isn't drawn.
It fires for line, area, scatter and bubble, but only bubble maps UPoint.size onto anything — chart_view.dart:216 uses the flat scatterRadius otherwise.
Reachable from real input, too: uspec.dart reads encoding.size before deciding the mark, so a Vega-Lite {"mark":"point", …, "size":{"field":"pop"}} announces "sizes 1000000 to 9000000" over a dozen identical 6px dots.
(So this one asks you to remove from the sentence while the heatmap comment above asks you to add — not indecision, just that one is missing information and the other is wrong information.)
| static String _formatValue(double value) { | ||
| if (!value.isFinite) return value.toString(); | ||
| // Also normalizes -0.0, which toStringAsFixed(0) renders as '-0'. | ||
| if (value == 0) return '0'; |
There was a problem hiding this comment.
Small negatives are read out as "minus zero".
This guard catches exact -0.0, but anything between -0.005 and 0 falls through to the last branch and comes back '-0' — I ran it, and -0.001, -0.004 and -1e-9 all do. A chart of small negative deltas announces "values -0 to -0".
This is the awkward one, since it's listed under 🐞 Fixed; the positive half genuinely was fixed. The guarding test uses y: -0, which is the one input that hits this short-circuit, so it passes with the bug live.
| /// | ||
| /// Depends on both, so a chart repaints when either changes. Never `null`; | ||
| /// with neither present every field derives from the ambient [ThemeData]. | ||
| static ChartThemeData of(BuildContext context) { |
There was a problem hiding this comment.
Nested scopes replace rather than layer.
dependOnInheritedWidgetOfExactType only finds the nearest scope, so wrapping a palette scope around a height scope silently drops the palette.
README line 202 says "the three compose, most specific first", which is true with one scope and not with two — this method's own doc gets it right. Either reword the README or add a ChartTheme.merge the way IconTheme does.
…label
Five review findings on roadmap 2.4, all confirmed against the code.
Pie slice labels no longer go through ThemeData.estimateBrightnessForColor,
which compares its 0.15 threshold to (luminance + 0.05)² and so switches at
0.34 — leaving white on mid blue (3.34:1), orange (2.85:1) and red (3.82:1),
under the 4.5:1 an 11px label needs. The label is now whichever of white and
black87 has the better WCAG contrast against the slice, composited first.
Five of the six default colors flip dark; purple keeps white, which reads
better on it. The widget test asserts the ratio over kDefaultChartSeriesColors
rather than a navy/yellow pair any threshold would pass — the CI goldens can't
cover this, since alchemist blocks fl_chart's canvas-painted slice titles.
ChartSemantics._formatValue rounds before it decides how to render. It caught
exact -0.0 only, so every value in (-0.005, 0) formatted as '-0.00' and was
stripped to '-0', voiced as "minus zero". The guarding test used y: -0, the
one input that hit the short-circuit.
A size range is gathered only for a bubble chart. Every other kind draws its
points at one flat radius, but the parsers fill UPoint.size in regardless of
the mark, so a Vega-Lite {"mark": "point", "encoding": {"size": …}} — a
scatter — announced "sizes 1000000 to 9000000" over identical dots. The gate
is in fromSpec, so no translation can reintroduce it.
A heatmap names its rows and columns instead of only counting them. It rarely
carries axis labels, the row and column are what identify a cell, and the
painted labels are excluded from the tree — so "2 rows by 4 columns" was the
whole description. Rows come from seriesNames, columns from the new
ChartSemantics.columnLabels. Counts are singularised too ("1 slice").
ChartTheme.merge layers onto an enclosing scope, since `of` reads the nearest
one only and nesting otherwise shadows the outer palette. Kept IconTheme's
behaviour and added its escape hatch; the README's "the three compose" claim
now says which three.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Self-audit of the comments added in the previous commit, since adding prose to fix a prose-accuracy problem is the same failure mode. - ChartSemantics.columnLabels said a heatmap draws them "down the side and along the bottom". It draws them along the bottom; the side is the rows, from seriesNames (heatmap_chart_view.dart:61-132). - ChartView._onSlice punctuated a paraphrase as a quotation. Flutter's comment reads "appears to bias more towards", not "biases more towards". - ChartTheme.merge claimed folding AITheme in would leave a widget outliving a change to it. It wouldn't — the Builder depends on AITheme through 'of' and rebuilds. The reason that holds is that it keeps 'data' the overrides its own doc describes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The previous two commits added 64 lines of comment against 20 of code. Trimmed to what a reader needs and can't infer: that estimateBrightnessForColor switches at 0.34 rather than the 0.15 it names, that black87 is composited before measuring, that only a bubble draws UPoint.size, that _formatValue rounds before deciding, and that nesting a ChartTheme replaces. Dropped the measured contrast tables, the ChartTheme.merge sample and the restated rationale — the ROADMAP entry carries all of it at length. Net comment lines across the review round: +64 -> +30. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A pass over the doc comments 2.4 added, not only the ones this review round touched. Cut the bug archaeology, which git blame holds better than a doc comment does: how the scatter axis used to index per series, how a Chart.js null gap shifted points left, how clamping raw bubble sizes flattened the encoding. Each is now one clause naming the trap rather than a paragraph recounting it. Also cut the restated rationale in AITheme, ChartThemeData, HeatmapChartView and ResolvedChartTheme — the ROADMAP entry carries all of it. Kept every 'don't simplify this back' note: why lerp swaps a one-sided field instead of interpolating, why the row-label gutter isn't themeable, why the pie label style may carry no colour, why the heatmap grid keys rows by label. Comment density across 2.4's lib additions: 38% -> 32%. AITheme's doc-only import of ai_translations.dart went with it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Brings in #9 (client-side tool calling) and #11 (chart value equality). Three conflicts, all additive: - The barrel: 2.4 added the theme exports, #9 the tools ones. The list is alphabetical, so both stay in order. - HeatmapChartView's class doc: both sides reworded the same paragraph. Took ours, which is main's content already tightened in 1c84463. - The ROADMAP status table: 2.4 marks its own row done and adds 2.5, #9 renames 3.1 to "Client-side tool-calling (`AIToolRegistry`)" and marks it done. All three rows kept. 459 tests pass, analyze and format clean, the committed CI goldens unchanged. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Follows #11, which gave USpec, USeries and UPoint value equality. ChartSemantics is derived from them and exported alongside them, so a host memoizing on one or asserting against one in a test was the odd case out. Matches uspec.dart's shape: identical-first, ListEquality for the two string lists, Object.hashAll for their deep hashes. The test pairs two specs per field, each differing in exactly one derived fact, and asserts both != and a different hashCode. A set of distinct variants would not do: a field dropped from == alone still hashes apart and lands in another bucket, so the collision never happens. Verified by removing three fields from == and three from hashCode and watching the matching test fail each time. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
ROADMAP 2.4 — chart theming and accessibility.
What 2.4 asked for
ChartView's presentation was entirely fixed — a hardcoded palette, height, bubble radius range,bucket count and white-on-slice pie label, with no way for a host to intervene. And charts carried
no
Semanticsat all:fl_chartpaints to a canvas, so to a screen reader aChartViewwas anempty box. 2.3 also deferred its
USpecParser'Series'/'Pie'fallback names here.Theming
There was nothing to hang a chart theme on: no
ThemeExtensionand noInheritedWidgetexisted inthis package outside 2.3's translations scope. So this lands the groundwork, with the chart theme as
its first component.
AITheme— aThemeExtensiona host registers onThemeData.extensions, read through anever-throwing
AITheme.of.ChartThemeData— thirteen nullable fields (palette, height, scatter/bubble radii, bucket count,axis/pie-label/title styles, grid line colour, three heatmap ramp stops) plus
ChartTheme, anInheritedThemewhoseofmerges a subtree's overrides overAITheme's.ChartView.theme/HeatmapChartView.themefor one chart. The three compose, most specific first.kDefaultChartSeriesColorsis the old six-colour set, exported so a host can extend it.An unset field derives from the ambient
ColorScheme, so overriding the palette leaves the gridline, axis labels and heatmap ramp following the app in both brightnesses.
The shape mirrors
stream_core_flutter'sStreamTheme—<Component>ThemeDataplus anInheritedTheme, defaults resolved in the widget'sbuild— so a host of both SDKs finds what itexpects. Three deliberate divergences, each argued in the ROADMAP entry:
AIThemecarries no brightness, colour scheme or typography. Every widget here alreadyresolves from the ambient
ColorScheme; a second brightness would be a second source of truthable to disagree with
Theme.of. It's a bag of overrides over Material, not a parallel designsystem — so no
.light()/.dark()either.copyWith/merge/lerp/==, nottheme_extensions_builder. One componenttheme doesn't justify a codegen dependency in a package whose premise is a short dependency list.
New ROADMAP item 2.5 notes where to reconsider.
BuildContextextension getters.stream_core_flutter's are allStream-prefixed;unprefixed ones would leak into every host file importing this barrel.
Also not the shape 2.3's draft of this item sketched. It said to mirror
AITranslations— anabstract class with a concrete default subclass. Only the
of-falls-back-to-a-default third carriedover: translations are behaviour you subclass, a theme is data that interpolates.
The lerp detail worth reading before "simplifying" it
A field set on only one side swaps at
t == 0.5rather than interpolating.MaterialAppanimates its
ThemeData, solerpruns on every light/dark switch, andnullhere means "derivefrom the theme" — not "transparent" and not "zero".
Color.lerp(null, c, t)would fade a grid linethrough transparency mid-transition;
lerpDouble(null, 220, t)would grow a chart up from zeroheight. Palettes of different lengths interpolate by wrapping the shorter one, the way it's cycled
at paint time, so no series loses its colour partway through.
Nothing added to the fence cache key
AIMarkdownBodyandStreamingMessageViewgained no parameter — the same call #8 made fortranslations, and for the same reason. A chart resolves the theme from its own
BuildContext, andthe inherited-dependency mechanism marks the cached element dirty regardless of widget identity.
ai_markdown_body_test.dartasserts exactly that: one identical widget instance, two palettes. AchartThemeconstructor argument left out of that global key would instead let twodifferently-themed bodies share one palette.
Accessibility
Each chart now carries one semantics node —
Semantics(container: true, excludeSemantics: true, label: …)— summarising the kind, title, axes, series, counts and value range, with a sentence perUSpecKind: "Bar chart, Messages per day, 5 categories, values 8 to 24".A heatmap is the one kind whose sentence names rather than counts — "Heatmap, 2 rows by 4 columns,
rows: Mon, Tue, columns: 9am, 12pm, 3pm, 6pm, values 2 to 14". It rarely carries axis labels, and
the row and column are what identify a cell, so with the painted labels excluded a bare
"2 rows by 4 columns" described the grid's shape and nothing in it.
The work is split so only the half that needs translating is translatable:
ChartSemantics.fromSpecgathers the facts and formats numbers for speech (
10.0reads as "10"), andAITranslations.chartSemanticsLabelcomposes the sentence. That's one method rather than a dozenphrase-sized ones, because a whole sentence's word order varies far more between languages than a
tooltip's does.
ChartView.semanticsLabelreplaces it; an empty string adds no node at all.This is also the first code in the package to read
USpec.xLabel/USpec.yLabel, which theparsers had been filling in for nobody.
Two changes to look at
Pie slice labels pick black or white per slice instead of always white, which disappears on
any light slice a host palette might carry. The choice is the better of the two WCAG contrast
ratios against that slice's own fill, with the label composited on first —
black87is0xDD000000, so what lands on screen is 87% black over the slice, not pure black. Under thedefault palette five of the six flip dark; purple keeps white, which reads better on it at 4.67:1
against
black87's 4.10:1. Setting a colour onChartThemeData.pieLabelStyletakes it back.(This is corrected from the first commit, which used
ThemeData.estimateBrightnessForColorandgot three of the six wrong — see the fourth commit below.)
excludeSemantics: truedrops the axis and heatmap labels from the accessibility tree. Righttrade in my view — the summary already carries the range, the counts and the axis names, and
walking those
Textwidgets gives a reader a run of bare numbers with nothing saying which axisthey belong to. But a host can only undo it by passing
semanticsLabel: ''and wrapping the chartthemselves, so it's flagged in the code, the README and the ROADMAP.
2.3's leftover
USpecParserleaves an unnamed series'nameempty instead of substituting'Series'/'Pie',with the name resolved at render time from
AITranslations.unnamedChartSeries. The'Pie'variantis dropped rather than translated — it reached the screen nowhere. Reading
USeries.nameoff aparse result now gives
''where it used to give one of those two words.Verification
melos run lint:allandmelos run lint:pubclean.the pie, since alchemist blocks
fl_chart's canvas-painted slice titles with an opaque paint thatignores the text colour. Confirmed with
CI=true melos run test:all.ai_theme_test.dart,chart_theme_test.dart,chart_semantics_test.dart, plus atheming group and a semantics group in
chart_view_test.dartand cases inuspec_test.dartandai_markdown_body_test.dart.label contrast and the translated row-name fallback are visible.
Review fixes (second and third commits)
A review of the first commit turned up two defects in the accessibility summary, both reaching the
screen reader as the only description a chart has — the
Semanticsnode excludes the painted axislabels, so there is nothing to fall back on.
_formatValuekept a dangling decimal point. The trailing-zero strip ran on the wholeformatted number rather than the fraction, so anything rounding to
x.00came out as100.,4.,1.— voiced as "one hundred point". Values below0.005collapsed to0.at both ends ofthe range, losing the range entirely.
ChartViewplots only the first seriesfor both kinds, but
ChartSemantics.fromSpecflattened every one — a two-series pie showing fourslices announced eight, with the largest slice's share taken against a total never drawn.
USpecParseraccepts a multi-series pie from model output. Deriving the described series list upfront also removed an unguarded
reducethat threwStateErrorinsidebuildon a pie whosefirst series was empty.
The third commit closes the coverage gaps that let those ship, each verified by breaking the code it
guards and watching the test fail: no test set a dark
Brightness(so_kDarkScalenever ran); theheatmap ramp test matched with
containsover an unordered bag, so a reversed ramp passed;copyWith/merge/==/hashCodewere spot-checked on 2 of 13 fields; andChartTheme.wrap— thereason the class is an
InheritedTheme— had no coverage at all. It also corrects several docs,including a
USeries.namethat still said "shown in legends".Review fixes (fourth commit)
@VelikovPetar's review found five defects, all of which reproduced. Its framing was the more useful
part: the description, CHANGELOG and doc comments were reasoned about carefully and not run, so
several described what the code was meant to do. Each fix below is verified by executing it.
The pie label picked the wrong colour on half the default palette.
ThemeData.estimateBrightnessForColorcompares itskThresholdof0.15to(luminance + 0.05)², so it switches at a luminance of 0.34, not 0.15, and its own commentconcedes it "biases more towards using light text than WCAG20 recommends". Four of the six shipped
colours sit below that line, so it left white on mid blue (3.34:1), orange (2.85:1) and red
(3.82:1) — all under the 4.5:1 that 11px semibold label needs as small text. The contrast argument
in this description was right and the code was taking the other branch.
The label is now whichever of white and
black87has the better ratio. Measured from the runningwidget, not from a table I wrote:
#4A90D9blue#E67E22orange#2ECC71green#E74C3Cred#9B59B6purple#1ABC9CtealSo "flips every label dark" was wrong twice over — purple keeps white, legitimately. The goldens
can't cover any of this (alchemist blocks
fl_chart's canvas-painted slice titles), and the widgettest used a navy/yellow pair far enough apart that any threshold passes. It now asserts the ratio
clears 4.5:1 across
kDefaultChartSeriesColors; against the old code it fails atActual: 3.3426590622839893.Small negatives were still read as "minus zero". The second commit's
_formatValueguardcaught exact
-0.0only. Everything in(-0.005, 0)is neither zero nor whole, so it reached thelast branch as
-0.00and was stripped to-0. The guarding test usedy: -0— the one inputthat hits the short-circuit — so it passed with the bug live.
_formatValuenow rounds first andthen decides how to render, which handles both edges in one place.
A line, area or scatter chart announced a size range it never drew. Only a bubble maps
UPoint.sizeonto anything; the rest draw one flat radius. The parsers fillsizein regardlessof the mark, so a Vega-Lite
{"mark": "point", "encoding": {"size": …}}— a scatter — described"sizes 1000000 to 9000000" over a dozen identical dots. The gate is in
fromSpec, sosizeMinisnull for every other kind and no translation can reintroduce it.
A heatmap never said what it was showing — see Accessibility above. Rows come from
seriesNames, columns from a newChartSemantics.columnLabels;columnCountis now a getterover it.
Nested
ChartThemescopes replaced rather than layered.ofreads the nearest scope only, soa palette scope wrapped around a height scope silently dropped the palette. That is
IconTheme'sbehaviour and it's kept;
ChartTheme.mergeis its escape hatch, merging the enclosing scope onlyso
AITheme's resolved fields aren't baked into a widget that outlives a change to them. TheREADME's "the three compose" now names which three.
Also from the review's unfiled notes: every count is singularised (
1 slice, not1 slices), andthe README's sample translation no longer reads
categoryCountfor every kind. The histogram'saxis-label skip is left alone — its drawn axes really are derived bins and counts.
Also in here
ROADMAP item 2.5 — theming for the remaining components, tabulating what still reads colours
straight from the
ColorScheme(composer, suggestions, attachment sheet, typing indicator, codeblock), noting that Swift's
Colorscovers only composer/suggestions/transcription, and flaggingthat at five or six component themes the hand-written boilerplate stops being cheaper than
theme_extensions_builder.🤖 Generated with Claude Code