#1921 Changed wpf implementation to use view model parameter sent wit…

[Bowman74]

…h request by new navigation service. Updated wfp event hooks sample application to be more complete.

✨ What kind of change does this PR introduce? (Bug fix, feature, docs update...)

Bug Fix

⤵️ What is the current behavior?

When using the new navigation service the view model created with the request is ignored, instead the original ShowViewModel behavior is used creating a second view model ignoring the one from the request that has the parameter passed to it.

🆕 What is the new behavior (if this is a feature change)?

Only one view model is created when the new navigation service is used and the parameter properly passed to it.

💥 Does this PR introduce a breaking change?

No

🐛 Recommendations for testing

Test in a variety of scenarios for navigation. I tested about three of the many possible scenarios

📝 Links to relevant issues/docs

#1921, #1943

🤔 Checklist before submitting

• All projects build
• Follows style guide lines (code style guide)
• Relevant documentation was updated (docs style guide)
• Nuspec files were updated (when applicable)
• Rebased onto current develop

[mention-bot]

@Bowman74, thanks for your PR! By analyzing the history of the files in this pull request, we identified @slodge, @martijn00 and @Cheesebaron to be potential reviewers.

[nickrandolph]

We're seeing a similar problem with using Master-Details in a Forms app when navigating between details pages. An alternative to this implementation is to move the check for existing view model into the LoadViewModel method - this would fix this issue for all platforms.

[martijn00]

@nickrandolph Hmm you might be right about that one! We have to look into it.

[nickrandolph]

I just tried this, which seems to solve this issue (in MvxPresenterHelpers), but this is specific to Mvvmcross for Forms. Wonder if it could be pushed into the viewModelLoader?

public static IMvxViewModel LoadViewModel(MvxViewModelRequest request)
{
if (request is MvxViewModelInstanceRequest instance) return instance.ViewModelInstance
var viewModelLoader = Mvx.Resolve();
var viewModel = viewModelLoader.LoadViewModel(request, null);
return viewModel;
}

[Bowman74]

@nickrandolph That would not work for all platforms. Not all platforms can pass the request instance to the view and instead keep the view model in a cache location that needs to be handled completely differently with the position in the cache passed to the view instead (UWP and Android both have variations on this).

[nickrandolph]

Yep fair enough. I didn't have time to do a full test on that suggestion (hence no pull request).

This issue is actually pretty severe as it is completely screwing up navigation on Forms using Master Details. You can see this if you attempt to navigate to a different view model from one of the details view models in the forms master-detail sample project

[Bowman74]

Unfortunately about to board a flight. I can look at that problem later. I suspect there may be a custom presenter in play that may need modification. Can you send me a link to the sample (just to be sure we are looking at the same one)?

[Bowman74]

@nickrandolph, I do not see a master detail example under TestProjects\Forms\ directory for WPF? Is there some WPF example in another location? Nor do I even find any MvvmCross support for Forms in general for WPF?

If not this PR only impacts WPF and is intended to fix a WFP specific bug so any separate issues should likely be handled under another issue.

[Cheesebaron]

There is no WPF support from Xamarin.Forms to begin with. So dunno how what @nickrandolph talks about relates to this issue.

[kjeremy]

Not yet, but I believe they're working on it xamarin/Xamarin.Forms#895

[Cheesebaron]

@kjeremy well still not related to this issue though. @nickrandolph if you have an issue with Forms, make a new issue and just link to this.

[Cheesebaron]

Seems to work as advertised. Thanks!

[nickrandolph]

I'll raise another issue later today but my observation stands. The issue which is fixed by this commit seems to be mirrored (and admittedly I don't know the code base well enough to know whether the root cause is the same) by breaking functionality when using Mvvmcross for forms with master detail.

[Bowman74]

@nickrandolph Yes a new issue would be great. If you look at what files this commit changes they are all in WPF specific projects which are not used for the Forms projects in any way, shape or form.