Convert jQuery data attribute to number #3899
Conversation
|
@spaghetticode nice finding. Did you have the chance to understand if this has different implications with other locales? |
|
@kennyadsl I did not. I'm assuming everyone is converting to 2 decimals as of today, if that's what concerns you about locales. It would not be that hard to change it to use the digits that are found in the data attribute numbers though. Also, I was able to find the reason for this strange behavior, see this line: https://github.com/jquery/jquery/blob/a32cf6324f8f2190e66a687e94be9687ebf840b7/src/data.js#L34 "12.01" === +"12.01" + ""
> true
"12.00" === +"12.00" + ""
> false |
|
🤦 Thanks for the fix @spaghetticode! As for the precision, I'm pretty sure there are currencies that use a different numbers of decimals (e.g., the Japanese Yen comes to mind — as far as I know, they use no decimals at all). Perhaps we can get this information from the Money gem? |
JQuery numeric string data attributes are not casted to numbers
consistently. There are instances when they are kept as string,
this behavior seems to be related to the presence of insignificant
zeros in the original string:
$('<div data-price="12.00"></div>').data('price')
> "12.00"
$('<div data-price="12.10"></div>').data('price')
> "12.10"
but:
$('<div data-price="12.1"></div>').data('price')
> 12.1
So, we need to enforce the typecast to number when summing those
attributes in the function `totalSelectedReturnItemAmount`.
After the sum, the number needs to be converted to a properly
localized amount. For this we leverage the existing function
`formatNumber` from the library `accounting.js`, see:
https://github.com/solidusio/solidus/blob/master/backend/vendor/assets/javascripts/solidus_admin/accounting.js
spaghetticode
force-pushed
the
spaghetticode/fix-total-amount
branch
2 times, most recently
from
a486ed6 to
1f6b44e
Compare
|
@aldesantis thanks for the input, it now works properly also for those countries that don't have decimal amounts, and the number is properly localized too. Unfortunately, we don't have access to the currency here, and adding it would make things a bit too complicated, still can leverage some locale information such as the currency delimiter and separator that IMHO will suffice. |
spaghetticode
added
the
changelog:solidus_backend
|
Failing spec is already addressed in master |
JQuery numeric string data attributes are not casted to numbers consistently.
There are instances when they are kept as string, this behavior seems to be related to the presence of insignificant
zeros in the original string:
but:
So, we need to enforce the typecast when summing those attributes in the function
totalSelectedReturnItemAmount.After the sum, since we want to show a 2 decimal number, we need to convert the total amount with
toFixed(2).Checklist: