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

with on page is not picking up changes #75

Closed
tedsteen opened this issue Nov 20, 2012 · 8 comments
Closed

with on page is not picking up changes #75

tedsteen opened this issue Nov 20, 2012 · 8 comments
Labels

Comments

@tedsteen
Copy link
Contributor

See this fiddle http://jsfiddle.net/xkCea/10/

When changing the with observable pagerjs with attribute on pages does not pickup the change as knockouts native with does.

  1. look at page "broken" by clicking "broken"
  2. click "change to VM2"
  3. Wrong - the context in the broken page is not updated with the new value.
  4. Right - the context in the page "working is.
@finnsson
Copy link
Owner

Hi,

the fiddle is working now. I'm not entirely sure how the internals of KnockoutJS is working. The solution in pager is somewhat different from the solution in KO's with-binding, but seems to work (pager is updating $data when the view model is changed, but leave $parent, $root, $page, etc untouched).

@tedsteen
Copy link
Contributor Author

tedsteen commented Dec 2, 2012

Yes the fiddle works fine but the old exponentially increasing mystery bug seems to be back again. This fiddle shows the problem
http://jsfiddle.net/xkCea/27/
Changing between the pages (klicking on Goto1, Goto2 etc. makes the sub pages in the page using a dynamic "with" grow exponentially.

I don't think this bug was introduced when this issue (75) was fixed, because I have seen the same behaviour whenever I re-render nodes containing pages f.ex using
<-- ko with: expr --><div data-bind="page:{...}">...</div><!-- /ko -->
or
<-- ko if: expr --><div data-bind="page:{...}">...</div><!-- /ko -->

It seems like if a page-binding is re-rendered something strange happens. That's my gut feeling anyway.

@tedsteen
Copy link
Contributor Author

tedsteen commented Dec 2, 2012

And with three sub-pages: http://jsfiddle.net/uL7AW/1/
Only one sub-page works fine tho... http://jsfiddle.net/uL7AW/

So each sub-page grows in number proportional to the total number of subpages?
Något att bita i.. :)

@finnsson
Copy link
Owner

finnsson commented Dec 5, 2012

I've posted a question (https://groups.google.com/d/topic/knockoutjs/k8OJk8dTJ7E/discussion) at the KnockoutJS forum about how I'm supposed to implement the with-binding.

@tedsteen
Copy link
Contributor Author

tedsteen commented Dec 7, 2012

I'm a little curious about your thoughs on using pagerjs like this, I could explain my use case if you want me to.
To keep a long story short; http://domain.com/show/player/playerId123/ playerid123 will match a wildcard and setup a new viewmodel for that page bound using the pager with-binding. This is where things break.

finnsson added a commit that referenced this issue Dec 8, 2012
@finnsson
Copy link
Owner

finnsson commented Dec 8, 2012

OK. I think with might be fixed now. I'll have to make an overview of withOnShow too. Combining an observable/computed with with source or sourceOnShow must also be reviewed.

@finnsson
Copy link
Owner

finnsson commented Dec 8, 2012

Regarding using the wildcard page I think I might need to supply a better way to handle your use case. I see other developers are using the wildcard pages in the same way while in reality you probably want different element-instances for the different sub-pages.

@finnsson
Copy link
Owner

I'm sorry so say that the current version of KO makes it very difficult for me to support with-bindings where the view model can be changed (the edge case above works, but withOnShow, iframes, etc does not...). The problem is that KO does not really support observable view models when creating new contexts. There are two alternatives: use Michael Bests fork of KnockoutJs or wait for KnockoutJS 2.3 (the feature is scheduled for the v2.3 release).

@DKhalil DKhalil closed this as completed May 15, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants