Skip to content
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

Numeric preset fields should localize numbers #3615

Closed
1ec5 opened this issue Nov 24, 2016 · 14 comments · Fixed by #8769
Closed

Numeric preset fields should localize numbers #3615

1ec5 opened this issue Nov 24, 2016 · 14 comments · Fixed by #8769
Assignees
Labels
localization Adapting iD across languages, regions, and cultures

Comments

@1ec5
Copy link
Collaborator

1ec5 commented Nov 24, 2016

The “Lanes” and “Max Height” (maxheight=*) fields accept numbers as input, but the placeholder string for “Max Height” highlights the fact that they only accept English-style numbers (European digits and a period as the decimal point). Numeric fields like these should convert between the language’s local number format and the format expected in OSM tags. (Note that floating-point fields like “Max Height” also tend to accept units: #3614.)

Related: #3597

@bhousel bhousel added the localization Adapting iD across languages, regions, and cultures label Nov 28, 2016
@1ec5
Copy link
Collaborator Author

1ec5 commented Oct 5, 2017

Per #3597 (comment), consider the Number module of Globalize.js for this task.

@1ec5
Copy link
Collaborator Author

1ec5 commented Dec 6, 2017

Number.toLocaleString() should be sufficient for converting these values back and forth between the user’s locale and en-US in modern browsers.

@1ec5 1ec5 added the new contributor opportunity Best for first-time contributors without prior experience. You won’t be assigned; PRs welcome! label Dec 6, 2017
@1ec5
Copy link
Collaborator Author

1ec5 commented Jan 15, 2018

Number.toLocaleString() should be sufficient for converting these values back and forth between the user’s locale and en-US in modern browsers.

Correction: Number.toLocaleString() only formats a number for display, but there isn’t a corresponding method for parsing localized floats built into the language. Globalize.js’s Globalize.parseFloat() is one option, but there are others as well, such as parse-decimal-number.

@Xavier-J-Ortiz
Copy link
Contributor

Xavier-J-Ortiz commented Mar 22, 2018

@1ec5 @bhousel

I'd like to take this one on as it would make me dive a bit more into the codebase of iD.

Where, or which file would I need to modify in order to effect these changes needed for this issue? I'll continue digging for the time being, and will updated in this thread if I get my bearings and realize where this is to be done.

But as of right now, I wouldn't know where I would need to create these changes in order to do the parsing and pass it along to the page.

Thanks!

@Xavier-J-Ortiz
Copy link
Contributor

I suspect that I should work on this file data/update_locales.js and add a function that will parse the decimals correctly in there.

Will work on this this evening.

@slhh
Copy link
Contributor

slhh commented Mar 23, 2018

Accepting localized numbers is useful, but iD should also accept the formally defined format of the tag value. Due to a restricted range of reasonable values, the resulting value seems to be unambiguous.

@bhousel
Copy link
Member

bhousel commented Mar 23, 2018

I suspect that I should work on this file data/update_locales.js and add a function that will parse the decimals correctly in there.

I don't think it would go there - that script is to pull down translation packs from Transifex.

You'll probably really need to do the conversion to/from local formatting in modules/ui/fields/* (all of the fields? I dont know.)

@1ec5 can you provide some more guidance? I'm not sure this should be labelled with "good first issue". It sounds like something with a lot of gotchas involved and requiring deeper knowledge of iD.

@manfredbrandl
Copy link
Contributor

@1ec5 Please remove "good first issue". Even @bhousel writes "I dont know".

@1ec5 1ec5 removed the new contributor opportunity Best for first-time contributors without prior experience. You won’t be assigned; PRs welcome! label Aug 3, 2018
@1ec5
Copy link
Collaborator Author

1ec5 commented Aug 3, 2018

Sorry, I forgot to remove that after posting #3615 (comment). For what it’s worth, #4964 would also introduce a dependency on an internationalization library.

@1ec5
Copy link
Collaborator Author

1ec5 commented Oct 25, 2021

Correction: Number.toLocaleString() only formats a number for display, but there isn’t a corresponding method for parsing localized floats built into the language. Globalize.js’s Globalize.parseFloat() is one option, but there are others as well, such as parse-decimal-number.

There’s probably an NPM package for this by now, but this compact solution is right in line with how native platforms have been exposing number formatting for eons, so it’s good enough for the numeric field’s needs.

@1ec5 1ec5 self-assigned this Oct 26, 2021
@1ec5
Copy link
Collaborator Author

1ec5 commented Oct 26, 2021

Accepting localized numbers is useful, but iD should also accept the formally defined format of the tag value. Due to a restricted range of reasonable values, the resulting value seems to be unambiguous.

I don’t think this is necessarily true. For example, in a locale like Spanish that uses the format #.###,#, an elevation of “1.609” meters and an elevation of 1.2 meters would both be reasonable. It would be straightforward to accept both formats. However, in case of ambiguity, I think it would be better for the numeric field to prefer the localized interpretation, since it caters to laypeople who already enjoy the localized interpretation outside of iD. Those familiar with OSM tagging conventions can always enter the POSIX format verbatim in the raw tag editor.

@1ec5
Copy link
Collaborator Author

1ec5 commented Nov 17, 2021

#8769 implements localized number formatting and parsing in numeric fields. However, there are a couple more things to take care of as tail work:

  • For locales that use non-European digits, translated placeholders defined in id-tagging-schema should be updated to use the localized number format instead of an English/POSIX number format. These locales include:
    • Arabic (this might be something to revisit whenever regional Arabic locales are introduced)
    • Bengali (apparently this was prematurely formatted, but now it’s correct)
    • Central Kurdish (prematurely formatted)
    • Nepali (prematurely formatted)
  • Add measurement field type ideditor/schema-builder#15 so that all fields that accept numbers can consistently format them, even if units come attached

@1ec5
Copy link
Collaborator Author

1ec5 commented May 26, 2023

Arabic (this might be something to revisit whenever regional Arabic locales are introduced)

There are quite a few placeholders to update, but I updated this placeholder proactively because otherwise it would mislead users into entering the wrong decimal separator and thus the wrong raw tag.

@1ec5
Copy link
Collaborator Author

1ec5 commented May 26, 2023

Actually, never mind: this is a text field so that it can incorporate the unusual syntax for miles.

https://github.com/openstreetmap/id-tagging-schema/blob/c0c94c0a6ddf58cd92a95cfa399bade027b28690/data/fields/railway/position.json#L3-L4

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
localization Adapting iD across languages, regions, and cultures
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants