Skip to content

Commit

Permalink
Fix Hot Reload with Popup (#1635)
Browse files Browse the repository at this point in the history
* Fix Hot Reload with Popup

Call AddLogicalChild/RemoveLogicalChild when the popup is
shown/closed so that popups are included in the logical (and visual)
tree, so that Hot Reload works for them. The Live Visual Tree VS
UI now shows popup content as well.

Fixes #620

As Shane explained to me, the rule as of MAUI in .NET8 is that
whenenever Parent is set on an Element, AddLogicalChild should
also be called to include the element in the logical children.
RemoveLogicalChild should be called when the Parent is set to null.
This may be more automatic in .NET9, but for .NET8 that's what's needed
so that the logical tree is kept up to date.

* Remove setting the Parent as that's not needed

Shane pointed out that Add/RemoveLogicalChild also updates the parent,
so there's no need for our code to do that as well.

* Protect against NREs for all calls to AddLogicalChild

---------

Co-authored-by: Brandon Minnick <[email protected]>
Co-authored-by: Pedro Jesus <[email protected]>
  • Loading branch information
3 people authored Feb 17, 2024
1 parent 3c26964 commit 9628467
Show file tree
Hide file tree
Showing 5 changed files with 47 additions and 5 deletions.
32 changes: 32 additions & 0 deletions src/CommunityToolkit.Maui.UnitTests/Views/Popup/PopupTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -323,6 +323,38 @@ public void NullColorThrowsArgumentNullException()
#pragma warning restore CS8625 // Cannot convert null literal to non-nullable reference type.
}

[Fact(Timeout = (int)TestDuration.Short)]
public async Task ShowPopup_IsLogicalChild()
{
var app = Application.Current ?? throw new NullReferenceException();

var page = new ContentPage
{
Content = new Label
{
Text = "Hello there"
}
};

// Make sure that our page will have a Handler
CreateViewHandler<MockPageHandler>(page);

app.MainPage = page;

// Make sure that our popup will have a Handler
CreateElementHandler<MockPopupHandler>(popup);

Assert.NotNull(popup.Handler);
Assert.NotNull(page.Handler);

Assert.Single(page.LogicalChildrenInternal);
page.ShowPopup((MockPopup)popup);
Assert.Equal(2, page.LogicalChildrenInternal.Count);

await((MockPopup)popup).CloseAsync(token: CancellationToken.None);
Assert.Single(page.LogicalChildrenInternal);
}

class MockPopup : Popup
{
public MockPopup()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,10 @@ static PageHandler CreatePageHandler(IPopup virtualView)
view.SetBinding(BindingContextProperty, new Binding { Source = virtualView, Path = BindingContextProperty.PropertyName });
var contentPage = new ContentPage
{
Content = view,
Parent = virtualView.Parent as Element
Content = view
};
var parent = virtualView.Parent as Element;
parent?.AddLogicalChild(contentPage);

return (PageHandler)contentPage.ToHandler(mauiContext);
}
Expand Down
6 changes: 5 additions & 1 deletion src/CommunityToolkit.Maui/Views/Popup/Popup.shared.cs
Original file line number Diff line number Diff line change
Expand Up @@ -351,7 +351,11 @@ protected virtual async Task OnClosed(object? result, bool wasDismissedByTapping
((IResourceDictionary)resources).ValuesChanged -= OnResourcesChanged;

await popupDismissedTaskCompletionSource.Task.WaitAsync(token);
Parent = null;

if (Parent is not null)
{
Parent.RemoveLogicalChild(this);
}

dismissWeakEventManager.HandleEvent(this, new PopupClosedEventArgs(result, wasDismissedByTappingOutsideOfPopup), nameof(Closed));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,10 @@ async void handler(object? sender, EventArgs args)
static void CreatePopup(Page page, Popup popup)
{
var mauiContext = GetMauiContext(page);
popup.Parent = page.GetCurrentPage();

var parent = page.GetCurrentPage();
parent?.AddLogicalChild(popup);

var platformPopup = popup.ToHandler(mauiContext);
platformPopup.Invoke(nameof(IPopup.OnOpened));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,9 @@ public static partial class PopupExtensions
static void PlatformShowPopup(Popup popup, IMauiContext mauiContext)
{
var window = mauiContext.GetPlatformWindow().GetWindow() ?? throw new NullReferenceException("Window is null.");
popup.Parent = ((Page)window.Content).GetCurrentPage();

Page parent = ((Page)window.Content).GetCurrentPage();
parent?.AddLogicalChild(popup);

var platform = popup.ToHandler(mauiContext);
platform?.Invoke(nameof(IPopup.OnOpened));
Expand Down

0 comments on commit 9628467

Please sign in to comment.