-
Notifications
You must be signed in to change notification settings - Fork 465
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix desaturation of greyscale colours #865
Conversation
It looks like the related functions also don't clamp results, e.g., |
@anlutro Could you come up with a test case which includes all edge cases? If so please add another PR to https://github.com/sass/sass-spec! |
@QuLogic I did that originally but thought it might be misleading because it is originally the saturation of the colour passed to the function. As for clamping
I'll see if I can make a PR to the spec. |
Spec added sass/sass-spec#249 |
@anlutro I've merged your specs which now causes this PR build to fail. Could you please investigate? |
Looks like I was wrong about |
if (hslcolorS <= 0) { | ||
hslcolorS = 0; | ||
} else { | ||
hslcolorS += amount->value(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, this is +=
now for desaturation?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yikes, stupid mistake. Glad you spotted it.
OK, there is one more problem I have to fix - |
Fixed and squashed commits, everything should be OK now. |
Great work @anlutro |
Fix desaturation of greyscale colours
See issue #864