Language support#491
Conversation
- refactor DRY widget - delete redundant provider - fix bad number formatting locale
|
Warning No auditable source files found in this PR's diff. |
…into beast/language-support
- use intl from sdk - import legacy provider
Setting already cleared in substarte service logout
Review: PR #491 – Language Support (EN/ID)Overall: Nice, well-scoped localization effort. Architecture is solid: a single Below are the issues I'd want addressed before merge, grouped by priority. High priority1.
|
…into beast/language-support
|
Here's my fresh review of PR #491 against the latest commit Review: PR #491 – Language Support (EN/ID), follow-up passOverall: The recent commits ( High priority
debugPrint('[PosQr] watching address=${active.account.accountId} expected=$expectedPlanck planck');
_txWatch.watch(
address: active.account.accountId,
onTransfer: (tx) {
debugPrint('[PosQr] onTransfer from=${tx.from} amount=${tx.amount} hash=${tx.txHash}');
...
debugPrint('[PosQr] amount mismatch (received=$received expected=$expectedPlanck), ignoring');Switching
SubstrateService().logout();
_ref.read(pendingTransactionsProvider.notifier).clear();
...
_ref.read(selectedAppLocaleProvider.notifier).reset();
_ref.read(selectedFiatCurrencyProvider.notifier).reset();
Future<void> select(AppLocale locale) async {
await _settings.setSelectedAppLocale(locale.languageCode);
state = locale;
}
void reset() {
state = AppLocale.fromCode(_defaultLocaleCode);
}
static String formatPaidAt(DateTime dt, String localeName) {
final date = DateFormat.yMMMd(localeName).format(dt);
final time = DateFormat.jm(localeName).format(dt);
return '$date, $time';
}
static String formatDateGroupLabel(DateTime dt, String localeName) {
return DateFormat.yMMMd(localeName).format(dt);
}The new helpers use named formatters ( Medium priority
this.continueLabel = '',Both current callers pass it, but any new caller that forgets renders a button with no label. Make this
debugPrint('error selecting locale: $e');
// Currently this is only for demo purposes
// We just return the demo warning for now
String _getDepositAddress(AppLocalizations l10n) {
return l10n.swapDepositDemoWarning;
}Then shared as Low priority / nits
continueLabel: l10n.createWalletCautionContinue,Functionally fine (the text is just "Continue"/"Lanjutkan"), but a
import 'dart:io';
...
static final String _defaultLocaleCode = Platform.localeName.split('_').first.toLowerCase();
String activityTxTimeHoursAgo(int hours) {
return '${hours}j lalu';
}
...
String activityTxTimeDaysAgo(int days) {
return '${days}h lalu';
}
Things that look great on this pass
If you'd like, I can hold off making any changes (per the "don't change the files" instruction) and just leave this review for the second reviewer to cross-check against. |
|
I agree these prints should be in kDebugMode ^^^ We don't want to have the logs contain anything b/c we're a privacy oriented chain... but we need these for debugging... |
PR #491 — Language Support (EN/ID)Latest commit: This is a third pass against your earlier reviews and the follow-up commits ( Resolved since the last review
High priority (still worth fixing)1. void logout(BuildContext context) {
_clearSettingsAndMemory(); // fire-and-forget
Navigator.pushAndRemoveUntil(...);
}
Suggestion: make 2.
If you see This is the main functional risk I’d verify manually before merge. 3. Rapid language changes can complete Suggestion: ignore stale completions (generation counter) or no-op when Medium priority4. POS debug logs still print full identifiers in debug
5. Fine when always preceded by 6. Pre-existing demo behavior; naming is misleading now that copy is localized. Rename to 7. Blocks web builds for that file. Prefer Low priority / nits
What looks strong
Merge recommendationClose to merge after:
Optional before merge: locale I would not block on deposit-screen naming, web If you want, I can implement #1–#3 on |
…into beast/language-support
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 8bbbfa7. Configure here.
| final localeNumberConfigProvider = Provider<LocaleNumberConfig>((ref) { | ||
| return LocaleNumberConfig.fromLocale(Platform.localeName); | ||
| final appLocale = ref.watch(selectedAppLocaleProvider); | ||
| return LocaleNumberConfig.fromLocale(appLocale.numberFormatLocale); |
There was a problem hiding this comment.
Notification amounts ignore locale
Low Severity
Local notification amounts still format via Platform.localeName, while the rest of the app uses localeNumberConfigProvider tied to the user’s selected language. After switching to Bahasa (or another locale), notification copy can show device-style number formatting instead of matching the chosen app locale.
Reviewed by Cursor Bugbot for commit 8bbbfa7. Configure here.


Summary
Support localization of language. Now, user can pick their preferred language. Currently only support EN and ID. We can add more as we go.
Screenshots
Note
Medium Risk
Introduces a new localization system and wires locale into
MaterialAppand number formatting, which can affect all user-facing text and formatting. Also changes logout behavior to asynchronously reset settings and guard navigation withcontext.mounted.Overview
Adds app-wide localization support (EN + Indonesian). Introduces Flutter gen-l10n configuration (
l10n.yaml), ARB files (app_en.arb,app_id.arb), and generatedAppLocalizationssources, plus anAppLocalemodel and a Riverpodl10nProvider/selectedAppLocaleProviderfor persisted locale selection (defaulting from device locale).Wires locale through the app runtime.
MaterialAppnow setslocaleand localization delegates/supported locales, number formatting is driven by the selected app locale (instead of a one-timePlatform.localeNamesnapshot), and initial widgets begin consuming localized strings (e.g.,AddressDetailsCard,NameField). Logout now resets locale and fiat currency selections via newreset()methods and only navigates whencontext.mounted.Reviewed by Cursor Bugbot for commit 8bbbfa7. Bugbot is set up for automated code reviews on this repo. Configure here.