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

allow transform #26

Closed
wants to merge 4 commits into from
Closed

allow transform #26

wants to merge 4 commits into from

Conversation

g-van-vreckem
Copy link

This is rather rough...
To avoid adding dependencies, I added a transform parameter, where the proper prefixed name for the transform (or false) must be supplied outside of the code.

Also, the focusPoint settings where not available in the adjustFocus function, so I simply added them as parameters, I'm sure that's not a good jQuery pattern.
Someone knowledgeable on jQuery plugin options and settings patterns better take a look at this.

But it is a proof of concept that the formula is good...

Note that the transform % are expressed in relation to the image size while the left/top % are expressed in relation to the container size.

replaced 'start', 'stop' with the new  'windowOn', 'windowOff'
Not suficiently tested, ugly way to pass the settings around, but it
should prove the formula validity.
@jonom
Copy link
Owner

jonom commented Sep 21, 2014

Thanks for this proof of concept, I will think about the best way to roll it in.

@g-van-vreckem
Copy link
Author

improved performance significantly by using scaling instead of width/height constraint.
Persisted the transform name in the element to allow the helper to work again.
Kept the bug correction I made available separately.
Optimized the calcShift in the case of the transform
Started to remove extraneous var

@jonom
Copy link
Owner

jonom commented Sep 22, 2014

@g-van-vreckem Thanks again, great to see you continuing to work on this. Finding it difficult to follow along with changes in the last commit though - would it be difficult to undo that last commit and separate it in to two separate ones, one for 'using scale instead of width/height' and a separate one for 'Started to remove extraneous var'?

@jonom
Copy link
Owner

jonom commented Sep 22, 2014

p.s. could you please change the pull request to target a new branch with a suitable name e.g. 'allow-transform'. I can't see a way to do it at my end.

@g-van-vreckem
Copy link
Author

I made the requested change in #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.

None yet

2 participants