search

streetcomplete / StreetComplete

Easy to use OpenStreetMap editor for Android
https://streetcomplete.app
GNU General Public License v3.0
3.83k stars 348 forks source link

Decimal separator wrong in German UI #4084

Closed hmartink closed 2 years ago

hmartink commented 2 years ago

I'm not sure if it is in the codebase or a translation issue, so please reassign.

Today I came by a bridge and SC asked me about the maximum height for vehicles below the bridge. And this bridge did have sign. So I tried entering the height. I could add the value before the decimal separator, but could not enter the "," which is the decimal separator in German. I fiddled with the GUI, because sometime the value is in a seperate field. However, I finally realized that the "." is used as decimal separator. I could then enter also the value after the separator.

It appears that the decimal separator is not localized to German.

To reproduce: Locate a bridge with a quest asking for the maximum height for vehicles below. Use (or probably switch to) the German UI. Try to enter the value as x,y and notice that the "," is not accepted; however, if x.y is entered, it works.

Android version: 10 SC version: v44.0

mnalis commented 2 years ago

I can confirm this behaviour. It never bothered me personally as I instinctively pressed . which was anyway at more expected place at numeric keyboard than , (which I wasn't even aware is actually shown in numeric keyboard, and was quite shocked to see it there when I went to check right now :smile:).

It might be simplest to accept them both and treat them as equivalent.

(Note: Croatian also uses , as official decimal separator; but as someone who has been exposed to computer programming, it would annoy me to no end if I had to actually use anything but . as decimal separator. Also, I use different language in SC compared to Android OS language, which is again different from keyboard settings - because reasons).

Helium314 commented 2 years ago

This behavior was introduced (somewhat) recently. In 38.2 (randomly chosen old version) both separators work.

westnordost commented 2 years ago

What is you Android OS system language?

hmartink commented 2 years ago

Deutsch(Deutschland)

westnordost commented 2 years ago

I can reproduce insofar as the software keyboard greyed out the "," button (Samsung keyboard). Which keyboard do you use?

westnordost commented 2 years ago

So, there are three possible different causes for the , not being accepted on a German locale that I know of:

  1. there was a bug in Android that had been fixed in Android 8 where the system simply didn't recognize that in other locales, other decimal separator than the . are used
  2. the Samsung keyboard (the Internet explorer of keyboards) apparently still has this issue, see for example this stackoverflow-post (funnily enough the , is shown, but greyed out, 🤣 ) or this (comment on this) issue on flutter
  3. if you changed the language within the app only to a language that uses , as a decimal separator but the system language is on English, then the keyboard will likely also show/use only the . as a separator because that app (the keyboard) is still in English.

At least for 1 and 2, there are known workarounds, though they are ugly. One could specify all "decimal number" input as "phone number inputs" (because apparently a , can be part of a phone number) to make the Samsung keyboard allow commas, but then it also displays all kind of other characters on the keyboard, like #. Plus, this breaks it for all other keyboards who interpret the "decimal number" input type correctly, so there needs to be some hacky way to somehow detect if a Samsung keyboard is used and only apply that then programmatically.

All in all, since the impact of this bug is just that you have to input . instead of , on German (etc) locales and it is not a bug in this project anyway, I prefer not doing anything about it. Not worth the effort and hackery involved.

Helium314 commented 2 years ago

This behavior was introduced (somewhat) recently. In 38.2 (randomly chosen old version) both separators work.

I tested a bit more: SC 41.0: both , and . work SC 42.0: only . works nothing was done between the tests, except upgrading SC Tested with English and German phone locale, with SC set to default or to German.

westnordost commented 2 years ago

Well, maybe there is a fourth possibility how this issue can be triggered. Maybe I removed some workaround code in between SC 41 and 42. You are free to git bisect and take a look, if the workaround has a minimal impact on the codebase, it could be reintroduced.

The input texts in question all have android:inputType="numberDecimal" correctly set. Using a Samsung device myself and using the standard Samsung keyboard, I was able to reproduce this issue and tried different easy possible workarounds, such as for example adding android:digits="0123456789,." but that didn't do the trick.

Helium314 commented 2 years ago

Won't have time for this in the next few days, but my first guess would be https://github.com/streetcomplete/StreetComplete/commit/e2a1e6fa09d9730350d88833c27cdd4709d9c4b6

FloEdelmann commented 2 years ago

In v41 and before, there was the EditText.allowOnlyNumbers() extension function, which was a one-line fix for exactly this issue: https://github.com/streetcomplete/StreetComplete/blob/v41.2/app/src/main/java/de/westnordost/streetcomplete/ktx/EditText.kt

It apparently didn't make it to the new LengthInputViewController: https://github.com/streetcomplete/StreetComplete/blob/v42.0/app/src/main/java/de/westnordost/streetcomplete/view/controller/LengthInputViewController.kt

westnordost commented 2 years ago

Good find... I am going to try this out...

hmartink commented 2 years ago

This is a Nokia 3.1, very close to stock android. No special keyboard.

westnordost commented 2 years ago

And which keyboard is it then?

westnordost commented 2 years ago

Adding editText.keyListener = DigitsKeyListener.getInstance("0123456789,.") does not solve issue 2. The , is still greyed out. Whether it would solve issue 1, I don't know, I don't have a device that old.

Helium314 commented 2 years ago

Adding editText.keyListener = DigitsKeyListener.getInstance("0123456789,.") does not solve issue 2. The , is still greyed out. Whether it would solve issue 1, I don't know, I don't have a device that old.

It does solve the issue for me (metersInput.keyListener = DigitsKeyListener.getInstance("0123456789,.") in LengthInputViewController). Is there any reason to not add this? Looks like it could improve the situation at least for some people.

westnordost commented 2 years ago

what about android:digits="0123456789,." for every EditText with android:inputType="numberDecimal". Does this also work for you?

Helium314 commented 2 years ago

I only tested for the quest_maxheight layout, and here it works.

westnordost commented 2 years ago

Ok, then I'd be fine to add this to every EditText that has numberDecimal in the project. Will you do this?

westnordost commented 2 years ago

nevermind, i did that now