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

Setting ViewModel property on a Fragment is not correct #636

Closed
dustin-graham opened this Issue Mar 20, 2014 · 8 comments

Comments

Projects
None yet
5 participants
@dustin-graham

dustin-graham commented Mar 20, 2014

Hello,
I'm just getting into MvvmCross and coming from several years of professional Android development. I noticed a pattern that is used in the fragging plugin and in almost all the other examples from the community I've seen thus far on Fragments which is incorrect as far as Android development is concerned. It has to do with the way the correct ViewModel for a fragment is found and then set to the ViewModel property on the fragment. Something like:

var viewModelLoader = Mvx.Resolve();
fragment.ViewModel = viewModelLoader.LoadViewModel(request, null);

This is incorrect because Fragments are managed by a FragmentManager and each Fragments lifecycle is at the mercy of that manager and Android. At any time your Fragment may be destroyed and the FragmentManager's job is to recreate it. When it gets recreated it will no longer have the ViewModel you gave it and this will cause problems in the program. This pattern works in ideal situations but will break in many others. The way around this sort of a thing in native development is to provide the Fragment what it needs to operate inside of an argument Bundle and then the Fragment can then request all other resources from the environment. I've been prototyping some things along this line with a bit of success but I'd like to see what your thoughts are on this. The partial solution that I've put together so far is to have an interface on my Activity called IFragmentHost which has a method for getting the MvxViewModel for a fragment tag. Then on my ViewModel, I have a dictionary of "Sub" ViewModel's keyed by the same tags. So my Activity will obtain the proper SubViewModel from it's ViewModel and give return it to the Fragment when the Fragment requests it during it's initialization phases of it's lifecycle (currently during onResume works best). This isn't terribly cross-platform yet but this should guarantee that a fragment, dynamic or static, will be able to get it's appropriate ViewModel no matter what Android decides to do. Thoughts?

@slodge

This comment has been minimized.

Contributor

slodge commented Mar 21, 2014

Thanks for the issue.

The main answer is that we've talked about this area many times. I completely agree that the framework currently leaves this area open for users to solve in their apps.

In some apps, I've personally plugged the hole by using a "cache" in the parent Activity - which might be similar to your IFragmentHost approach - and also by using code which hooks into the Activity/Fragment lifecycles. (When I have to properly support Android rotation, it always makes my day!)

Personally, I'd still like us to investigate some full solutions for this area and to come up with some "best practice" advice - which may include code changes too. Ideally I'd like to revisit Mvx's support around savedInstanceState - it feels like we could do much better in this area...

It sounds like you have some good experience and some good ideas here... very happy to look at how to integrate them.

Thanks again

Stuart

@nihilnihilnihil

This comment has been minimized.

nihilnihilnihil commented Jul 3, 2014

I'd love to see some best practice advice around fragment handling in MvvmCross - I've openend a Stack Overflow question here: http://stackoverflow.com/questions/24546571/mvvmcross-proper-configuration-change-handling-in-android

@martijn00

This comment has been minimized.

Member

martijn00 commented Aug 9, 2015

This is fixed in the MvxCachingFragmentActivity. See this sample: https://github.com/MvvmCross/MvvmCross-AndroidSupport/tree/master/Samples

@martijn00 martijn00 closed this Aug 9, 2015

@martijn00 martijn00 added this to the 4.0.0 milestone Aug 11, 2015

@andresdsep

This comment has been minimized.

andresdsep commented Jan 5, 2016

Has this been updated in the nuget package? MvvmCross.Droid.Support.V7.Fragging.Caching doesn't seem to exist in 4.0.0-beta7

@martijn00

This comment has been minimized.

Member

martijn00 commented Jan 5, 2016

You need to add the MvvmCross Android-Support Fragging package to be able to use this in beta7.

@andresdsep

This comment has been minimized.

andresdsep commented Jan 6, 2016

This is what I have installed https://www.nuget.org/packages/Cirrious.MvvmCross.Droid.Support.Fragging/4.0.0-beta7/.

Am I doing something wrong?

@martijn00

This comment has been minimized.

Member

martijn00 commented Jan 6, 2016

The sample is a bit further then the beta7 package. On the next nuget update this will be aligned again. For now you can look at the commit history to see the previous sample.

@andresdsep

This comment has been minimized.

andresdsep commented Jan 7, 2016

Thanks man :)

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