Refactor improve localizations #30
Conversation
- Added labels and descriptions for: - Followed items limit - Saved headlines limit - Ad frequency - Ad placement interval - Articles before interstitial ads
- Replaced hardcoded labels with localized strings. - Replaced hardcoded descriptions with localized strings. - Improved readability and maintainability. - Simplified code by using localized strings. - Updated UI to reflect localized text.
- Imported l10n package. - Used localized string for retry button.
- Use AppLocalizations for labels/descriptions - Added helper methods for better readability - Improved code structure and organization - Simplified descriptions in some cases - Handle different user roles consistently
- Added "Link Account" - Added "Rate App" - Added "Follow Topics" - Added "Follow Sources" - Added "Enable Notifications"
- Removed redundant formatting function. - Use switch statement for localized labels. - Improved clarity and readability. - Localized description using localized action type. - Simplified label generation.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary of Changes
Hello @fulleni, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request undertakes a significant refactoring of the application's localization infrastructure. My primary goal was to centralize and improve the management of user-facing text by moving static strings into dedicated localization files and introducing mechanisms for dynamic content based on user context. This enhancement lays the groundwork for better internationalization and maintainability of the UI text.
Highlights
- Localization Refactor: I've migrated numerous hardcoded UI strings for labels and descriptions, particularly within configuration forms, to the application's localization system. This includes fields related to user preferences (e.g., 'Followed Items Limit', 'Saved Headlines Limit'), ad configurations (e.g., 'Ad Frequency', 'Ad Placement Interval'), and account action prompts.
- Dynamic Localization: I've implemented dynamic localization for certain UI elements, allowing labels and descriptions to vary based on the user's role (Guest, Standard, Premium) or the specific type of in-app action prompt. This is achieved through new helper methods that select the appropriate localized string.
- Localization File Updates: I've updated the core localization interface (
app_localizations.dart) and its concrete implementations (app_localizations_ar.dart,app_localizations_en.dart), along with the ARB resource files (app_ar.arb,app_en.arb), to include all newly localized strings and their respective translations. - Error State Localization: I've localized the 'Retry' button text within the
FailureStateWidgetto ensure consistency with the application's internationalization efforts.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Code Review
Good work on this pull request! The refactoring to move hardcoded strings into localization files is a great improvement for maintainability and internationalization. The changes are well-organized and cover all necessary files, including the .arb source files and the generated Dart files.
I've identified a couple of areas for improvement:
- Code Duplication: In
app_configuration_page.dart, there's an opportunity to refactor some repetitiveswitchstatements into a more declarative, map-based approach. This will make the code cleaner and easier to maintain. - Translation Inconsistency: There's a minor inconsistency in the Arabic translation for one of the updated keys.
Details and suggestions are in the comments below. Keep up the great work!
| @@ -371,8 +371,7 @@ class AppLocalizationsAr extends AppLocalizations { | |||
| String get androidStoreUrlLabel => 'رابط متجر Android'; | |||
|
|
|||
| @override | |||
| String get androidStoreUrlDescription => | |||
| 'رابط التطبيق على متجر Google Play Store.'; | |||
| String get androidUpdateUrlDescription => 'رابط تحديثات تطبيق Android.'; | |||
There was a problem hiding this comment.
There's an inconsistency between the English and Arabic localizations for androidUpdateUrlDescription. The English version was updated to use the description for the Google Play Store URL ("URL to the app on the Google Play Store."), but the Arabic version still uses the old text ("رابط تحديثات تطبيق Android.", which translates to "URL for Android app updates.").
To ensure consistency, the Arabic translation should also refer to the Google Play Store URL.
String get androidUpdateUrlDescription => 'رابط التطبيق على متجر Google Play Store.';
| String _getFollowedItemsLimitLabel(AppLocalizations l10n) { | ||
| switch (widget.userRole) { | ||
| case AppUserRole.guestUser: | ||
| return l10n.guestFollowedItemsLimitLabel; | ||
| case AppUserRole.standardUser: | ||
| return l10n.standardUserFollowedItemsLimitLabel; | ||
| case AppUserRole.premiumUser: | ||
| return l10n.premiumFollowedItemsLimitLabel; | ||
| } | ||
| } | ||
|
|
||
| String _getFollowedItemsLimitDescription(AppLocalizations l10n) { | ||
| switch (widget.userRole) { | ||
| case AppUserRole.guestUser: | ||
| return l10n.guestFollowedItemsLimitDescription; | ||
| case AppUserRole.standardUser: | ||
| return l10n.standardUserFollowedItemsLimitDescription; | ||
| case AppUserRole.premiumUser: | ||
| return l10n.premiumFollowedItemsLimitDescription; | ||
| } | ||
| } | ||
|
|
||
| String _getSavedHeadlinesLimitLabel(AppLocalizations l10n) { | ||
| switch (widget.userRole) { | ||
| case AppUserRole.guestUser: | ||
| return l10n.guestSavedHeadlinesLimitLabel; | ||
| case AppUserRole.standardUser: | ||
| return l10n.standardUserSavedHeadlinesLimitLabel; | ||
| case AppUserRole.premiumUser: | ||
| return l10n.premiumSavedHeadlinesLimitLabel; | ||
| } | ||
| } | ||
|
|
||
| String _getSavedHeadlinesLimitDescription(AppLocalizations l10n) { | ||
| switch (widget.userRole) { | ||
| case AppUserRole.guestUser: | ||
| return l10n.guestSavedHeadlinesLimitDescription; | ||
| case AppUserRole.standardUser: | ||
| return l10n.standardUserSavedHeadlinesLimitDescription; | ||
| case AppUserRole.premiumUser: | ||
| return l10n.premiumSavedHeadlinesLimitDescription; | ||
| } | ||
| } |
There was a problem hiding this comment.
These four methods are very repetitive, each containing a similar switch statement. This leads to code duplication and makes maintenance harder, especially if new user roles are added in the future.
A more declarative and maintainable approach would be to use Maps to associate user roles with their corresponding localization functions. You could replace these four methods with static maps inside the _UserPreferenceLimitsFormState class.
For example:
static final _followedItemsLimitLabels = <AppUserRole, String Function(AppLocalizations)>{
AppUserRole.guestUser: (l10n) => l10n.guestFollowedItemsLimitLabel,
AppUserRole.standardUser: (l10n) => l10n.standardUserFollowedItemsLimitLabel,
AppUserRole.premiumUser: (l10n) => l10n.premiumFollowedItemsLimitLabel,
};
// ... and similar maps for descriptions and saved headlines.Then, in your build method, you would access the localized strings like this:
// For followed items limit
label: _followedItemsLimitLabels[widget.userRole]!(l10n),
description: _followedItemsLimitDescriptions[widget.userRole]!(l10n),
// For saved headlines limit
label: _savedHeadlinesLimitLabels[widget.userRole]!(l10n),
description: _savedHeadlinesLimitDescriptions[widget.userRole]!(l10n),This refactoring would significantly reduce code duplication and improve the code's readability and maintainability.
Status
READY/IN DEVELOPMENT/HOLD
Description
Type of Change