decimalFormatter does not always work with decimalSeparator=',' and thousandSeparator='.'

[fheck]

I'm submitting a Bug report

Your Environment

Software	Version(s)
Angular	8.2.3
Angular-Slickgrid	2.17.10
TypeScript	3.5.3

Describe the Bug

If we configure the FormatterOptions with decimalSeparator: ',' and thousandSeparator: '.', the formatting does not take affect for values like 1000.5, while it will work for 1.5 and for 1000

Steps to Reproduce

Standard grid with a column with FiledType.number values, and a Formatters.decimalFormatter,
FormatterOptions with decimalSeparator: ',' and thousandSeparator: '.'.

Expected Behavior

1000.5, -> 1.000,5
1.5 -> 1,5
1000 -> 1.000

Current Behavior

1000.5, -> 1,000.5
1.5 -> 1,5
1000 -> 1.000

Possible Solution

I strongly suspect that this is due to the fact that the config is exactly reversed to the defaults (decimalSeparator: '.' and thousandSeparator: ',').
Since both decimalFormatted and thousandSeparatorFormatted use string replacement and the bug only occurs if both functions need to run, my assumption is that the second function cancels out the replacement of the first one.

[ghiscoding]

In my use case it works, but we are only using the thousandSeparator and the decimal is default to dot so I guess that is why I don't configure it.

I don't really have time to look into this, so if you could troubleshoot the problem, that would help. All the formatters end up using the formatNumber function that is defined in the utilities.ts file here. There's a bunch of unit tests as well, so I'm a bit surprised unless I don't have unit tests to cover your use case, they're all here

[fheck]

As I said, I highly suspect that the reason for this is that we use this exact combination, with both values essentially swapped. I took a quick look at the unit tests, and I did not find this exact combination there, so this corner case could have gone unnoticed.

For now, I've written a workaround specific to our project, since I'm also a bit short on time but I might get around to it in a couple of weeks.

[ghiscoding]

Quick investigation, this issue is happening only when both properties are used at the same time and works flawlessly when only 1 of the 2 is used. Not sure why yet though but that is the first thing I can see.

[ghiscoding]

Ok I found it, the code was basically cancelling each other when both were provided

previous code

export function decimalFormatted(...) {
  // do we want to display our number with a custom separator in each thousand position
  if (thousandSeparator) {
    amount = thousandSeparatorFormatted(amount, thousandSeparator);
  }

  // when using a separator that is not a dot, replace it with the new separator
  if (decimalSeparator !== '.') {
    amount = amount.replace('.', decimalSeparator);
  }

The thousand separator was being changed by a decimal (.) but then was being cancelled by the second call of replace of decimal separator.

I'm rewriting the code to instead just do a text split by the decimal and then combine integer + separator + decimal value and now it works for all cases.

Considering the print screen below where

1. first column has thousandSeparator: '.' and decimalSeparator: ','
2. second column has thousandSeparator: ',' and decimalSeparator: '.'

now it seems all fine, PR #415 is coming done

[fheck]

Thank you very much for the fast response and even for fixing this so quickly!
I suspected this was the case, but I couldn't pinpoint the issue exactly.

[ghiscoding]

now shipped under new version 2.17.11

Please upvote if you haven't already ⭐️
Cheers