Color functions #94
Merged
Color functions #94
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Adds support for alpha into a subclass of ColorExpression, which then uses the rgba() format to output to the CSS.
The implementation of ARGB uses a new node called Anonymous. I copied this approach from the less.js source.
Hue & spin don't quite match the sample yet.
Still doesn't resolve hue not matching in functions-color.less -> .css. I have also tried another HSL->RGB->HSL converter and got the same result, so this example may not be correct. I will check to see how less.js handles it.
…lpha, luma, mix, greyscale
The almost-css21.less has an example of a filter declaration that invokes an alpha function. Now that we've implemented the alpha function this test fails as it's not supposed to be a function. Best to just not process filter declarations I think. References issue [SomMeri#90]
so we mimic the accuracy of less.js under computation such as to and from HSL. Also change to use the existing formatting for numbers (didn't realise I could pass null for originalString), which tidies up the output by removing extra .0s on the ends of numbers. These accuracy changes mean much more of the test case functions-color.less is now correct. There is one case of too many decimal places of accuracy that I still need to address. Also I have removed the first set of functions from the test case as they don't appear to be in less.js either?
Note that there is a bug in less.js (I am submitting a bug report) in the contrast function when a threshold is given, so I have changed the expected result and added more test-cases.
output. A test-case to support this is coming, it was in less.js test-cases.
parameter isn't a color, to support filters I guess. Test-cases updated with additional color testing cases from less.js. This matches the test-case from less.js, included in functions-color.less now. The bug in less.js is unclear actually, the documentation suggests that the threshold parameter to contrast should be a percentage but it doesn't work if it is. Less.js has updated their test-cases to use decimal numbers rather than percentages. I have included those cases, but left in the percentage cases as we handle those "correctly", but don't match less.js.
I didn't notice that they round them in less.js! Now the color functions test-cases all pass.
I have added a convertTo function to NumberExpression, based on the implementation in less.js.
remove _color("evil red").
These apparently are just test functions, perhaps to test custom
functions?
and arguably more correct? ;-)
|
I've added to this pull request to include more math functions and some other misc functions. This wraps up #66. |
This function doesn't take parameters, which was not supported in the grammar. I have changed the grammar (added a ?) and modified TermBuilder to recognise the change and created EmptyExpression to represent the lack of parameters. The pi test is now returned to functions.less.
In particular the round() error messages now match their test-cases so all the tests pass. Color functions now return FaultyExpression rather than null in the event of errors. Except for contrast() which accepts a faulty argument as per less.js.
ghost
assigned
SomMeri
added a commit
that referenced
this pull request
Jan 23, 2013
Color functions, math functions and filter declarations problem.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
All of the color functions are now implemented and tested using the latest test-cases from less.js. [#66]
I am going to go on to the other functions to wrap this up.
Note this also includes the fix described in [#90]. If that isn't the right approach to that problem let me know and I can exclude it from this pull-request. That fix was needed to pass tests due to the implementation of the alpha function that was used in a filter (as referenced in [#90]). It would be possible to solve this another way, like contrast() has solved it by silently failing if the arguments are of the wrong type... I believe that filters are always filter: rules, so that should work safely!