Notify depending widgets when translations are loaded #756
Conversation
WalkthroughThe changes update English translation files, modify notification logic in localization provider and controller classes to better signal when translations are loaded, and add or update tests to verify correct widget updates and translation loading behavior. Minor test description typos are also corrected. Changes
Sequence Diagram(s)sequenceDiagram
participant Widget
participant EasyLocalizationProvider
participant EasyLocalizationController
Widget->>EasyLocalizationProvider: dependOnInheritedWidgetOfExactType
EasyLocalizationProvider->>EasyLocalizationController: Check if translations loaded
EasyLocalizationController->>EasyLocalizationProvider: Load translations (async)
EasyLocalizationController->>EasyLocalizationProvider: notifyListeners()
EasyLocalizationProvider-->>Widget: updateShouldNotify triggers rebuild if translations loaded
Widget->>EasyLocalizationProvider: didChangeDependencies called
Assessment against linked issues
Poem
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
test/easy_localization_init_test.dart (2)
10-19: Remove duplicate logger configuration.The logger configuration is set twice with identical values. This is redundant and should be consolidated.
EasyLocalization.logger.enableLevels = <LevelMessages>[ LevelMessages.error, LevelMessages.warning, ]; SharedPreferences.setMockInitialValues({}); -EasyLocalization.logger.enableLevels = <LevelMessages>[ - LevelMessages.error, - LevelMessages.warning, -];
29-29: Consider using a more robust path for translations.The path
'../../i18n'is relative and could be fragile depending on where tests are executed from. Consider using a more explicit path or ensuring the test environment is consistent.-path: '../../i18n', +path: 'test/resources/i18n', // or another more explicit path
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
i18n/en-US.json(1 hunks)i18n/en.json(1 hunks)lib/src/easy_localization_app.dart(3 hunks)lib/src/easy_localization_controller.dart(1 hunks)test/easy_localization_init_test.dart(1 hunks)test/easy_localization_widget_test.dart(6 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (10)
i18n/en-US.json (1)
2-2: Good test data differentiation.Changing the test key value to "test_en-US" helps distinguish between different locale files in tests, making it easier to verify that the correct locale-specific translations are being loaded.
i18n/en.json (1)
2-2: Consistent locale differentiation.The change to "test_en" maintains consistency with the en-US locale file and helps distinguish the base English locale in tests.
lib/src/easy_localization_controller.dart (1)
127-127: Excellent fix for the core issue.Adding
notifyListeners()after translation loading ensures that dependent widgets are properly notified when translations become available. The placement is correct - after both main and fallback translations are loaded, and only on successful loading.This directly addresses the PR objective of notifying widgets when translations are loaded.
test/easy_localization_widget_test.dart (2)
132-132: Correct test updates for new translation values.The test expectations have been properly updated from 'test' to 'test_en-US' to align with the changes in the translation files. This ensures tests will pass with the new locale-specific translation values.
Also applies to: 136-136, 158-158, 163-163, 208-208, 213-213, 244-244, 249-249, 792-792
227-227: Good typo fixes.Fixed the typo "loacle" to "locale" in test descriptions, improving code quality and readability.
Also applies to: 282-282
lib/src/easy_localization_app.dart (3)
232-232: Good addition to track translation loading state.Adding the
_translationsLoadedfield provides a clean way to track whether translations have been loaded, which is essential for the notification mechanism.
259-259: Correct initialization of translation loading state.The initialization logic correctly determines if translations are loaded by checking if
_localeState.translationsis not null. This provides the initial state for the notification mechanism.
295-296: Excellent enhancement to notification logic.The updated
updateShouldNotifymethod now correctly triggers notifications when either the locale changes OR when the translation loading state changes. This ensures that dependent widgets are notified immediately when translations become available, which perfectly addresses the PR objective.test/easy_localization_init_test.dart (2)
39-77: Well-designed test observer for translation loading verification.The
I18nObserveris an excellent approach to test the notification mechanism. It correctly:
- Uses
didChangeDependenciesto detect inherited widget changes- Calls
EasyLocalization.of(context)to establish dependency- Tests both the initial untranslated state and the subsequent translated state
The test logic effectively verifies that dependent widgets are notified when translations are loaded.
7-7:✅ Verification successful
Verify the dependency on easy_localization_context_test.dart.
The test imports
MyAppfromeasy_localization_context_test.dart. Ensure this dependency is intentional and thatMyAppis the appropriate widget for this test scenario.
🏁 Script executed:
#!/bin/bash # Check if MyApp is defined in the imported file and verify its usage fd "easy_localization_context_test.dart" --exec cat {} | grep -A 10 -B 5 "class MyApp"Length of output: 565
🏁 Script executed:
#!/bin/bash # Display the contents of the test to see how MyApp is used fd "easy_localization_init_test.dart" --exec cat {} | sed -n '1,200p'Length of output: 2110
Dependency Verified: MyApp is defined and used intentionally
The import ofeasy_localization_context_test.dartis correct—MyAppis declared there as the top-level widget for this test and is used appropriately withinEasyLocalization. No changes needed.
|
Wow that was an interesting review. |
|
Thank you @aissat for merging my fix so quickly. Do you have an ETA for the next update? |
Fixes #755
Summary by CodeRabbit
Bug Fixes
Tests
Chores