less / less.js

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

Color functions flawed (?) #282

Closed jox closed 12 years ago

jox commented 13 years ago

In my opinion the color functions (like lighten(), darken(), desaturate()) don't work right. At least in the way it would be logical and useful and one would expect them to function (I think).

To make it short: they work with absolute amounts, not with relative (which would be logical when giving a percentage value).

For example the following function call

lighten(@color, 50%)

will not increase the lightness of the color by 50%. Instead it is increasing the lightness by a fixed (absolute) amount of 50 (lightness being a value between 0 and 100) no matter what the value was before.

If the lightness of a color is 50 and it will be increased by 50% I would expect it to become 75. Instead it will be 100 (white).

Example:

/* gray (50% lightness) */
@color: #808080;

/* will be white */
lighten(@color, 50%);

/* will be black */
darken(@color, 50%);

The color red would also be white or black when lightend or darkend by 50%.

A workaround woud be:

/* a 50% lighter gray */
darken(@color, lightness(@color) * 0.5);

/* a 50% darker gray */
lighten(@color, (100 - lightness(@color)) * 0.5);

Mathematically it is currently like this:

decreasing: value -= amount
increasing: value += amount
(absolute)

(value and amount being values between 0.0 and 1.0) But should be like this:

decreasing: value -= value * amount
increasing: value += (1.0 - value) * amount
(relative)

The current functionality could still be useful though. So the best would be to have a choice.

Perhaps the most logical would be to allow an absolute or relative amount.

/* absolute */
lighten(@color, 50);
or
lighten(@color, 0.5);

/* relative */
lighten(@color, 50%);

But this would of course massively break compatibility.

One solution (maybe the easiest) would be to add another set of functions, like for example:

lighten_rel(@color, 50%);
darken_rel(@color, 50%);
...

Another option would be to establish a global setting (with absolute as default for compatibility).

set_amount_method(relative);
set_amount_method(absolute);

Or something like that.

jox

jamesfoster commented 13 years ago

it would be cool if you could define your own functions within the .less file. Or have some way of plugging in your own functions!

I have wanted to have relative adjustments before, but I've always either just used the calculation or simply gone with the absolute adjustment.

it would also be good if you could do this with a mixin...

.lighten_rel(@prop, @color, @amount) {
{
    @{prop}: lighten(@color, lightness(@amount) * @amount);
}
.darken_rel(@prop, @color, @amount) {
{
    @{prop}: darken(@color, lightness(@amount) * @amount);
}

but I don't how/whether @cloudhead plans to implement property name interpolation. I'd definitely look at implementing that myself if I know what syntax to go for.

boomyjee commented 13 years ago

you can use your own functions through less.tree.functions.myFunc = function (param) { ... return new less.tree.Anonymous("some value"); }

guilhermeaiolfi commented 13 years ago

Thanks boomyjee. I've been adding my functions directly into the code for some time. I didn't know about that, maybe it's not documented. Or maybe it is, I don't know. Updating to new version now will be a lot easier. :)

chriseppstein commented 13 years ago

See the change-color, scale-color, and adjust-color functions in Sass for how we deal with this issue. http://sass-lang.com/docs/yardoc/Sass/Script/Functions.html#other_color_functions

cloudhead commented 13 years ago

The reason for this is pretty simple: if you use multiplication, you can't lighten black, as 0 * n = 0. If the color is almost black, it will work, but you'll have to use huge values to get visible results. This applies to all the other color functions too.

chriseppstein commented 13 years ago

Our scale-color function multiples the delta between the current color value and the endpoint. This handles the 0 case you mention here. (100 - n) * percent

topherfangio commented 13 years ago

For reference, you can grab the hue, saturation and brightness from a color, and create a new color based off of those values. So, you should be able to grab the hue/saturation and modify the brightness of the black to lighten it.

OlsonDev commented 12 years ago

I ran into this today. What's wrong with chriseppstein's solution?? darken(rgba(0.10 * 255, 0.10 * 255, 0.10 * 255, 1), 10%) is not rgba(0, 0, 0, 1); color functions are useless if they're not correct.

Synchro commented 12 years ago

In those SASS docs, they give this example:

darken(hsl(25, 100%, 80%), 30%) => hsl(25, 100%, 50%)

That is clearly using an absolute change (80 - 30 == 50); if it was relative the lightness would be changed to 80 - ((80/100) * 30) == 56.

In your example, rgba(0.10 * 255, 0.10 * 255, 0.10 * 255, 1) is the same as hsla(0,0,10%,1), and it is darkened by an absolute 10%, resulting in 0 for all color components.

There is no problem here; SASS' darken works the same way as LESS', and SASS has some other functions that do something different. If you want to add those other functions, open another ticket.

OlsonDev commented 12 years ago

I understand that. Isn't that the point of this ticket though? From the ticket:

One solution (maybe the easiest) would be to add another set of functions, like for example:

Why open another ticket when this one can just be reopened?

Synchro commented 12 years ago

Mostly because the subject is wrong for that purpose and the description is misleading: these color functions work as advertised (and the same way as SASS) and on that basis this was closed.

A ticket should try to do only one thing - make it a bug or a feature request, not both. This comment history is already cluttered with confusion between the two (even your own comment came from a 'this is a bug' direction). By all means reference this issue in a new one, and cherry-pick the most useful bits out of here so it's clear and unambiguous; reopening this one won't help your cause. A good title might be "Add color scaling functions".