In many Europe countries, we often use comma "," as decimal delimiter in numbers. Commerce use periods ".", commas does not like for pricing.

Commerce should support option to use commas.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

rszrama’s picture

Version: 7.x-1.2 » 7.x-1.x-dev
Status: Active » Postponed

We support commas for currency formatting, but for data entry right now we just support decimals. I'm not sure it's worth the pain that will introduce, but we can leave it open as a postponed feature request. We probably won't be putting this support into 1.x. The big challenge is to ensure that this is possible on every form where a price might be entered, not just product forms (such as Rules configuration forms, Views exposed filters, etc.).

Honza Pobořil’s picture

It's quite problem form me, because I have to import products by Feeds from file with prices with commas.

rszrama’s picture

Ahh, well, even if we allowed comma entry on forms, that wouldn't automatically translate into entry for Feeds. The best thing to do for that would be to find a module like Feeds Tamper (or implement a custom hook) to convert the comma to a decimal prior to entry. Storage in the database will always be decimal based.

googletorp’s picture

Title: Support for comma as decimal delimiter » Support for comma as decimal delimiter for field input
Status: Postponed » Needs review
FileSize
835 bytes

I don't know if I'm hijacking this issue - anyways I would like to put some focus on this one.

I know that we it would be hard to control every UI that in some way or another could be used for price input like

* Rules
* Views
* Exposed forms

and that consistency is a good thing. I would however like to advocate for the solution that inputs on price fields also accepts comma as decimal separator. It will help a lot of people (12 currencies uses , as decimal separator so that's a lot of users (about 1 billion people)). The price fields are also very different from Rules and Views UI since it's mostly used by consumers - create products or enter a price via line item price fields.

I really think it's a huge usability win to allow users to user their native decimal separator for this many people, and it's highly unlikely that they will experience any inconsistency - unless they have a high tech level (are site builders) and could be expected to understand why such a difference could occur.

I would really urge you to think about this. Imagine the usability failure you would have to deal with in the states if you expected every one to use comma as the decimal separator and simply replied with enter a numeric amount if a (.) was use as decimal separator. We can do better than this and we should.

rszrama’s picture

Status: Needs review » Needs work

Just to recap a bit of discussion in IRC - I'm not opposed to this idea, but I am a little concerned about it. How helpful is it to resolve the issue for price field input but not have corresponding fixes for Rules / Views UI input elements? Or, could it cause problems on multi-currency sites where the same price field might use either a decimal point or a comma for the decimal separator?

If we're going to move forward with this, I do think we at least need the str_tr() to be currency specific. In other words, instead of just blindly changing commas to decimal points, we should load the currency and swap out the currency's decimal separator for a decimal point. While we're at it, we can also swap out the currency's thousands separator for an empty string. We would also want to update the price field form so that default values use the currency specific formatting, though this could result in currency switching errors.

Anonymous’s picture

Status: Needs work » Needs review

#4: comma-as-decimal-1450736-4.patch queued for re-testing.

rszrama’s picture

Status: Needs review » Needs work

Despite the automated code review, my comments above still stand.

Simon Georges’s picture

Status: Needs work » Needs review
Issue tags: +Usability
FileSize
1.59 KB

Hi,

I modified the patch following rszarma's remark in #5, which is perfectly justified.
I've also tried to alter the form widget to have the price displayed WITH the decimal separator as well, so the user doesn't need to think (http://www.sensible.com/dmmt.html ;-)), I hope I didn't break anything...

checker’s picture

I tested your patch (#8). I can type in the defined decimal separator instead of . but the input form element still displays the price with a .

It never goes into the if clause.

Simon Georges’s picture

Of course, you're right... I'll try again tomorrow.

Simon Georges’s picture

It seems better to me with this one.
I've removed the if that I suppose was purely there for performance reason, I'm wondering if that'll effect other things. Please review.

This patch is part of the #1day1patch initiative.

Status: Needs review » Needs work

The last submitted patch, commerce-decimal_separator_1450736_11.patch, failed testing.

Simon Georges’s picture

Hum... I was almost there, only 1 fail ;-)
I don't really understand why it fails, though... I think I'll need some CommerceGuys input on this.
In the meantime, can somebody test the patch and give some feedback about potential issues on other parts of Commerce?

checker’s picture

Patch #11works now.
I guess the test fails because of the price format settings in testCommerceOrderUIUpdateLineItems().

Simon Georges’s picture

Ok, I get it... In the order UI, there is another need for Price alteration. But it implies modifying more the logic of the line items, so it'll take me some more time.

Simon Georges’s picture

Title: Support for comma as decimal delimiter for field input » Support for currency decimal separator as decimal delimiter for field input

Change title so it reflects the current effort on the patch.

guy_schneerson’s picture

This may be a bit distracting as it looks like you are cumming up with a pragmatic solution, however I think it is worth mentioning, that this issue is not unique to the currency decimal separator.
I have a similar issue #1787182: comma instead of dot on the Commerce decimal quantities module and iAugur found one other potentially related issue on the webform issue Q #1813200: Validation of field type number fails for decimal separator
It would be great to have a consolidated approach to handle localized of numbers.
I have been looking at using the https://drupal.org/project/format_number module, this provides a form element that works great with Commerce decimal quantities: providing definition for the decimal symbol, default formatting, right alignment of numbers and even client side validation.
I am not saying we must use this module especially as it is on Dev and no commits in the last year but I agree with @googletorp that this is impotent and I think that like dates Drupal should provide a common approach for both editing and the display of numbers.

Simon Georges’s picture

It seemed to be pragmatic, but in the end, it totally broke the order admin interface, so I'm a little blocked right now. I think everything you can throw at it is good, especially if several area of Commerce are impacted.

blainelang’s picture

I needed a solution for a client that was using the decimal number field on a form, and their users were pasting values into the form that contained comma's as separators for the 1,000's.

I decided the quickest solution was to use jQuery and on tabbing out the field, remove the comma's.

The following javascript will automatically remove all comma's on any decimal type input field.

        $('.field-type-number-decimal input[type=text]').blur(function() {
          $(this).val($(this).val().replace(/,/g,''));s
        });
geek-merlin’s picture

Issue summary: View changes
Status: Needs work » Needs review
FileSize
3.93 KB

OK let's get some love again into this.
* with D8 above the horizon i'm fine with the pragmatic approach
* the code itself looks totally fine
* the if-clause mentioned in the last commit is gone anyway in dev so no fuss about that

So i did a re-roll of #11 and gave a shot fixing the tests.

This is a PITA in some commerce use cases so let's go fix that.

geek-merlin’s picture

Cool! Lights are green.

Any manual tester that can test this and set RTBC?

bojanz’s picture

I am extremely nervous about such a change, and feel it probably makes more sense to leave this to per-site hacks (or educating clients on how prices must be entered).

Commerce 2.x solves this properly via https://github.com/commerceguys/intl, but the approach is not backportable because it introduces the concept of locale-based amount formatting and parsing.

geek-merlin’s picture

Priority: Normal » Major

@bojanz: it's not only clients, but there are plenty of sites where prices are displayed and/or entered by customers (the calassic donation site example) so i still claim for EU this is a major PITA.

It's also a PITA selling commerce to customers if some people tell them "Commerce won't accept your decimal comma, it's that US company that doesn't care for EU." (sic.)

I wonder what are your concerns? Would it calm your nervosity if we add a configuration variable for this behavior so nothing will change if this is OFF?

bojanz’s picture

Well, we're a French company too ;)

Yes, making it off by default would make me happy.
My concern is that by changing how we parse prices we alter the behavior of existing sites.
A dot yesterday is a comma today. Prices are interpreted differently.
It has the potential for trouble.

geek-merlin’s picture

Status: Needs review » Needs work

OK i support this.
People using CI shouldn't have to change gazillions of tests.

TODO: make behavior (globally) optional, defaulting to OFF.

tfranz’s picture

I applied the patch in #20 and it seems to work for me – thank you!

Anybody’s picture

Same problem here, I can confirm that the priority is still major. We're a German company und our customers are very unhappy with the current solution. The patch works for me. What can we do here, @commerce maintainers?

ferriol’s picture

Yes, this is important for our project. I hope we can define the decimal separator, or it can be defined for the language or currency. Is this considered in commerce 2.x ?

lomasr’s picture

FileSize
107.45 KB

Tested the patch #20 . it worked for me . Please see the 'Price' column in screenshot.

lomasr’s picture

Status: Needs work » Needs review
geek-merlin’s picture

@anybody #27
> The patch works for me. What can we do here, @commerce maintainers?

read #24/#25, do it or get one that does it. ;-)