update scroll offset before updating pin state

[kasparsj]

Hi Jan, I'm working on a website where I change the scroll position if the window gets resized. I do that so that the user sees the same section even though the contents shift. Doing so I run into a bug: if the current section was pinned (fixed), it got a wrong offset.
Basically this fix makes sure that updateScrollOffset() gets called before updatePinState().

[janpaepke]

Mh. Actually it should be called before updatePinState as it is.
And your code would actually mean it is called after scene.update(), which would quite definitely cause it to be updated to the wrong values.

It would help if you could create a fiddle of what you are experiencing...

[kasparsj]

No, because construct() gets called after the feature-pinning event listeners are added, therefore at the moment updateScrollOffset() is called after updatePinState().

[janpaepke]

you're right again!
But maybe this is making bad design (on my part) even worse?

Maybe we should move the whole event-listener-adding out of the constructor?

Or – maybe even better – make it so that nothing will be executed outside of the constructor (which technically sounds more correct).
We could do this by creating an array of functions that should be executed in order on-construct.

what do you think?

[kasparsj]

I think the easy way - move them out of the constructor.
That way it would be least difficult to understand what's going.

[janpaepke]

Mh. I know what you mean.
But I kind of dislike that it messes up the nice order.

let me reflect on this a little...

[janpaepke]

So I think I found a nice way.
I changed the code of Scene.js to be this:

[...]
/**
 * Internal constructor function of the ScrollMagic Scene
 * @private
 */
var construct = function () {
    for (var key in _options) { // check supplied options
        if (!DEFAULT_OPTIONS.hasOwnProperty(key)) {
            log(2, "WARNING: Unknown option \"" + key + "\"");
            delete _options[key];
        }
    }
    // add getters/setters for all possible options
    for (var optionName in DEFAULT_OPTIONS) {
        addSceneOption(optionName);
    }
    // validate all options
    validateOption();
};

// @include('Scene/event-management.js')

// @include('Scene/core.js')

// @include('Scene/update-params.js')

// @include('Scene/getters-setters.js')

// @include('Scene/feature-pinning.js')

// @include('Scene/feature-classToggles.js')

// INIT
construct();
return Scene;
[...]

And put all the event-listener attachments at the top of core.js
It kinda makes sense, because they reference the functions defined there, just like in feature-pinning.js

What do you think?
I really appreciate your input on this. :)

[kasparsj]

great! :)