-
Notifications
You must be signed in to change notification settings - Fork 348
feat(l10n): Add complete Bengali (bn) translation support #630
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Release v3.0.0
|
🎉 Welcome @codealchemist007!
We appreciate your contribution! 🚀 |
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughAdds Bengali (bn) localization: inserts bn into iOS CFBundleLocalizations, adds bn to l10n.yaml, adds Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@coderabbitai pls review. |
|
@codealchemist007: I'll review the Bengali translation PR for you now. ✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (2)
lib/main.dart (1)
61-69: Confirm whether making Bengali the global default locale is intentionalChanging the fallback from
"en"to"bn"means that on first launch (no storedlanguageLocale), all users will see the app in Bengali, regardless of their device language. That’s a behavioral change beyond simply “adding bn support”.If the goal is just to support Bengali, a safer pattern would be:
- Try stored
languageLocale.- Else fall back to device locale if supported.
- Else fall back to a neutral default (often English).
For example (pseudo-logic):
final storage = FlutterSecureStorage(); languageLocale = await storage.read(key: "languageLocale") ?? WidgetsBinding.instance.platformDispatcher.locale.languageCode ?? "en";You can then clamp this to
AppLocalizations.supportedLocalesif needed.Please double-check product requirements; if bn is not meant to be the universal default, consider reverting to
"en"or adopting a device-locale–driven fallback.ios/Runner/Info.plist (1)
7-14: iOS localization list correctly updated to include BengaliAdding
bntoCFBundleLocalizations(and reordering the array) is consistent with enabling Bengali support and won’t affect runtime behavior beyond letting iOS know the app supports this language.If you later localize system-facing strings (e.g., permission prompts via
InfoPlist.strings), make sure to also create a correspondingbn.lproj, but for Flutter-only UI localization this change is sufficient.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
ios/Runner/Info.plist(1 hunks)l10n.yaml(1 hunks)lib/l10n/app_bn.arb(1 hunks)lib/main.dart(1 hunks)
🔇 Additional comments (1)
l10n.yaml (1)
4-7: Bengali locale wiring in l10n config looks correctAdding
bntosupported-localesis correct and consistent with the new ARB file and other changes in this PR.
4e52452 to
2647373
Compare
cc3808b to
9be7ab7
Compare
9be7ab7 to
2247123
Compare
| <category android:name="android.intent.category.DEFAULT" /> | ||
| <category android:name="android.intent.category.BROWSABLE" /> | ||
| <data android:scheme="appwrite-callback-${PROJECT_ID}" /> | ||
| <data android:scheme="appwrite-callback-resonate" /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why was this changed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The android:scheme was changed to appwrite-callback-resonate to ensure the OAuth 2.0 callback URL is correctly configured. This scheme must match the callback URL registered on the Appwrite server to securely and correctly redirect the user back to the Resonate app after external authentication (e.g., Google or GitHub login). This change was required to fix the token passing issue during the login flow.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is no token parsing issue. The use of dynamic variables is necessary there so that Project IDs can be injected at compile time, as they differ for projects on the cloud
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please change it back as this is a breaking change
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hmm now I get it & I've also made the necessary change for this.
android/app/build.gradle.kts
Outdated
| val keyAliasProp = keystoreProperties.getProperty("keyAlias") | ||
| val keyPasswordProp = keystoreProperties.getProperty("keyPassword") | ||
| val storeFileProp = keystoreProperties.getProperty("storeFile") | ||
| val storePasswordProp = keystoreProperties.getProperty("storePassword") | ||
|
|
||
| if (!keyAliasProp.isNullOrBlank()) { | ||
| keyAlias = keyAliasProp | ||
| } | ||
| if (!keyPasswordProp.isNullOrBlank()) { | ||
| keyPassword = keyPasswordProp | ||
| } | ||
| if (!storeFileProp.isNullOrBlank()) { | ||
| storeFile = file(storeFileProp) | ||
| } | ||
| if (!storePasswordProp.isNullOrBlank()) { | ||
| storePassword = storePasswordProp |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why was this changed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
where I didn't get it and I'm sure that I didn't make any changes here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please revert the file
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah I've reverted this file as you said.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please revert these changes, they are still a part of the PR
| 'title': 'Live Chapter Starting!', | ||
| 'body': | ||
| "${authStateController.displayName} is starting a Live Chapter in ${storyName}: ${liveChapterData.chapterTitle}. Tune In!", | ||
| "${authStateController.displayName} is starting a Live Chapter in $storyName: ${liveChapterData.chapterTitle}. Tune In!", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change is not part of the scope of this PR
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oops! sorry for that could have been my mistake.
lib/main.dart
Outdated
| Get.put(AboutAppScreenController()); | ||
| languageLocale = | ||
| await FlutterSecureStorage().read(key: "languageLocale") ?? "en"; | ||
| await FlutterSecureStorage().read(key: "languageLocale") ?? "bn"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is the default locale set to Bengali?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ohh yes, you are correct about the default locale. I've fixed this by changing the hardcoded fallback from 'bn' to 'en' in the secure storage read operation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please change it back
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
already done pls check it.
untranslated.txt
Outdated
| "bn": [ | ||
| "exploreDiverseConversations" | ||
| ], | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is an untranslated string in bengali, please translate it as well
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yep done this as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now there are 4 untranslated strings
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this too big of an issue if there are a few untranslated strings? cause in other languages also there are lots of them right in that untranslated.txt file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are adding Bengali for the first time, so yes, it is an issue. For the rest of the languages, the untranslated strings came after new features and strings were added, and as a result, contributors for that language are yet to add translations for those strings. For new languages, it is required to translate all strings. Either way, why is it an issue to translate all of the strings?
|
@codealchemist007 please note, do not make anymore force pushes as that overwrites review history |
untranslated.txt
Outdated
| "bn": [ | ||
| "exploreDiverseConversations" | ||
| ], | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now there are 4 untranslated strings
|
Please re-request review when ready |
untranslated.txt
Outdated
| "bn": [ | ||
| "exploreDiverseConversations" | ||
| ], | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are adding Bengali for the first time, so yes, it is an issue. For the rest of the languages, the untranslated strings came after new features and strings were added, and as a result, contributors for that language are yet to add translations for those strings. For new languages, it is required to translate all strings. Either way, why is it an issue to translate all of the strings?
|
Hi @M4dhav, I hope you’re having a great week! I just wanted to slightly follow up on this PR. I’ve implemented the changes we discussed regarding the Bengali translations. Please let me know if there’s anything else that needs adjusting, or if this is ready for the next step. Thanks again for your time! |
android/app/build.gradle.kts
Outdated
| val keyAliasProp = keystoreProperties.getProperty("keyAlias") | ||
| val keyPasswordProp = keystoreProperties.getProperty("keyPassword") | ||
| val storeFileProp = keystoreProperties.getProperty("storeFile") | ||
| val storePasswordProp = keystoreProperties.getProperty("storePassword") | ||
|
|
||
| if (!keyAliasProp.isNullOrBlank()) { | ||
| keyAlias = keyAliasProp | ||
| } | ||
| if (!keyPasswordProp.isNullOrBlank()) { | ||
| keyPassword = keyPasswordProp | ||
| } | ||
| if (!storeFileProp.isNullOrBlank()) { | ||
| storeFile = file(storeFileProp) | ||
| } | ||
| if (!storePasswordProp.isNullOrBlank()) { | ||
| storePassword = storePasswordProp |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please revert these changes, they are still a part of the PR
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 7
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pubspec.lockis excluded by!**/*.lock
📒 Files selected for processing (6)
.metadata(1 hunks)ios/Runner/Info.plist(1 hunks)l10n.yaml(1 hunks)lib/l10n/app_bn.arb(1 hunks)lib/l10n/app_localizations.dart(4 hunks)lib/l10n/app_localizations_bn.dart(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- l10n.yaml
- ios/Runner/Info.plist
- lib/l10n/app_bn.arb
🔇 Additional comments (4)
lib/l10n/app_localizations.dart (1)
8-8: Bengali locale integration looks correct.The Bengali (bn) locale has been properly integrated into the localization delegate:
- Import added for
app_localizations_bn.dartLocale('bn')added tosupportedLocales'bn'added toisSupportedchecklookupAppLocalizationscorrectly returnsAppLocalizationsBn()for the 'bn' caseThis file appears to be auto-generated by
flutter gen-l10n, and the changes are consistent with adding a new locale.Also applies to: 102-102, 2509-2509, 2525-2526
lib/l10n/app_localizations_bn.dart (3)
1-9: Class structure is correct.The
AppLocalizationsBnclass properly extendsAppLocalizationsand passes the 'bn' locale to the superclass constructor. The ignore comments are standard for Flutter-generated localization files.
82-89: ICU message formats implemented correctly.The
Intl.selectLogicandIntl.pluralLogicimplementations are properly structured:
- Select messages correctly handle the different cases with appropriate Bengali translations
- Plural messages properly use
localeNameand handle singular (one) and plural (other) forms- Bengali numerals are used appropriately in singular forms (e.g., "১ মিনিট আগে")
Also applies to: 1082-1112
1-1369: Translation coverage is complete.The Bengali localization file provides translations for all UI strings defined in the abstract
AppLocalizationsclass. The file structure is correct, all method signatures match the parent class, and the ICU message formats (plurals/selects) are properly implemented.Please address the critical translation errors flagged above (the "যৌন"/"যোগ" confusion) before merging, as they would display inappropriate content to Bengali-speaking users.
Consider having a native Bengali speaker review the translations for accuracy and natural phrasing, particularly for:
- Technical terms (AI model names, UI elements)
- Complex sentences with multiple clauses
|
Hey @codealchemist007 could you take a look at these please? If the comments are accurate then please make the necessary changes, otherwise lmk if they are hallucinations |
|
@M4dhav, yeah I will fix these small typos now only. Thanks for asking! |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (2)
lib/l10n/app_localizations_bn.dart (2)
338-339: Incorrect translation for "Base" model name remains unfixed.The word 'বেশ' means "quite" or "pretty" in Bengali, not "Base". For technical terms like AI model names, transliteration is preferred.
🔎 Proposed fix
@override - String get whisperModelBase => 'বেশ'; + String get whisperModelBase => 'বেস';
982-985: Trailing colon in startedFollowingYou message remains unfixed.The Bengali translation has a trailing colon (
:) that doesn't appear in the English source and is inconsistent with similar notification messages.🔎 Proposed fix
@override String startedFollowingYou(String username) { - return '$username আপনাকে ফলো করা শুরু করেছে:'; + return '$username আপনাকে ফলো করা শুরু করেছে'; }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
lib/l10n/app_bn.arb(1 hunks)lib/l10n/app_localizations_bn.dart(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- lib/l10n/app_bn.arb
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Run Software Tests
🔇 Additional comments (1)
lib/l10n/app_localizations_bn.dart (1)
1-1369: Good progress on fixing critical translation issues!The critical translation errors flagged in previous reviews (incorrect use of 'যৌন' instead of 'যোগ' for "join") have been properly fixed. The Bengali localization is comprehensive and correctly uses the intl package for pluralization and selection logic.
However, two minor issues from previous reviews remain unfixed (lines 339 and 984), and one new minor issue was found (inconsistent transliteration of "Resonate"). Please address these text quality issues before merging.
|
@codealchemist007 please take care of this as well |
|
@M4dhav, yeah sorry for that I will just fix these as well np. |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (4)
lib/l10n/app_bn.arb (2)
979-995: Fix typo in shareRoomMessage placeholder text.There's a typo in the Bengali word for "participants": "অংশগ্রহণকারীরদেড়" should be "অংশগ্রহণকারীদের".
Note: The generated Dart file (
app_localizations_bn.dartline 773) appears to have the correct spelling, but since the ARB file is the source of truth, regenerating localizations will reintroduce this typo. Please fix it in the ARB source.🔎 Proposed fix
- "shareRoomMessage": "🚀এই অসাধারণ রুমটি দেখুন: {roomName}!\n\n📖 বিবরণ: {description}\n👥 এখনই {participants} অংশগ্রহণকারীরদেড় সাথে যোগ দিন!", + "shareRoomMessage": "🚀এই অসাধারণ রুমটি দেখুন: {roomName}!\n\n📖 বিবরণ: {description}\n👥 এখনই {participants} অংশগ্রহণকারীদের সাথে যোগ দিন!",
1298-1307: Remove trailing colon from startedFollowingYou.The Bengali translation includes a trailing colon (
:) that doesn't appear in the English source. This creates display inconsistency.🔎 Proposed fix
- "startedFollowingYou": "{username} আপনাকে ফলো করা শুরু করেছে:", + "startedFollowingYou": "{username} আপনাকে ফলো করা শুরু করেছে",lib/l10n/app_localizations_bn.dart (2)
338-339: Incorrect translation for technical model name."বেশ" means "quite" or "pretty" in Bengali, not "Base". For technical terms like AI model names, transliteration is typically preferred.
🔎 Proposed fix
Since this is a generated file, fix the source ARB file and regenerate:
In
app_bn.arb, change:- "whisperModelBase": "বেশ", + "whisperModelBase": "বেস",Then run
flutter gen-l10nto regenerate this file.
982-985: Remove trailing colon for consistency.The trailing colon (
:) doesn't appear in the English source and creates display inconsistency.🔎 Proposed fix
Since this is a generated file, fix the source ARB file (already flagged in separate comment) and regenerate this file by running
flutter gen-l10n.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
lib/l10n/app_bn.arb(1 hunks)lib/l10n/app_localizations_bn.dart(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Run Software Tests
🔇 Additional comments (2)
lib/l10n/app_bn.arb (1)
1-1778: Excellent work adding Bengali translations!The ARB file now contains proper Bengali translations throughout, addressing the critical blocker from the previous review where all strings were still in English. This represents significant progress toward genuine Bengali localization support.
However, there are two remaining typos that need correction (see separate comments below).
lib/l10n/app_localizations_bn.dart (1)
1-1369: Great progress fixing critical translation errors!Most of the critical issues from the previous review have been successfully addressed:
- ✅
joinRoomnow correctly uses "যোগ দিন" instead of the inappropriate word- ✅
unableToJoinRoomandunableToReconnectproperly fixed- ✅ Transliteration of "Resonate" is now consistent as "রেজোনেট"
- ✅
shareRoomMessagetypo fixed in this generated fileTwo minor issues remain (see separate comments below).
M4dhav
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the contribution
|
✅ PR Closed - Thank You, @codealchemist007!
We appreciate your effort and look forward to more contributions from you! 🤝 |
|
Thank you for the review and for catching those issues earlier! Glad to see the translations are ready. Looking forward to contributing more to Resonate in the future. |
Description:
This Pull Request introduces complete localization support for the Bengali (bn) language in the Resonate app, making the application accessible to a wider user base.
The translation covers all existing UI strings and messages present in the app_en.arb template file.
Fixes #621:
Type of change:
[x] New feature (non-breaking change which adds functionality)
How Has This Been Tested:
The following steps were taken to verify the changes:
Localization Generation: Ran flutter gen-l10n to validate the JSON syntax of the new app_bn.arb file and ensure correct Dart code generation. The command ran successfully without errors.
Configuration Check: Verified and updated project configuration files: l10n.yaml, lib/main.dart, and ios/Runner/Info.plist to officially register the bn locale.
Visual Confirmation (Manual Testing): The app was run on an Android emulator and an actual physical Android device. The device locale was temporarily set to Bengali (bn).
{username}) were correctly preserved and populated dynamically.Test Configuration: Flutter SDK [3.38.3] on Android Emulator.
Checklist:
[x] My code follows the style guidelines of this project
[x] I have performed a self-review of my own code
[x] My changes generate no new warnings
[x] I have checked my code and corrected any misspellings
Maintainer Checklist:
[ ] closes #621
[ ] Tag the PR with the appropriate labels
Summary by CodeRabbit
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.