Posted by Bobík on February 22, 2012 at 4:52pm
12 followers
| Project: | Drupal Commerce |
| Version: | 7.x-1.x-dev |
| Component: | Price |
| Category: | feature request |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | needs work |
| Issue tags: | Usability |
Issue Summary
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.
Comments
#1
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.).
#2
It's quite problem form me, because I have to import products by Feeds from file with prices with commas.
#3
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.
#4
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.
#5
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.
#6
#4: comma-as-decimal-1450736-4.patch queued for re-testing.
#7
Despite the automated code review, my comments above still stand.
#8
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...
#9
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.
#10
Of course, you're right... I'll try again tomorrow.
#11
It seems better to me with this one.
I've removed the
ifthat 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.
#12
The last submitted patch, commerce-decimal_separator_1450736_11.patch, failed testing.
#13
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?
#14
Patch #11works now.
I guess the test fails because of the price format settings in testCommerceOrderUIUpdateLineItems().
#15
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.
#16
Change title so it reflects the current effort on the patch.