[BUG] Popup doesn't work with hot reload

[mavispuford]

Is there an existing issue for this?

• I have searched the existing issues

Current Behavior

Popup xaml changes aren't reflected with hot reload. I'm not sure if this was part of the original intended functionality, but it can be quite tedious to iterate on Popup design changes.

Expected Behavior

Popup xaml changes should ideally be reflected during runtime, just like other Views in .NET MAUI.

Steps To Reproduce

1. Open and run the solution in the project repository
2. Click the button to show a popup
3. Make changes in the popup's xaml during runtime
4. Observe that your changes have not been reflected

Link to public reproduction project repository

https://github.com/mavispuford/PopupHotReloadNotWorking

Environment

- .NET MAUI CommunityToolkit: 1.2.0
- OS:
Edition	Windows 10 Enterprise
Version	20H2
OS build	19042.1889
Experience	Windows Feature Experience Pack 120.2212.4180.0
- .NET MAUI: 6.0.486

Anything else?

I've also reproduced this bug in Android. I haven't tested platforms outside of Windows/Android.

[pictos]

Hey @drasticactions, in an overview can you tell if this is on us?

[mavispuford]

I got a bit curious and looked at the .NET MAUI source code. Looks like classes have to implement IHotReloadableView in order to be reloaded.

The View class implements IHotReloadableView.

Same with ContentPage.

See also:

• https://github.com/dotnet/maui/blob/10be7d53f9fc8b3c7e1bc99fc6eb8528289b7085/src/Core/src/HotReload/HotReloadHelper.cs
• https://github.com/dotnet/maui/tree/10be7d53f9fc8b3c7e1bc99fc6eb8528289b7085/src/Core/src/HotReload

[drasticactions]

@mavispuford IHotReloadabieView has nothing to do with XAML Hot Reload, it has to do with Comet. Implementing it won't do anything for XAML Hot Reload.

@pictos For XAML Hot Reload to work, you need to implement IVisualTreeElement, https://github.com/dotnet/maui/blob/main/src/Core/src/Core/IVisualTreeElement.cs

Most base MAUI controls (Apart from ListView) implement this out of the box. XAML Hot Reload checks if the control implements this and uses it to then go through the children. If your control doesn't implement this, it will not show up in the Live Visual Tree, and will then not be tracked at all for XAML Hot Reload.

[pictos]

@drasticactions. Thanks for the answer, I'll look further to implement this.

And all the views should implement this?

[drasticactions]

It depends on the control, but generally you should only need to place it on the top most level, unless the control handles child elements in a weird way. Basically, if you don't see the child elements in the Live Visual Tree, it is not tracked and can't be hot reloaded.

You may also need to make sure whenever the popup is invoked, that VisualDiagnostics OnVisualTree changes are fired, https://github.com/dotnet/maui/blob/main/src/Core/src/VisualDiagnostics/VisualDiagnostics.cs#L77-L80

[drasticactions]

Also, I would leave an issue in the MAUI repo about docs for this, since none of that is obvious unless you're me, lol.

[pictos]

since none of that is obvious unless you're me, lol.

or if the person know that you know these secrets xP

I'll find time to fill out an issue

[LennoxP90]

has any changes happened for this?

[pictos]

@LennoxP90 not yet

[czmirek]

@drasticactions So, I'm trying to create a workaround and implement the Hot Reload support for my specific popup directly using your instructions...but I can't make it work.

I tried to look inside the MAUI source code for other controls but I'm kinda lost....any advice or push in some direction would be greatly appreciated.

public partial class MySpecificPopup : Popup, IVisualTreeElement
{
    public PoiDetailPopup()
    {
        InitializeComponent();
        VisualDiagnostics.OnChildAdded(Application.Current!.MainPage!, this);
    }

    IReadOnlyList<IVisualTreeElement> IVisualTreeElement.GetVisualChildren() => new List<IVisualTreeElement>() { this.Content! }.AsReadOnly();
    IVisualTreeElement? IVisualTreeElement.GetVisualParent() => Application.Current!.MainPage!;
}

[drasticactions]

Popup needs to be visible within MAUIs Visual Tree, and for that you need to register your control with VisualDiagnostics.OnChildAdded and VisualDiagnostics.OnChildRemoved. But how you would invoke it is tough to tell...

https://github.com/dotnet/maui/pull/14006/files

You can see this in this PR. You need to get your control into MAUIs visual tree, or else Hot Reload can't work since it's not visible to it. In your case, I don't know what the right approach is. I would guess that something needs to add Popup to the Page Children when it gets invoked and removed when you dismiss it. @PureWeen do you have any thoughts?

[czmirek]

I just want to clarify that I have the simplest use case, just simply showing the Popup on Android using PopupExtensions.ShowPopup in the page like this.

public partial class MyPage : ContentPage
{
	public MyPage()
	{
		InitializeComponent();
		this.ShowPopup(new MyPopup());
    }
}

Here I can see that the page is set as the Popup parent but I guess that does not translate directly into adding it to the Visual Tree. Not sure how that would work ....I thought that ContentPage can have a single child only.

[PureWeen]

Popup needs to be visible within MAUIs Visual Tree, and for that you need to register your control with VisualDiagnostics.OnChildAdded and VisualDiagnostics.OnChildRemoved. But how you would invoke it is tough to tell...

https://github.com/dotnet/maui/pull/14006/files

You can see this in this PR. You need to get your control into MAUIs visual tree, or else Hot Reload can't work since it's not visible to it. In your case, I don't know what the right approach is. I would guess that something needs to add Popup to the Page Children when it gets invoked and removed when you dismiss it. @PureWeen do you have any thoughts?

This circles back to what we've been chatting about.

Currently adding LogicalChildren to Page is nearly impossible.
Basically, what you're doing for TitleView we need a way for external libraries to do the same thing.

I worked with @pictos a little bit to try and hack it together but we weren't super successful.

[BretJohnson]

Now that LogicalChildren APIs are in MAUI .NET8, calling addlogicalchildren should make this work. But as that's a .NET8 only API, it'll need a .NET8 version of the communitytoolkit, which we don't yet have. So that's the next step, I believe, for this issue - multitargeting to .NET8.

[Redth]

While I wouldn't normally suggest this approach, given .NET 7 is not really going to change anything in this area of the code, it is probably safe enough to use some reflection if you really want to multitarget this support. Here's what I used in Maui.VirtualListView before the methods were added in .NET 8:

internal static class LogicalChildrenHelpers
{
	static MethodInfo removeLogicalChildMethod = null;

	internal static void RemoveLogicalChild(this Element parent, IView view)
	{
		if (view is Element elem)
		{
			removeLogicalChildMethod ??= GetLogicalChildMethod(parent, "RemoveLogicalChildInternal", "RemoveLogicalChild");
			removeLogicalChildMethod?.Invoke(parent, new[] { elem });
		}
	}

	static MethodInfo addLogicalChildMethod = null;

	internal static void AddLogicalChild(this Element parent, IView view)
	{
		if (view is Element elem)
		{
			addLogicalChildMethod ??= GetLogicalChildMethod(parent, "AddLogicalChildInternal", "AddLogicalChild");
			addLogicalChildMethod?.Invoke(parent, new[] { elem });
		}
	}

	static MethodInfo GetLogicalChildMethod(Element parent, string internalName, string publicName)
	{
		var internalMethod = parent.GetType().GetMethod(
				internalName,
				BindingFlags.Instance | BindingFlags.NonPublic,
				new[] { typeof(Element) });

		if (internalMethod is null)
		{
			internalMethod = parent.GetType().GetMethod(
				publicName,
				BindingFlags.Instance | BindingFlags.Public,
				new[] { typeof(Element) });
		}

		return internalMethod;
	}
}

[BretJohnson]

While I wouldn't normally suggest this approach, given .NET 7 is not really going to change anything in this area of the code, it is probably safe enough to use some reflection if you really want to multitarget this support. Here's what I used in Maui.VirtualListView before the methods were added in .NET 8:

internal static class LogicalChildrenHelpers
{
	static MethodInfo removeLogicalChildMethod = null;

	internal static void RemoveLogicalChild(this Element parent, IView view)
	{
		if (view is Element elem)
		{
			removeLogicalChildMethod ??= GetLogicalChildMethod(parent, "RemoveLogicalChildInternal", "RemoveLogicalChild");
			removeLogicalChildMethod?.Invoke(parent, new[] { elem });
		}
	}

	static MethodInfo addLogicalChildMethod = null;

	internal static void AddLogicalChild(this Element parent, IView view)
	{
		if (view is Element elem)
		{
			addLogicalChildMethod ??= GetLogicalChildMethod(parent, "AddLogicalChildInternal", "AddLogicalChild");
			addLogicalChildMethod?.Invoke(parent, new[] { elem });
		}
	}

	static MethodInfo GetLogicalChildMethod(Element parent, string internalName, string publicName)
	{
		var internalMethod = parent.GetType().GetMethod(
				internalName,
				BindingFlags.Instance | BindingFlags.NonPublic,
				new[] { typeof(Element) });

		if (internalMethod is null)
		{
			internalMethod = parent.GetType().GetMethod(
				publicName,
				BindingFlags.Instance | BindingFlags.Public,
				new[] { typeof(Element) });
		}

		return internalMethod;
	}
}

This was a good tip. But as the CommunityToolkit no longer supports .NET7 (something I've since learned), my fix PR #1635 just calls the newly exposed .NET8 APIs directly, keeping things simple.