-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Localize numbers in numeric fields #8769
Conversation
9c3f92e
to
a733225
Compare
@@ -391,5 +391,35 @@ export function coreLocalizer() { | |||
return code; // if not found, use the code | |||
}; | |||
|
|||
localizer.floatFormatter = (locale) => { | |||
if (!('Intl' in window && 'NumberFormat' in Intl && | |||
'formatToParts' in Intl.NumberFormat.prototype)) { |
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.
Displaying and accepting localized numbers is limited to browsers that support Intl.NumberFormat.prototype.formatToParts
.
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.
Thanks for this PR! It looks great! 😍
I've not yet looked in too much detail into roadspeed.js
and roadheight.js
, but please find a few comments inline below.
cd22a74
to
17d8f50
Compare
cfe1422
to
01a4e58
Compare
I rebased this branch atop the latest develop branch. The conflicts were a bit tricky this time due to f573c37, which I think obsoleted some of the code review feedback above, but I went through the same cases as before and the functionality still seems to work correctly. |
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.
Let's get this merged!
The only small concern I currently still have with this one is what happens when users enter numbers in the "raw" (English) format, and the field is expecting localized input. This might occur regularly because mappers are likely to assume that numbers need to be entered in the same was as they are encoded in OSM tags.
For example, when entering 2.5
(for example in a road width field) when on a locale which uses ,
as the decimal delimiter (such as vi
or de
for example), this results in the field value to be set to 25
, which is not ideal.
I think we need to somehow special-case the situation when a "raw" decimal is entered.
There's also this bug when using the +/- buttons:
This suggestion came up previously, but unfortunately I think it would be difficult to have it both ways, unless we assume the user won’t use their own language’s digit-grouping character for some reason: #3615 (comment). We could require the digit-grouping character to be used only to group three digits at a time, but we’d have to be careful not to disadvantage South Asian users who use crore and lakh – even in English. #8238 introduced a monospaced font for raw OSM tags, so I suspect a new user would intuit that this field would accept localized numbers due to its variable-width font. Granted, it will require some getting used to for longtime users, but that problem will only grow over time. Do you think we should “prime” longstanding users by first styling these fields in monospaced font for a release before landing this feature?
Haha wow! I tested this case previously, but I guess it crept back in at some point. |
01a4e58
to
6419def
Compare
Co-authored-by: Martin Raifer <[email protected]>
e884929
to
4e11297
Compare
The float formatter function now takes a number of fraction digits to return.
This comment was marked as resolved.
This comment was marked as resolved.
@tyrasd, I have misgivings about sniffing the format from the user’s input – differing behavior based on opaque rules is a recipe for user confusion. Your example is the easy case: no language uses a digit grouping symbol to separate the tens from the ones. But an equally realistic example of “2.500” could go either way. In my opinion, the fields should be biased towards the user’s language, since the user can always enter raw values in the raw tag editor below. Anyways, I came up with this fancy regular expression to detect likely “raw” numbers. It catches raw numbers like “1.23”, “0.123”, “1.2345”, and “1234.567” but not formatted numbers like “1.234” and “123.456”. It would be trivial to short-circuit the float parser when the input matches this regular expression: /^-?(0\.\d*|\d*\.\d{0,2}(\d{4,})?|\d{4,}\.\d{3})$/ Unfortunately, this special case actually turns out to be counterproductive. Fields commit their values continuously on each keystroke rather than waiting for the user to finish typing, so a numeric field reformats the number on each keystroke. In the cases where there are two correct answers, we would instead choose a third, incorrect answer. For example, if a German speaker intends to enter “1.234” to tag Continuously validating a field causes other problems too, such as preventing the user from prepending more than one character at a time: #9233. But eliminating that behavior might resurrect a previously persistent issue that the field would fail to commit after the user finishes typing and navigates to another part of the UI. /ref #1361 |
b20b1a3
to
249771d
Compare
when a user enters a decimal number using the "international"/English/OSM-raw-data formatting (e.g. as in `0.5`), it is parsed using the basic, non-localized, number parser. In such cases, the content of the input field should not be overwritten with the localized formatting, as that would cause unexpected glitches and make editing harder (e.g. when thousands-grouping characters seemingly "magically" disappear or appear while typing). see #8769 ff.
FYI: I was still concerned that users might enter number in the "raw" format, which I addressed in the following way:
I think this will work quite well in almost all cases of numeric osm tag fields, as they are typically large integers (e.g. |
I’m very happy for this fix, though this PR wouldn’t have affected the
I considered making this change too, but in the past, the downside was that some browsers would fail to fire the blur event in some cases, unexpectedly aborting the change: #1361. Are we confident that this is no longer a risk?
For a peak’s height, no, but I’m also concerned about users expecting to enter raw tag values in these fields. Though the changes you made seem to work, I think we can reinforce the expected user behavior with a small style change: #9677. |
As far as I can tell, what I changed should not interfere with this: The
Yeah, valid concern. A real example of a potentially ambiguous value would be something like https://www.openstreetmap.org/node/9591246259 (if the number field were to support entering numbers with a measurement unit in the future). I'd say, let's definitely keep this potential issue in mind and reevaluate when such fields are introduced.
👍 Footnotes
|
Fields of type
number
,roadheight
, androadspeed
now interpret their input as a number or semicolon-delimited list of numbers according to the UI language. If the input value can’t be parsed as a localized number, the field falls back to the raw value verbatim. Regardless of how the field displays its value, it always outputs tag values containing numbers in English/POSIX format for consistency with OSM tagging conventions.Width field in English
Width field in Vietnamese
Width field in Arabic
Max Height in Vietnamese
roadspeed
-type fields contain combo boxes with suggested values, which are now localized:Speed Limit field in Arabic
When using a locale that uses non-European numerals, such as Arabic, you can enter the value using European digits. The numerals are converted into localized digits on the fly as you type.
Ideally, this PR would be nothing more than changing the
<input>
tag’stype
attribute tonumber
instead oftext
. (In fact, there’s anumber
input box in the Issues panel and it works great.)iD/modules/ui/fields/input.js
Line 98 in 810ce60
However, browsers such as Firefox are a bit too strict about what can go into a
number
input box. If a feature is taggedmaxheight=below_default
ordirection=0;180
, the built-innumber
field would be blank. So instead, I’ve attempted to emulate the built-in behavior.For languages that use non-English number formats, translated placeholders defined in id-tagging-schema should be updated to use the localized number format instead of an English/POSIX number format.
Fixes #3615.