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

Access to Page in withOnShow handler #27

Closed
Shildrak opened this Issue Sep 6, 2012 · 2 comments

Comments

Projects
None yet
2 participants
@Shildrak
Contributor

Shildrak commented Sep 6, 2012

First, thanks for creating a really nice library!

It would be nice to have access to the Page object when dynamically fetching viewmodels in "withOnShow". I modified the source for withOnShow to pass "this", just like you are currently doing for sourceOnShow:
I replaced
572 - }, this));
572 + }, this), this);

This allow me to retrieve specific data for my views based on the route, eg

http://domain.name.com/order/56

 <div data-bind="page: {id: '?', withOnShow: MyNamespace.Order.GetModel()}"></div>

MyNamespace.Order = {
    GetModel: function () {
        return function (callback, page) {
            $.getJSON('/api/orders/' + page.currentId, function (data) {
                var viewModel = new MyNamespace.Order.ViewModel(data);
                callback(viewModel);
            });
        };
    }
};

Off course, ability to use wildcards would be even better, to say something like:

 <div data-bind="page: {id: '?', withOnShow: MyNamespace.Order.GetModel({1})}"></div>

But I don't think this is an easy addition? However, I hope you will consider adding the page parameter as described above.

@finnsson

This comment has been minimized.

Show comment
Hide comment
@finnsson

finnsson Sep 7, 2012

Owner

Hi,

I checked out your fork and found that you had another interesting addition (the cache control). Reminds me that I need to review all the parameters in different callbacks (withOnShow etc) and try to unify their parameters. At the moment it's basically accidental design which parameters are sent to a callback... .

I fetched your repo and merged with my repo so now your changes are in. Thanks for sharing!

-- Oscar

Owner

finnsson commented Sep 7, 2012

Hi,

I checked out your fork and found that you had another interesting addition (the cache control). Reminds me that I need to review all the parameters in different callbacks (withOnShow etc) and try to unify their parameters. At the moment it's basically accidental design which parameters are sent to a callback... .

I fetched your repo and merged with my repo so now your changes are in. Thanks for sharing!

-- Oscar

@Shildrak

This comment has been minimized.

Show comment
Hide comment
@Shildrak

Shildrak Sep 7, 2012

Contributor

Thanks for merging the changing into the master branch. That also makes it easier for me to stay up-to-date with your library.

Contributor

Shildrak commented Sep 7, 2012

Thanks for merging the changing into the master branch. That also makes it easier for me to stay up-to-date with your library.

@Shildrak Shildrak closed this Sep 7, 2012

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