-
Notifications
You must be signed in to change notification settings - Fork 348
Feat: Added metadata to all keys and removed duplicate translations #532
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
WalkthroughAdds supported-locales to l10n.yaml. Expands ARB metadata extensively, removes duplicates/obsolete keys, and renames pushNewChapters to newChapters. Updates AppLocalizations API and per-locale implementations to reflect removals/renames. Adjusts UI screens to use updated localization keys. Minor template string fixes and doc comment updates. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
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 |
|
🎉 Welcome @uju09!
We appreciate your contribution! 🚀 |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
@M4dhav I request you to please review it. |
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: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
lib/l10n/app_en.arb (1)
1432-1473: ICU placeholder bug: use {username}, not ${username}.These three strings won’t bind the placeholder; gen-l10n will error or treat ${username} as literal.
Apply this diff:
-"friendRequestAcceptedTo": "You are now friends with ${username}.", +"friendRequestAcceptedTo": "You are now friends with {username}.", ... -"friendRequestDeclinedTo": "You have declined the friend request from ${username}.", +"Friend request declined to": "You have declined the friend request from {username}.", ... -"callDeclinedTo": "User ${username} declined the call.", +"callDeclinedTo": "User {username} declined the call.",lib/l10n/app_localizations_en.dart (1)
1082-1103: Broken string interpolation: username rendered literallyEscaped $ prevents interpolation, yielding “$username”. Remove the backslash.
- return 'You are now friends with \$$username.'; + return 'You are now friends with $username.'; ... - return 'You have declined the friend request from \$$username.'; + return 'You have declined the friend request from $username.'; ... - return 'User \$$username declined the call.'; + return 'User $username declined the call.';
🧹 Nitpick comments (14)
l10n.yaml (2)
4-6: Use preferred-supported-locales (or remove the block).gen-l10n doesn’t have a supported-locales option; use preferred-supported-locales to control default order, or rely on ARB filenames to infer locales.
Apply this diff:
-supported-locales: - - en - - hi +preferred-supported-locales: + - en + - hi
1-7: Remove unused localization key aboutSection from lib/l10n/app_en.arb
aboutSection is defined in lib/l10n/app_en.arb but not referenced in code — remove it (and its @-metadata) or restore usage. No missing keys were found in lib/l10n/app_hi.arb.lib/l10n/app_en.arb (4)
884-893: Pluralize participantsCount.Show “1 Participant” vs “2 Participants” using ICU plurals.
Apply this diff:
-"participantsCount": "{count} Participants", +"participantsCount": "{count, plural, =1{1 Participant} other{{count} Participants}}",(keep the existing placeholders block)
548-551: Capitalize Discord.Minor copy fix.
Apply this diff:
-"joinDiscordServer": "Join discord server", +"joinDiscordServer": "Join Discord server",
742-781: Month naming consistency.Some months are full (“March”, “April”) while others are abbreviated (“Jan”, “Feb”). Pick one style for all to avoid UI inconsistency.
820-833: Minor copy polish (optional).
- “Scheduled Date Time cannot be in past” → “in the past”.
- Consider “Required field” vs “This field is required” for consistency with other errors.
Also applies to: 668-675, 672-675
lib/views/screens/about_app_screen.dart (1)
73-77: Localize “Stable”.You already have stable in ARB; avoid hard-coded English.
Apply this diff:
- "${aboutAppScreenController.appVersion} | ${aboutAppScreenController.appBuildNumber} | Stable", + "${aboutAppScreenController.appVersion} | ${aboutAppScreenController.appBuildNumber} | ${AppLocalizations.of(context)!.stable}",untranslated.txt (1)
1-5: Remove stale key from untranslated list.aboutSection is being retired; keeping it here adds noise.
Apply this diff (or regenerate via flutter gen-l10n):
-{ - "hi": [ - "aboutSection" - ] -} +{ + "hi": [] +}lib/views/screens/email_verification_screen.dart (2)
56-57: Prefer theme semantic color instead of manual brightness checkUse colorScheme.onBackground for better theming/contrast across schemes.
- color: - Theme.of(context).brightness == Brightness.light ? Colors.black : Colors.white, + color: Theme.of(context).colorScheme.onBackground,
156-162: Use runtime text direction for a11y announcementsHardcoding TextDirection.ltr may break future RTL locales. Derive from context.
- SemanticsService.announce( - AppLocalizations.of(context)!.otpMismatch, - TextDirection.ltr, - ); + SemanticsService.announce( + AppLocalizations.of(context)!.otpMismatch, + Directionality.of(context), + );lib/l10n/app_localizations.dart (1)
1163-1234: Month strings mix full names and abbreviationsIn EN: Mar–Jul are full names; Aug–Dec are abbreviations. Consider standardizing (or splitting into monthShort/monthLong) to avoid inconsistent UI.
lib/l10n/app_localizations_en.dart (1)
430-431: Brand capitalizationCapitalize “Discord”.
- String get joinDiscordServer => 'Join discord server'; + String get joinDiscordServer => 'Join Discord server';lib/l10n/app_hi.arb (2)
1297-1300: Placeholder example likely incorrectchooseTheme.placeholder.category example is "classic" but keys use "classicTheme", "timeTheme", etc. Update the example to a real key to guide contributors.
- "example": "classic" + "example": "classicTheme"
731-778: Align month metadata and values (abbrev vs full-name)
- Findings: lib/l10n/app_en.arb mixes abbreviations (Jan/Feb, Aug–Dec) with full names (Mar–Jul); lib/l10n/app_hi.arb uses full Hindi names but several @month* descriptions still say "Abbreviation" (monthJan, monthFeb, monthAug–monthDec).
- Action: Choose one style (all short or all full), update ARB values and the corresponding @month* description entries for monthJan..monthDec in lib/l10n/app_en.arb and lib/l10n/app_hi.arb, then regenerate localization artifacts.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
l10n.yaml(1 hunks)lib/l10n/app_en.arb(25 hunks)lib/l10n/app_hi.arb(23 hunks)lib/l10n/app_localizations.dart(2 hunks)lib/l10n/app_localizations_en.dart(1 hunks)lib/l10n/app_localizations_hi.dart(1 hunks)lib/views/screens/about_app_screen.dart(1 hunks)lib/views/screens/add_chapter_screen.dart(1 hunks)lib/views/screens/chapter_play_screen.dart(2 hunks)lib/views/screens/edit_profile_screen.dart(3 hunks)lib/views/screens/email_verification_screen.dart(4 hunks)lib/views/screens/story_screen.dart(2 hunks)untranslated.txt(1 hunks)
🔇 Additional comments (11)
lib/views/screens/edit_profile_screen.dart (2)
300-305: LGTM: camera tooltip key rename hooked up.
321-323: LGTM: gallery tooltip key rename hooked up.lib/views/screens/add_chapter_screen.dart (1)
169-170: LGTM: uses newChapters key after rename.lib/views/screens/chapter_play_screen.dart (1)
181-183: LGTM: switched from aboutSection to about.lib/views/screens/about_app_screen.dart (1)
68-71: LGTM: app title now uses title key.lib/views/screens/story_screen.dart (1)
281-282: LGTM: switched from aboutStory to about.lib/views/screens/email_verification_screen.dart (1)
104-115: Key rename applied correctly (verificationCompleteMessage)Looks consistent with ARB/API changes and a11y announce mirrors the snackbar body.
Run gen-l10n and a quick build to ensure no stale references remain.
lib/l10n/app_localizations_hi.dart (1)
444-445: newChapters translation looks goodMatches the EN intent and PR objective to consolidate keys.
lib/l10n/app_localizations_en.dart (1)
444-445: Rename to newChapters confirmedString value and getter align with base API.
lib/l10n/app_hi.arb (1)
567-570: newChapters metadata added—niceClear description improves DX with gen-l10n.
lib/l10n/app_localizations.dart (1)
935-940: Approve: rename to newChaptersPublic API and ARB/generation are in sync — newChapters is present in lib/l10n/*.arb and generated files and is used via AppLocalizations.of(context)!.newChapters (lib/views/screens/add_chapter_screen.dart). No stale localization keys found in lib/l10n; occurrences of pickImageFromGallery are method calls (lib/controllers/edit_profile_controller.dart, lib/views/screens/edit_profile_screen.dart), not l10n references.
|
Hey @uju09 , thanks for the PR, please implement review feedback as suggested by coderabbit about removing the aboutSection key, and run |
|
@M4dhav Sure I will follow the suggestion of coderabbit and commit the changes. |
|
@CodeRabbit 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: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
lib/l10n/app_en.arb (2)
1428-1437: Fix ICU placeholders: use {username}, not ${username}.gen-l10n won’t substitute
${username}; it must be{username}.-"friendRequestAcceptedTo": "You are now friends with ${username}.", +"friendRequestAcceptedTo": "You are now friends with {username}.", -"friendRequestDeclinedTo": "You have declined the friend request from ${username}.", +"friendRequestDeclinedTo": "You have declined the friend request from {username}.", -"callDeclinedTo": "User ${username} declined the call.", +"callDeclinedTo": "User {username} declined the call.",Also applies to: 1442-1451, 1460-1469
1-1550: Replace Dart-style ${...} placeholders with ICU placeholders in EN ARB
- lib/l10n/app_en.arb — replace ${username} → {username} for keys: friendRequestAcceptedTo (line 1428), friendRequestDeclinedTo (line 1442), callDeclinedTo (line 1460).
- No usages of pushNewChapters found (OK).
- EN/HI key parity: no missing keys.
♻️ Duplicate comments (1)
lib/l10n/app_en.arb (1)
392-395: Resolved: removed duplicate “aboutSection”.The duplicate key noted earlier is gone; using “about” as canonical is correct.
🧹 Nitpick comments (11)
lib/l10n/app_en.arb (11)
880-889: Pluralize participantsCount correctly.Shows “1 Participants” today.
-"participantsCount": "{count} Participants", +"participantsCount": "{count, plural, =1{1 Participant} other{{count} Participants}}",
738-757: Standardize month names (abbr vs full).Mixed “Mar/Apr/Jun/Jul” full vs others abbreviated; pick one. Suggest all abbreviations for consistency and tighter UI.
-"monthMar": "March", +"monthMar": "Mar", @@ - "description": "Full name for March." + "description": "Abbreviation for March." -"monthApr": "April", +"monthApr": "Apr", @@ - "description": "Full name for April." + "description": "Abbreviation for April." @@ - "description": "Full name for May." + "description": "Abbreviation for May." -"monthJun": "June", +"monthJun": "Jun", @@ - "description": "Full name for June." + "description": "Abbreviation for June." -"monthJul": "July", +"monthJul": "Jul", @@ - "description": "Full name for July." + "description": "Abbreviation for July."Alternative: drop these keys and format dates with DateFormat.MMM() at runtime.
216-219: Capitalize GitHub properly.-"oauthUsersMessage": "(Only for users who logged in using Google or Github)", +"oauthUsersMessage": "(Only for users who logged in using Google or GitHub)",
548-551: Capitalize brand name: Discord.-"joinDiscordServer": "Join discord server", +"joinDiscordServer": "Join Discord server",
668-671: Grammar fix.-"scheduledDateTimePast": "Scheduled Date Time cannot be in past", +"scheduledDateTimePast": "Scheduled Date Time cannot be in the past",
1470-1473: Natural phrasing.-"checkForUpdates": "Check Updates", +"checkForUpdates": "Check for Updates",
570-573: Improve CTA tone.-"helpToGrow": "Help to grow", +"helpToGrow": "Help us grow",
822-825: Polish the deletion warning text.-"deleteAccountPermanent": "This action will Delete Your Account Permanently. It is irreversible process. We will delete your username, email address, and all other data associated with your account. You will not be able to recover it.", +"deleteAccountPermanent": "This action will delete your account permanently. It is an irreversible process. We will delete your username, email address, and all other data associated with your account. You will not be able to recover it.",
1356-1359: Grammar fix (article + casing).-"chooseUser": "Choose User to chat with", +"chooseUser": "Choose a user to chat with",
1198-1201: Fix “Check out” + “GitHub” capitalization.-"resonateOpenSourceProject": "Resonate is an open source project maintained by AOSSIE. Checkout our github to contribute.", +"resonateOpenSourceProject": "Resonate is an open source project maintained by AOSSIE. Check out our GitHub to contribute.",
680-683: Inconsistent “canceled/cancelled” spelling.Mix of US and UK variants across keys/values (“canceled”, “cancelled”). Pick one house style (recommend US English) and align these strings.
Also applies to: 1398-1401, 1491-1497
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
lib/l10n/app_en.arb(25 hunks)
🔇 Additional comments (2)
lib/l10n/app_en.arb (2)
684-687: Is “english” meant for a UI label or a code?Value is “en”. If this appears in UI, consider “English”; if it’s internal, keep as-is. Please confirm usage.
566-569: Rename confirmationpushNewChapters → newChapters looks good; ensure all call-sites updated.
Run the checks below.
|
@M4dhav I have made changes. I request you to review it. |
|
Hey @uju09 can you fix the Outside Diff range comments suggested by CodeRabbit? Thanks |
|
@CodeRabbit 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: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
lib/l10n/app_localizations_en.dart (1)
733-735: Method present in implementation but removed from base class.The
addNewChaptersToStorymethod exists here but appears to be removed from the baseAppLocalizationsclass. This could cause compilation errors.Either restore the method in the base class or remove it from all implementation classes:
- @override - String addNewChaptersToStory(String storyName) { - return 'Add New Chapters to $storyName'; - }
♻️ Duplicate comments (1)
lib/l10n/app_en.arb (1)
566-569: Duplicate key still present: RemovenewChapters(usepushNewChapterseverywhere).The PR objective is to remove duplicate translation keys. However, I notice that
newChaptersappears to be a renamed version ofpushNewChapters, but both keys may still exist in the codebase. This defeats the goal of removing duplicates.Apply this diff to delete the duplicate key and its metadata:
-"newChapters": "New Chapters", -"@newChapters": { - "description": "Header or label for the section to add or view new chapters of a story." -},
🧹 Nitpick comments (1)
lib/l10n/app_localizations_hi.dart (1)
735-737: Verify if this method should be removed.The
addNewChaptersToStorymethod is present in the Hindi localization but appears to be removed from the baseAppLocalizationsclass. This inconsistency could cause runtime errors.Consider removing this method to maintain consistency with the base class:
- @override - String addNewChaptersToStory(String storyName) { - return '$storyName में नए चैप्टर्स जोड़ें'; - }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
lib/l10n/app_en.arb(25 hunks)lib/l10n/app_hi.arb(23 hunks)lib/l10n/app_localizations.dart(2 hunks)lib/l10n/app_localizations_en.dart(3 hunks)lib/l10n/app_localizations_hi.dart(1 hunks)
🔇 Additional comments (5)
lib/l10n/app_hi.arb (2)
27-34: LGTM! Metadata addition improves developer experience.Good job adding metadata descriptions for the password-related keys. The descriptions clearly explain the purpose of each translation string, which will help future developers understand their usage.
567-570: LGTM! Key rename aligns with UI simplification.The rename from
pushNewChapterstonewChaptersis cleaner and more concise. The Hindi translation "नए चैप्टर्स जोड़ें" (Add new chapters) is appropriate.lib/l10n/app_localizations.dart (2)
929-933: LGTM! Key rename properly reflected in the API.The rename from
pushNewChapterstonewChaptersis correctly implemented in the base localization class with updated documentation.
1461-1465: Incorrect — addNewChaptersToStory still exists and is used.
Declared: lib/l10n/app_localizations.dart:1465. Implemented: lib/l10n/app_localizations_en.dart:733, lib/l10n/app_localizations_hi.dart:735. Referenced: lib/views/screens/add_chapter_screen.dart:46. Original removal-impact concern is not applicable.Likely an incorrect or invalid review comment.
lib/l10n/app_localizations_en.dart (1)
1080-1088: LGTM! Fixed string interpolation bugs.Good catch fixing the string interpolation issues. The previous code had
\$$usernamewhich would have displayed a literal dollar sign instead of the username value.Also applies to: 1099-1099
|
@CodeRabbit review |
✅ Actions performedReview triggered.
|
|
@M4dhav All the conflicts has been resolved and repo is updated as per suggestion from CodeRabbit. I request you to review it. |
|
Great work, thanks for the PR @uju09 |
|
✅ PR Closed - Thank You, @uju09!
We appreciate your effort and look forward to more contributions from you! 🤝 |
Description
This pull request resolves Issue #530 by adding comprehensive metadata to all translation keys and removing duplicate values across both the English and Hindi localization files.
The key changes include:
app_en.arb) and Hindi (app_hi.arb) files now has a corresponding metadata block with a description. This greatly improves the developer experience by making the purpose of each string clear and consistent across all languages.l10n.yamlfile, formalizing the setup for both supported languages.Type of change
How Has This Been Tested?
The changes have been thoroughly tested locally on an iOS Simulator. The testing procedure was as follows:
flutter gen-l10nsuccessfully to ensure bothapp_en.arbandapp_hi.arbfiles are correctly formatted, in sync, and generate the necessary Dart localization code.flutter run.Checklist:
Fixes #530
Summary by CodeRabbit
New Features
Changes
Bug Fixes
Chores