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 support for percentage values #69

Merged
merged 4 commits into from Nov 29, 2015
Merged

Add support for percentage values #69

merged 4 commits into from Nov 29, 2015

Conversation

andrepolischuk
Copy link
Contributor

Add percentage values for translate, fix #40

move('.square')
  .to('50%', '25%')
  .end();

@yields
Copy link
Contributor

yields commented May 4, 2015

nice! can we keep the conditionals on one line? otherwise looks good to merge!

@andrepolischuk
Copy link
Contributor Author

I fixed to one line

@A
Copy link
Member

A commented Nov 28, 2015

@andrepolischuk good catch! But IMHO checking for percent/px by the type of value looks confusing for users. I think, the better way is to add px to the value only if user hasn't specified units. For example if user set -100% or 20rem we're just passing it. But if user has just set only digits (no matter string it or integer), then we will add px to the value before passing.

Please, can you update behaviour to make it more flexible?

@A A self-assigned this Nov 28, 2015
@A
Copy link
Member

A commented Nov 28, 2015

And one little note — can you move this logic into helper function in the bottom of the file? It's pretty nice place for DRY 'cause of many duplications :D

@andrepolischuk
Copy link
Contributor Author

Sure

@andrepolischuk
Copy link
Contributor Author

@shuvalov-anton, I fixed

@A
Copy link
Member

A commented Nov 29, 2015

Thanks)

A added a commit that referenced this pull request Nov 29, 2015
Add support for percentage values
@A A merged commit ed57570 into visionmedia:master Nov 29, 2015
@iamstarkov
Copy link

👍

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.

Add support for percentage values
4 participants