Skip to content

Conversation

seven-phases-max
Copy link
Member

This quick-fix is inspired by issue found in #1665:
Color functions with amount parameter now distinguish between % and non-% values.
The following functions are changed:

saturate
desaturate
lighten
darken
fadein
fadeout
fade
mix

Known issues: To keep things simple I reused existing functions.js:number goody. This function tests only for % and non-% values so for example fade(#111, 1px) gives same result as fade(#111, 1.0) with no error thrown.

seven-phases-max added a commit to seven-phases-max/less-docs that referenced this pull request Nov 20, 2013
seven-phases-max added a commit to seven-phases-max/less-docs that referenced this pull request Nov 24, 2013
seven-phases-max added a commit to seven-phases-max/less-docs that referenced this pull request Nov 24, 2013
seven-phases-max added a commit to seven-phases-max/less-docs that referenced this pull request Nov 25, 2013
@lukeapage
Copy link
Member

So I agree it should have been done like this in the beginning. But is this a breaking change? anyone who has been lazy and left off % will get a surprise. In these functions in particular does over 100% make sense? We could possibly convert with a warning? Otherwise I'm afraid we will have to wait with this until we have a breaking change release.

@seven-phases-max
Copy link
Member Author

Yes, this is breaking change so if you feel that "one who left off % shouldn't be surprised" it's better to wait. Though I'd say such code is ill-formed and works only by mistake, since the current documentation specifies that:

Parameters: ... amount: A percentage 0-100%.

...

In these functions in particular does over 100% make sense?

No, since they either use +, - or simply replace corresponding hsla channel.

We could possibly convert with a warning?

Yes, this makes sense if you expect such "left off %" code to be spread out there (i.e. does it deserve a dedicated conversion function? Though we need it anyway if we don't also want fade(#111, 1px)).

@seven-phases-max
Copy link
Member Author

@lukeapage I thought of your idea about a "left off %" warning, indeed that's a good idea (we could first enable a warning without making any changes in next LESS version, and then we'll be free to make breaking changes later). The problem is that we don't have any "warning" facility yet :) How might that work? (won't this conflict with any tools built on top of the compiler? will they be able to distinguish warnings and errors? We could make warnings optional of course (e.g. with some command-line option) but that most likely would mean that nobody ever reads them :)

@seven-phases-max
Copy link
Member Author

Closed as outdated (I'll probably include this fix as part of some "bigger" feature later).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants