Add scaffold for localizations (l10n) in Sharezone #1798
Conversation
flowhorn
requested review from
Jonas-Sander and
nilsreichardt
as code owners
|
|
github-actions
bot
added
documentation
dependencies
…ese files are generated
…whorn/sharezone-app into sharezone_localizations_demo
github-actions
bot
added
the
code quality
nilsreichardt
changed the title
.vscode/tasks.json
Outdated
| // We also format the files after generating the l10n files because | ||
| // the generated files use a different formatting style than the | ||
| // rest of the project. | ||
| "command": "fvm flutter gen-l10n && fvm dart format .", |
There was a problem hiding this comment.
Later, we can create a sz command for this: #1799
| import 'package:flutter/foundation.dart'; | ||
| import 'package:sharezone/util/cache/streaming_key_value_store.dart'; | ||
|
|
||
| class FeatureFlagl10n extends ChangeNotifier { |
There was a problem hiding this comment.
Just a temporary class. Will be removed when we're done with translating.
| Stream<AppLocales> getLocale() { | ||
| final defaultValue = jsonEncode(featureFlagl10n.isl10nEnabled | ||
| ? AppLocales.system.toMap() | ||
| : AppLocales.en.toMap()); | ||
| return keyValueStore | ||
| .getString('locale', defaultValue: defaultValue) | ||
| .map((event) => AppLocales.fromMap(jsonDecode(event))); | ||
| } |
There was a problem hiding this comment.
Later, we will stream the locale from Firebase. Will be done in a future pull request.
| return { | ||
| 'isSystem': isSystem(), | ||
| 'languageTag': _toLanguageTag(), | ||
| }; |
There was a problem hiding this comment.
This is a proposal on how we could store it in the database. We need the isSystem to differentiate if the language is the system language or the selected language.
|
@Jonas-Sander You can review now 👍 |
|
Visit the preview URL for this PR (updated for commit 16da52f): https://sharezone-test--pr1798-sharezone-localizati-80ev1bk7.web.app (expires Tue, 31 Dec 2024 16:08:32 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 Sign: 4cb3ae61e1e018abfd9841fd3239f5b49ccc034b |
nilsreichardt
added
the
safe to test
github-actions
bot
removed
the
safe to test
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 14
🧹 Nitpick comments (17)
lib/sharezone_localizations/lib/sharezone_localizations.dart (1)
9-13: LGTM! Well-organized modular architecture.The export structure follows a clean architecture pattern with clear separation of concerns:
- Generated localizations for string management
- Gateway/Provider pattern for locale state management
- Model for locale representation
- Context extensions for convenient access
This modular approach will make it easier to:
- Test components in isolation
- Maintain and update localization logic
- Add new locale-related features
Consider adding a CHANGELOG.md file to track version changes and a barrel file (index.dart) in the src directory to manage internal exports more effectively.
lib/sharezone_localizations/l10n/app_en.arb (1)
1-13: Enhance ARB file with additional metadataConsider adding the following improvements to make the localization more maintainable:
- Add descriptions for each string using
@keynotation- Add placeholders for dynamic values
- Add pluralization examples
- Add RTL support indicators where needed
Example:
{ "@@locale": "en", "@app_name": { "description": "The name of the application" }, "common_actions_cancel": "Cancel", "@common_actions_cancel": { "description": "Label for cancel buttons throughout the app" } }lib/sharezone_localizations/lib/localizations/sharezone_localizations_en.gen.dart (1)
13-15: Add class-level documentationConsider adding more comprehensive documentation for the generated class, including:
- Usage examples
- Notes about regeneration
- Link to the source ARB file
/// The translations for English (`en`). +/// +/// This class is generated from lib/sharezone_localizations/l10n/app_en.arb. +/// Do not modify this file manually. +/// +/// To regenerate, run: `flutter gen-l10n` in the package root. class SharezoneLocalizationsEn extends SharezoneLocalizations {app/lib/l10n/feature_flag_l10n.dart (1)
27-29: Consider using a constant for the feature flag keyExtract 'l10n_enabled' into a constant to maintain consistency and ease future updates.
+ static const _l10nEnabledKey = 'l10n_enabled'; final StreamingKeyValueStore keyValueStore; late StreamSubscription<bool> _subscription; bool isl10nEnabled = false;lib/sharezone_localizations/lib/src/app_locale_provider.dart (1)
14-15: Enhance class documentationWhile the basic purpose is documented, consider adding more details about:
- The relationship with AppLocaleProviderGateway
- Expected usage patterns
- Example code
lib/sharezone_localizations/lib/src/app_locale_gateway.dart (2)
11-22: Consider enhancing documentation with usage examplesThe documentation clearly explains the purpose, but adding code examples would make it even more helpful for developers implementing this gateway.
Example addition to the documentation:
/// Example implementation: /// ```dart /// class MyLocaleGateway extends AppLocaleProviderGateway { /// @override /// Stream<AppLocale> getLocale() => Stream.value(AppLocale.system); /// /// @override /// Future<void> setLocale(AppLocale locale) async { /// // Implementation /// } /// } /// ```
32-35: Consider using BehaviorSubject for better testing capabilitiesThe current implementation using
Stream.value()only emits once. For testing scenarios where you need to verify stream behavior with multiple emissions, consider using aBehaviorSubjectfrom the RxDart package.Example implementation:
import 'package:rxdart/rxdart.dart'; class MockAppLocaleProviderGateway implements AppLocaleProviderGateway { final _localeSubject = BehaviorSubject<AppLocale>(); @override Stream<AppLocale> getLocale() => _localeSubject.stream; @override Future<void> setLocale(AppLocale locale) async { _localeSubject.add(locale); } void dispose() { _localeSubject.close(); } }app/lib/l10n/flutter_app_local_gateway.dart (1)
15-42: Architecture is ready for future Firebase integrationThe current gateway implementation using a key-value store abstraction will make it easy to add Firebase support in the future, as mentioned in previous reviews. The clean separation of concerns will allow for a smooth transition.
lib/sharezone_localizations/lib/src/app_locale.dart (1)
49-52: Consider caching system locale to prevent frequent platform callsThe
getSystemLocale()method directly accessesPlatformDispatcheron each call. Consider caching the result and implementing a listener for system locale changes to improve performance.static Locale getSystemLocale() { - return PlatformDispatcher.instance.locale; + return _cachedSystemLocale ??= PlatformDispatcher.instance.locale; } + +static Locale? _cachedSystemLocale; + +static void _initializeSystemLocaleListener() { + PlatformDispatcher.instance.onLocaleChanged = () { + _cachedSystemLocale = PlatformDispatcher.instance.locale; + }; +}app/lib/settings/src/subpages/language/language_page.dart (1)
23-27: Consider adding search for languagesAs more languages are added, consider implementing a search functionality to help users find their language quickly.
This will be particularly important as the number of supported locales grows. Consider implementing a search bar in the
ListViewheader.lib/sharezone_localizations/README.md (2)
60-60: Consider formatting technical paths.Technical terms like
ios/Runner.xcworkspaceshould be enclosed in backticks for better readability.-First, open your project's ios/Runner.xcworkspace Xcode workspace file. +First, open your project's `ios/Runner.xcworkspace` Xcode workspace file.
92-93: Format package name consistently.The package name
arb_translateshould be formatted with backticks for consistency with other technical terms.-5. Repeat the above steps in each relevant .arb file (e.g., app_de.arb, app_es.arb, etc.) to keep translations up to date across your app. (Optionally you can use packages like arb_translate for auto translations) +5. Repeat the above steps in each relevant .arb file (e.g., app_de.arb, app_es.arb, etc.) to keep translations up to date across your app. (Optionally you can use packages like `arb_translate` for auto translations)app/lib/activation_code/src/bloc/enter_activation_code_bloc.dart (2)
122-127: Enhance the success message for better clarity.The current success message could be more descriptive about what l10n means and what changes users should expect.
SuccessfulEnterActivationCodeResult( 'l10n', - 'l10n wurde ${!currentValue ? 'aktiviert' : 'deaktiviert'}. Starte die App neu, um die Änderungen zu sehen.', + 'Localization (l10n) wurde ${!currentValue ? 'aktiviert' : 'deaktiviert'}. Starte die App neu, um die Spracheinstellungen zu aktualisieren.', ),
118-121: Consider tracking feature flag changes.For better analytics and debugging, consider tracking when the l10n feature flag is toggled.
void _togglel10nFeatureFlag() { final currentValue = featureFlagl10n.isl10nEnabled; featureFlagl10n.toggle(); + analytics.log('l10n_feature_flag_toggled', parameters: { + 'new_value': !currentValue, + });lib/sharezone_localizations/lib/localizations/sharezone_localizations.gen.dart (1)
171-185: Enhance error handling for unsupported locales.The current error handling could be more specific about which locales are supported.
throw FlutterError( - 'SharezoneLocalizations.delegate failed to load unsupported locale "$locale". This is likely ' - 'an issue with the localizations generation tool. Please file an issue ' - 'on GitHub with a reproducible sample app and the gen-l10n configuration ' - 'that was used.'); + 'SharezoneLocalizations.delegate failed to load unsupported locale "$locale". ' + 'Supported locales are: ${supportedLocales.map((l) => l.languageCode).join(", ")}. ' + 'If you believe this is an error, please file an issue on GitHub with a reproducible ' + 'sample app and the gen-l10n configuration that was used.');app/lib/settings/settings_page.dart (1)
151-152: Consider adding a11y labels for language selectionThe language selection UI follows the existing pattern, but could benefit from accessibility improvements.
Add accessibility labels to help screen readers:
if (featureFlagl10n.isl10nEnabled) const _SettingsOption( title: "Sprache", icon: Icon(Icons.language), + semanticsLabel: "Spracheinstellungen ändern", tag: LanguagePage.tag, ),Also applies to: 175-180
app/lib/main/sharezone_bloc_providers.dart (1)
Line range hint
1-800: Consider documenting the localization feature flag integrationThe addition of the localization feature flag to the providers suggests this is part of a larger localization strategy. Consider adding documentation in the README or relevant documentation files about:
- The purpose and scope of the localization feature flag
- How it integrates with the activation code system
- Guidelines for other developers who might need to add similar localization support to their components
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (8)
.vscode/tasks.jsonis excluded by!**/*.jsonapp/pubspec.lockis excluded by!**/*.lock,!**/*.lockapp/pubspec.yamlis excluded by!**/*.yamllib/sharezone_localizations/analysis_options.yamlis excluded by!**/*.yamllib/sharezone_localizations/l10n.yamlis excluded by!**/*.yamllib/sharezone_localizations/pubspec.lockis excluded by!**/*.lock,!**/*.locklib/sharezone_localizations/pubspec.yamlis excluded by!**/*.yamllicenses_config.yamlis excluded by!**/*.yaml
📒 Files selected for processing (22)
.gitignore(1 hunks)app/lib/activation_code/src/bloc/enter_activation_code_bloc.dart(5 hunks)app/lib/activation_code/src/bloc/enter_activation_code_bloc_factory.dart(2 hunks)app/lib/l10n/feature_flag_l10n.dart(1 hunks)app/lib/l10n/flutter_app_local_gateway.dart(1 hunks)app/lib/main/sharezone.dart(4 hunks)app/lib/main/sharezone_app.dart(2 hunks)app/lib/main/sharezone_bloc_providers.dart(3 hunks)app/lib/main/sharezone_material_app.dart(3 hunks)app/lib/settings/settings_page.dart(2 hunks)app/lib/settings/src/subpages/language/language_page.dart(1 hunks)lib/sharezone_localizations/README.md(1 hunks)lib/sharezone_localizations/l10n/app_de.arb(1 hunks)lib/sharezone_localizations/l10n/app_en.arb(1 hunks)lib/sharezone_localizations/lib/localizations/sharezone_localizations.gen.dart(1 hunks)lib/sharezone_localizations/lib/localizations/sharezone_localizations_de.gen.dart(1 hunks)lib/sharezone_localizations/lib/localizations/sharezone_localizations_en.gen.dart(1 hunks)lib/sharezone_localizations/lib/sharezone_localizations.dart(1 hunks)lib/sharezone_localizations/lib/src/app_locale.dart(1 hunks)lib/sharezone_localizations/lib/src/app_locale_gateway.dart(1 hunks)lib/sharezone_localizations/lib/src/app_locale_provider.dart(1 hunks)lib/sharezone_localizations/lib/src/context_extension.dart(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- .gitignore
- lib/sharezone_localizations/l10n/app_de.arb
🧰 Additional context used
🪛 LanguageTool
lib/sharezone_localizations/README.md
[style] ~64-~64: To form a complete sentence, be sure to include a subject.
Context: ... locale changes: - AppLocaleProvider can be injected in your widget tree to hand...
(MISSING_IT_THERE)
[uncategorized] ~104-~104: Use a comma before ‘so’ if it connects two independent clauses (unless they are closely connected and short).
Context: ....arb files, regenerate the localizations so Flutter can reflect the changes. ### F...
(COMMA_COMPOUND_SENTENCE_2)
🔇 Additional comments (14)
lib/sharezone_localizations/lib/sharezone_localizations.dart (1)
1-8: LGTM! Proper license header implementation.
The license header is well-structured with all necessary components: copyright notice, license identifier (EUPL-1.2), license URL, and SPDX identifier.
lib/sharezone_localizations/lib/localizations/sharezone_localizations_en.gen.dart (1)
27-27: Fix typo in language_page_title getter
The typo from the ARB file has propagated to the generated code.
This will be fixed automatically when the ARB file is corrected and regenerated.
lib/sharezone_localizations/lib/localizations/sharezone_localizations_de.gen.dart (1)
13-14: LGTM! Documentation is clear and follows conventions.
The class documentation clearly indicates the language and follows Flutter's localization patterns.
app/lib/l10n/feature_flag_l10n.dart (1)
14-14: Document temporary status of this class
Previous review indicates this is a temporary class. Consider adding a TODO comment to track its removal.
app/lib/activation_code/src/bloc/enter_activation_code_bloc_factory.dart (1)
15-15: LGTM! Clean integration of the l10n feature flag
The addition of FeatureFlagl10n dependency is properly integrated into the factory class.
Also applies to: 22-22, 29-29, 38-38
lib/sharezone_localizations/lib/src/app_locale.dart (2)
21-27: LGTM: Clean pattern matching implementation
The toLocale() implementation using pattern matching is clean and maintainable. The use of switch expression is a good choice for enum handling.
15-18: Consider future extensibility for supported locales
The enum currently supports system, English, and German locales. Consider documenting the process of adding new locales or implementing a more dynamic approach for future language additions.
Let's check if there are any other locale references in the codebase:
✅ Verification successful
Let me gather more information about the localization implementation to better understand the architecture and potential extensibility concerns.
Let me check for any localization generation setup or documentation about adding new locales.
Let me check for the localization generation configuration and any documentation about the process.
Based on the gathered information from the codebase, I can now provide a final response:
Documentation exists but needs enhancement for locale additions
The project already has a well-structured localization setup using Flutter's gen-l10n tool with:
- Clear ARB file structure in
l10n/directory - Template-based generation using
app_en.arb - Automated generation configuration in
l10n.yaml
To address the extensibility concern:
- Add a section in
lib/sharezone_localizations/README.mddocumenting the step-by-step process for adding new locales:- Create new
app_[locale].arbfile inl10n/directory - Add translations following the template structure from
app_en.arb - Add locale to
AppLocaleenum inapp_locale.dart - Run
flutter gen-l10nto generate the localization files
- Create new
The current implementation with ARB files and gen-l10n is already designed for extensibility - no architectural changes needed.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for hardcoded locale references
rg -g '!*.{arb,lock}' -i "locale.*['\"](?:en|de)['\"]"
Length of output: 1137
Script:
#!/bin/bash
# Check for locale-related documentation
fd README -e md -x cat {}
# Look for locale configuration or generation setup
fd -e yaml -x cat {}
# Check for locale-related generation code or utilities
ast-grep --pattern 'class $_ implements LocalizationsDelegate<$_> {
$$$
}'
Length of output: 66127
Script:
#!/bin/bash
# Check for localization generation setup
cat lib/sharezone_localizations/l10n/app_en.arb
cat lib/sharezone_localizations/l10n/app_de.arb
# Look for locale-related code
ast-grep --pattern 'class AppLocale {
$$$
}'
Length of output: 833
Script:
#!/bin/bash
# Check for l10n.yaml config
cat lib/sharezone_localizations/l10n.yaml
# Look for any documentation about localization process
fd -t f -e md -e txt -x grep -l -i "local\|translation\|l10n" {} \;
# Check for locale-related code implementation
ast-grep --pattern 'class SharezoneLocalizations {
$$$
}'
Length of output: 730
app/lib/main/sharezone_material_app.dart (1)
38-38: Verify localization delegate initialization
The localization setup looks correct, but ensure that SharezoneLocalizations.localizationsDelegates includes all required delegates (e.g., material, cupertino).
Also applies to: 48-50
✅ Verification successful
Localization delegates are properly configured
The implementation includes all required delegates:
- GlobalMaterialLocalizations.delegate
- GlobalCupertinoLocalizations.delegate
- GlobalWidgetsLocalizations.delegate
- Custom Sharezone delegate
The MaterialApp is correctly configured to use these delegates along with the supported locales.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check SharezoneLocalizations implementation
ast-grep --pattern 'class SharezoneLocalizations {
static List<LocalizationsDelegate> localizationsDelegates = $$$;
}'
Length of output: 122
Script:
#!/bin/bash
# Let's try a broader search for localization delegates
rg -l "localizationsDelegates" --type dart
# Then search for SharezoneLocalizations class
rg "class SharezoneLocalizations" --type dart -A 10
Length of output: 3914
Script:
#!/bin/bash
# Let's check the MaterialApp configuration and localization delegates
rg "localizationsDelegates" -A 5 app/lib/main/sharezone_material_app.dart
# Also check the context extension for any delegate setup
rg "localizationsDelegates" -A 5 lib/sharezone_localizations/lib/src/context_extension.dart
Length of output: 607
Script:
#!/bin/bash
# Let's check the actual implementation of localizationsDelegates in the generated file
rg "localizationsDelegates" -B 5 -A 10 lib/sharezone_localizations/lib/localizations/sharezone_localizations.gen.dart
Length of output: 1190
lib/sharezone_localizations/lib/localizations/sharezone_localizations.gen.dart (1)
104-108: LGTM: Locale support implementation.
The implementation of supported locales is clean and follows Flutter's best practices.
app/lib/main/sharezone.dart (2)
143-151: LGTM: Provider setup follows best practices
The ChangeNotifierProvider setup for localization is well-structured:
- Feature flag provider is correctly placed before the locale provider
- Dependencies are properly injected into AppLocaleProvider
79-80: Consider initializing state variables atomically
The late initialization of streamingKeyValueStore and featureFlagl10n could lead to race conditions. Consider initializing them together in the constructor or using a dedicated initialization method.
Consider implementing a dedicated initialization method that ensures atomic initialization of all state variables:
- late StreamingKeyValueStore streamingKeyValueStore;
- late FeatureFlagl10n featureFlagl10n;
+ StreamingKeyValueStore? _streamingKeyValueStore;
+ FeatureFlagl10n? _featureFlagl10n;
+
+ StreamingKeyValueStore get streamingKeyValueStore => _streamingKeyValueStore!;
+ FeatureFlagl10n get featureFlagl10n => _featureFlagl10n!;
+
+ Future<void> _initializeState() async {
+ _streamingKeyValueStore = FlutterStreamingKeyValueStore(
+ widget.blocDependencies.streamingSharedPreferences,
+ );
+ _featureFlagl10n = FeatureFlagl10n(_streamingKeyValueStore!);
+ }Also applies to: 103-106
app/lib/main/sharezone_app.dart (1)
208-208: LGTM: Route registration is consistent
The LanguagePage route registration follows the established pattern and is correctly placed in the routes map.
app/lib/main/sharezone_bloc_providers.dart (2)
87-87: LGTM!
The import for the localization feature flag follows the existing import organization pattern.
597-597: LGTM!
The addition of featureFlagl10n parameter to EnterActivationCodeBlocFactory is implemented correctly using context.read.
Let's verify the integration of the feature flag:
✅ Verification successful
Integration of FeatureFlagl10n is properly implemented
The verification confirms that:
FeatureFlagl10nis correctly declared as a final field in bothEnterActivationCodeBlocandEnterActivationCodeBlocFactory- The dependency injection in
sharezone_bloc_providers.dartusingcontext.readmatches the factory's requirements - The feature flag is properly integrated across the activation code related files
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the FeatureFlagl10n usage in EnterActivationCodeBlocFactory
# and related files
# Check how FeatureFlagl10n is used in the activation code related files
ast-grep --pattern 'class EnterActivationCodeBlocFactory {
$$$
FeatureFlagl10n $$$
$$$
}'
# Check for any direct usage of the feature flag
rg -l "FeatureFlagl10n" --type dart
Length of output: 458
Script:
#!/bin/bash
# Let's check how FeatureFlagl10n is used in the activation code bloc
ast-grep --pattern 'class EnterActivationCodeBloc {
$$$
FeatureFlagl10n $$$
$$$
}'
# Check the factory implementation
ast-grep --pattern 'class EnterActivationCodeBlocFactory {
$$$
final FeatureFlagl10n $_
$$$
}'
# Check the actual usage of FeatureFlagl10n in these files
rg "FeatureFlagl10n" -A 3 app/lib/activation_code/src/bloc/enter_activation_code_bloc.dart app/lib/activation_code/src/bloc/enter_activation_code_bloc_factory.dart
Length of output: 1090
lib/sharezone_localizations/lib/localizations/sharezone_localizations_en.gen.dart
Outdated
Show resolved
Hide resolved
lib/sharezone_localizations/lib/localizations/sharezone_localizations_de.gen.dart
Outdated
Show resolved
Hide resolved
lib/sharezone_localizations/lib/localizations/sharezone_localizations.gen.dart
Outdated
Show resolved
Hide resolved
WalkthroughThis pull request introduces comprehensive localization and feature flag functionality to the Sharezone application. The changes span multiple files across the project, adding support for dynamic language selection, implementing a feature flag for localization, and creating a robust internationalization framework. The modifications include new classes for managing locales, feature flags, and localization delegates, along with updates to existing components to integrate these new capabilities. Changes
Sequence DiagramsequenceDiagram
participant User
participant LanguagePage
participant AppLocaleProvider
participant AppLocaleProviderGateway
participant FeatureFlagL10n
User->>LanguagePage: Select Language
LanguagePage->>AppLocaleProvider: Set Locale
AppLocaleProvider->>AppLocaleProviderGateway: Persist Locale
AppLocaleProvider->>FeatureFlagL10n: Check L10n Enabled
alt L10n Enabled
AppLocaleProvider->>User: Update UI with New Locale
else L10n Disabled
AppLocaleProvider->>User: Show Feature Flag Disabled Message
end
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (3)
app/lib/l10n/feature_flag_l10n.dart (1)
14-25:⚠️ Potential issuePotential race condition on initialization
This class retrieves the initial value for "l10n_enabled" asynchronously from the stream, which may lead to a transient mismatch between the real flag value and theisl10nEnabledfield until the first stream event arrives. If this is critical, consider performing a synchronous fetch of the initial value before setting up the subscription.lib/sharezone_localizations/lib/src/app_locale_provider.dart (2)
15-24:⚠️ Potential issuePlease add onError handling to the subscription
If the gateway’s stream emits errors, the app can crash unless errors are handled. Forwarding errors to a reporting service or at least logging them can help diagnose issues.
26-26: 🛠️ Refactor suggestionMark 'gateway' as final
The gateway field is required and set only in the constructor, so it’s safer to mark it as final to prevent reassignment.
🧹 Nitpick comments (12)
lib/sharezone_localizations/lib/src/app_locale_gateway.dart (1)
24-41: Consider broadcasting locale changes
The mock returns only a single-value stream. If future iterations require dynamic locale updates, consider using a broadcast stream or a subject that pushes new locales to existing subscribers in real time.app/lib/activation_code/src/bloc/enter_activation_code_bloc_factory.dart (1)
22-22: Field naming is consistent
Using descriptive naming (featureFlagl10n) clearly communicates the field’s purpose.app/lib/settings/src/subpages/language/language_page.dart (1)
19-29: Add error handling or user feedback for potential locale update issues.
When onChanged is triggered, any exceptions from updating the locale could silently fail without user-facing feedback. Consider adding basic error handling or a message in case of failures.Possible approach:
onChanged: (value) { try { localeProvider.locale = value!; } catch (e) { ScaffoldMessenger.of(context).showSnackBar( SnackBar(content: Text(context.sl.language_change_error)), ); } },app/lib/main/sharezone_material_app.dart (1)
48-50: Simplify the usage of localizations delegates & supported locales if seldom changed.
If these lists are static or rarely updated, consider centralizing them in one place to avoid duplication across the app.lib/sharezone_localizations/lib/src/app_locale.dart (2)
54-59: Check for map validity before accessing keys.
The code assumesmap['isSystem']always exists and is a bool. Consider adding type checks or null checks to avoid runtime exceptions.if (map == null || map['isSystem'] == null || map['isSystem'] == true) { return system; }
84-90: Add a safety check for empty language tags in fromLanguageTag.
When the language tag is empty or malformed, the code falls back to system. For clarity, check explicitly if the tag is empty or incorrectly formatted.Would you like me to open a GitHub issue to track the validation improvement for malformed language tags?
app/lib/activation_code/src/bloc/enter_activation_code_bloc.dart (2)
95-98: **Adding 'l10n' activation code trigger **
Relying on a specific string ('l10n') for toggling this feature might be adequate for testing and internal usage, but ensure that it's not inadvertently accessible in production or by end-users.
118-128: **Toggle locale feature flag **
The toggling logic is fine, but consider adding logging or analytics for better visibility into how frequently users enable/disable localization.lib/sharezone_localizations/lib/localizations/sharezone_localizations.gen.dart (1)
171-185: **Unsupported locale fallback **
Instead of throwing an error, consider gracefully defaulting to English or the next best locale to avoid runtime crashes.lib/sharezone_localizations/README.md (3)
64-64: Minor grammar improvement for clarity.
The sentence introduces how to manage locale changes but lacks a complete subject. Consider rewriting to be more clear.- To manage or observe locale changes: + To manage or observe locale changes, you can:🧰 Tools
🪛 LanguageTool
[style] ~64-~64: To form a complete sentence, be sure to include a subject.
Context: ... locale changes: -AppLocaleProvidercan be injected in your widget tree to hand...(MISSING_IT_THERE)
92-92: Insert a comma for clarity.
Add a small comma to separate the clauses and improve readability.- ... across your app. (Optionally you can use packages like arb_translate ... + ... across your app. (Optionally, you can use packages like arb_translate ...🧰 Tools
🪛 LanguageTool
[uncategorized] ~92-~92: Possible missing comma found.
Context: ...anslations up to date across your app. (Optionally you can use packages like arb_translate...(AI_HYDRA_LEO_MISSING_COMMA)
104-104: Use a comma before “so” to connect independent clauses.
Insert a comma between the two independent clauses for better flow.- After updating or creating .arb files, regenerate the localizations so Flutter can reflect the changes. + After updating or creating .arb files, regenerate the localizations, so Flutter can reflect the changes.🧰 Tools
🪛 LanguageTool
[uncategorized] ~104-~104: Use a comma before ‘so’ if it connects two independent clauses (unless they are closely connected and short).
Context: ....arb files, regenerate the localizations so Flutter can reflect the changes. ### F...(COMMA_COMPOUND_SENTENCE_2)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (8)
.vscode/tasks.jsonis excluded by!**/*.jsonapp/pubspec.lockis excluded by!**/*.lock,!**/*.lockapp/pubspec.yamlis excluded by!**/*.yamllib/sharezone_localizations/analysis_options.yamlis excluded by!**/*.yamllib/sharezone_localizations/l10n.yamlis excluded by!**/*.yamllib/sharezone_localizations/pubspec.lockis excluded by!**/*.lock,!**/*.locklib/sharezone_localizations/pubspec.yamlis excluded by!**/*.yamllicenses_config.yamlis excluded by!**/*.yaml
📒 Files selected for processing (22)
.gitignore(1 hunks)app/lib/activation_code/src/bloc/enter_activation_code_bloc.dart(5 hunks)app/lib/activation_code/src/bloc/enter_activation_code_bloc_factory.dart(2 hunks)app/lib/l10n/feature_flag_l10n.dart(1 hunks)app/lib/l10n/flutter_app_local_gateway.dart(1 hunks)app/lib/main/sharezone.dart(4 hunks)app/lib/main/sharezone_app.dart(2 hunks)app/lib/main/sharezone_bloc_providers.dart(3 hunks)app/lib/main/sharezone_material_app.dart(3 hunks)app/lib/settings/settings_page.dart(2 hunks)app/lib/settings/src/subpages/language/language_page.dart(1 hunks)lib/sharezone_localizations/README.md(1 hunks)lib/sharezone_localizations/l10n/app_de.arb(1 hunks)lib/sharezone_localizations/l10n/app_en.arb(1 hunks)lib/sharezone_localizations/lib/localizations/sharezone_localizations.gen.dart(1 hunks)lib/sharezone_localizations/lib/localizations/sharezone_localizations_de.gen.dart(1 hunks)lib/sharezone_localizations/lib/localizations/sharezone_localizations_en.gen.dart(1 hunks)lib/sharezone_localizations/lib/sharezone_localizations.dart(1 hunks)lib/sharezone_localizations/lib/src/app_locale.dart(1 hunks)lib/sharezone_localizations/lib/src/app_locale_gateway.dart(1 hunks)lib/sharezone_localizations/lib/src/app_locale_provider.dart(1 hunks)lib/sharezone_localizations/lib/src/context_extension.dart(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- lib/sharezone_localizations/l10n/app_de.arb
- lib/sharezone_localizations/lib/sharezone_localizations.dart
🧰 Additional context used
📓 Learnings (1)
app/lib/settings/src/subpages/language/language_page.dart (1)
Learnt from: nilsreichardt
PR: SharezoneApp/sharezone-app#1798
File: app/lib/settings/src/subpages/language/language_page.dart:39-54
Timestamp: 2024-12-22T22:54:33.905Z
Learning: `RadioListTile` (in Flutter) already includes necessary semantics for accessibility, making an extra `Semantics` wrapper often redundant.
🪛 LanguageTool
lib/sharezone_localizations/README.md
[style] ~64-~64: To form a complete sentence, be sure to include a subject.
Context: ... locale changes: - AppLocaleProvider can be injected in your widget tree to hand...
(MISSING_IT_THERE)
[uncategorized] ~92-~92: Possible missing comma found.
Context: ...anslations up to date across your app. (Optionally you can use packages like arb_translate...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~104-~104: Use a comma before ‘so’ if it connects two independent clauses (unless they are closely connected and short).
Context: ....arb files, regenerate the localizations so Flutter can reflect the changes. ### F...
(COMMA_COMPOUND_SENTENCE_2)
🔇 Additional comments (45)
lib/sharezone_localizations/l10n/app_en.arb (2)
10-12: Looks good
These keys (“languageSystemName”, “languageDeName”, “languageEnName”) are consistent and correctly spelled.
9-9:
Fix the typo in “Langauge” → “Language”
The key "languagePageTitle" has a typo in its value. This can cause confusion and inconsistency in your translated strings.
Apply this diff to correct it:
- "languagePageTitle": "Langauge",
+ "languagePageTitle": "Language",Likely invalid or redundant comment.
lib/sharezone_localizations/lib/src/context_extension.dart (1)
19-19: Handle null safety more gracefully
Force unwrapping the return value from SharezoneLocalizations.of(this) can lead to a runtime crash if not properly initialized. This mirrors a previous comment.
Suggested approach:
- SharezoneLocalizations get sl => SharezoneLocalizations.of(this)!;
+ SharezoneLocalizations get sl {
+ final localizations = SharezoneLocalizations.of(this);
+ if (localizations == null) {
+ throw FlutterError(
+ 'SharezoneLocalizations not found.\n'
+ 'Did you forget to add SharezoneLocalizations.delegate '
+ 'to your localizationsDelegates?'
+ );
+ }
+ return localizations;
+ }lib/sharezone_localizations/lib/localizations/sharezone_localizations_en.gen.dart (1)
27-27: Typo detected in "languagePageTitle"
This is still spelled "Langauge" instead of "Language." Same issue referenced previously.
- String get languagePageTitle => 'Langauge';
+ String get languagePageTitle => 'Language';lib/sharezone_localizations/lib/localizations/sharezone_localizations_de.gen.dart (1)
1-37: All translations look correct in German
No spelling or functional issues found.
app/lib/l10n/feature_flag_l10n.dart (2)
29-35: Implementation of toggle logic looks good
The toggle method neatly flips the current boolean state and notifies listeners. Unless there is a specific reason to batch toggles, this direct approach is straightforward and maintainable.
37-41: Dispose method is correctly implemented
The subscription is canceled appropriately, preventing leaks and ensuring clean tear down.
lib/sharezone_localizations/lib/src/app_locale_provider.dart (1)
32-38: Setter approach is appropriate
The setter checks if the updated locale matches the existing one, reducing redundant updates and notifications. This is a good pattern for performance.
lib/sharezone_localizations/lib/src/app_locale_gateway.dart (1)
16-22: Interface is well-defined
The contract for retrieving and setting locales is clear and provides an extensible foundation for more advanced gateway implementations.
app/lib/activation_code/src/bloc/enter_activation_code_bloc_factory.dart (3)
15-15: Clean import of FeatureFlagl10n
Importing this dependency ensures the factory can manage the localization feature flag. Looks consistent.
29-29: Constructor injection of featureFlagl10n
Ensuring the constructor requires this dependency promotes clearer usage and testability.
38-38: Bloc instantiation is clear
Passing featureFlagl10n as an argument to EnterActivationCodeBloc maintains consistent dependency injection.
app/lib/l10n/flutter_app_local_gateway.dart (2)
35-41:
Await the async storage operation in setLocale.
The method signature is async, but the write operation to the keyValueStore is not awaited. This can lead to race conditions or data loss if the operation fails or is interrupted.
Future<void> setLocale(AppLocale locale) async {
final value = jsonEncode(locale.toMap());
- keyValueStore.setString(
+ await keyValueStore.setString(
'locale',
value,
);
}Likely invalid or redundant comment.
24-32: 🛠️ Refactor suggestion
Add try-catch blocks to handle malformed or empty JSON for getLocale.
If the JSON stored is invalid or empty, calling jsonDecode can throw an error. It would be beneficial to handle exceptions gracefully to avoid potential runtime crashes.
Possible enhancement:
- .map((event) => AppLocale.fromMap(jsonDecode(event)));
+ .map((event) {
+ try {
+ return AppLocale.fromMap(jsonDecode(event));
+ } catch (e) {
+ // Log error or handle gracefully
+ return featureFlagl10n.isl10nEnabled ? AppLocale.system : AppLocale.en;
+ }
+ });Likely invalid or redundant comment.
app/lib/main/sharezone_material_app.dart (1)
38-38: Validate locale conversion for potential errors.
If the user somehow selects an invalid or unsupported locale, the .toLocale() method might return unexpected results. Adding a try-catch or a fallback here can help prevent runtime crashes.
Would you like me to generate a script to search for other toLocale() calls in the codebase to confirm if all call sites handle potential exceptions or fallback gracefully?
app/lib/activation_code/src/bloc/enter_activation_code_bloc.dart (3)
18-18: **Import statement seems correct **
No issues spotted with adding this import.
30-30: **New field for feature flag management **
Introducing this field effectively integrates the localization feature flag with the Bloc.
39-39: **Constructor injection of FeatureFlagl10n **
Constructor-based injection promotes testability and decoupling. Good addition.
lib/sharezone_localizations/lib/localizations/sharezone_localizations.gen.dart (11)
1-7: **License header **
All looks good here.
9-10: **Imports for localization **
Imports appear correct and necessary for the generated localizations.
11-15: **Core Flutter imports **
No issues found in these lines.
16-17: **Locale-specific generated files **
References to the DE/EN localizations are correct.
18-20: **Ignore lint directives **
No concerns here.
21-37: **Documentation references **
The documentation is detailed and helpful for integrators.
38-51: **Pubspec instructions **
Clear instructions for adding dependencies. Looks good.
53-71: **iOS configuration notes **
Well-documented steps for iOS app locale setup.
72-127: **Abstract class for localizations **
Overall structure is consistent with Flutter's recommended patterns.
128-132: **Fix the "Langauge" typo **
Matches the past review comment. Please correct “Langauge” to “Language”.
153-169: **LocalizationsDelegate implementation **
Mechanics for loading localizations appear valid.
app/lib/main/sharezone.dart (7)
24-24: **Introduced import for feature flag **
No concerns here; the feature flag functionality is needed.
25-25: **Importing FlutterAppLocaleGateway **
Good approach for bridging the streaming store with the localization layer.
33-33: **Importing StreamingKeyValueStore **
Necessary for persistent feature flag management.
79-79: **Storing a streaming key-value store **
Storing it as a local field is consistent with the rest of the class.
80-80: **FeatureFlagl10n field **
Late initialization is acceptable here.
102-106: **Initialization of streaming key-value store & FeatureFlagl10n **
No issues spotted. This is a clean approach to hooking up the feature flags with persistent storage.
143-151: **Providers for featureFlagl10n & AppLocaleProvider **
Wrapping them in ChangeNotifierProviders allows the UI and logic to react to locale feature flag changes seamlessly.
app/lib/main/sharezone_app.dart (2)
49-49: **Added import for LanguagePage **
Necessary for referencing the new language settings screen.
208-208: **Added route for LanguagePage **
Routing to LanguagePage correctly references the declared static tag.
app/lib/main/sharezone_bloc_providers.dart (3)
87-87: New import for feature flag localization.
No issues here. The import statement is consistent with the new feature flag l10n usage.
501-501: Provider for KeyValueStore seems fine.
Exposing the KeyValueStore via Provider is a straightforward approach. Ensure that all child widgets correctly consume this store as intended.
597-597: Verify availability of 'FeatureFlagl10n' upstream.
Confirm that the 'FeatureFlagl10n' provider is created higher up in the widget tree so that 'context.read()' never fails here.
.gitignore (1)
14-14: Allow tracking of .vscode/tasks.json.
This change looks intentional. Confirm that you want .vscode/tasks.json to be versioned, as it may contain user-specific tasks.
app/lib/settings/settings_page.dart (3)
13-14: Imports for provider and feature flag localization.
These imports are aligned with the new feature flag system.
21-21: New import for LanguagePage.
Importing the new LanguagePage is consistent with the localization feature.
151-180: Conditional language settings option.
Rendering the “Sprache” option behind a feature flag is a clean approach. Be mindful of user feedback if the feature flag toggles frequently.
lib/sharezone_localizations/lib/localizations/sharezone_localizations_en.gen.dart
Outdated
Show resolved
Hide resolved
nilsreichardt
added
the
safe to test
github-actions
bot
removed
the
safe to test
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
lib/sharezone_localizations/README.md (4)
1-31: Consider enhancing the introduction with essential package information.The introduction effectively explains the package's purpose. Consider adding:
- Version compatibility information
- Minimum Flutter/Dart SDK requirements
- Installation instructions (e.g., how to add the package to pubspec.yaml)
34-61: Enhance usage examples with additional scenarios.The current example effectively demonstrates basic usage. Consider adding examples for:
- Error handling (e.g., missing translations)
- Plural forms usage
- Gender-specific translations
- Handling formatting (dates, numbers, currency)
68-101: Add guidelines for string management best practices.Consider enhancing this section with:
- Key naming conventions (e.g., category_feature_description)
- Guidelines for string reuse and modularization
- String validation process
- Best practices for handling long texts and rich text formatting
102-122: Enhance generation instructions with configuration options and troubleshooting.Consider adding:
- Available configuration options for
gen-l10n- Troubleshooting guide for common generation issues
- CI/CD integration examples
Also, fix the grammar in line 104:
-After updating or creating .arb files, regenerate the localizations so Flutter can reflect the changes. +After updating or creating .arb files, regenerate the localizations, so Flutter can reflect the changes.🧰 Tools
🪛 LanguageTool
[uncategorized] ~104-~104: Use a comma before ‘so’ if it connects two independent clauses (unless they are closely connected and short).
Context: ....arb files, regenerate the localizations so Flutter can reflect the changes. ### F...(COMMA_COMPOUND_SENTENCE_2)
lib/sharezone_localizations/lib/src/context_extension.dart (1)
13-19: Enhance documentation with usage examplesWhile the documentation clearly explains the requirements, it would be more helpful to include example usage. This helps developers understand how to properly integrate the extension in their code.
Add example usage to the documentation:
/// Extension on [BuildContext] to access the [SharezoneLocalizations] object. /// /// Requires the SharezoneLocalizations to be available in the context, /// otherwise it will throw an exception. Add /// [SharezoneLocalizations.delegate] to the underlying App /// localizationsDelegates, for access in the context of the app. + /// + /// Example usage: + /// ```dart + /// Text(context.l10n.welcomeMessage), + /// ``` + /// + /// Make sure to add the delegate to your app: + /// ```dart + /// MaterialApp( + /// localizationsDelegates: [ + /// SharezoneLocalizations.delegate, + /// ...GlobalMaterialLocalizations.delegates, + /// ], + /// // ... + /// ) + /// ``` SharezoneLocalizations get l10n => SharezoneLocalizations.of(this)!;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
app/lib/settings/src/subpages/language/language_page.dart(1 hunks)lib/sharezone_localizations/README.md(1 hunks)lib/sharezone_localizations/l10n/app_en.arb(1 hunks)lib/sharezone_localizations/lib/localizations/sharezone_localizations.gen.dart(1 hunks)lib/sharezone_localizations/lib/localizations/sharezone_localizations_en.gen.dart(1 hunks)lib/sharezone_localizations/lib/src/app_locale.dart(1 hunks)lib/sharezone_localizations/lib/src/context_extension.dart(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- lib/sharezone_localizations/lib/localizations/sharezone_localizations_en.gen.dart
- app/lib/settings/src/subpages/language/language_page.dart
- lib/sharezone_localizations/l10n/app_en.arb
- lib/sharezone_localizations/lib/src/app_locale.dart
- lib/sharezone_localizations/lib/localizations/sharezone_localizations.gen.dart
🧰 Additional context used
🪛 LanguageTool
lib/sharezone_localizations/README.md
[style] ~64-~64: To form a complete sentence, be sure to include a subject.
Context: ... locale changes: - AppLocaleProvider can be injected in your widget tree to hand...
(MISSING_IT_THERE)
[uncategorized] ~104-~104: Use a comma before ‘so’ if it connects two independent clauses (unless they are closely connected and short).
Context: ....arb files, regenerate the localizations so Flutter can reflect the changes. ### F...
(COMMA_COMPOUND_SENTENCE_2)
🔇 Additional comments (4)
lib/sharezone_localizations/README.md (1)
1-122: Documentation is well-structured and informative.
The README provides a solid foundation for using the localization package. While there are opportunities for enhancement (as noted in previous comments), the current documentation effectively covers the essential aspects of the package's functionality and usage.
🧰 Tools
🪛 LanguageTool
[style] ~64-~64: To form a complete sentence, be sure to include a subject.
Context: ... locale changes: - AppLocaleProvider can be injected in your widget tree to hand...
(MISSING_IT_THERE)
[uncategorized] ~104-~104: Use a comma before ‘so’ if it connects two independent clauses (unless they are closely connected and short).
Context: ....arb files, regenerate the localizations so Flutter can reflect the changes. ### F...
(COMMA_COMPOUND_SENTENCE_2)
lib/sharezone_localizations/lib/src/context_extension.dart (3)
1-8: LGTM: License header is properly formatted
The copyright notice and EUPL-1.2 license are correctly specified with the current year.
19-19: Improve null safety handling in the extension
The force-unwrap operator (!) could lead to runtime crashes if the localizations delegate isn't properly initialized.
9-10: Verify localization delegate setup
Let's verify that the required localization delegate is properly set up in the app and that the SharezoneLocalizations.of() method is correctly implemented.
Also applies to: 19-19
✅ Verification successful
Let me gather more information to verify the localization setup.
Let me do one final check to verify the complete setup.
Localization integration appears to be properly implemented
Based on the verification results, I can confirm that:
- The
SharezoneLocalizations.of()method is correctly implemented in the generated code usingLocalizations.of<SharezoneLocalizations>. - The localization extension is actively used throughout the codebase (e.g., in
LanguagePage,AppLocale). - The required
.arbfiles exist for both English and German localizations. - The implementation follows Flutter's standard localization patterns with proper delegate setup.
While we couldn't verify the exact MaterialApp setup due to file access limitations, the presence of working localization usage (context.l10n) in multiple files indicates that the delegate is properly configured in the app.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for proper delegate setup in MaterialApp/CupertinoApp
echo "Checking for SharezoneLocalizations.delegate usage..."
rg "SharezoneLocalizations.delegate" -A 5
# Verify SharezoneLocalizations.of implementation
echo "Checking SharezoneLocalizations.of implementation..."
ast-grep --pattern 'static SharezoneLocalizations? of(BuildContext context) {
$$$
}'
Length of output: 2243
Script:
#!/bin/bash
# Search for MaterialApp/CupertinoApp configurations with localizationsDelegates
echo "Checking for localizationsDelegates setup..."
rg "localizationsDelegates.*\[" -A 5
# Check the SharezoneLocalizations.of implementation in the generated file
echo "Checking generated SharezoneLocalizations implementation..."
rg "class SharezoneLocalizations" -A 20 lib/sharezone_localizations/lib/localizations/sharezone_localizations.gen.dart
# Look for actual usage of the l10n extension
echo "Checking for l10n extension usage..."
rg "context.l10n" -A 2
Length of output: 3248
Script:
#!/bin/bash
# Check for the app's localization setup
echo "Checking app's localization setup..."
rg "MaterialApp|CupertinoApp" -A 10 app/lib/main.dart
# Check for arb files
echo "Checking for .arb files..."
fd .arb$ lib/sharezone_localizations
# Check for delegate usage in the app
echo "Checking delegate usage in app..."
rg "SharezoneLocalizations.delegate" app/lib/
Length of output: 514
| To manage or observe locale changes: | ||
|
|
||
| - `AppLocaleProvider` can be injected in your widget tree to handle locale switching logic. | ||
|
|
||
| --- |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Expand the locale management section with implementation details.
The section would benefit from:
- Complete implementation examples of locale switching
- Instructions for persisting locale preferences
- Handling of default locales
- System locale detection
Also, consider rephrasing line 64 to form a complete sentence:
-AppLocaleProvider can be injected in your widget tree to handle locale switching logic.
+The AppLocaleProvider can be injected in your widget tree to handle locale switching logic.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| To manage or observe locale changes: | |
| - `AppLocaleProvider` can be injected in your widget tree to handle locale switching logic. | |
| --- | |
| To manage or observe locale changes: | |
| - The AppLocaleProvider can be injected in your widget tree to handle locale switching logic. | |
| --- |
🧰 Tools
🪛 LanguageTool
[style] ~64-~64: To form a complete sentence, be sure to include a subject.
Context: ... locale changes: - AppLocaleProvider can be injected in your widget tree to hand...
(MISSING_IT_THERE)
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
app/test_goldens/settings/settings_page_test.dart (1)
33-37: Great approach using a Provider forFeatureFlagl10n.
InjectingFeatureFlagl10ninto the widget tree improves testability and modularity. However, if more dependencies are added later, consider grouping them in a dedicated provider or using multi-providers for scalability.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
app/test_goldens/settings/settings_page_test.dart(1 hunks)app/test_goldens/settings/settings_page_test.mocks.dart(3 hunks)lib/sharezone_localizations/lib/src/context_extension.dart(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- lib/sharezone_localizations/lib/src/context_extension.dart
🔇 Additional comments (7)
app/test_goldens/settings/settings_page_test.dart (2)
14-15: All good with newly introduced imports.
No issues detected. They seem necessary for the new provider and feature-flag functionality.
Also applies to: 18-18
23-26: Extension of generated mocks is well-structured.
Including FeatureFlagl10n in the mocks ensures comprehensive testing opportunities. No concerns here.
app/test_goldens/settings/settings_page_test.mocks.dart (5)
6-7: No issues with Dart UI import.
Importing dart:ui for VoidCallback usage is valid, and there's no concern about conflicts or performance overhead.
11-14: Consistent imports for new feature flag and streaming key-value store.
These imports align with the introduction of FeatureFlagl10n and the streaming key-value store functionalities used in the mocks.
86-95: New _FakeStreamingKeyValueStore_5 mock class seems correctly scoped.
The class correctly extends SmartFake and implements StreamingKeyValueStore. Since this file is auto-generated by Mockito, manual edits are typically discouraged.
100-100: MockSharezoneContext update is compatible with the new alias.
The updated implementation uses SharezoneContext from _i8 instead of the old alias. This is aligned with the refactored import paths.
176-261: MockFeatureFlagl10n provides thorough coverage for testing.
All relevant methods—getters, setters, and no-op lifecycle methods—are mocked here. This is consistent with generated Mockito patterns and appears ready for usage in test scenarios.
nilsreichardt
added
the
safe to test
github-actions
bot
removed
the
safe to test
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
This pull request introduces a new package, sharezone_localizations, which provides a streamlined way of managing and accessing localized strings in the Sharezone-App using Flutter’s internationalization features. Below is an overview of the core additions:
1. New Package: sharezone_localizations
• README Documentation: Explains how to add, generate, and use translatable strings.
• Internationalization Setup: Based on Flutter’s gen-l10n tooling, enabling multiple locales and seamless code generation.
2. Locale Management
•
AppLocaleProvider: Facilitates dynamic switching of app locales.3. Usage Guidelines
• String Access: Via the context.sl extension, which simplifies referencing keys from the .arb files.
• Adding/Updating Strings: Detailed steps on modifying .arb files, including placeholder usage and recommended auto-translation tools.
4. Generating Localizations
• Flutter gen-l10n Command: Simple CLI approach to regenerate localizations after .arb updates.
• VS Code Task: Option to run a dedicated VS Code Task (“Generate l10n for sharezone_localizations”) for developers who prefer an IDE-based workflow.
Overall, this package consolidates translation logic, enhances maintainability by centralizing locale management, and simplifies how developers interact with localized strings.
Things, we need to do in future pull requests:
sz l10ncommand to our repo CLI #1799/app/l10n/.arbfile, usearb_translate, makes native changes -Info.plistneeds changed)Summary by CodeRabbit
Summary by CodeRabbit
LanguagePagefor users to choose their preferred language.FeatureFlagl10nclass.AppLocaleProviderfor managing application locale settings.SharezoneLocalizations.sharezone_localizationspackage..gitignoreto manage ignored files more effectively.Part of #346