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 option for restoring the original positions upon disable. #40

Merged
merged 2 commits into from Apr 30, 2013

Conversation

magebarf
Copy link
Contributor

This is quite useful when your page has multiple sections where you won't be seeing all of them at the same time.

In this case you wouldn't want the entire set of plax layers to be enabled all the time, and as such the clearLayers option is great.

However, moving back and forth from different sections of the page, there's a risk the objects aren't in their original position when clearing the layers, and when moving back and re-enabling plax on them, they would get a new initial position, which would be stacking up for each time you move back and forth to the section.

New option simply makes sure that each layer's original position is restored upon disable, normally combined with clearLayers, but there are possible uses without clearing the layers as well.

@@ -94,6 +94,8 @@ Disable parallaxing.

__Parameters__

`restorePositions` — Boolean: (optional) resets all previously defined layers to their original positions. Useful when combining Plax with functions to hide and show various parts of the page, a good example being [turn.js](http://www.turnjs.com/) [(github repo)](https://github.com/blasten/turn.js).
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

http://www.turnjs.com/ doesn't use plax, which is a little confusing. I think we could get away with something simpler like:

Boolean: (optional) resets all previously defined layers to their original positions when plax is deactivated. 

@cameronmcefee
Copy link
Owner

Nice, I dig this. If you don't mind updating the one part in the documentation, I'm down to merge this.

@magebarf
Copy link
Contributor Author

Yup, it should be a quite straight forward option, not needing too much of an explanation.

My ambiguous description was intended to convey the message that it was useful when you were using a combination of Plax and Turn.js, as you might want to enable parallax scrolling for elements only on the page you are currently watching, and disabling it in between.

Another thing that I started to wonder a bit about today, is why both the $(this).position() and this.offset[Left[Top] are used in the initialization of the non-background layers, and if possibly the layer.origin variables should be set to the $(this).position values instead.

As far as I can tell though, after refreshing my memory of the jQuery documentation is that they should be the same values. Reading up a bit on the jQuery code, the only potential difference I can see is that the .position() values deducts the margin, which I'm unsure if the offset values handles?

Also, I'm unsure if Ender differs in this aspect?

Please pitch your thoughts on this... It works just fine in my use cases with these commits in place, but I might be missing something outside of my scenario...

@magebarf
Copy link
Contributor Author

And to fill in after a bit more research on Ender, and more specifically Jeesh/Bonzo (which I assume is the parts of Ender meant):

It does actually not have any .position() function, and thus I believe that commit 918b527 broke ender/Jeesh compatibility (modifying the examples att current head to load Jeesh instead of jQuery logs "Uncaught TypeError: Object [object Array] has no method 'position'" to the console).

@cameronmcefee
Copy link
Owner

I originally accepted the Ender support without really considering what that would mean (I was very new to open-source at the time) as far as supporting it myself. Having never used Ender before or since then, it's more of a hurdle for maintaining the code on my end. If the Ender support is actually broken, my vote would be to just remove it and not sweat it.

Another thing that I started to wonder a bit about today, is why both the $(this).position() and this.offset[Left[Top] are used in the initialization of the non-background layers, and if possibly the layer.origin variables should be set to the $(this).position values instead.

It's entirely possible this was done arbitrarily. When I originally wrote plax I was pretty green as far as my javascript chops. It may have just been a terrible decision on my part. One of these days I may just rewrite the things, but for now I've been letting stuff sit as is until I or someone else can really focus on it.

@magebarf
Copy link
Contributor Author

I'll make sure to put it up as a separate issue, as its not really related to this pull request.
I can prepare a small patch to remove the usage of .position instead of the offset properties, and it'll be back on track again, as that as far as I can tell is the only part incompatible with the default Ender package (Jeesh).

@cameronmcefee
Copy link
Owner

🔥 thanks.

cameronmcefee pushed a commit that referenced this pull request Apr 30, 2013
Add option for restoring the original positions upon disable.
@cameronmcefee cameronmcefee merged commit bf789ce into cameronmcefee:master Apr 30, 2013
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