-
Notifications
You must be signed in to change notification settings - Fork 528
Edge UI: Use browser Language as Default #3415
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Conversation
Codecov Report❌ Patch coverage is ❌ Your patch check has failed because the patch coverage (37.04%) is below the target coverage (75.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## develop #3415 +/- ##
=============================================
+ Coverage 59.61% 59.62% +0.01%
Complexity 112 112
=============================================
Files 2894 2895 +1
Lines 124658 124664 +6
Branches 9343 9332 -11
=============================================
+ Hits 74304 74320 +16
+ Misses 47534 47524 -10
Partials 2820 2820 🚀 New features to boost your workflow:
|
…-browser-language # Conflicts: # ui/src/assets/i18n/de.json # ui/src/assets/i18n/en.json
lukasrgr
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thx for this incremental improvement
| edgeShortName: "OpenEMS", | ||
| edgeLongName: "Open Energy Management System", | ||
| defaultLanguage: "de", | ||
| defaultLanguage: "en", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
das solltest du wahrscheinlich nicht ändern
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
en macht aber mehr sinn als de
| ], | ||
| "fileReplacements": [ | ||
| { | ||
| "replace": "src/environments/dummy.ts", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For consistency we should name this file karma.ts dont you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
dummy.ts ist ja die Datei, die ich ersetzen will.
| const userLangKey = Language.getByKey(authenticateResponse.user.language?.toLowerCase()); | ||
| const demoLangKey = Language.getByKey(localStorage.DEMO_LANGUAGE); | ||
|
|
||
| const language = demoLangKey ?? userLangKey ?? Language.SYSTEM ?? Language.DEFAULT; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
willst du das dann nicht auch auslagern? const userLangKey,
const demoLangKey
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Was genau meinst du mit auslagern? Wohin auslagern?
AuthenticateResponseprovides no language.detoendein Karma tests.Address language issue in #3411