Use browser locale for floating point numbers formatted String.format() in UI #857

a-sayyed · 2019-06-21T08:56:09Z

No description provided.

a-sayyed · 2019-06-28T11:49:08Z

hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/rollout/rollout/DefineGroupsLayout.java

-            targetPercentage.setValue(String.format(getCurrentLocale(), "%.2f", group.getTargetPercentage()));
+            // necessary due the format must contain a dot and other locals might
+            // use a comma
+            targetPercentage.setValue(String.format(Locale.ENGLISH, "%.2f", group.getTargetPercentage()));


stefbehl · 2019-07-08T11:08:35Z

hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/utils/HawkbitCommonUtil.java

-        final UI currentUI = UI.getCurrent();
-        return currentUI == null ? Locale.getDefault() : currentUI.getLocale();
+        final Page page = Page.getCurrent();
+        return page == null ? Locale.getDefault() : page.getWebBrowser().getLocale();


pls double-check if defaultLocale config property is still accepted...

if prop is set, it takes precedence... otherwise browser locale, otherwise system locale

hawkbit-bot · 2019-07-09T08:12:59Z

hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/UiProperties.java


        /**
         * Default localization
         */
-        private String defaultLocal = "en";
+        private String defaultLocal = null;


Remove this initialization to "null", the compiler will do that for you.

hawkbit-bot · 2019-07-09T08:13:00Z

hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/UiProperties.java

-         * List of available localizations
-         */
-        private List<String> availableLocals = Collections.singletonList("en");
+        public Localization(final List<Resource> locales) {


Document this public constructor by adding an explicit description.

hawkbit-bot · 2019-07-09T08:13:03Z

SonarQube analysis reported 2 issues

2 minor

Watch the comments in this conversation to review them.

a-sayyed · 2019-07-17T11:14:58Z

hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/UiProperties.java

+     *          localization files (message.properties) as resources
+     */
+    public UiProperties(@Value("classpath*:messages_*.properties") final Resource[] localesResources) {
+        this.localization = new Localization(Arrays.asList(localesResources));


check with the new spring vaadin integration if there is another solution

a-sayyed · 2019-07-22T11:06:30Z

hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/utils/HawkbitCommonUtil.java

@@ -596,7 +596,7 @@ public static Locale getLocaleToBeUsed(final UiProperties.Localization localizat
     */
    public static void initLocalization(final UI ui, final Localization localizationProperties,
            final VaadinMessageSource i18n) {
-        ui.setLocale(HawkbitCommonUtil.getLocaleToBeUsed(localizationProperties, ui.getSession().getLocale()));
+        ui.setLocale(getLocaleToBeUsed(localizationProperties, Page.getCurrent().getWebBrowser().getLocale()));


do we really need to get the current page to get the desired locale?

You can use ui.getPage() instead of Page.getCurrent() here, not to search for the UI one more time.

hawkbit-ui/src/test/java/org/eclipse/hawkbit/ui/utils/HawkbitCommonUtilTest.java

StefanKlt · 2019-08-05T13:53:00Z

hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/utils/HawkbitCommonUtil.java

-        return new Locale(localizationProperties.getDefaultLocal());
+        return localizationProperties.getDefaultLocal() == null
+                ? desiredLocale
+                : localizationProperties.getDefaultLocal();


Doesn't this change the logic, because before the browser-locale had the highest priority.
Would it be possible so set fallbackToSystemLocale=false and completely rely on spring's MessageSource-management. Then that the code and properties used to determine the locale would become obsolete?

StefanKlt · 2019-08-05T13:58:06Z

hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/utils/HawkbitCommonUtil.java

@@ -596,7 +595,7 @@ public static Locale getLocaleToBeUsed(final UiProperties.Localization localizat
     */
    public static void initLocalization(final UI ui, final Localization localizationProperties,
            final VaadinMessageSource i18n) {
-        ui.setLocale(HawkbitCommonUtil.getLocaleToBeUsed(localizationProperties, ui.getSession().getLocale()));
+        ui.setLocale(getLocaleToBeUsed(localizationProperties, ui.getPage().getWebBrowser().getLocale()));


I think ui.getSession().getLocale() was the right choice here. According to the doc this returns "the preferred locale of the user (...). In most cases it is read from the browser"

StefanKlt · 2019-08-28T07:50:24Z

hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/UiProperties.java

@@ -63,26 +63,26 @@ public void setFixedTimeZone(final String fixedTimeZone) {
        /**
         * Default localization
         */
-        private String defaultLocal = "en";
+        private Locale defaultLocal;


the default used to be English, is it intended that it is now null?

laverman · 2019-08-28T08:03:42Z

@a-sayyed please check the commit history, if all commits have a 'signed-off' to it

Signed-off-by: Ahmed Sayed <ahmed.sayed@bosch-si.com>

a-sayyed commented Jun 28, 2019

View reviewed changes

schabdo added this to the 0.3.0M5 milestone Jul 1, 2019

stefbehl reviewed Jul 8, 2019

View reviewed changes

hawkbit-bot reviewed Jul 9, 2019

View reviewed changes

a-sayyed force-pushed the feature/add_locale_to_String_format branch from 0c569a0 to 033dcf0 Compare July 9, 2019 11:28

a-sayyed commented Jul 17, 2019

View reviewed changes

a-sayyed commented Jul 22, 2019

View reviewed changes

hawkbit-ui/src/test/java/org/eclipse/hawkbit/ui/utils/HawkbitCommonUtilTest.java Show resolved Hide resolved

schabdo modified the milestones: 0.3.0M5, 0.3.0M6 Jul 29, 2019

StefanKlt reviewed Aug 5, 2019

View reviewed changes

StefanKlt reviewed Aug 28, 2019

View reviewed changes

Added localization to String.format

7d59344

Signed-off-by: Ahmed Sayed <ahmed.sayed@bosch-si.com>

a-sayyed force-pushed the feature/add_locale_to_String_format branch from d917985 to 7d59344 Compare August 28, 2019 08:32

a-sayyed changed the title ~~added Locale.ENGLISH to String.format() for numbers with floating point~~ use browser Locale in String.format() in UI for numbers with floating point Aug 30, 2019

StefanKlt approved these changes Aug 30, 2019

View reviewed changes

stefbehl merged commit a85dafa into eclipse:master Sep 2, 2019

stefbehl deleted the feature/add_locale_to_String_format branch September 2, 2019 06:45

schabdo changed the title ~~use browser Locale in String.format() in UI for numbers with floating point~~ Use browser locale for floating point numbers formatted String.format() in UI Jan 16, 2020

schabdo added the enhancement label Jan 16, 2020

AmmarBikic pushed a commit to bosch-io/hawkbit that referenced this pull request Oct 2, 2020

Added localization to String.format (eclipse#857)

874014f

Signed-off-by: Ahmed Sayed <ahmed.sayed@bosch-si.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use browser locale for floating point numbers formatted String.format() in UI #857

Use browser locale for floating point numbers formatted String.format() in UI #857

a-sayyed commented Jun 21, 2019

a-sayyed Jun 28, 2019

stefbehl Jul 8, 2019

stefbehl Jul 8, 2019

hawkbit-bot Jul 9, 2019

hawkbit-bot Jul 9, 2019

hawkbit-bot commented Jul 9, 2019

a-sayyed Jul 17, 2019

a-sayyed Jul 22, 2019

bogdan-bondar Jul 23, 2019

StefanKlt Aug 5, 2019

StefanKlt Aug 5, 2019

StefanKlt Aug 28, 2019

laverman commented Aug 28, 2019

Use browser locale for floating point numbers formatted String.format() in UI #857

Use browser locale for floating point numbers formatted String.format() in UI #857

Conversation

a-sayyed commented Jun 21, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hawkbit-bot commented Jul 9, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

laverman commented Aug 28, 2019