Color conversion not behaving as expected

[reck1610]

Description of Change

• Updated xunit to latest version to use cultured tests.
• Created tests to ensure color conversion behaves the same on different locales
• Removed test that test for wrong output
• Fixed test data generated differs depending on locale
• Fixed Color conversion not behaving as documented

Every locale produces the documented output.

Linked Issues

• Fixes [BUG] Color conversion not behaving as expected #2873

PR Checklist

• Has a linked Issue, and the Issue has been approved(bug) or Championed (feature/proposal)
• Has tests (if omitted, state reason in description)
• Has samples (if omitted, state reason in description)
• Rebased on top of main at time of PR
• Changes adhere to coding standard
• Documentation created or updated: https://github.com/MicrosoftDocs/CommunityToolkit/pulls

Additional information

[reck1610]

@dotnet-policy-service agree company="Hartburg GmbH & Co. KG"

[bijington]

Thanks for this. I wonder though - rather than marking the old methods as Obsolete can't we let developers provide the culture if they wish but introduce your new methods to shortcut to Invariant?

[reck1610]

Other locales produce broken output like RGBA(0,0,0,0,5).

An RGBA string like that is unusable. I think the format of this should be strict, because it is not unlikely to get interpreted at some point. E.g. passed as CSS. I think localized output can only hurt in this case. I don't think this was ever supposed to be localized in the first place.

[reck1610]

I would like to get some feedback on this method. I don't really like it, but I wanted to ensure the same rounding behavior as before my commit.

If the change in rounding behavior is ok. I would prefer to use only "0%", remove the method and update the failing tests.

failed CommunityToolkit.Maui.UnitTests.Converters.ColorToHslaStringConverterTests.ColorToRgbStringConverterValidInputTest(red: 0,25, green: 1, blue: 0,25, alpha: 1, expectedResult: "HSLA(120,100%,62%,1)")[en-US] (0ms)
  Assert.Equal() Failure: Values differ
  Expected: HSLA(120,100%,62%,1)
  Actual:   HSLA(120,100%,63%,1)

[reck1610]

I implemented the change in a separate commit (fce5451).
If this change in behavior is not wanted, I will just drop the commit.

@TheCodeTraveler Would this be considered a breaking change? If so, do I need to do something in that case, e.g.: mark the commit with "BREAKING CHANGE"?

[VladislavAntonyuk]

We should maintain the existing methods as they are. It's up to me which culture I provide in parameters and what the expected output format is. As for the tests, we should set the invariant culture to the culture info parameter.

[reck1610]

Yes its up you which culture you provide. But its the up the method to ignore it if its not relevant.

The culture info was passed as fix for the tests in #552.
They were not passed to allow localized output, just to fix tests.
Localized output for these methods makes no sense, It just breaks the output.

Those methods are supposed to create well defined output.
Here is some example output for those methods:

Method	en-US	de-DE (uk-UA should be the same)	Invariant
ToRgbString()	RGB(255,0,0)	RGB(255,0,0)	RGB(255,0,0)
ToRgbaString()	RGBA(255,0,0,0.5)	RGBA(255,0,0,0,5)	RGB(255,0,0,0.5)
ToCmykString()	CMYK(0%,100%,100%,0%)	CMYK(0 %,100 %,100 %,0 %)	CMYK(0 %,100 %,100 %,0 %)
ToCmykaString()	CMYKA(100%,100%,0%,100%,0.5)	CMYKA(100 %,100 %,0 %,100 %,0,5)	CMYKA(100 %,100 %,0 %,100 %,0.5)
ToHslString()	HSL(0,100%,50%)	HSL(0,100 %,50 %)	HSL(0,100 %,50 %)
ToHslaString()	HSLA(0,100%,50%,0.5)	HSLA(0,100 %,50 %,0,5)	HSLA(0,100 %,50 %,0.5)

RGBA, CMYKA and HSLA are just broken in de-DE. The comma as decimal point should not be used in comma separated list.

There is no upside in localizing this. This can only cause bugs. The methods are already documented to have a well defined behavior.

Here is a full list of the possible output for CMYKA

CMYKA(100 %,65 %,52 %,0 %,0.625)
CMYKA(100%,65%,52%,0%,0.625)
CMYKA(100%,65%,52%,0%,0,625)
CMYKA(100٪؜,65٪؜,52٪؜,0٪؜,0٫625)
CMYKA(100‎%‎,65‎%‎,52‎%‎,0‎%‎,0,625)
CMYKA(100 %,65 %,52 %,0 %,0,625)
CMYKA(100 ٪,65 ٪,52 ٪,0 ٪,0٫625)
CMYKA(% 100,% 65,% 52,% 0,0,625)
CMYKA(100٪,65٪,52٪,0٪,0٫625)
CMYKA(%100,%65,%52,%0,0,625)
CMYKA(100%,65%,52%,0%,0٫625)

I think the 4. example contains a control character making the values appear on the wrong side of the comma.