withOnShow not updating on navigation #143

Closed
majkiee opened this Issue Aug 24, 2013 · 7 comments

Projects

None yet

4 participants

@majkiee
majkiee commented Aug 24, 2013

I'm using withOnShow to bind a viewmodel based on a wildcard id.

The showView will use pager.getActivePage().currentId to get the current id and request the corresponding viewmodel data.

It works fine on the first page load (/view/1) but if I click on a link to /view/2 the viewModel is not updated, the first one is still used. But if i point my browser to /view/2 directly the correct viewModel loads.

Have I missed anything or can I force pagerjs to refresh to the new viewModel?

I have logged the viewModel returned by the showView function and it will return the correct vm on link navigation.

@majkiee
majkiee commented Aug 24, 2013

Did a quick jsFiddle to show the problem.
Click on one of the links and then try to navigate to the other id with the new links.
The viewModel that is returned has the correct id but the binding value is not updated.
Fiddle: http://jsfiddle.net/xU4Uu/

@CuinnWylie
Contributor

Hi,

I don't want to step on Oscar's toes, but I'm pretty sure that this happens because the page attempts to rebind the second time you view it as the source is not being loaded in, but you are changing the viewmodel with the withOnShow parameter.

You could get around this by modifying the page to also use sourceOnShow or in my case I've added some code that copies the page contents into another field and then if this specific case is matched I replace the contents before it's bound again (not the best approach possibly, but it works).

Cuinn.

@monsanto

+1 on fixing this (I wish I understood @CuinnWylie 's description of what is going wrong).

@monsanto

This broke at exactly

commit 17508a1772892c319506f908d1d98d3e22119904
Author: Michael Best <mbest@dasya.com>
Date:   Thu Jan 3 16:53:38 2013 -1000

    prevent applying bindings more than once to the same node

on knockoutjs. In pagerjs, applyBindingsToDescendants throws an exception saying "can't bind to the same element more than once"`.

I'm continuing to debug this. At the moment, pagerjs is virtually unusable for me, so let me know how I can be of more assistance.

(@finnsson I hate to tag you, but you haven't responded to this issue yet and it has been 3 months, so I am just making sure you didn't miss it. Sorry to be annoying!!!)

@monsanto

Instead of applying the bindings more than once, can we

  • in p.init, change m.ctx = {} to m.ctx = ko.observable({})
  • in p.init, call applyBindingsToDescendants, instead of in p.loadWithOnShow
  • in p.loadWithOnShow, instead of binding the descendants, replace m.ctx = vm with m.ctx(vm).
  • fix up p.augmentContext to assume m.ctx is observable

?

@CuinnWylie
Contributor

I think that will result in sourceOnShow implementations breaking. As a quick fix and until there is a better solution you can have a look at the code that I've implemented.

At the top of p.init but after the declaration of m insert:

m.cleanElement = m.element.innerHTML;

and then in the loadWithOnShow sub insert:

if (!me.val('sourceOnShow') && me.withOnShowLoaded) {
    ko.cleanNode($(me.element));
   $(me.element).empty();
   me.element.innerHTML = me.cleanElement;
}

and move the with onShowLoaded to the bottom so that it looks like:

        p.loadWithOnShow = function () {
            var me = this;
            if (!me.withOnShowLoaded || me.val('sourceCache') !== true) {
                me.val('withOnShow')(function (vm) {
                    if (!me.val('sourceOnShow') && me.withOnShowLoaded) {
                        ko.cleanNode($(me.element));
                        $(me.element).empty();
                        me.element.innerHTML = me.cleanElement;
                    }
                    var childBindingContext = me.bindingContext.createChildContext(vm);
                    me.ctx = vm;
                    // replace the childBindingContext
                    me.childBindingContext = childBindingContext;
                    me.augmentContext();
                    ko.utils.extend(childBindingContext, {$page: me});
                    applyBindingsToDescendants(me);
                    me.showElementWrapper();
                    // what is signaling if a page is active or not?
                    if (me.route) {
                        me.childManager.showChild(me.route);
                    }
                }, me);
                me.withOnShowLoaded = true;
            } else {
                me.showElementWrapper();
            }
        };

ps. I consider this to be a bit nasty and I'm looking into a better way to do it. Alternatively you could implement sourceOnShow for the page. I'll post back if I come up with a better solution or let me know if you come up with something good.

@finnsson finnsson closed this in cf75f5e Nov 15, 2013
@finnsson
Owner

Thanks @CuinnWylie. I took your solution.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment