Skip to content
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

Add tint and shade functions, fixes #944 #947

Closed
wants to merge 9 commits into from
Closed

Add tint and shade functions, fixes #944 #947

wants to merge 9 commits into from

Conversation

Synchro
Copy link
Member

@Synchro Synchro commented Sep 10, 2012

No description provided.

@ricardobeat
Copy link
Contributor

Maybe the shade() helper should invert the amount value, so that tint/shade are symmetric: tint(10%) and shade(10%) should result in two colors equally spaced apart.

I also don't think the default amount of 10 is needed, all the other color functions don't have defaults.

@lukeapage
Copy link
Member

Not sure what @ricardobeat means by inverting the amount value? (or the example without colours?)

I agree that a default of 10% is a bit OTT, I'd prefer people have to be explicit about how much shading they are adding.. the only defaults are where the value can be absolute

@Synchro
Copy link
Member Author

Synchro commented Sep 11, 2012

Initially I agreed about the sign of the amount, but then I realised that the direction of the change is determined by the color it's blending with - tint goes towards white, shade towards black - so it doesn't need negating.
I removed the default value processing, and that stray space.

@lukeapage
Copy link
Member

good but don't check-in the less-1.3.0.js file...

could you revert that file and rebase it into 1 commit? I can do this if your not sure how.

Direction of shade should be same as for tint because its direction is altered by the color it's mixing with
@Synchro
Copy link
Member Author

Synchro commented Sep 12, 2012

Sigh. I thought this would be a good learning opportunity, but I think I completely screwed that up... I did (after about 20 failed attempts):

git rebase -i 53fad4f^
(set first commit to p, others to s, except for 79d801f set to e)
git reset HEAD^1 dist/less-1.3.0.js
git checkout -- dist/less-1.3.0.js
git rebase --continue
git push origin

This all made some kind of sense until the final merge (why a merge? this should be replacing?), and I've ended up with a whole new bunch of commits that seem to have ignored the squashing and reverting of that js file. What did I do wrong?

@lukeapage
Copy link
Member

no idea, thats what I would do.

@Synchro
Copy link
Member Author

Synchro commented Sep 12, 2012

Gave up on git. I trashed that entire fork and made my changes again in a clean fork/branch/pull and now it all looks happy. See #953

@Synchro Synchro closed this Sep 12, 2012
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.

3 participants