@melker opened this Issue on February 16th 2022

Posting two screenshots:

Displayed in English (Correct)

Displayed in Swedish (with some highlighted areas by me)

Bugs in Swedish version:

  • Negative values should be red
  • Negative values should have "-" (minus) not "+-" (plus and minus)
  • Zero-value should be "grey-ish" just as in the English version.
@sgiehl commented on February 16th 2022 Member

Hi @melker
Thanks for creating the issue. I was able to reproduce it locally. Guess that is due to the fact, that in Swedish it doesn't use a "english" minus sign and such can't detect that the number is lower 0. Guess this might happen for other languages as well.

@tsteur commented on February 16th 2022 Member

@sgiehl do you know how easy it is to fix this one?

@sgiehl commented on February 16th 2022 Member

Maybe it's enough to fetch unformatted data for the sparklines and format them only for the display values. Or have unformatted data as metadata like we have for totals. Would need to have a look at that to say more

@melker commented on February 18th 2022

I'd say the main problem is the weird translation. It should be the same signs as in the English version, at least in Swedish. (Other languages may have other signs.)

@sgiehl commented on February 18th 2022 Member
@melker commented on February 18th 2022

@sgiehl not really....

It says:
"minusSign": "−",

and on the page (my screenshot) you have:
"+-" (both plus and munis)

(reservation if I enter the wrong dash-sybol)

@sgiehl commented on February 18th 2022 Member

@melker That minus sign differs from the english one. Even though it's not really good visible in some fonts.
In my browser a can see a small difference in the length of the signs below:

"minusSign": "−",
"minusSign": "-",

And the reason that you see a plus and minus sign is something that happens in the code. Normally numbers don't have a plus sign in front. So we are adding one if the number is not negative. But the check for negative numbers fails in that case, as it already uses the formatted number, which uses another minus sign...

@melker commented on February 18th 2022

That look bit weird to my the the English uses the hyphen-minus and not the formal minus character while the Swedish version uses the formal minus character... well I and not a person really into those things... but still It seems weird to me. Now moving forward instead...

So you need to make the pos/neg check locale aware.

How about the zeros? How come they get the same treatment as negative values? (I don't see what is used on the Unicode CLDR)


@sgiehl commented on February 18th 2022 Member

So you need to make the pos/neg check locale aware.

Exactly. For the zero value that is a similar problem. The formatted percent value contains a non breaking space, which might also break the numeric detection. As noted somewhere above, we need to do such comparisons with unformatted numbers to fix all those issues.

@tsteur commented on February 20th 2022 Member

@sgiehl if there is a different sign, does that mean we can fix that one easily? At least for some languages by putting in the translated minusSign to detect negative values?

@sgiehl commented on February 21st 2022 Member

@tsteur yes that might work. Even though it would be more a workaround than a proper fix.

@sgiehl commented on February 21st 2022 Member

@tsteur I have quickly set up a draft, which should fully fix it (at least for comparison). It adds an additional column for the comparison metrics, that indicates the trend. As this is done prior to formatting the number, this should work for all languages.

@tsteur commented on February 21st 2022 Member

@sgiehl I'll put the issue for now in 4.10 as it's quite a big change and adds heaps more information to the API output etc.

If any possible, we should check if there's a smaller change that fixes this issue as well for Swedish maybe like in https://github.com/matomo-org/matomo/pull/18832/files#diff-9e27247dd682afe6f5fbc3c43bc37873fe2b5583a69ce15d233edfea3377aa3aR4 checking if the value contains the minusSign or translated minus sign for example. Then we could fix it sooner without needing to add so much information to the API and it would work in most countries.

@sgiehl commented on February 21st 2022 Member

If the additional api output is a problem, we can simply remove it by default and only add it for internal requests when needed

@tsteur commented on February 22nd 2022 Member

Not having the additional API output makes it better. I've moved this for now into 4.10 as the PR is a bigger review and quite complex and we have other more important issues in comparison. If there's an easy solution to it, then happy to put it in an earlier milestone.

This Issue was closed on March 8th 2022
Powered by GitHub Issue Mirror