{"action":"created","issue":{"url":"https://api.github.com/repos/Windows-XAML/Template10/issues/629","repository_url":"https://api.github.com/repos/Windows-XAML/Template10","labels_url":"https://api.github.com/repos/Windows-XAML/Template10/issues/629/labels{/name}","comments_url":"https://api.github.com/repos/Windows-XAML/Template10/issues/629/comments","events_url":"https://api.github.com/repos/Windows-XAML/Template10/issues/629/events","html_url":"https://github.com/Windows-XAML/Template10/pull/629","id":131959371,"number":629,"title":"Supporting Proxied/Wrapped ViewModels","user":{"login":"1iveowl","id":12871120,"avatar_url":"https://avatars.githubusercontent.com/u/12871120?v=3","gravatar_id":"","url":"https://api.github.com/users/1iveowl","html_url":"https://github.com/1iveowl","followers_url":"https://api.github.com/users/1iveowl/followers","following_url":"https://api.github.com/users/1iveowl/following{/other_user}","gists_url":"https://api.github.com/users/1iveowl/gists{/gist_id}","starred_url":"https://api.github.com/users/1iveowl/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/1iveowl/subscriptions","organizations_url":"https://api.github.com/users/1iveowl/orgs","repos_url":"https://api.github.com/users/1iveowl/repos","events_url":"https://api.github.com/users/1iveowl/events{/privacy}","received_events_url":"https://api.github.com/users/1iveowl/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":10,"created_at":"2016-02-07T11:16:05Z","updated_at":"2016-02-10T22:09:51Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/Windows-XAML/Template10/pulls/629","html_url":"https://github.com/Windows-XAML/Template10/pull/629","diff_url":"https://github.com/Windows-XAML/Template10/pull/629.diff","patch_url":"https://github.com/Windows-XAML/Template10/pull/629.patch"},"body":"Please read this carefully this before you dismiss this PR? The comments, I got the previous two times I tried left me with the feeling that we are not entirely on the same page, so I try once more.\r\n\r\nFrom our conversation I get sense is that the idea of wrapped/proxied ViewModels feels unfamiliar. I get that; nevertheless a MVVM pattern using this approach such as that supported by Assisticant (https://www.nuget.org/packages/Assisticant/) have been downloaded ~3000 times so “yikes”, there are other “crazy”, “complicated” people out there ;-)\r\n  \r\nFor me this pattern helps make ViewModels cleaner. I don’t like complexity either.\r\n\r\nThe changes proposed are:\r\n\r\n**NavigationService.cs:**\r\n\r\n#590 addressed NavigateToAsync only. However, I see that `page.DataContext as INavigable` since yesterday is now also used in NavigateFromAsync and NavigatedFromAsync.\r\n\r\nI’m glad we agree that this is an improvement over checking DataContext for null, as all three methods are in fact assuming INavigable to work as intended. The new code is an improvement, but it is still insufficient for dealing with wrapped ViewModels. \r\n\r\nUsing ResolveForPage is indeed the right approach and I do apologies that my first suggestion did not recognize this. Now, let’s make sure that all three methods do in fact use ResolveForPage to deal with situations where DataContext is not INavigable. Currently only NativateToAsync does. The two others just ignore any further action if DataContext is not INavigable. \r\n\r\nBecause all three methods need to get hold of the right INavigable DataContext/ViewModel, I think the best approach is to create one single private method called “ResolveForPage(Page page)” in NavigationService.cs. The rational is reducing complexity – i.e. write the code once, not thrice. \r\n\r\nPlease notice that I’m passed the **Page object** and not just the Type of the page to ResolveForPage. There is an important reason for that, which leads me to the changes proposed to Bootstrapper.cs.\r\n\r\n**Bootstrapper.cs.**\r\n\r\nHere I propose to overload the ResolveForPage with a new method signature that accepts the **Page object**: `public virtual INavigable ResolveForPage(Page page, NavigationService navigationService)`\r\n\r\nIn the PR the new overloaded ResolveForPage gets the Type of the page and then passes the Type on to the original ResolveForPage method. This is just to ensure that any current implementation that might be overriding the original ResolveForPage(Type type…) is unaffected by the change. \r\n\r\nBut why overload the ResolveForPage? Because, this new overloaded ResolveForPage(Page page…) is what enables anyone working with wrapped ViewModels gain access to the Page object so that they can write code to upwrap the DataContext in order to expose the inner object that satisfy the INavigable assumption needed for the NavigationService. The existing ResoveForPage(Type type…) is insufficient for this purpose. \r\n\r\nJust as an example: using Assisticant with the proposed modifications would look something like this:\r\n\r\n    public class BootStrapperEx : BootStrapper\r\n    {\r\n\r\n        public override INavigable ResolveForPage(Page page, NavigationService navigationService)\r\n        {\r\n            return ForView.Unwrap<dynamic>(page.DataContext) as INavigable;\r\n        }\r\n\r\n    }\r\n\r\nI’ve tested it and this works as intended. And just as importantly it opens Template10 to the exotic bread of developers who like to wrap their ViewModels. We are people too you know ;-)\r\n\r\nOh and finally, I don’t see this breaking anything.\r\n\r\nThank you for your understanding."},"comment":{"url":"https://api.github.com/repos/Windows-XAML/Template10/issues/comments/182602264","html_url":"https://github.com/Windows-XAML/Template10/pull/629#issuecomment-182602264","issue_url":"https://api.github.com/repos/Windows-XAML/Template10/issues/629","id":182602264,"user":{"login":"1iveowl","id":12871120,"avatar_url":"https://avatars.githubusercontent.com/u/12871120?v=3","gravatar_id":"","url":"https://api.github.com/users/1iveowl","html_url":"https://github.com/1iveowl","followers_url":"https://api.github.com/users/1iveowl/followers","following_url":"https://api.github.com/users/1iveowl/following{/other_user}","gists_url":"https://api.github.com/users/1iveowl/gists{/gist_id}","starred_url":"https://api.github.com/users/1iveowl/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/1iveowl/subscriptions","organizations_url":"https://api.github.com/users/1iveowl/orgs","repos_url":"https://api.github.com/users/1iveowl/repos","events_url":"https://api.github.com/users/1iveowl/events{/privacy}","received_events_url":"https://api.github.com/users/1iveowl/received_events","type":"User","site_admin":false},"created_at":"2016-02-10T22:09:51Z","updated_at":"2016-02-10T22:09:51Z","body":"Thanks for the input. Sound interesting. \r\n\r\nCan you be more specific about what you mean by \"be more gentle about it\"? Is that not to throw an exception or something different? I'm not familiar with the Caliburn.Micro implementation."}}