Implement robust localization infrastructure (l10n)#241
Conversation
- Configured pubspec.yaml with generate: true. - Created l10n.yaml for localization settings. - Added initial English and Spanish ARB files in lib/l10n. - Integrated AppLocalizations in lib/main.dart, replacing hardcoded strings. - Added unit tests for localization verification.
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (9)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request implements internationalization support for English and Spanish by adding ARB files and integrating the Flutter localization framework into the main application and navigation components. Feedback highlights that generated localization files should not be manually checked into the repository when the automatic generation flag is enabled. Additionally, the reviewer advised against manual version downgrades in the lock file, suggesting that dependency management should be handled via the Flutter CLI.
| import 'dart:async'; | ||
|
|
||
| import 'package:flutter/foundation.dart'; | ||
| import 'package:flutter/widgets.dart'; | ||
| import 'package:flutter_localizations/flutter_localizations.dart'; | ||
| import 'package:intl/intl.dart' as intl; | ||
|
|
||
| import 'app_localizations_en.dart'; | ||
| import 'app_localizations_es.dart'; | ||
|
|
||
| // ignore_for_file: type=lint |
There was a problem hiding this comment.
It appears that app_localizations.dart and its associated implementation files (_en.dart, _es.dart) are generated code that has been manually added to the repository (indicated by the type=lint ignore and boilerplate structure).
Since generate: true is enabled in pubspec.yaml, Flutter's gen-l10n tool will automatically create these files in the .dart_tool/flutter_gen/ directory. Checking them into the source tree is generally discouraged as it leads to maintenance overhead and potential synchronization issues when .arb files are updated. For instance, the isSupported method (line 155) and lookupAppLocalizations (line 161) are currently hardcoded and would require manual updates whenever a new language is added.
Recommendation: Remove these files from lib/l10n/ and import from the synthetic package instead.
|
|
||
| import 'package:flutter/material.dart'; | ||
| import 'package:flutter_localizations/flutter_localizations.dart'; | ||
| import 'l10n/app_localizations.dart'; |
There was a problem hiding this comment.
If the manually added localization files are removed in favor of the automatically generated ones (as suggested in lib/l10n/app_localizations.dart), this import should be updated to point to the synthetic package generated by Flutter.
| import 'l10n/app_localizations.dart'; | |
| import 'package:flutter_gen/gen_l10n/app_localizations.dart'; |
| url: "https://pub.dev" | ||
| source: hosted | ||
| version: "0.12.19" | ||
| version: "0.12.18" |
There was a problem hiding this comment.
The version of matcher (and test_api at line 1496) is being downgraded. Manually editing the pubspec.lock file to reduce "noise" is risky and can lead to version mismatches or regressions. It is recommended to let the flutter pub tool manage this file. If you need to revert accidental upgrades, it is safer to adjust the constraints in pubspec.yaml and run flutter pub get again.
| @@ -0,0 +1,47 @@ | |||
| import 'package:flutter/material.dart'; | |||
| import 'package:flutter_test/flutter_test.dart'; | |||
| import 'package:orionhealth_health/l10n/app_localizations.dart'; | |||
There was a problem hiding this comment.
Update this import to use the generated localization package if the manual files are removed from the source tree.
| import 'package:orionhealth_health/l10n/app_localizations.dart'; | |
| import 'package:flutter_gen/gen_l10n/app_localizations.dart'; |
1 participant
I have implemented the requested localization infrastructure for the OrionHealth app.
Key changes:
generate: trueinpubspec.yamlto support Flutter's localization tool.l10n.yamland established thelib/l10ndirectory for ARB files.app_en.arbandapp_es.arbwith base strings (app title, home page content, navigation labels).lib/main.dartto useAppLocalizationsfor its strings and configured the necessarylocalizationsDelegatesandsupportedLocales.test/l10n_test.dartthat verifies correct string loading for both English and Spanish.I also addressed code review feedback regarding the generation of localization files and dependency noise in
pubspec.lock.Fixes #239
PR created automatically by Jules for task 3378336104619687335 started by @iberi22