Skip to content

Reorder popup/view model instantiation #3071

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.

Sign up for GitHub

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

Merged
TheCodeTraveler merged 2 commits into from
Feb 7, 2026
Merged

Reorder popup/view model instantiation #3071

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c6d286b
Reorder popup/view model instantiation
bijington Feb 6, 2026
6667539
Add Regression Tests
TheCodeTraveler Feb 7, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/CommunityToolkit.Maui.UnitTests/BaseViewTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ static void InitializeServicesAndSetMockApplication(out IServiceProvider service
appBuilder.Services.AddTransientPopup<LongLivedSelfClosingPopup, LongLivedMockPageViewModel>();
appBuilder.Services.AddTransientPopup<ShortLivedSelfClosingPopup, ShortLivedMockPageViewModel>();
appBuilder.Services.AddTransientPopup<GarbageCollectionHeavySelfClosingPopup, MockPageViewModel>();
appBuilder.Services.AddTransientPopup<SingleConstructionPopup, SingleConstructionViewModel>();

appBuilder.Services.AddTransientPopup<MockPopup>();
#endregion
Expand Down
88 changes: 74 additions & 14 deletions src/CommunityToolkit.Maui.UnitTests/Services/PopupServiceTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -513,9 +513,51 @@ public async Task ClosePopupAsyncT_ShouldClosePopupUsingPageAndReturnResult()
Assert.Equal(expectedResult, popupResult.Result);
Assert.False(popupResult.WasDismissedByTappingOutsideOfPopup);
}

[Fact]
public void ShowPopup_WithRegisteredPopup_ShouldOnlyConstructViewModelOnce()
{
// Arrange
SingleConstructionViewModel.ConstructorCallCount = 0;
Assert.Equal(0, SingleConstructionViewModel.ConstructorCallCount);

if (Application.Current?.Windows[0].Page is not Page page)
{
throw new InvalidOperationException("Page cannot be null");
}

var popupService = ServiceProvider.GetRequiredService<IPopupService>();

// Act
popupService.ShowPopup<SingleConstructionViewModel>(page.Navigation);

// Assert
Assert.Equal(1, SingleConstructionViewModel.ConstructorCallCount);
}

[Fact]
public async Task ShowPopupAsync_WithRegisteredPopup_ShouldOnlyConstructViewModelOnce()
{
// Arrange
SingleConstructionViewModel.ConstructorCallCount = 0;
Assert.Equal(0, SingleConstructionViewModel.ConstructorCallCount);

if (Application.Current?.Windows[0].Page is not Page page)
{
throw new InvalidOperationException("Page cannot be null");
}

var popupService = ServiceProvider.GetRequiredService<IPopupService>();

// Act
await popupService.ShowPopupAsync<SingleConstructionViewModel>(page.Navigation, cancellationToken: TestContext.Current.CancellationToken);

Copilot AI Feb 7, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This call uses a named argument cancellationToken, but the ShowPopupAsync<T>(INavigation, ...) overload’s parameter is named token. As written this won’t compile; use the correct parameter name or pass the token positionally.

Suggested change
await popupService.ShowPopupAsync<SingleConstructionViewModel>(page.Navigation, cancellationToken: TestContext.Current.CancellationToken);
await popupService.ShowPopupAsync<SingleConstructionViewModel>(page.Navigation, token: TestContext.Current.CancellationToken);

Copilot uses AI. Check for mistakes.

// Assert
Assert.Equal(1, SingleConstructionViewModel.ConstructorCallCount);
}
}

class GarbageCollectionHeavySelfClosingPopup(MockPageViewModel viewModel, object? result = null) : MockSelfClosingPopup(viewModel, TimeSpan.FromMilliseconds(500), result)
sealed class GarbageCollectionHeavySelfClosingPopup(MockPageViewModel viewModel, object? result = null) : MockSelfClosingPopup(viewModel, TimeSpan.FromMilliseconds(500), result)
{
protected override void HandlePopupOpened(object? sender, EventArgs e)
{
Expand All @@ -532,6 +574,24 @@ protected override void HandlePopupClosed(object? sender, EventArgs e)
}
}

file class PopupViewModel : INotifyPropertyChanged
{
public event PropertyChangedEventHandler? PropertyChanged;

public Color? Color
{
get;
set
{
if (!Equals(value, field))
{
field = value;
PropertyChanged?.Invoke(this, new PropertyChangedEventArgs(nameof(Color)));
}
}
} = new();
}

sealed class LongLivedSelfClosingPopup(LongLivedMockPageViewModel viewModel) : MockSelfClosingPopup(viewModel, TimeSpan.FromMilliseconds(1500), "Long Lived");

sealed class ShortLivedSelfClosingPopup(ShortLivedMockPageViewModel viewModel) : MockSelfClosingPopup(viewModel, TimeSpan.FromMilliseconds(500), "Short Lived");
Expand Down Expand Up @@ -619,20 +679,20 @@ void IQueryAttributable.ApplyQueryAttributes(IDictionary<string, object> query)

sealed class MockPopup : Popup;

sealed file class PopupViewModel : INotifyPropertyChanged
sealed class SingleConstructionViewModel : MockPageViewModel
{
public event PropertyChangedEventHandler? PropertyChanged;
public static int ConstructorCallCount { get; set; }

public Color? Color
public SingleConstructionViewModel()
{
get;
set
{
if (!Equals(value, field))
{
field = value;
PropertyChanged?.Invoke(this, new PropertyChangedEventArgs(nameof(Color)));
}
}
} = new();
ConstructorCallCount++;
}
}

sealed class SingleConstructionPopup : MockSelfClosingPopup
{
public SingleConstructionPopup(SingleConstructionViewModel viewModel) : base(viewModel, TimeSpan.FromSeconds(2))
{
BindingContext = viewModel;
}
}
25 changes: 14 additions & 11 deletions src/CommunityToolkit.Maui/Services/PopupService.shared.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ public void ShowPopup<T>(INavigation navigation, IPopupOptions? options = null)
{
ArgumentNullException.ThrowIfNull(navigation);

var popupContent = GetPopupContent(serviceProvider.GetRequiredService<T>());
var popupContent = GetPopupContent<T>();

navigation.ShowPopup(popupContent, options);
}
Expand All @@ -57,7 +57,7 @@ public void ShowPopup<T>(Shell shell, IPopupOptions? options = null, IDictionary
{
ArgumentNullException.ThrowIfNull(shell);

var popupContent = GetPopupContent(serviceProvider.GetRequiredService<T>());
var popupContent = GetPopupContent<T>();
shell.ShowPopup(popupContent, options, shellParameters);
}

Expand All @@ -77,7 +77,7 @@ public Task<IPopupResult> ShowPopupAsync<T>(INavigation navigation, IPopupOption

token.ThrowIfCancellationRequested();

var popupContent = GetPopupContent(serviceProvider.GetRequiredService<T>());
var popupContent = GetPopupContent<T>();

return navigation.ShowPopupAsync(popupContent, options, token);
}
Expand All @@ -89,7 +89,7 @@ public Task<IPopupResult> ShowPopupAsync<T>(Shell shell, IPopupOptions? options,

token.ThrowIfCancellationRequested();

var popupContent = GetPopupContent(serviceProvider.GetRequiredService<T>());
var popupContent = GetPopupContent<T>();

return shell.ShowPopupAsync(popupContent, options, shellParameters, token);
}
Expand All @@ -111,7 +111,7 @@ public Task<IPopupResult<TResult>> ShowPopupAsync<T, TResult>(INavigation naviga
ArgumentNullException.ThrowIfNull(navigation);

token.ThrowIfCancellationRequested();
var popupContent = GetPopupContent(serviceProvider.GetRequiredService<T>());
var popupContent = GetPopupContent<T>();

return navigation.ShowPopupAsync<TResult>(popupContent, options, token);
}
Expand All @@ -122,7 +122,7 @@ public Task<IPopupResult<TResult>> ShowPopupAsync<T, TResult>(Shell shell, IPopu
ArgumentNullException.ThrowIfNull(shell);

token.ThrowIfCancellationRequested();
var popupContent = GetPopupContent(serviceProvider.GetRequiredService<T>());
var popupContent = GetPopupContent<T>();

return shell.ShowPopupAsync<TResult>(popupContent, options, shellParameters, token);
}
Expand Down Expand Up @@ -178,16 +178,19 @@ public Task<IPopupResult<T>> ClosePopupAsync<T>(INavigation navigation, T result
services.TryAdd(new ServiceDescriptor(typeof(TPopupViewModel), typeof(TPopupViewModel), lifetime));
}

View GetPopupContent<T>(T bindingContext)
View GetPopupContent<T>()
{
if (bindingContext is View view)
if (viewModelToViewMappings.TryGetValue(typeof(T), out var viewType)
&& serviceProvider.GetRequiredService(viewType) is View content)
{
return view;
return content;
}

if (serviceProvider.GetRequiredService(viewModelToViewMappings[typeof(T)]) is View content)
var bindingContext = serviceProvider.GetRequiredService(typeof(T));

if (bindingContext is View view)
{
return content;
return view;
}

throw new InvalidOperationException($"Could not locate {typeof(T).FullName}");
Expand Down
Loading