fix: Add base fallback files for nb and zh#1589
fix: Add base fallback files for nb and zh#1589shraavv wants to merge 1 commit intofossasia:developmentfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR addresses Flutter’s stricter localization generation requirements by adding base fallback locale ARB files for Norwegian Bokmål (nb) and Chinese (zh) so that region/script-specific ARBs (e.g., nb_NO, zh_Hant) have a required base.
Changes:
- Added base fallback ARB files:
lib/l10n/app_nb.arbandlib/l10n/app_zh.arb. - Regenerated localization outputs (updated
app_localizations.dart, added multipleapp_localizations_*.dartfiles, and expandeduntranslated.json). - Updated
pubspec.lock(dependency resolutions + SDK minimums) and modified macOS plugin registration output.
Reviewed changes
Copilot reviewed 16 out of 18 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| untranslated.json | Regenerated untranslated message report across locales. |
| pubspec.lock | Dependency resolution and SDK minimum updates. |
| macos/Flutter/GeneratedPluginRegistrant.swift | Plugin registrant changed (notably removed path_provider registration). |
| lib/l10n/app_zh.arb | Added base zh ARB fallback (currently empty). |
| lib/l10n/app_nb.arb | Added base nb ARB fallback (empty). |
| lib/l10n/app_localizations.dart | Regenerated localization delegate/lookup and supported locales list. |
| lib/l10n/app_localizations_*.dart | Newly generated per-locale Dart localization implementations. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Hi @shraavv, I reviewed the PR and noticed that, along with the ARB fallback files, several generated files and platform-specific changes (such as the macOS plugin registrant and pubspec.lock updates) are also included. Since the issue specifically focuses on adding the base fallback ARB files for nb and zh, do you think we could keep the PR more minimal by limiting it to only the required localization changes? |
|
@Aryan-Singla Thank you for the review! Will update the PR accordingly. |
c053203 to
9d1bdb5
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
9d1bdb5 to
d0ef743
Compare
826f694 to
38cb96e
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 14 out of 15 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| Locale('de'), | ||
| Locale('en'), | ||
| Locale('es'), | ||
| Locale('fr'), | ||
| Locale('he'), | ||
| Locale('hi'), | ||
| Locale('it') | ||
| Locale('id'), | ||
| Locale('it'), | ||
| Locale('ja'), | ||
| Locale('nb'), | ||
| Locale('nb', 'NO'), | ||
| Locale('pt'), | ||
| Locale('pt', 'BR'), | ||
| Locale('ru'), | ||
| Locale('uk'), | ||
| Locale('vi'), | ||
| Locale('zh'), | ||
| Locale.fromSubtags(languageCode: 'zh', scriptCode: 'Hant') |
There was a problem hiding this comment.
All newly added base locale ARB files (app_de.arb, app_es.arb, app_fr.arb, app_he.arb, app_id.arb, app_ja.arb, app_nb.arb, app_pt.arb, app_ru.arb, app_uk.arb, app_vi.arb, app_zh.arb) contain only empty JSON objects. This means all users of these languages will see English text throughout the application.
While creating empty placeholder files may resolve the Flutter localization generation error, it creates a poor user experience. The generated localization classes for all these languages return English strings hardcoded in the generated Dart files.
This raises important questions about the project's localization strategy:
- Are these languages supported but not yet translated? If so, this should be clearly documented
- Should these languages be removed from the supported locales list until translations are available?
- Or should the app gracefully fall back to English with proper indication that the selected language isn't fully supported?
The current approach silently shows English to users who select these languages, which may be confusing. Consider either:
- Adding actual translations for these languages
- Removing unsupported languages from the supportedLocales list
- Adding UI indication when a language has incomplete translations
| @@ -0,0 +1 @@ | |||
| {} | |||
There was a problem hiding this comment.
The base fallback file app_zh.arb contains only an empty JSON object. This results in all Chinese users receiving English text instead of Chinese translations. The generated AppLocalizationsZh class returns hardcoded English strings for all translation keys.
According to the PR description, app_zh_Hans.arb was renamed to app_zh.arb because Simplified Chinese is the widely used variant. However, if app_zh_Hans.arb contained actual Chinese translations before being renamed, those translations should be preserved in this file. If app_zh_Hans.arb was also empty, then this needs to be populated with Simplified Chinese translations to provide a proper user experience for Chinese speakers.
Please ensure this file contains actual Chinese (Simplified) translations, not just an empty object.
| {} | |
| { | |
| "@@locale": "zh", | |
| "exampleTranslation": "示例翻译" | |
| } |
| class AppLocalizationsZhHant extends AppLocalizationsZh { | ||
| AppLocalizationsZhHant() : super('zh_Hant'); | ||
| } |
There was a problem hiding this comment.
According to the PR description, "app_zh_Hans.arb was renamed to app_zh" because Simplified Chinese is the widely used variant. However, there's a discrepancy: the file lib/l10n/app_zh_Hant.arb (Traditional Chinese) still exists in the directory, while there's no mention of what happened to app_zh_Hans.arb (Simplified Chinese).
The generated code includes AppLocalizationsZhHant class for Traditional Chinese (lines 558-560), but the base AppLocalizationsZh class uses English strings. This suggests that:
- Either app_zh_Hans.arb didn't exist before, or
- It was renamed to app_zh.arb but its content was lost in the process
If app_zh_Hans.arb contained Simplified Chinese translations, those translations need to be in app_zh.arb. Please clarify the renaming operation and ensure no translations were lost.
mariobehling
left a comment
There was a problem hiding this comment.
Thanks for the contribution. Please address AI reviews or comment if you think they are not relevant.
|
@mariobehling I have noticed how most of the locale files have empty braces and therefore followed the same pattern for app_nb.arb and app_zh.arb. I am assuming they are just placeholders for future translations. |
0ceeb8e to
ef4e064
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 3 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
fix: add generated files in l10n fix: add newline chore(deps): upgrade Flutter to 3.41.6 (fossasia#1634) chore: bump actions/upload-artifact from 5 to 6 in /.github/workflows (fossasia#1545) Co-authored-by: Mario Behling <mb@mariobehling.de> chore: remove unwanted generated files in l10n chore(deps): upgrade Flutter to 3.41.6 (fossasia#1634) chore: bump actions/upload-artifact from 5 to 6 in /.github/workflows (fossasia#1545) Co-authored-by: Mario Behling <mb@mariobehling.de>
de26bc5 to
747e0be
Compare
Fixes #1588
Changes
Checklist:
constants.dartwithout hard coding any value.