Skip to content

Commit 3ba59e6

Browse files
AlejoTorresLeonAlejoTorresLeon
authored
Cleanup Popup Resources When Closed (#2971)
1 parent 4149f2e commit 3ba59e6

File tree

6 files changed

+60
-66
lines changed

6 files changed

+60
-66
lines changed

‎src/CommunityToolkit.Maui.Core/Primitives/Defaults/RatingViewDefaults.shared.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ static class RatingViewDefaults
2929

3030
/// <summary>Default Fill When Tapped</summary>
3131
public const RatingViewFillOption FillWhenTapped = RatingViewFillOption.Shape;
32-
32+
3333
/// <summary>Default Fill Option</summary>
3434
public const RatingViewFillOption FillOption = RatingViewFillOption.Shape;
3535

‎src/CommunityToolkit.Maui.UnitTests/BaseViewTest.cs‎

Lines changed: 0 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -15,22 +15,6 @@ protected BaseViewTest()
1515

1616
protected IServiceProvider ServiceProvider { get; }
1717

18-
protected override async ValueTask DisposeAsyncCore()
19-
{
20-
await base.DisposeAsyncCore();
21-
22-
#region Cleanup Popup Tests
23-
24-
Application.Current.Should().NotBeNull();
25-
var navigation = Application.Current.Windows[0].Page?.Navigation ?? throw new InvalidOperationException("Unable to locate Navigation Stack");
26-
27-
while (navigation.ModalStack.Any())
28-
{
29-
await navigation.PopModalAsync();
30-
}
31-
#endregion
32-
}
33-
3418
protected static TElementHandler CreateElementHandler<TElementHandler>(IElement view, bool doesRequireMauiContext = true)
3519
where TElementHandler : IElementHandler, new()
3620
{

‎src/CommunityToolkit.Maui.UnitTests/Extensions/PopupExtensionsTests.cs‎

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -229,21 +229,22 @@ public void ShowPopupAsync_Shell_WithViewType_ShowsPopup()
229229
Assert.Equal(shellParameterViewModelTextValue, viewWithQueryable.BindingContext.Text);
230230
}
231231

232-
[Fact(Timeout = (int)TestDuration.Short)]
232+
[Fact(Timeout = (int)TestDuration.Long)]
233233
public async Task ShowPopupAsync_AwaitingShowPopupAsync_EnsurePreviousPopupClosed()
234234
{
235235
// Arrange
236236
var selfClosingPopup = ServiceProvider.GetRequiredService<ShortLivedSelfClosingPopup>() ?? throw new InvalidOperationException();
237+
var longLivedSelfClosingPopup = ServiceProvider.GetRequiredService<LongLivedSelfClosingPopup>() ?? throw new InvalidOperationException();
237238

238239
// Act
239-
await navigation.ShowPopupAsync<object?>(selfClosingPopup, PopupOptions.Empty, TestContext.Current.CancellationToken);
240+
await navigation.ShowPopupAsync<object?>(longLivedSelfClosingPopup, PopupOptions.Empty, TestContext.Current.CancellationToken);
240241
await navigation.ShowPopupAsync<object?>(selfClosingPopup, PopupOptions.Empty, TestContext.Current.CancellationToken);
241242

242243
// Assert
243244
Assert.Empty(navigation.ModalStack);
244245
}
245246

246-
[Fact(Timeout = (int)TestDuration.Short)]
247+
[Fact(Timeout = (int)TestDuration.Long)]
247248
public async Task ShowPopupAsync_Shell_AwaitingShowPopupAsync_EnsurePreviousPopupClosed()
248249
{
249250
// Arrange
@@ -255,9 +256,10 @@ public async Task ShowPopupAsync_Shell_AwaitingShowPopupAsync_EnsurePreviousPopu
255256

256257
var shellNavigation = Shell.Current.Navigation;
257258
var selfClosingPopup = ServiceProvider.GetRequiredService<ShortLivedSelfClosingPopup>() ?? throw new InvalidOperationException();
259+
var longLivedSelfClosingPopup = ServiceProvider.GetRequiredService<LongLivedSelfClosingPopup>() ?? throw new InvalidOperationException();
258260

259261
// Act
260-
await shell.ShowPopupAsync<object?>(selfClosingPopup, PopupOptions.Empty, shellParameters, TestContext.Current.CancellationToken);
262+
await shell.ShowPopupAsync<object?>(longLivedSelfClosingPopup, PopupOptions.Empty, shellParameters, TestContext.Current.CancellationToken);
261263
await shell.ShowPopupAsync<object?>(selfClosingPopup, PopupOptions.Empty, shellParameters, TestContext.Current.CancellationToken);
262264

263265
// Assert
@@ -351,9 +353,10 @@ public void ShowPopup_MultiplePopupsDisplayed()
351353
{
352354
// Arrange
353355
var selfClosingPopup = ServiceProvider.GetRequiredService<ShortLivedSelfClosingPopup>() ?? throw new InvalidOperationException();
356+
var longLivedSelfClosingPopup = ServiceProvider.GetRequiredService<LongLivedSelfClosingPopup>() ?? throw new InvalidOperationException();
354357

355358
// Act
356-
navigation.ShowPopup(selfClosingPopup, PopupOptions.Empty);
359+
navigation.ShowPopup(longLivedSelfClosingPopup, PopupOptions.Empty);
357360
navigation.ShowPopup(selfClosingPopup, PopupOptions.Empty);
358361

359362
// Assert
@@ -365,6 +368,7 @@ public void ShowPopup_Shell_MultiplePopupsDisplayed()
365368
{
366369
// Arrange
367370
var selfClosingPopup = ServiceProvider.GetRequiredService<ShortLivedSelfClosingPopup>() ?? throw new InvalidOperationException();
371+
var longLivedSelfClosingPopup = ServiceProvider.GetRequiredService<LongLivedSelfClosingPopup>() ?? throw new InvalidOperationException();
368372
var shell = new Shell();
369373
shell.Items.Add(new MockPage(new MockPageViewModel()));
370374

@@ -374,7 +378,7 @@ public void ShowPopup_Shell_MultiplePopupsDisplayed()
374378
var shellNavigation = Shell.Current.Navigation;
375379

376380
// Act
377-
shell.ShowPopup(selfClosingPopup, PopupOptions.Empty, shellParameters);
381+
shell.ShowPopup(longLivedSelfClosingPopup, PopupOptions.Empty, shellParameters);
378382
shell.ShowPopup(selfClosingPopup, PopupOptions.Empty, shellParameters);
379383

380384
// Assert

‎src/CommunityToolkit.Maui.UnitTests/Services/PopupServiceTests.cs‎

Lines changed: 42 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ public void ShowPopupAsync_UsingNavigation_WithViewType_ShowsPopup()
5757
var popupService = ServiceProvider.GetRequiredService<IPopupService>();
5858

5959
// Act
60-
popupService.ShowPopup<ShortLivedMockPageViewModel>(navigation);
60+
popupService.ShowPopup<MockPageViewModel>(navigation);
6161

6262
// Assert
6363
Assert.Single(navigation.ModalStack);
@@ -76,22 +76,22 @@ public void ShowPopupAsync_UsingPage_WithViewType_ShowsPopup()
7676
}
7777

7878
// Act
79-
popupService.ShowPopup<ShortLivedSelfClosingPopup>(page);
79+
popupService.ShowPopup<MockPopup>(page);
8080

8181
// Assert
8282
Assert.Single(navigation.ModalStack);
8383
Assert.IsType<PopupPage>(navigation.ModalStack[0]);
8484
}
8585

86-
[Fact(Timeout = (int)TestDuration.Short)]
86+
[Fact(Timeout = (int)TestDuration.Long)]
8787
public async Task ShowPopupAsync_AwaitingShowPopupAsync_EnsurePreviousPopupClosed()
8888
{
8989
// Arrange
9090
var popupService = ServiceProvider.GetRequiredService<IPopupService>();
9191

9292
// Act
9393
await popupService.ShowPopupAsync<ShortLivedSelfClosingPopup>(navigation, PopupOptions.Empty, TestContext.Current.CancellationToken);
94-
await popupService.ShowPopupAsync<ShortLivedSelfClosingPopup>(navigation, PopupOptions.Empty, TestContext.Current.CancellationToken);
94+
await popupService.ShowPopupAsync<LongLivedSelfClosingPopup>(navigation, PopupOptions.Empty, TestContext.Current.CancellationToken);
9595

9696
// Assert
9797
Assert.Empty(navigation.ModalStack);
@@ -110,7 +110,7 @@ public async Task ShowPopupAsync_UsingPage_AwaitingShowPopupAsync_EnsurePrevious
110110

111111
// Act
112112
await popupService.ShowPopupAsync<ShortLivedMockPageViewModel>(page, PopupOptions.Empty, TestContext.Current.CancellationToken);
113-
await popupService.ShowPopupAsync<ShortLivedMockPageViewModel>(page, PopupOptions.Empty, TestContext.Current.CancellationToken);
113+
await popupService.ShowPopupAsync<LongLivedMockPageViewModel>(page, PopupOptions.Empty, TestContext.Current.CancellationToken);
114114

115115
// Assert
116116
Assert.Empty(navigation.ModalStack);
@@ -124,7 +124,7 @@ public void ShowPopup_NavigationModalStackCountIncreases()
124124
Assert.Empty(navigation.ModalStack);
125125

126126
// Act
127-
popupService.ShowPopup<ShortLivedMockPageViewModel>(navigation, PopupOptions.Empty);
127+
popupService.ShowPopup<MockPopup>(navigation, PopupOptions.Empty);
128128

129129
// Assert
130130
Assert.Single(navigation.ModalStack);
@@ -137,8 +137,8 @@ public void ShowPopup_MultiplePopupsDisplayed()
137137
var popupService = ServiceProvider.GetRequiredService<IPopupService>();
138138

139139
// Act
140-
popupService.ShowPopup<LongLivedMockPageViewModel>(navigation, PopupOptions.Empty);
141-
popupService.ShowPopup<ShortLivedMockPageViewModel>(navigation, PopupOptions.Empty);
140+
popupService.ShowPopup<MockPopup>(navigation, PopupOptions.Empty);
141+
popupService.ShowPopup<MockPopup>(navigation, PopupOptions.Empty);
142142

143143
// Assert
144144
Assert.Equal(2, navigation.ModalStack.Count);
@@ -160,7 +160,7 @@ public void ShowPopupAsync_WithCustomOptions_AppliesOptions()
160160
};
161161

162162
// Act
163-
popupService.ShowPopup<ShortLivedSelfClosingPopup>(navigation, options);
163+
popupService.ShowPopup<MockPopup>(navigation, options);
164164

165165
var popupPage = (PopupPage)navigation.ModalStack[0];
166166
var popupPageLayout = popupPage.Content;
@@ -243,7 +243,6 @@ public async Task ShowPopupAsyncShouldValidateProperBindingContext()
243243
// Act
244244
await popupService.ShowPopupAsync<LongLivedMockPageViewModel, object?>(page.Navigation, PopupOptions.Empty, TestContext.Current.CancellationToken);
245245

246-
247246
// Assert
248247
Assert.NotNull(popupInstance.BindingContext);
249248
Assert.IsType<LongLivedMockPageViewModel>(popupInstance.BindingContext);
@@ -534,40 +533,42 @@ protected override void HandlePopupClosed(object? sender, EventArgs e)
534533
}
535534
}
536535

537-
class LongLivedSelfClosingPopup : MockSelfClosingPopup
538-
{
539-
public LongLivedSelfClosingPopup(LongLivedMockPageViewModel viewModel)
540-
: base(viewModel, TimeSpan.FromMilliseconds(1500), "Long Lived")
541-
{
542-
}
543-
}
536+
sealed class LongLivedSelfClosingPopup(LongLivedMockPageViewModel viewModel) : MockSelfClosingPopup(viewModel, TimeSpan.FromMilliseconds(1500), "Long Lived");
544537

545-
class ShortLivedSelfClosingPopup : MockSelfClosingPopup
546-
{
547-
public ShortLivedSelfClosingPopup(ShortLivedMockPageViewModel viewModel)
548-
: base(viewModel, TimeSpan.FromMilliseconds(500), "Short Lived")
549-
{
550-
}
551-
}
538+
sealed class ShortLivedSelfClosingPopup(ShortLivedMockPageViewModel viewModel) : MockSelfClosingPopup(viewModel, TimeSpan.FromMilliseconds(500), "Short Lived");
552539

553-
#pragma warning disable CA1001
554-
class MockSelfClosingPopup : Popup<object?>, IQueryAttributable
555-
#pragma warning restore CA1001
540+
class MockSelfClosingPopup : Popup<object?>, IQueryAttributable, IDisposable
556541
{
557-
readonly TimeSpan displayDuration;
542+
readonly TaskCompletionSource popupClosedTCS = new();
543+
558544
CancellationTokenSource? cancellationTokenSource;
559545

560-
public MockSelfClosingPopup(MockPageViewModel viewModel, TimeSpan displayDuration, object? result = null)
546+
protected MockSelfClosingPopup(MockPageViewModel viewModel, TimeSpan displayDuration, object? result = null)
561547
{
562-
this.displayDuration = displayDuration;
563548
BackgroundColor = DefaultBackgroundColor;
549+
DisplayDuration = displayDuration;
564550
BindingContext = viewModel;
565551
Result = result;
566552
Opened += HandlePopupOpened;
567553
Closed += HandlePopupClosed;
568554
}
555+
556+
~MockSelfClosingPopup()
557+
{
558+
Dispose(false);
559+
}
560+
561+
public object? Result { get; }
562+
563+
public TimeSpan DisplayDuration { get; }
569564

570565
public static Color DefaultBackgroundColor { get; } = Colors.White;
566+
567+
public void Dispose()
568+
{
569+
Dispose(true);
570+
GC.SuppressFinalize(this);
571+
}
571572

572573
protected virtual void HandlePopupClosed(object? sender, EventArgs e)
573574
{
@@ -585,7 +586,7 @@ protected virtual async void HandlePopupOpened(object? sender, EventArgs e)
585586

586587
Console.WriteLine($@"{DateTime.Now:O} HandlePopupOpened {BindingContext.GetType().Name}");
587588

588-
await Task.Delay(displayDuration);
589+
await Task.Delay(DisplayDuration);
589590

590591
if (cancellationTokenSource?.IsCancellationRequested is true)
591592
{
@@ -599,9 +600,17 @@ protected virtual async void HandlePopupOpened(object? sender, EventArgs e)
599600

600601
Console.WriteLine(
601602
$@"{DateTime.Now:O} Closed {BindingContext.GetType().Name} - {Application.Current?.Windows[0].Page?.Navigation.ModalStack.Count}");
603+
604+
popupClosedTCS.SetResult();
605+
}
606+
607+
protected virtual void Dispose(bool disposing)
608+
{
609+
if (disposing)
610+
{
611+
cancellationTokenSource?.Dispose();
612+
}
602613
}
603-
604-
public object? Result { get; }
605614

606615
void IQueryAttributable.ApplyQueryAttributes(IDictionary<string, object> query)
607616
{

‎src/CommunityToolkit.Maui/Views/Popup/Popup.shared.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -122,9 +122,9 @@ private protected PopupPage GetPopupPage()
122122

123123
while (parent is not null)
124124
{
125-
if (parent.Parent is PopupPage popuppage)
125+
if (parent.Parent is PopupPage popupPage)
126126
{
127-
return popuppage;
127+
return popupPage;
128128
}
129129

130130
parent = parent.Parent;

‎src/CommunityToolkit.Maui/Views/Popup/PopupPage.shared.cs‎

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,12 @@ public async Task CloseAsync(PopupResult result, CancellationToken token = defau
114114
token.ThrowIfCancellationRequested();
115115
await Navigation.PopModalAsync(false).WaitAsync(token);
116116

117+
// Clean up Popup resources
118+
base.Content.GestureRecognizers.Clear();
119+
popup.PropertyChanged -= HandlePopupPropertyChanged;
120+
117121
PopupClosed?.Invoke(this, result);
122+
popup.NotifyPopupIsClosed();
118123
}
119124

120125
protected override bool OnBackButtonPressed()
@@ -125,14 +130,6 @@ protected override bool OnBackButtonPressed()
125130
return true;
126131
}
127132

128-
protected override void OnNavigatedFrom(NavigatedFromEventArgs args)
129-
{
130-
popup.NotifyPopupIsClosed();
131-
base.Content.GestureRecognizers.Clear();
132-
popup.PropertyChanged -= HandlePopupPropertyChanged;
133-
base.OnNavigatedFrom(args);
134-
}
135-
136133
protected override void OnNavigatedTo(NavigatedToEventArgs args)
137134
{
138135
base.OnNavigatedTo(args);

0 commit comments

Comments
 (0)