less / less.js

Less. The dynamic stylesheet language.
http://lesscss.org
Apache License 2.0
17k stars 3.41k forks source link

hsl() not getting colorized #3625

Closed PRR24 closed 3 years ago

PRR24 commented 3 years ago

According to documentation https://lesscss.org/functions/#color-definition-hsl hsl() function should output a color, but it does not. rgb() and hsv() behave as documented.

$ npx lessc --version
lessc 4.1.1 (Less Compiler) [JavaScript]
$ cat problem.less 
.problem {
  color: rgb(128, 255, 0);
  background-color: hsl(90, 100%, 50%);
  border-color: hsv(90, 100%, 50%);
}
$ npx lessc problem.less 
.problem {
  color: #80ff00;
  background-color: hsl(90, 100%, 50%);
  border-color: #408000;
}
PRR24 commented 3 years ago

Triangulated:

$ npx less@3.8.0 problem.less 
.problem {
  color: #80ff00;
  background-color: #80ff00;
  border-color: #408000;
}
$ npx less@3.8.1 problem.less 
.problem {
  color: #80ff00;
  background-color: hsl(90, 100%, 50%);
  border-color: #408000;
}
matthew-dean commented 3 years ago

Not sure what the issue is. hsl() is a valid color value in CSS. Less, in general, tries to preserve original color formats.

matthew-dean commented 3 years ago

@PRR24 If the issue is that it doesn't match documentation, it's more an issue of the documentation.

PRR24 commented 3 years ago

Well, rgb() is also a valid color, but its gets converted nevertheless... I don't mind hsl(), but problem is consistency. Currently some color functions return hsl while come return color.

@color-base: hsl(90, 100%, 50%);
@color-dark: darken(@color-base, 20%);
@color-avg: average(@color-base, @color-dark);

.problem {
  color: @color-base;
  background-color: @color-dark;
  border-color: @color-avg;
}
$ npx less@4.1.1 problem.less 
.problem {
  color: hsl(90, 100%, 50%);
  background-color: hsl(90, 100%, 30%);
  border-color: #66cc00;
}

If one has an additional use for the computed values, for example creating a color list for external use, it is bit of a mess.

If you find that hsl() handling in its current form is better (and there are understandable arguments for it), please adjust the documentation and concider adjusting all possible color functions to return computed value according to the format of its parameters, so that average() in the example above would return hsl(90, 100%, 40%)

Thanks.

iChenLei commented 3 years ago

All Color process logic located at packages/less/src/less/tree/color.js and packages/less/src/less/functions/color.js .

If you find that hsl() handling in its current form is better (and there are understandable arguments for it), please adjust the documentation and concider adjusting all possible color functions to return computed value according to the format of its parameters, so that average() in the example above would return hsl(90, 100%, 40%)

If you are interested in this work, please help us to improve less doc. Thanks

less doc repo -> https://github.com/less/less-docs , example PR https://github.com/less/less-docs/pull/551

arronKler commented 3 years ago

There really have some updates about colors on v3.8.1. hsl() will keep instead of converting to hex color. The document (https://lesscss.org/functions/#color-definition-hsl) is outdated.

Changelog: https://github.com/less/less.js/blob/master/CHANGELOG.md#v381-2018-08-08 Related Pull request: https://github.com/less/less.js/pull/3291 Related Issue Comment: https://github.com/less/less.js/issues/3290#issuecomment-406821050