Skip to content

Language support#491

Merged
dewabisma merged 28 commits into
mainfrom
beast/language-support
May 23, 2026
Merged

Language support#491
dewabisma merged 28 commits into
mainfrom
beast/language-support

Conversation

@dewabisma
Copy link
Copy Markdown
Collaborator

@dewabisma dewabisma commented May 20, 2026

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

  • English
Simulator Screenshot - iPhone 8 - 2026-05-20 at 12 35 09
  • Bahasa
Simulator Screenshot - iPhone 8 - 2026-05-20 at 12 35 01
  • Language Picker
Simulator Screenshot - iPhone 8 - 2026-05-20 at 12 35 04

Note

Medium Risk
Introduces a new localization system and wires locale into MaterialApp and number formatting, which can affect all user-facing text and formatting. Also changes logout behavior to asynchronously reset settings and guard navigation with context.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 generated AppLocalizations sources, plus an AppLocale model and a Riverpod l10nProvider/selectedAppLocaleProvider for persisted locale selection (defaulting from device locale).

Wires locale through the app runtime. MaterialApp now sets locale and localization delegates/supported locales, number formatting is driven by the selected app locale (instead of a one-time Platform.localeName snapshot), and initial widgets begin consuming localized strings (e.g., AddressDetailsCard, NameField). Logout now resets locale and fiat currency selections via new reset() methods and only navigates when context.mounted.

Reviewed by Cursor Bugbot for commit 8bbbfa7. Bugbot is set up for automated code reviews on this repo. Configure here.

@dewabisma dewabisma requested a review from n13 May 20, 2026 04:36
@v12-auditor
Copy link
Copy Markdown

v12-auditor Bot commented May 20, 2026

Warning

No auditable source files found in this PR's diff.

Comment thread mobile-app/lib/l10n/app_id.arb Outdated
Comment thread mobile-app/lib/l10n/app_id.arb Outdated
Comment thread mobile-app/lib/providers/l10n_provider.dart
Comment thread mobile-app/lib/v2/screens/pos/pos_qr_screen.dart Outdated
- use intl from sdk
- import legacy provider
Comment thread mobile-app/lib/providers/l10n_provider.dart
Comment thread mobile-app/lib/services/logout_service.dart Outdated
@n13
Copy link
Copy Markdown
Collaborator

n13 commented May 20, 2026

Review: PR #491 – Language Support (EN/ID)

Overall: Nice, well-scoped localization effort. Architecture is solid: a single selectedAppLocaleProvider + l10nProvider pair, a documented "use ref.watch(l10nProvider) not AppLocalizations.of(context)" convention, and a generic SettingsPickerScreen<T> that already DRYs language and currency pickers. 77 files, 322 keys, fully present in both locales. The iOS SwiftPM + Xcode prepare pre-action migration is standard and needed to make flutter_localizations build cleanly.

Below are the issues I'd want addressed before merge, grouped by priority.


High priority

1. SettingsPickerScreen loading guard is broken (new bug)

In mobile-app/lib/v2/screens/settings/settings_picker_screen.dart lines 104–108 the onSelect future is fire-and-forget, so _isLoading flips back to false synchronously before the await even starts:

                              if (_isLoading) return;

                              setState(() => _isLoading = true);
                              widget.onSelect(item);
                              setState(() => _isLoading = false);

This makes the guard against rapid taps a no-op and is the underlying cause of the "Locale select race condition" Bugbot flagged. Fix:

onTap: () async {
  if (_isLoading) return;
  setState(() => _isLoading = true);
  try {
    await widget.onSelect(item);
  } finally {
    if (mounted) setState(() => _isLoading = false);
  }
},

This also fixes the same race for the currency picker (same widget).

2. Indonesian biometrics copy says "unlock the device" (Bugbot, still open)

  "authUseDeviceBiometricsToUnlock": "Gunakan biometrik untuk membuka perangkat",

The English string says "unlock" (the wallet); the Indonesian says "unlock the device". Suggested: "Gunakan biometrik perangkat untuk membuka" or "Gunakan biometrik untuk membuka dompet".

3. Inconsistent error prefix across ID strings (Bugbot, still open)

  "homeError": "Eror: {error}",
  "activityError": "Error: {error}",
  "posQrError": "Error: {error}",

Three different conventions in three keys ("Eror:", "Error:", and elsewhere "Gagal …"). Pick one. "Eror" is informal/loanword Indonesian; "Galat:" is the formal term; "Gagal" (used elsewhere) reads most naturally. I'd standardize on "Gagal: {error}" to match the rest of the file.


Medium priority

4. qr_scanner_page.dart uses an account-specific key as a generic title

          Positioned(top: 20, left: 24, right: 24, child: V2AppBar(title: l10n.addHardwareAccountScanQr)),

QrScannerPage is reused in several flows (send recipient, hardware account, etc.), but the title key is addHardwareAccountScanQr. Add a componentQrScannerTitle (already have componentQrScannerNoCode) and use that here. Easy win and DRYs future use.

5. _getDepositAddress returns the localized demo warning, not an address

  String _getDepositAddress(AppLocalizations l10n) {
    return l10n.swapDepositDemoWarning;
  }

  void _copyAddress(AppLocalizations l10n) {
    context.copyTextWithToaster(_getDepositAddress(l10n));
  }

This is pre-existing demo behavior, but it's now even more confusing because the function name says "address" while it returns a localized warning. Either rename to _getDisplayedDepositText or pull the demo string into a local variable in the build method. Not blocking, but worth a follow-up.

6. debugPrint of sender address/amount/hash in pos_qr_screen.dart (Bugbot)

    debugPrint('[PosQr] watching address=${active.account.accountId} expected=$expectedPlanck planck');
        debugPrint('[PosQr] onTransfer from=${tx.from} amount=${tx.amount} hash=${tx.txHash}');

Note debugPrint is not stripped in release builds in Flutter — it only loses rate-limiting wrt print. For privacy I'd at minimum truncate the address (you already have AddressFormattingService.formatAddress) and drop the amount and txHash from the production path, or wrap in if (kDebugMode).

7. Default-locale fallback uses Platform.localeName (mobile-only)

  static final String _defaultLocaleCode = Platform.localeName.split('_').first.toLowerCase();

Importing dart:io here breaks Flutter web compilation. If mobile-app isn't built for web today, fine — but the SDK is shared, so worth a comment or using WidgetsBinding.instance.platformDispatcher.locale (which works on all platforms) instead.

Also: reset() snaps to device locale, which is a UX choice. Worth confirming with design that wallet-reset really should reapply system language vs. keeping the user's last selection.


Low priority / nits

8. DRY between LanguagePickerScreenV2 and CurrencyPickerScreenV2

The two pickers have identical try/catch/toaster/navigate logic — only the label keys differ. Could lift into a helper inside SettingsPickerScreen (e.g. errorMessageBuilder callback) so callers just pass the strings. Not a must-have given there are only two callers today.

9. accountsSheetLoading reused as a generic "Loading…" outside the accounts sheet

                loading: () => _buildTappableRow(e.value, subtitle: l10n.accountsSheetLoading, trailing: trailing),

Works fine, but the key name lies. Consider a generic commonLoading key.

10. sendInputAmountBalance ("{balance} {symbol}") reused as a value formatter in review_send_screen.dart

Same DRY-ish concern as above — using a key named for one screen as a general balance formatter. Either rename to commonAmountWithSymbol or move the formatting helper into NumberFormattingService so callers don't need a localization key for trivial concatenation. (You already have formatBalance there with addSymbol: true — could often replace this entirely.)

11. Hardcoded numberFormatLocale per app locale

  en(languageCode: 'en', displayName: 'English', numberFormatLocale: 'en_US'),
  id(languageCode: 'id', displayName: 'Bahasa Indonesia', numberFormatLocale: 'id_ID'),

Fine for two locales. When you add more, derive numberFormatLocale from languageCode (e.g. via Intl.canonicalizedLocale) so you're not maintaining two parallel lists.

12. Tests

send_screen_logic_test.dart was updated to thread l10n through — great. There's no test for SelectedAppLocaleNotifier.select/reset or for the picker's loading guard. Given the bug in #1, a small widget test that double-taps the picker would have caught it.

13. Authorization \n Required looks weird but is OK

"authAuthorizationRequired": "Authorization \n Required" (and Indonesian equivalent) renders with a leading space on the second line due to \n having a space on each side. Intentional? If yes, fine. If you wanted clean centering, drop the leading space: "Authorization\n Required" or just "Authorization\nRequired".

14. Duplicate Package.resolved

mobile-app/ios/Runner.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved
mobile-app/ios/Runner.xcworkspace/xcshareddata/swiftpm/Package.resolved

Both new files are byte-identical. That's Xcode's normal behavior with SwiftPM under a .xcworkspace, not your bug, but if your .gitignore lets you commit only one, you'll avoid drift later.


Things that look great

  • selectedAppLocaleProvider + l10nProvider pattern with the explicit library doc is excellent — please keep that as the project convention.
  • SettingsPickerScreen<T> is exactly the right generic abstraction; reused immediately for both pickers.
  • Threading AppLocalizations into pure logic (SendScreenLogic.getButtonText, TxItemData.from, dateGroupLabel) instead of touching BuildContext is the right call.
  • The formatPaidAt/formatDateGroupLabel additions in quantus_sdk/lib/src/services/datetime_formatting_service.dart use locale-aware DateFormat.yMMMd(localeName) — chef's kiss.
  • 322/322 key parity between EN and ID — no missing translations.

If you want, I can push a fixup commit for the await widget.onSelect(item) guard and the three ID-translation fixes in a single small change — just say the word (and confirm you want me to commit/push since the rule is to ask).

Original PR: Language support · Pull Request #491 · Quantus-Network/quantus-apps

Comment thread mobile-app/lib/providers/l10n_provider.dart
@n13
Copy link
Copy Markdown
Collaborator

n13 commented May 21, 2026

Here's my fresh review of PR #491 against the latest commit 245b865.

Review: PR #491 – Language Support (EN/ID), follow-up pass

Overall: The recent commits (fix: resolve review issues, feat: address reset bug, feat: remove redundant setting clear, fix: reset depend on setting clear) cleared out most of the high/medium priority items from the previous review. The DRY picker abstraction, race fix, common keys, and Indonesian wording fixes are all in. Below are what remains and a couple of new things spotted on this pass.

High priority

  1. pos_qr_screen.dart still logs sender PII in release builds
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 printdebugPrint only changed the print sink and added rate-limiting; debugPrint is NOT stripped in release builds. Sender addresses, amounts, and tx hashes still end up in device logs (and any log uploader) on prod. Either wrap in if (kDebugMode) or truncate via AddressFormattingService.formatAddress and drop amount/txHash in non-debug.

  1. SubstrateService().logout() not awaited → race with locale/currency reset
SubstrateService().logout();
_ref.read(pendingTransactionsProvider.notifier).clear();
...
_ref.read(selectedAppLocaleProvider.notifier).reset();
_ref.read(selectedFiatCurrencyProvider.notifier).reset();

SubstrateService.logout() is Future<void> and internally awaits _settingsService.clearAll(). Today's LogoutService.logout(...) is Future<void> Function(BuildContext) declared as sync (no async), so SubstrateService().logout() runs without await. After the recent change that removed SettingsService().clearAll(), the only thing clearing the locale/currency keys is this un-awaited future. By the time Navigator.pushAndRemoveUntil(...) runs, the prefs clear may still be in flight, and the in-memory reset() snaps state to device locale even though the persisted preference hasn't been wiped yet. If SubstrateService.logout() ever fails (it has no try/catch around the prefs clear) the user's locale silently survives a "wallet reset", which is exactly what the previous review flow tried to fix.

  • Either make the chain async and await SubstrateService().logout(); before the resets, or have SelectedAppLocaleNotifier.reset()/SelectedFiatCurrencyNotifier.reset() also call _settings.remove(...) so the persisted state is guaranteed to match the in-memory state regardless of the substrate call. The current notifier API only persists on select(...):
Future<void> select(AppLocale locale) async {
  await _settings.setSelectedAppLocale(locale.languageCode);
  state = locale;
}

void reset() {
  state = AppLocale.fromCode(_defaultLocaleCode);
}
  1. DateFormat.yMMMd('id_ID') likely needs initializeDateFormatting
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 (yMMMd, jm) with a non-default locale (id_ID). I couldn't find an initializeDateFormatting() call anywhere in the repo. With flutter_localizations configured for Locale('id'), calling DateFormat.yMMMd('id_ID') typically throws LocaleDataException: Locale data has not been initializedLocale('id') and 'id_ID' are two different keys in intl's locale store. Worth a quick smoke test on the POS "paid" screen and the Activity date headers in Indonesian — and either call await initializeDateFormatting('id_ID', null) once at startup, or switch the numberFormatLocale to bare 'id' to match the language code. (Same for 'en_US' vs 'en'.)

Medium priority

  1. SettingsCautionScaffold.continueLabel defaults to '' — easy footgun
this.continueLabel = '',

Both current callers pass it, but any new caller that forgets renders a button with no label. Make this required (and required at the type level forces the localized string at the call site).

  1. _filenameOf("locale") error log in the generic picker
debugPrint('error selecting locale: $e');

SettingsPickerScreen<T> is reused for both language and currency, but the debug log hardcodes "locale". Either drop the qualifier or include T.toString() so currency errors aren't logged under "locale".

  1. _getDepositAddress still returns the demo warning string (pre-existing, but now more visible)
// 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 address: ${_getDepositAddress(l10n)} inside swapDepositShareContent, which is confusing. The added comment helps, but renaming to _getDisplayedDepositText or inlining for the demo path would prevent surprises when the real address ships.

Low priority / nits

  1. continueLabel reuses a "createWallet…" key for the wallet-reset flow
continueLabel: l10n.createWalletCautionContinue,

Functionally fine (the text is just "Continue"/"Lanjutkan"), but a commonContinue key would avoid the cross-feature coupling — same pattern as commonLoading / commonAmountBalance you already extracted in this PR.

  1. SelectedAppLocaleNotifier still imports dart:io for Platform.localeName
import 'dart:io';
...
static final String _defaultLocaleCode = Platform.localeName.split('_').first.toLowerCase();

WidgetsBinding.instance.platformDispatcher.locale.languageCode would work on web too. Not a blocker if web isn't a target, but adds a future-compat footgun in a file that otherwise looks like pure Dart UI plumbing.

  1. No tests for the new behavior most likely to regress

    • SelectedAppLocaleNotifier.select / .reset (especially the new reset-on-logout flow)
    • SettingsPickerScreen<T> rapid-tap guard (the bug that did exist, now fixed)
      send_screen_logic_test.dart was nicely updated — same treatment for these two would catch the next regression cheaply.
  2. Indonesian time abbreviations are correct but ambiguous when switching locales

String activityTxTimeHoursAgo(int hours) {
  return '${hours}j lalu';
}
...
String activityTxTimeDaysAgo(int days) {
  return '${days}h lalu';
}

j = jam (hours), h = hari (days) is correct. Just flagging that 5h means "5 days" in ID and "5 hours" in EN — a user toggling locales will get surprised once. Not a bug; possibly worth a usability check with a native speaker if you want to consider 5j / 5hr or full words.

  1. continueLabel argument ordering nit: SettingsCautionScaffold now has continueLabel as an optional named arg sandwiched between required ones — easy to miss when scanning. Minor; only matters if you make it required (Fix warnings1 #4).

Things that look great on this pass

  • The race-condition fix in SettingsPickerScreen<T> is exactly what was asked for — onSelect is Future<void> Function(T), awaited inside a try/finally, and _isLoading is correctly toggled both ways.
  • errorMessageBuilder lifted into SettingsPickerScreen<T> removed the duplicated try/catch from both pickers; both now read as tiny 15-line ConsumerWidgets. Lovely.
  • The commonAmountBalance and commonLoading extractions + componentQrScannerTitle kill the "key named for one screen, used everywhere" smell from the previous review.
  • Indonesian error prefixes are now consistently Gagal: {error} across homeError, activityError, posQrError, and friends.
  • authUseDeviceBiometricsToUnlock ID now says "untuk mengakses wallet" — matches the English intent.
  • SettingsService.setSelectedAppLocale / getSelectedAppLocale are minimal, single-responsibility, and the comment on the getter is exactly right.

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.

@n13
Copy link
Copy Markdown
Collaborator

n13 commented May 21, 2026

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...

@n13
Copy link
Copy Markdown
Collaborator

n13 commented May 22, 2026

PR #491 — Language Support (EN/ID)

Latest commit: e8afa78 (feat: resolve PR issues)
Branch: beast/language-supportmain
Scope: ~77 files, +7.5k / −812, 322 ARB keys (EN/ID parity)
CI: Analyze, Cursor Bugbot, Security Reviewer — all green on e8afa78

This is a third pass against your earlier reviews and the follow-up commits (e92bb03, aeaad0d, e8afa78). Most of the earlier blockers are addressed; a few items remain before I’d call it fully merge-ready.


Resolved since the last review

Item Status
SettingsPickerScreen loading / double-tap race Fixed — await widget.onSelect, try/finally, _isLoading
Indonesian biometrics / error prefixes Fixed — consistent Gagal: / wallet wording
QR scanner generic title Fixed — componentQrScannerTitle
PII in release logs (POS) Fixed — quantusDebugPrintkDebugMode only (matches your agreement with dewabisma)
SubstrateService().logout() ordering Fixed — await inside _clearSettingsAndMemory() before locale/currency reset()
continueLabel footgun Fixed — required on SettingsCautionScaffold
commonContinue Added; wallet-reset / recovery flows use it
Picker error log says "locale" Fixed — generic [SettingsPickerScreen] message
DRY pickers + errorMessageBuilder In good shape

High priority (still worth fixing)

1. logout() still does not await cleanup before navigation

void logout(BuildContext context) {
  _clearSettingsAndMemory();  // fire-and-forget
  Navigator.pushAndRemoveUntil(...);
}

_clearSettingsAndMemory() correctly awaits SubstrateService().logout() (which clears prefs via clearAll()), then resets providers — but the welcome screen can appear while that future is still running. That can briefly show the old locale (Bugbot’s “stale locale after reset” class of issue).

Suggestion: make logout async and await _clearSettingsAndMemory() before pushAndRemoveUntil, or keep navigation but block the welcome UI until cleanup finishes (loader on reset path).

2. DateFormat.yMMMd('id_ID') without initializeDateFormatting

DatetimeFormattingService.formatPaidAt / formatDateGroupLabel use appLocale.numberFormatLocale (id_ID, en_US). There is still no initializeDateFormatting anywhere in the repo. On a real device in Indonesian, smoke-test:

  • Activity date group headers
  • POS “paid” timestamp

If you see LocaleDataException, either call initializeDateFormatting once at startup (for supported locales) or align locale strings with Flutter’s Locale('id') / 'en' instead of 'id_ID' / 'en_US'.

This is the main functional risk I’d verify manually before merge.

3. SelectedAppLocaleNotifier.select — out-of-order persistence (Bugbot, still plausible)

Rapid language changes can complete select() calls out of order so persisted prefs and UI disagree. The picker guard helps within one sheet, but not if the user changes language twice quickly from different entry points.

Suggestion: ignore stale completions (generation counter) or no-op when state == locale after await.


Medium priority

4. POS debug logs still print full identifiers in debug

quantusDebugPrint correctly strips release builds, but debug builds still log full accountId, from, amount, and txHash. Acceptable for local debugging; optional hardening: truncate addresses via AddressFormattingService.formatAddress even in debug.

5. reset() only updates memory, not prefs

Fine when always preceded by clearAll() (logout path). If reset() is ever called alone, stored locale can disagree with in-memory state. Either document that contract or have reset() clear _selectedAppLocaleKey when you intend a true reset.

6. _getDepositAddress still returns the demo warning

Pre-existing demo behavior; naming is misleading now that copy is localized. Rename to _getDisplayedDepositText when you touch swap again.

7. dart:io / Platform.localeName in l10n_provider.dart

Blocks web builds for that file. Prefer WidgetsBinding.instance.platformDispatcher.locale.languageCode if web matters later.


Low priority / nits

  • No unit tests for SelectedAppLocaleNotifier or picker double-tap (regression-prone areas you already hit once).
  • Indonesian 5h = hari vs English 5h = hours — fine linguistically; optional UX pass with a native speaker.
  • numberFormatLocale per enum entry is OK for two locales; derive when you add more.
  • Duplicate Package.resolved under iOS workspace — Xcode norm, not a PR defect.

What looks strong

  • l10nProvider + library doc — clear “don’t use AppLocalizations.of(context)” convention.
  • SettingsPickerScreen<T> — good generic abstraction for language and currency.
  • Logic layerAppLocalizations passed into SendScreenLogic, TxItemData, etc.
  • 322/322 EN/ID key parity — no missing translations.
  • send_screen_logic_test.dart — updated and passing on the PR branch.

Merge recommendation

Close to merge after:

  1. Manual smoke test: Indonesian activity dates + POS paid screen (catches Feature/dilithium signatures #2).
  2. Either await logout cleanup before welcome navigation (Rust Bindings #1), or explicitly accept the brief stale-locale window and document it.

Optional before merge: locale select() ordering guard (#3).

I would not block on deposit-screen naming, web Platform.localeName, or extra tests unless you want stricter regression coverage.

If you want, I can implement #1#3 on beast/language-support in a small follow-up commit (only if you ask me to change the branch).

@dewabisma
Copy link
Copy Markdown
Collaborator Author

dewabisma commented May 22, 2026

I tested the #2 raised issue, and there is nothing wrong. It's properly localize to bahasa.

I will fix the #1 issue by waiting the settings, so we can have proper order instead of race condition.

Copy link
Copy Markdown
Collaborator

@n13 n13 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GTG

@dewabisma dewabisma merged commit 4e391bc into main May 23, 2026
3 checks passed
Copy link
Copy Markdown

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ 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);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 8bbbfa7. Configure here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants