Add option for restoring the original positions upon disable. #40

Merged
merged 2 commits into from Apr 30, 2013

Projects

None yet

2 participants

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.

@cameronmcefee cameronmcefee commented on an outdated diff Apr 25, 2013
@@ -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).
cameronmcefee
cameronmcefee Apr 25, 2013 Owner

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. 
Owner

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

Contributor

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...

Contributor

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).

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.

Contributor

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).

Owner

🔥 thanks.

@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