#1936: Improve localization of gui#1979
Conversation
- Added testing class for modals
- Added logging to IdeGuiStateManager. - Added functionality, that selecting a different project now switches the IdeContext to the new project.
- Added testing class for modals
- Added logging to IdeGuiStateManager. - Added functionality, that selecting a different project now switches the IdeContext to the new project.
…t-for-gui' into devonfw#1785-implement-modals-in-idecontext
- Added functionality, that selecting a different project now switches the IdeContext to the new project.
…plementation' into devonfw#1802-state-management-implementation
…r other ui feature branches
- added DI for IdeGuiStateManager.switchContext
…reading the list of workspaces/projects instead of reading those from the UI
…nager, when switchContext(Path rootDirectory, ...) is called.
This reverts commit 6f92d93.
…plementation' into devonfw#1802-state-management-implementation
…tateManager is now set when calling getInstance(), allowing us to provide a getInstance() method with a DI parameter
… getInstance()) (see previous commit)
…can be extended by tests
…plementation' into devonfw#1802-state-management-implementation
- Add interactive i18n support: app can switch locales (English/Deutsch) via combo box - I18nService manages locale state with listener notifications for UI text updates - MainController registers locale change listener to refresh all UI labels and prompts on language switch Testing: - I18nServiceTest: verify service behavior - AppBaseTest: new interactive UI test
Coverage Report for CI Build 27685523323Coverage decreased (-0.04%) to 71.243%Details
Uncovered ChangesNo uncovered changes found. Coverage Regressions34 previously-covered lines in 4 files lost coverage.
Coverage Stats💛 - Coveralls |
laim2003
left a comment
There was a problem hiding this comment.
looks good! just left some thoughts😊
|
|
||
| private static final Logger LOG = LoggerFactory.getLogger(I18nService.class); | ||
|
|
||
| private static final String BUNDLE_NAME = "i18n.messages"; |
There was a problem hiding this comment.
i would say that naming this something like "localization" would probably be a bit more readable (altough i understand where you are coming from with i18n)
At least that what I would think coming from an android development background, where this is how they name it😅
There was a problem hiding this comment.
I agree with you, it would be clearer to be named as "Localization", done ✅
| public static synchronized I18nService getInstance() { | ||
|
|
||
| if (instance == null) { | ||
| return getInstance(null); | ||
| } | ||
| return instance; | ||
| } |
There was a problem hiding this comment.
@hohwille said that we should avoid Singleton patterns where possible and use Dependency Injection. I know i used this pattern a lot in my own state management implementation, but I will also be moving away from Singeltons there. Maybe we can discuss about whether its okay to use them in some instances in the Next daily, probably would also help me clear up some questions. I would probably leave this how it is here for now, but then open a second issue for the future where we update this to DI.
| try { | ||
| return this.resourceBundle.getString(key); | ||
| } catch (MissingResourceException e) { | ||
| LOG.warn("Missing translation key: {} for locale: {}", key, this.locale); |
There was a problem hiding this comment.
I think if no string is found, this probably should be an error rather than a warning. Maybe you can help me here, basically if the string is not found in ANY translation, this error will occur? Or does it already occur if the key is simply not found in the specified Locale?
There was a problem hiding this comment.
I noticed actually that if a resource key is missing, the application fails to start. This is because fxmlLoader.load() in the application's start method throws an exception before the custom exception in the get method is reached.
Resulting error:
Failed to load the application: Resource "button.open" not found
/C:/.../main-view.fxml
This PR fixes #1936
Implemented changes:
Testing instructions
Please add conscise, understandable instructions on how a reviewer can test/verify the functionality of your contribution here:
/ide-guiAppLauncher, interact with the third combo box (Language) -- check if language is correctly switchedmessages.propertiesandmessages_de.properties--> runI18nServiceTestChecklist for this PR
Make sure everything is checked before merging this PR. For further info please also see
our DoD.
mvn clean testlocally all tests pass and build is successful#«issue-id»: «brief summary»(e.g.#921: fixed setup.bat). If no issue ID exists, title only.In Progressand assigned to you or there is no issue (might happen for very small PRs)with
internal