Prism: [Enhancement] Support Dynamic Partial Views

[andreinitescu]

Description

Partial view is a great feature!

However, I noticed some critical issues:

1. The way OnAutowirePartialViewChanged is implemented when the AutowirePartialView attached property is set to null on a partial view, is to set Page PartialViewsProperty to null, removing all other partial views from the Page:

    private static void OnAutowirePartialViewChanged(BindableObject bindable, object oldValue, object newValue)
    {
        if (oldValue == newValue)
            return;

        if(oldValue is Page oldPage)
        {
            var oldPartials = (List<BindableObject>)oldPage.GetValue(PartialViewsProperty);
            oldPartials?.Clear();
            oldPage.SetValue(PartialViewsProperty, null);
        }
     ....

See https://github.com/PrismLibrary/Prism/blob/master/Source/Xamarin/Prism.Forms/Mvvm/ViewModelLocator.cs#L70

1. The Page holds a strong reference to the partial views.
   If a partial view needs to be removed while Page is displayed, an instance to the partial view is kept by the Page. Just setting null to the AutowirePartialView on the partial view doesn't work, we stumble into the previous issue. PrismPartialViewsProperty is internal so I can't read the property and remove the partial view from the collection.
   Maybe a solution is to have PrismPartialViewsProperty use a list of WeakReference to the partial views. The list can be purged each time when hitting the property.

2. Adding a partial view dynamically (like in the following example or using DataTemplate/ControlTemplate) doesn't fire INavigatedAware on the view-model of the partial view:

        private void Button_Clicked_1(object sender, System.EventArgs e)
        {
            var myView = new MyView();
            ViewModelLocator.SetAutowirePartialView(myView, this);
            _layout.Children.Add(myView);
        }
 ```

I understand why this happens, but for the view-model of a partial view, which follows same Prism pattern as for Page, not getting the methods called is a problem, the view-model needs to be initialized.

Maybe there should be a new interface meant to notify the view-model of the Page that partial view-model has been created? This way the Page view-model  could call the partial view model to initialize.

interface IPartialViewModelAware
{
void OnPartialViewModelCreated(IPartialViewModel partialViewModel);
}

interface IPartialViewModel
{
void Initialize(object context);
}

  Something similar needs also to be done for "un-initializing".... (but what does un-initialize means, does it mean when partial view gets removed from the view?)

<!-- REQUIRED -->
<!-- Issues reporting a bug, but lacking a Reproduction will be closed!
     Please ask questions on StackOverflow or on Slack. Issues opened
     that are questions will be closed without comment.  -->

### Steps to Reproduce

1. Add a partial view:

x:Name="_myView" />

2. Remove partial view on a clicked event handler implemented in the Page:

public partial class MainPage : ContentPage
{
public MainPage()
{
InitializeComponent();
}

    private void Button_Clicked(object sender, System.EventArgs e)
    {
        (_myView2.Parent as Layout<View>).Children.Remove(_myView2);
    }
}

3. When navigating to a different Page, notice how MyView is still called:

class MyViewModel : BindableObject, INavigatedAware
{
    public MyViewModel()
    {
    }

    public void OnNavigatedFrom(INavigationParameters parameters)
    {
    }

    public void OnNavigatedTo(INavigationParameters parameters)
    {
    }
}

```

Expected Behavior

MyViewModel should not be called, MyView should not be kept in memory after removal from parent

Actual Behavior

MyView is kept in memory

Basic Information

• Version with issue: (latest released) 7.1.0.431
• Last known good version:
• Xamarin.Forms version: 3.6.0.220655
• IDE: VS2017

[andreinitescu]

If project owners give feedback and we agree on how things need to be fixed, I could help with a pull request. I could just do a pull-request but without discussion it might end be a waste of effort.

[andreinitescu]

I added a 3rd issue.

[dansiegel]

@andreinitescu do you the results of a profiler to go along with this? It should be getting cleaned up.

[andreinitescu]

@dansiegel This isn't related to leaking if that's what you're referring to

[dansiegel]

@andreinitescu sorry the way I read this was that you were describing a memory leak... I think I understand your issue now... you seem to be using multiple partial views on a page and have the need to add/remove these views Dynamically... does that sound about right?

[andreinitescu]

Correct. And there are different issues related to having dynamically added partial views, which I tried to describe. Let me know if something is not clear

[andreinitescu]

I think this feature was designed only with static partial views, meaning child views which are never removed from the Page.

[dansiegel]

You are correct... it was only architected with static Partial Views in mind... Now that I have a better understanding of your issue I do have some ideas around how we can support this.

[dansiegel]

I do want to point out here that any support we do add will simply let you add/remove them at run time... it will be on you to initialize them as we won't be able to pass in NavigationParameters after the fact... and OnAppearing which comes from the Page will have already fired.

[andreinitescu]

I wonder, am I the only one who stumbled on this? How's everyone else doing dynamic partial views? Usually when it seems it's just you, it might be a sign you're doing something wrong :)

it will be on you to initialize them as we won't be able to pass in NavigationParameters after the fact... and OnAppearing which comes from the Page will have already fired.

Right. Which make sense to some extent, INavigationAware/INavigatedAware are for pages. Which is something I mentioned above.
Buuut... While I'm aware of IActiveAware pattern, the thing is, in practice, you need a way to pass data and initialize partial view-models.
For this, I wonder how about adding some new pattern/interfaces? See my IPartialViewModelAware / IPartialViewModel idea from above.

[dansiegel]

what you're beginning to describe goes beyond the scope of Partial Views, and ventures into the depths of Regions... which is something I would like to add support for generally... but it's pretty complex subject that will mean it's several months away... in the mean time I'd suggest you perhaps use the EventAggregator to pass some context around.

[andreinitescu]

While regions involve quite complex scenarios, why not use this opportunity to come up with something much more simpler?
I am thinking about simple interfaces and a custom behavior on partial views (similar to existing behaviors) which call methods when child is added/removed from a parent view.
We can monitor for example the "Parent" property on a partial view to know when view is being added/removed. This can give the opportunity to ask the "host" view of the partial view (the host can be any view, not just Page, it can be a value set by a custom attached property on the partial view) to call an initialization/un-initializationmethod on the view/view-model (something like IPartialAware.Initialize(initParams))
What do you think?

[dansiegel]

We can do the "PartialView" support because the intent is really about it being more static. When you start getting into how do we initialize etc like I said this is really Region territory and that is the only real way we're going to be looking at to support what you're talking about.

For the part of your issue that has to do with allowing you to unhook a single Partial View without clearing out the rest of them I'll leave this issue open. For the other part of your issue you'll want to track #1072

[scharada]

@andreinitescu no you are not alone on this. there are quiet few questions out there regarding the subject .. i have a "pseudo" solution for classic forms project but which works fine and is very fast and clean but it is neither the wished partialview nor does it work with prism which is the framework i used in my project.
sometimes, these limitations make me rethink about the choices i made. i am facing that issue now with the new XF shell feature which has much of the navigation goodies that prism offers. i wonder if there is a plan to support this in prism and when. this would be a tough decision that i will have to make very soon.

@dansiegel prism has helped a lot of people since its birth. no doubt about it. however, for people who have chosen it to build large projects, features like partialviews becomes a key element among others. that's something that makes a whole lot of difference.
it also becomes crucial to have a roadmap to support major platform changes such as shell ... (unless you have that planned and i missed it, then please point me to it)

thanks

[scharada]

probably spoke too soon about Shell support. just saw this and will give it a try.
https://github.com/dansiegel/ShellAppTemplate

[dansiegel]

@scharada this is off topic. The repo you are referencing has absolutely ZERO to do with Xamarin Forms Shell. In its current state Shell is not something I’m interested in pursuing until such time as the required changes are made by the Xamarin team.

[mackayn]

Funnily enough, ran into a scenario today where regions would have been great, essentially plugins from other modules (ContentViews) need to be dynamically swapped in and out of a container, not when the page is built.

All I could think of was doing was having the views in a carousel or layout and show then if enabled but the modules should register they have a plug in ContentView and they are registered in a collection of views and rendered on demand, have to see if the memory management will be a problem, it's a bit like scenario mentioned by @andreinitescu

Partial view support is great though, use it all the time, twice yesterday to quickly prototype different navigation schemes for an app.

[dansiegel]

@andreinitescu if you want to put together a PR on this I'm happy to review and look to merge, but I won't have time for the foreseeable future to work on this issue specifically. Just please keep it limited in scope. A lot of what you're wanting will be better served from Regions.

We will need tests to ensure:

• When a page is popped off the NavigationStack (i.e. GoBack) all of the partial Views are removed and there isn't a memory leak caused
• That the scenario you are specifically looking to support here where you're directly managing different partial Views is supported without clearing everything.

[dinisvieira-xo]

I also run into a scenario today where I need to dynamically add/remove PartialViews from a Page.
In my case the parent Page already has the responsibility of taking care of the initialization/un-initialization as needed (which is the 3rd issue I think).
@andreinitescu Do you still think the WeakReference would be a good choice to fix your 2nd issue? (which I think is also enough to fix the issue I'm having)

[dinisvieira-xo]

@dansiegel Just thinking, simply creating a public static method for removing a PartialView from the page it's attached to is something that could be added to the ViewModelLocator?
Something like:

        public static void RemoveAutoWirePartialView(BindableObject bindable)
        {
            var page = (Page)bindable.GetValue(AutowirePartialViewProperty);
            var partialViews = page.GetValue(PartialViewsProperty) as List<BindableObject>;
            partialViews.Remove(bindable);
        }

If not, do you have any guidance on how I can implement this?

[mackayn]

@andreinitescu

Ran into the same issue, wrote a container you can navigate regions in and out and drop it into a page, works well but all the partials views are retained in memory until the pages pops, then all the view get garbage collected fine as I had implemented IDestructible and took care of unhooking behaviors etc.

Would be cool if we could have a quick and dirty way to unhook partials so they can be garbage collected and we could manually take care of handling IDestructible and passing navigation parameters.

Mine works like this:

  public override async void OnNavigatedTo(INavigationParameters parameters)
        {
            base.OnNavigatedTo(parameters);
            Initialised = true;
            await _regionService.NavigateToRegion(KnownRegions.MyPictures, parameters);
        }

[mr5z]

Same here. My solution for this is to manually switch the View of ContentView in the code-behind since there is a noticeable UI bug from Xamarin which is too long to explain. I'm sure the solution should be coming from Xamarin Forms itself but just wanted support that this idea is in need.

[andreinitescu]

@mr5z What's the "noticeable UI bug when you switch the View of ContentView" ?

[dansiegel]

@andreinitescu I'm not sure I can get to this very soon. If you're able to do take on this issue we'd love to add the PR. We do need to validate that we don't leak memory which was the reason why we were trashing all of the Partial Views. Feel free to DM me if you have any questions.

[andreinitescu]

Did some work on this weeks ago, here's the PR #1843

Even if it doesn't solve all the issues described here, it gives a possibility for to support dynamic partial views easier.

[dansiegel]

fixed by #1843

[lock[bot]]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.