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

Format BigDecimal values in hidden fields and drop-down options according to locale #235

Closed
wants to merge 1 commit into from

Conversation

jenshp
Copy link
Member

@jenshp jenshp commented Jul 17, 2017

When received and processed in a transition, these field's values will be treated as if in user's locale, so this change is required for a DB roundtrip through the browser to work smoothly. This solves the server part of #49
It affects the value of hidden fields and options, as well as the current value in drop-down fields. The developer should, however, be locale aware if using the expandable key and text attributes of tags like entity-option and list-option, adding "${ec.l10n.format(keyName, null)} instead of just "${keyName}" for the code to work using a locale with comma as decimal separator. Making this aspect transparent to the developer would require to modify the behavior of ResourceFacade.expand(...) to make the transformation into the locale-aware format, which could affect other uses of that method.

When received at transition, it will be treated as if in user's locale and transformed back so this is required for a DB roundtrip through the browser to work smoothly.
@jonesde
Copy link
Member

jonesde commented Nov 19, 2017

Honestly, I'm never going to merge this. It is a partial solution to a much bigger issue that needs a lot more effort for testing different scenarios (which need to be defined) and overall handling of numbers with comma as the decimal separator. Even supporting this for only BigDecimal is inadequate, what about Double and other decimal number objects? What about pretty much every other instance of call to the toPlainString() method? There are lots of them about.

@jonesde jonesde closed this Nov 19, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants