Repository navigation
feat(ai): make every string the package renders translatable - #8
Conversation
Introduces AITranslations (abstract), DefaultAITranslations (the English the widgets already rendered) and AITranslationsScope, replacing the fourteen literals that were hardcoded across ChatComposer, its attachment sheet, the voice-input button and CodeBlockView. AITranslations.of falls back to the default rather than asserting, so a host that adds no scope sees no change. Ten of the fourteen are Tooltip messages on icon-only buttons, so they are also the only accessible label those buttons expose — a screen reader outside English read English or nothing. A scope resolves through the widget's own BuildContext rather than being threaded as a constructor argument. Twelve of the literals live in private leaf widgets two to four layers below a public one, and CodeBlockView's two are constructed inside the top-level _buildFenceCached, which has no BuildContext at all; threading would have meant ten new parameters plus a new component in the fence cache key. Translations deliberately are not part of that key — the cached object is an unbuilt widget configuration, and the string is resolved later from the element's own context, which ai_markdown_body_test asserts directly. The one place the mechanism needs help is the attachment sheet: it is pushed with showModalBottomSheet, so _AttachmentButton reads the translations before pushing and re-provides them inside the sheet's route. Ships English only — this is the seam, not a set of translations. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…tedTheme Review follow-up on the localization scaffolding. `AITranslationsScope` extends `InheritedTheme` and overrides `wrap` instead of being a plain `InheritedWidget`. `showModalBottomSheet`, `showDialog` and `showMenu` all call `InheritedTheme.capture` on the way to the Navigator, so the scope now crosses a route push on its own and `_AttachmentButton`'s hand-rolled snapshot-and-re-provide goes away. That also closes the case the old approach could not reach: a host presenting a `ComposerAttachmentSheet` itself no longer owes the re-provision, so the caveat is gone from the class doc and the README rather than merely being documented. Also from the review: - `allowPhotoAccess` was the one string of the fourteen with no test, and the one `_TestTranslations` in `chat_composer_test.dart` left at its default while claiming to override every string the composer and its sheet render. The sheet never leaves its spinner in tests without a permission answer, so the photo_manager channel is mocked into refusing to reach the branch. - `AITranslationsScope.of` joins `maybeOf` on the widget, per the usual Flutter shape; `AITranslations.of` stays as the shorthand every call site uses. - `AITranslations.of` no longer claims to register a dependency in the case where it finds no scope, which it does not. - Dropped the `chat_composer.dart` import that existed only for two doc links and put a cycle under the lowest-level file in the package. - README shows the `Localizations.localeOf` hookup, which is what a host actually needs and the place the const-instance rule bites. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
0494cb2 to
d8a70d6
Compare
AITranslationsScope alone meant an app covering several languages had to map locale to instance by hand, in a MaterialApp.builder, and re-provide it. AITranslationsDelegate is a plain LocalizationsDelegate<AITranslations> that goes on MaterialApp.localizationsDelegates, so Flutter resolves the locale, picks the instance, reloads on a locale change, and carries the result across routes -- the same mechanism stream_chat_flutter resolves its own Translations through. AITranslations.of now reads scope, then delegate, then the English defaults, and AITranslationsScope.of is gone: two public lookups with different resolution orders is a trap, so the scope keeps only maybeOf. A scope outranks the delegate, being the narrower of the two. Two deliberate differences from stream_chat_localizations. The map is keyed by String, not Locale: Locale overrides ==, and Dart forbids such a type as a const map key, so a Locale-keyed delegate could never be const -- which is exactly what the scope's identity comparison asks for. And isSupported returns true unconditionally, because WidgetsApp warns in debug about any supportedLocales entry a delegate refuses, and "this locale renders the English defaults" is not a refusal; load resolves the fallback instead. resolve() is the honest answer for callers that need one, and a debug-only check rejects a key that is not in Locale.toString()'s form rather than letting it silently never match. Also in here, from reviewing the rebase onto 2.2: - The literal counts were stale. 2.2 had already collapsed the enabled and disabled send tooltips into one call site, so there were fourteen literals to cover, not fifteen, and ten of them (not twelve) sit in private leaf widgets. Corrected in the ROADMAP, the CHANGELOG and a test comment. - The composer hint is the one string resolved outside the widget that renders it, since 2.2 made it a props field whose default is a const. Documented where it happens, and pinned by two tests: the hint follows a scope swapped at runtime, and a custom input slot is handed the translated string rather than the constant. - @immutable on AITranslations, so a subclass with a mutable field is an analyzer error rather than a paragraph of documentation. - The const rationale is stated once per place instead of explained. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The debug-only key check sat in load, so a malformed key threw there while resolve -- the method documented as the honest answer about what is registered -- returned null for it in silence. Two answers to one question, and the quiet one is the misleading half: null means "this locale is not translated", which is the wrong story for a locale that was translated under a key with a typo in it. Moved into resolve, which load goes through, so both report it. Nothing changes for a well-formed map, and the check stays debug-only. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
| /// `package:flutter/widgets.dart`. `GlobalMaterialLocalizations` above comes | ||
| /// from `flutter_localizations`, which a host translating the rest of its app | ||
| /// will already have. | ||
| class AITranslationsDelegate extends LocalizationsDelegate<AITranslations> { |
There was a problem hiding this comment.
Thinking if it makes sense to actually use a AITranslationsDelegate inside the sample app (even just the default one) - might serve as a decent documentation/example on how to customise this.
There was a problem hiding this comment.
Good call — the example had no localization at all, so the feature only ever existed in README prose. It now registers the delegate:
localizationsDelegates: const [
AITranslationsDelegate({'nl': DutchTranslations()}),
GlobalMaterialLocalizations.delegate,
GlobalWidgetsLocalizations.delegate,
GlobalCupertinoLocalizations.delegate,
],
supportedLocales: const [Locale('en'), Locale('nl')],with a full DutchTranslations at the bottom of main.dart. The locale comes from the device, so run it on a device set to Dutch to see the placeholder and tooltips follow.
One thing that had to go with it: the example passed hintText: 'Ask anything' to ChatComposer, which wins over the translation — it would have demonstrated the feature while visibly defeating it.
|
|
||
| /// The shape [AITranslationsDelegate.translations]'s keys have to take: | ||
| /// `nl`, `pt_BR`, `zh_Hant_TW`. | ||
| final _localeKey = RegExp(r'^[a-zA-Z]{2,8}(_[a-zA-Z0-9]{2,8})*$'); |
There was a problem hiding this comment.
Curious if this needs to be case-sensitive? As far as I understand, this regex would match something like: NL -> But will that successfully match any Locale?
There was a problem hiding this comment.
You're right, and it was a real hole in the check. Locale compares its subtags verbatim — languageCode only remaps deprecated codes (iw→he), never case — so 'NL' passed the old [a-zA-Z] pattern and could then never match Locale('nl'). That's exactly the "silently renders English" failure the assert exists to catch, and it sailed through.
The pattern now mirrors what Locale.toString() can actually produce — language, optional script, optional region, in that order, with the casing each carries:
final _localeKey = RegExp(r'^[a-z]{2,8}(_[A-Z][a-z]{3})?(_([A-Z]{2}|\d{3}))?$');The error message calls casing out explicitly now. Two tests came with it: one rejecting NL, pt_br, PT_BR, zh_hant_TW, and a guard against over-tightening that accepts nl, pt_BR, zh_Hant, zh_Hant_TW, es_419.
Worth flagging one judgment call: this also rejects a key matching a deliberately mis-cased Locale, e.g. Locale('nl', 'be') → 'nl_be'. Flutter's own docs call that Locale malformed, so I took rejecting it as correct — happy to loosen if you'd rather be permissive there.
| // constructed ChatComposerInputProps still defaults to that constant, so two | ||
| // copies of the English hint could drift apart. | ||
| @override | ||
| String get composerHint => ChatComposerInputProps.defaultHintText; |
There was a problem hiding this comment.
I find this a bit strange - having a dependency on the ChatComposerInputProps here.
Wouldn't it be possible to:
- Define here:
String get composerHint => 'Ask anything...' - Make the
ChatComposerInputProps.hintTextnullable - Fallback to the localised text when reading the prop:
props.hintText ?? AITranslations.of(context).composerHint - Drop the
ChatComposerInputProps.defaultHintText
I believe this way we can avoid having the ChatComposerInputProps.defaultHintText + its reference here.
There was a problem hiding this comment.
Agreed — the dependency pointed the wrong way, and the localization file needed an import of the composer props purely for that one literal. Done as you described: the literal lives in DefaultAITranslations.composerHint, hintText is nullable, and defaultHintText is gone.
We went one step further than your step 3. Rather than ChatComposer resolving the fallback and handing the input a non-null string, it now forwards hintText unresolved and ChatComposerInput does the ?? itself:
hintText: props.hintText ?? AITranslations.of(context).composerHint,So the hint resolves the way every other string in the package does — at the widget that renders it — instead of being the one threaded exception. Two knock-on effects:
props.hintText == nullnow means "the host expressed no preference", which a custom input can act on. Previously it couldn't tell a hint the host chose from the package's own default.- A factory that wraps the input in its own
AITranslationsScopegets the hint from that scope rather than the outer one, matching the tooltips beside it.
The cost is that a custom input forwarding props.hintText straight into an InputDecoration renders no placeholder unless it repeats the fallback — the README example did exactly that, so it's been updated. That seemed acceptable on the grounds that a custom input generally brings its own hint anyway.
ChatComposerInputProps.hintText becoming String? is a breaking change for a custom input reading it, so it's noted as one in the changelog.
Three points from the review on #8. Reject a mis-cased delegate key. `Locale` compares its subtags verbatim and never normalizes case, so `'NL'` matches no locale at all — exactly the "silently renders English" failure the debug check exists to catch, and it passed. The key pattern now mirrors what `Locale.toString()` can actually produce: lowercase language, optional title-case script, optional uppercase or numeric region, in that order. Point the composer hint's dependency the right way round. The literal lived on `ChatComposerInputProps.defaultHintText` and `DefaultAITranslations.composerHint` reached up into the composer layer for it, import and all. The literal now lives in the translations, `hintText` is nullable, and `ChatComposerInput` resolves `null` against `AITranslations.composerHint` itself. `ChatComposer` still resolves the hint eagerly, so a custom input forwarding `props.hintText` straight through is translated without repeating the fallback. Wire the delegate up in the example app, which had no localization at all — the feature was only ever shown in README prose. It now registers a Dutch translation and puts a language button in the app bar, so the whole path can be seen running. Its hardcoded `hintText` is gone; it won over the translation and would have made the demo contradict itself. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Three points from the review on #8. Reject a mis-cased delegate key. `Locale` compares its subtags verbatim and never normalizes case, so `'NL'` matches no locale at all — exactly the "silently renders English" failure the debug check exists to catch, and it passed. The key pattern now mirrors what `Locale.toString()` can actually produce: lowercase language, optional title-case script, optional uppercase or numeric region, in that order. Point the composer hint's dependency the right way round. `DefaultAITranslations.composerHint` reached up into the composer layer for its literal, import and all. The literal now lives in the translations and `ChatComposerInputProps.hintText` is nullable, `null` meaning the host expressed no preference — a breaking change for a custom input reading it. `ChatComposerInput` falls back to `AITranslations.composerHint`, which is how every other string in the package resolves: at the widget that renders it. The composer forwards `hintText` unresolved, so a factory wrapping the input in its own scope gets a hint from that scope rather than the outer one, matching the tooltips beside it. Wire the delegate up in the example app, which had no localization at all — the feature was only ever shown in README prose. Its hardcoded `hintText` is gone; it won over the translation and would have made the demo contradict itself. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
75e2050 to
36849dc
Compare
Implements ROADMAP item 2.3 — Localization scaffolding.
What
Every string this package renders itself now resolves through an injectable object, so a host can
translate them, or override any one of them, without forking a widget. New
lib/src/localization/ai_translations.dart, exported from the barrel:AITranslations— abstract,constconstructor,@immutable, 14 members for the 14 literalsthat were hardcoded.
clearOption(String option)is a method rather than a getter, being the oneinterpolated string.
DefaultAITranslations— the English the widgets already rendered, byte-identical.AITranslationsDelegate— aLocalizationsDelegate<AITranslations>forMaterialApp.localizationsDelegates. This is how an app covers several languages.AITranslationsScope— anInheritedTheme, for pinning one language over a subtree.AITranslations.of(context)reads the scope, then the delegate, thenconst DefaultAITranslations().It never throws and never asserts, so an app that registers neither sees no change at all.
Ships English only — this is the seam, not a set of translations. No
flutter_localizations, nointl, no.arb, no new dependency:LocalizationsandLocalizationsDelegateare both inpackage:flutter/widgets.dart. (GlobalMaterialLocalizationsabove isflutter_localizations,which a host translating the rest of its app already has.)
Why it matters more than it looks
Ten of the fourteen are
Tooltipmessages on icon-only buttons — send, stop, mic, "+", copy and thetwo dismiss controls — which makes them the only accessible label those buttons expose. Before this,
a screen reader outside English read English or nothing. That is the larger half of the change.
Decisions worth reviewing
The delegate, and two places it diverges from
stream_chat_localizationsThe mechanism is the one
stream_chat_flutteruses — resolve throughLocalizations, fall back toan English default (
StreamChatLocalizations.of(context) ?? DefaultTranslations.instance) — so ahost of both SDKs registers two delegates side by side. Two differences, each forced rather than
chosen:
String, notLocale.Localeoverrides==, and Dart forbids such atype as a
constmap key, so aLocale-keyed delegate could never beconst— which is exactlywhat the scope's identity comparison asks for. Keys take
Locale.toString()'s form:'nl', or'pt_BR', which beats a plain'pt'entry. A debug-only check rejects a key that could nevermatch (
'nl-BE', say) instead of leaving it to render English unexplained.isSupportedreturnstrueunconditionally.WidgetsAppwarns, in debug, about anysupportedLocalesentry that some delegate refuses — so answering honestly would put a warning inthe console of every app that supports more locales than it has translated this package into,
which is every app that adds the delegate first and translates later. "This locale renders the
English defaults" is not a refusal;
loadresolves the fallback, andresolve()is the honestanswer for anything that needs one.
There is no separate
stream_chat_ai_localizationspackage and no shipped locales: fourteen stringsare a seam, not a translation project.
A context lookup, not constructor threading
Ten of the fourteen literals live in private leaf widgets two to four layers below a public one
(
_TrailingControl,_AttachmentThumbnail,_SelectedOptionChip,_AttachmentButton,_CameraTile,_MicButton,_CopyButtonState), three more inComposerAttachmentSheet's privateState, andCodeBlockView's two are constructed inside the top-level_buildFenceCached, whichhas no
BuildContextat all. Threading would have meant ~10 new parameters plus a new component inthe fence cache key.
Translations deliberately are not part of that cache key, unlike
codeBackgroundColor/codeForegroundColor, which must be because they are constructor arguments baked into the cachedinstance. This looks like an oversight and isn't: the cached object is an unbuilt widget
configuration, the string is resolved later in
_CopyButtonState.buildfrom that element's owncontext, and the inherited-dependency mechanism marks the element dirty on a scope change regardless
of widget identity.
ai_markdown_body_test.dartasserts the claim directly —same(fence)on thecached instance while the rendered tooltip changes.
The composer hint is the one exception, and it is a seam
2.2 made
hintTexta field ofChatComposerInputProps, non-nullable, defaulting to thedefaultHintTextconstant — and aconstcannot consult a scope. SoChatComposer.buildresolvesit and hands the input an already-localized string, which is also what makes a custom
buildInputtranslated without doing the lookup itself. The consequence: a props object a host builds by
hand, outside the composer, carries the English default even inside a scope, while that same
widget's tooltips translate. Documented on
defaultHintText; two tests pin the composer's path(the hint follows a scope swapped at runtime, and a factory slot receives the translated string).
Routes are handled by the framework, not by hand
The attachment sheet is pushed with
showModalBottomSheet, so it sits under theNavigatorratherthan under whatever wraps the composer. Rather than snapshot the translations at the push and
re-provide them inside the sheet — which works, but only for the one call site that remembers to —
AITranslationsScopeextendsInheritedThemeand overrideswrap.showModalBottomSheet,showDialogandshowMenuall callInheritedTheme.captureon the way to theNavigator, so thescope crosses on its own, and a sheet a host pushes itself is translated too. Three tests pin it:
the composer's own sheet, a host-presented one, and a
showDialogproving the mechanism is generalrather than bottom-sheet-specific.
The one seam left is that
InheritedTheme.capturesnapshots at push time, so a scope swapped whilesuch a route is already open reaches it on the next open, not immediately. Documented on the scope
and in the README. The delegate has no such seam — its strings sit in
MaterialApp's ownLocalizations, above theNavigator.Naming
AITranslations, not theStreamAiTranslationsthe roadmap drafted. The package dropped theStreamprefix when it dropped thestream_chatdependency, and its AI-specific types carry acapital
AItoken (AIMarkdownBody,AISuggestionsView,AITypingIndicatorView).Other notes
ChatComposer.hintTextstill wins overAITranslations.composerHint— a hint written for onecomposer is more specific than an app-wide string. Documented and tested.
AITranslationsScopeexposesonly
maybeOf: two public lookups with different resolution orders would be a trap, soAITranslations.ofis the single documented entry point.DefaultAITranslations, don't implementAITranslations. Both work, but a stringadded in a later version arrives as an untranslated default for the former and a compile error for
the latter. The docs point at the former.
constconstructor. A scope compares instances to decide whether tonotify, so a fresh instance per
buildrebuilds every dependent; aconstmap of them also makesthe delegate itself
const.AITranslationsis@immutable, so a subclass with a mutable fieldis an analyzer error rather than a line of documentation.
uspec.dart's'Series'(×4) and'Pie'(×2) fallbacks. Theyare parser defaults assigned to
USeries.nameand the parser has noBuildContext; only'Series'is visible today, as a heatmap row label. Recorded under 2.4, which already owns chartpresentation.
Tests
263 passing, up from 216.
test/src/localization/ai_translations_test.dart— 20 cases. Twelve cover the delegate: thelocale's entry, a locale it does not carry falling back to English, a locale change swapping the
strings, country-vs-language key precedence, a scope outranking it, a pushed route, the
unconditional
isSupportedand why,shouldReload, and the debug-only key check. The other eightcover the scope: the no-scope fallback, an enclosing scope, nearest-of-nested,
updateShouldNotify,a dependent not rebuilding when the same
constinstance is re-supplied, a one-string subclasskeeping the rest, and a
showDialogcrossing a route both with a scope and without one.chat_composer_test.dart— every composer string, the composer's own sheet across itsroute, a host-presented sheet, the hint following a runtime scope swap, a factory slot receiving
the translated hint, and the denied-photo-access branch (which needs the
photo_managerchannelmocked into refusing, since without a permission answer the sheet never leaves its spinner).
speech_to_text_test.dart,code_block_view_test.dart, and the cached-fence case inai_markdown_body_test.dart.Every one of the fourteen strings is exercised.
melos run lint:all,CI=true melos run test:alland
melos run lint:pub(0 warnings) are green. No goldens regenerated — the English defaultsare unchanged and tooltips don't paint.
Rebased onto 2.2
This was rebased after #6 landed, which moved the composer's input into the public
ChatComposerInput. Worth knowing when reading the diff: the strings that used to live inchat_composer.dartare now inchat_composer_input.dart, and #6 had already collapsed theenabled and disabled send tooltips into a single call site — so this PR covers fourteen literals
with fourteen members, one-to-one, rather than collapsing anything itself.
Not verified
The manual example-app pass was skipped: the route-crossing case it was meant to catch is covered by
passing widget tests. Worth a hand-driven check if a reviewer wants one.
Local (
goldens/macos/) golden tests fail in a fresh worktree because that set is untracked andnever generated there — pre-existing and unrelated.
CI=truepasses.🤖 Generated with Claude Code