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

Fix wrong axis when checking for blocked axis #26

Merged
merged 2 commits into from
Jun 23, 2017

Conversation

chipironcin
Copy link
Contributor

@chipironcin chipironcin commented Jun 23, 2017

This PR fixes wrong axis blocking.

From documentation:

    axis:              null,   // What axis should be disabled. Can be X or Y.

Currently, when setting data-tilt-axis='x', y axis gets blocked and viceversa.

With this change, setting data-tilt-axis='x' will block x axis.

@micku7zu micku7zu merged commit fb1c4af into micku7zu:master Jun 23, 2017
@micku7zu
Copy link
Owner

How did it get so far without anyone noticing this? Thanks @chipironcin!
I need to make a new version and update the landing page as well... tomorrow 😄

@micku7zu
Copy link
Owner

Actually, I just reversed it until tomorrow when I will have a fresh look at all this axis thing 😄
Thanks!

I want to be in sync with tilt.js original library, and I will check tomorrow because it's too late right now.

@chipironcin
Copy link
Contributor Author

After taking a look at tilt.js and its main page, it seems it is not the code which is wrong.
But the documentation is a bit misleading.

TL;DR
Setting data-tilt-axis you are saying you want to tilt along that axis, which is equals to disabling that axis in terms of rotation.


axis: null, // What axis should be disabled. Can be X or Y.

If you set data-tilt-axis='x', you are disabling the X axis, disabling the rotation on X axis. But in terms of tilt, you are saying you want to tilt along the X axis.

The rotateX() method rotates an element around its X-axis at a given degree
"rotateX(" + (this.settings.axis === "x" ? 0 : values.tiltY) + "deg) "

So the code is fine as it was.
The above snippet means that if we said we want tilt on x axis, it will set rotateX to 0 (no X axis rotation = no Y axis tilt)


I am sorry for the confusion this PR may have generated and I am happy you reverted it right away.

I will create another PR for fixing only the documentation.
Will do the same to tilt.js project ;)

Thanks for this awesome work!

@chipironcin chipironcin deleted the fix_blocked_axis branch June 23, 2017 23:38
@micku7zu
Copy link
Owner

Hi @chipironcin, I still didn't have time to have a look, and I saw that the original project didn't merge the PR. I will automatically merge the PR if it will be merged in tilt.js or if I will have some time to clarify this.

Sorry for the delay :)

Thanks again for your help!

@chipironcin
Copy link
Contributor Author

chipironcin commented Jun 25, 2017

Dont worry, I will ping you
gijsroge/tilt.js#26
#28

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