finnsson / pagerjs

The Single Page Application Framework for KnockoutJS
http://pagerjs.com/
MIT License
271 stars 68 forks source link

Problem with afterShow event and child bindings #77

Closed CuinnWylie closed 11 years ago

CuinnWylie commented 11 years ago

When using the afterShow event on a page in order to do some custom modifications I have run into a problem where the bindings on the page (custom bindings from the knockout-kendo library) have not finished binding completely.

I have introduced a delay into the afterShow call of 50 ms using setInterval. This fixes the problem for me as the bindings have time to complete before I try to access elements on the page. I don't know if this is the right way to be doing this however. I made this change in the showElementWrapper function in the page object.

I'd be interested in your comment on this and would be happy to provide samples if needed.

Thanks.

finnsson commented 11 years ago

I had the same problem in another part of the code. It helped to put that code in

setTimeout(function() { someCode(); }, 0);

so the code someCode(); is executed after the current event loop is finished. I'll take a look at this tonight. Test-cases are always welcome.

finnsson commented 11 years ago

I've looked at the code. afterShow and afterHide should be possible to put on the event loop using setTimeout. Could you provide a JsFiddle or similar that demonstrates the bug? It would be much appreciated.

CuinnWylie commented 11 years ago

Hi,

Sample at http://jsfiddle.net/CuinnWylie/rJJp2/. If you take a look at the VM.AS function you can see the two lots of code. I fixed this in my app by modifying the code as below.

        p.showElementWrapper = function (callback) {
            self = this;
            if (this.val('beforeShow')) {
                this.val('beforeShow')(this);
            }
            this.showElement(callback);
            if (this.val('afterShow')) {
                setTimeout(function() {self.val('afterShow')(self);},0);
            }
        };
finnsson commented 11 years ago

I think the issue if actually with Kendo UI. I read some of the source code of kendoMenu and it is checking some async-option to see if it should do the binding inside a setTimeout(..., 0). All child bindings inside a page are processed before afterShow is fired, but the Kendo UI-binding is deciding to postpone the kendoMenu-binding even further (performance reasons?).

So I think the order turns out to be something like:

Event loop number:
1st loop:
    1. page-binding is processed. afterShow it put of the event loop queue.
    2. kendoMenu is processed. setup for kendoMenu is put on the event loop quese (after afterShow).
2nd loop:
    1. afterShow is triggered. Your code AddSimpleMenuItem is put on the event loop queue since it is inside a setTimeout.
    2. setup from kendoMenu is triggered.
3rd loop:
    1. AddSimpleMenuItem is processed.

I think the fix in this case is that I update the documentation - trying to make it clear that some bindings (such as bindings from Kendu UI) inside the page might be async.

If you grow tired of adding setTimeouts everywhere you can subclass the Page-binding and override showElementWrapper, maybe adding something like:

loremIpsumPage.prototype.showElementWrapper = function(callback) {
    pager.page.prototype.showElementWrapper.call(this, callback);
    if(this.val('asyncAfterShow')) {
        setTimeout(function() {
            this.val('asyncAfterShow')(this);
        }, 0);
    }
};

Any thoughts?

CuinnWylie commented 11 years ago

That seems like a good way to address it. I'll have a play around with that this week and see how it goes. I'm starting to run into a few issues with the kendo controls so I might need to re-think my strategy anyway.

Thanks for looking into this and providing such a good explanation. I agree that the fix is probably a documentation update in this case.