Incorrect conversion from HEX to HSL* with this color.

[overclokk]

Hi, I just found this issue trying to convert #DC3545 to an hsl value, the hue returned is -5.748502994012 instead of 354.

You can try this method inside FactoryTest::class:

    public function test_it_should_return_correct_hsa_value_from_hex() {

        $sut = Factory::fromString('#dc3545' );

	$this->assertStringMatchesFormat( '#dc3545', (string) $sut->toHex(), '' );
	$this->assertStringMatchesFormat( 'rgb(220,53,69)', (string) $sut->toRgb(), '' );
        $this->assertStringMatchesFormat( 'hsl(354,70%,54%)', (string) $sut->toHsl(), '' );
        $this->assertStringMatchesFormat( 'hsla(354,70%,54%,1)', (string) $sut->toHsla(), '' );
    }

This is part of the tests output:

There was 1 failure:

1) Spatie\Color\Test\FactoryTest::test_it_should_return_correct_hsa_value_from_hex
Failed asserting that string matches format description.
--- Expected
+++ Actual
@@ @@
-hsl(354,70%,54%)
+hsl(-6,70%,54%)

Maybe the issue is here

color/src/Convert.php

Line 104 in fafbf89

$hue = 60 * fmod(($g - $b) / $delta, 6);

The hex to rgb conversion is fine, the rgb value returned is 220, 53, 69, so it should be only the rgb to hsl in the line above.

[overclokk]

I just found another conversion issue from hsl to hex or rgb but I think it's better to open another issue for that.

[horuskol]

The maths used for HEX/RGB to HSL conversion does sometimes output a negative value for hue, but hsl(354, 70%, 54%) and hsl(-6, 70%, 54%) will produce the same shade of red on display. Because of the cyclic nature of hue, values like 714 and -366 would also be equivalent.

However, the hue could be normalised before being so that it is always between 0 and 360.

while ($hue < 0) {
    // can sometimes be negative
    $hue += 360;
}

while ($hue > 360) {
  // I don't think this is likely, but for balance
  $hue -= 360;
}

edit: I just saw that the markdown preview for hsl(-6, 70%, 54%) didn't work. Interesting. So far I've never had a problem with negative hue whenever I've used it.

[overclokk]

I just created a pull-request with a fix for this issue, it simply normalizes the negative value for the hue.

[freekmurze]

Thanks for the PR! 🙌