Skip to content

Commit 85da926

Browse files
authored
Merge pull request #3038 from unoplatform/dev/mara/fix-dialog-nav
fix: close dialog and escape orphaned region on root navigation
2 parents 2c0c5e0 + 8bb2d8c commit 85da926

File tree

12 files changed

+153
-1
lines changed

12 files changed

+153
-1
lines changed

src/Uno.Extensions.Navigation.UI/Navigator.cs

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -399,6 +399,15 @@ await navAncestor.Navigator.CanNavigate(depRequest.Route))
399399
// Required for Test: Given_PageNavigationRegistered.When_PageNavigationRegisteredRoot
400400
if (request.Route.IsRoot())
401401
{
402+
// If we're inside a ClosableNavigator (dialog/flyout) whose region is not parented
403+
// in the main region tree, delegate through CloseAndNavigateAsync so the request
404+
// escapes to the navigator that opened the dialog.
405+
if (Region.Parent is null && this is ClosableNavigator closable)
406+
{
407+
if (Logger.IsEnabled(LogLevel.Trace)) Logger.LogTraceMessage($"Closing closable navigator and redirecting root request via source");
408+
return closable.CloseAndNavigateAsync(request);
409+
}
410+
402411
if (Region.Parent?.Parent is null)
403412
{
404413
// If parent's Parent is null, then parent is the root

src/Uno.Extensions.Navigation.UI/Navigators/ClosableNavigator.cs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,4 +52,10 @@ protected ClosableNavigator(ILogger logger, IDispatcher dispatcher, IRegion regi
5252
/// </summary>
5353
/// <returns>Task that can be awaited</returns>
5454
protected abstract Task CloseNavigator();
55+
56+
/// <summary>
57+
/// Closes the navigator without any subsequent navigation.
58+
/// Used internally when navigating away from a page that has open dialogs/flyouts.
59+
/// </summary>
60+
internal Task ForceCloseAsync() => CloseNavigator();
5561
}

src/Uno.Extensions.Navigation.UI/Navigators/ContentControlNavigator.cs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,10 @@ protected override async Task<bool> RegionCanNavigate(Route route, RouteInfo? ro
6060

6161
try
6262
{
63+
// Close any active dialogs/flyouts before clearing children,
64+
// otherwise they persist as overlays even after navigation
65+
Region.CloseActiveClosableNavigators();
66+
6367
// Clear all child navigation regions
6468
Region.Children.Clear();
6569

src/Uno.Extensions.Navigation.UI/Navigators/ContentDialogNavigator.cs

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,5 +86,19 @@ services is null ||
8686
}
8787

8888

89+
protected override Task CloseNavigator()
90+
{
91+
var dialog = _activeDialog;
92+
_activeDialog = null;
93+
94+
// Cancel the ShowTask before calling Hide() so the ContinueWith
95+
// continuation sees a canceled task and skips the back-navigation.
96+
var result = base.CloseNavigator();
97+
98+
dialog?.Hide();
99+
100+
return result;
101+
}
102+
89103
protected override Task CheckLoadedAsync() => _activeDialog is not null ? _activeDialog.EnsureLoaded() : Task.CompletedTask;
90104
}

src/Uno.Extensions.Navigation.UI/Navigators/FrameNavigator.cs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,10 @@ viewType is null ||
104104
childRoute = CaptureActiveChildRoute();
105105
}
106106

107+
// Close any active dialogs/flyouts before clearing children,
108+
// otherwise they persist as overlays even after navigation
109+
Region.CloseActiveClosableNavigators();
110+
107111
// Detach all nested regions as we're moving away from the current view
108112
Region.Children.Clear();
109113

src/Uno.Extensions.Navigation.UI/Regions/RegionExtensions.cs

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,4 +77,25 @@ public static IEnumerable<IRegion> FindChildren(this IRegion region, Func<IRegio
7777
}
7878
}
7979
}
80+
81+
/// <summary>
82+
/// Closes any active closable navigators (ContentDialogs, MessageDialogs, Flyouts)
83+
/// in the region's child hierarchy. This should be called before clearing children
84+
/// when navigating away, to ensure overlays don't persist on screen.
85+
/// </summary>
86+
internal static void CloseActiveClosableNavigators(this IRegion region)
87+
{
88+
// Materialize to array first since closing may modify the children collection
89+
var closableRegions = region.FindChildren(
90+
r => r.Services?.GetService<INavigator>() is ClosableNavigator { CanGoBack: true }
91+
).ToArray();
92+
93+
foreach (var childRegion in closableRegions)
94+
{
95+
if (childRegion.Services?.GetService<INavigator>() is ClosableNavigator closable)
96+
{
97+
_ = closable.ForceCloseAsync();
98+
}
99+
}
100+
}
80101
}

testing/TestHarness/TestHarness.UITest/Ext/Navigation/TabBar/Given_TabBar_ClearBackStack.cs

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -124,4 +124,34 @@ public async Task When_StandardBack_From_Deep_Page()
124124
var isTabTwoVisible = App.Marked("TabTwoSection").IsVisible();
125125
isTabTwoVisible.Should().Be(true, "TabTwo section should be visible since we went back from Details");
126126
}
127+
128+
/// <summary>
129+
/// Test 6: Open a ContentDialog from the deep page, then navigate to /Root/Home
130+
/// from inside the dialog. The dialog should be dismissed and we should end up
131+
/// at the tabbed root.
132+
/// Reproduces: ContentDialog not closed during root navigation
133+
/// </summary>
134+
[Test]
135+
public async Task When_ContentDialog_Open_And_Navigate_Root()
136+
{
137+
NavigateToDetails();
138+
139+
// Open the ContentDialog
140+
App.WaitThenTap("NavTest6ShowDialogButton");
141+
142+
// Wait for the dialog to appear
143+
App.WaitElement("DialogContentText");
144+
App.WaitElement("DialogNavToRootButton");
145+
146+
// Navigate to /Root/Home from inside the dialog
147+
App.WaitThenTap("DialogNavToRootButton");
148+
149+
// The dialog should be dismissed and we should be at the tabbed root
150+
AssertBackAtTabbedRoot();
151+
152+
// Verify the dialog is actually dismissed (not just hidden behind the root page).
153+
// Use WaitForNoElement to allow time for the dialog overlay to be removed.
154+
App.WaitForNoElement("DialogContentText", "ContentDialog content should be dismissed after navigating to root");
155+
App.WaitForNoElement("DialogNavToRootButton", "ContentDialog button should be dismissed after navigating to root");
156+
}
127157
}

testing/TestHarness/TestHarness/Ext/Navigation/TabBar/TabBarClearBackStack/TabBarClearBackStackDetailPage.xaml

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,12 @@
7070
uen:Navigation.Request="-"
7171
HorizontalAlignment="Stretch" />
7272

73+
<!-- ContentDialog test: dialog should be closed by root navigation -->
74+
<Button Content="6) Show Dialog (navigate from inside)"
75+
AutomationProperties.AutomationId="NavTest6ShowDialogButton"
76+
Click="NavTest6_Click"
77+
HorizontalAlignment="Stretch" />
78+
7379
<TextBlock x:Name="StatusText"
7480
AutomationProperties.AutomationId="DetailStatusText"
7581
TextWrapping="Wrap"

testing/TestHarness/TestHarness/Ext/Navigation/TabBar/TabBarClearBackStack/TabBarClearBackStackDetailPage.xaml.cs

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,4 +78,19 @@ private async void NavTest5_Click(object sender, RoutedEventArgs e)
7878
}
7979
}
8080

81+
private async void NavTest6_Click(object sender, RoutedEventArgs e)
82+
{
83+
try
84+
{
85+
// Approach 6: open a ContentDialog, then navigate to root from inside it.
86+
// The dialog should be dismissed automatically when root navigation occurs.
87+
var nav = this.Navigator()!;
88+
await nav.NavigateViewAsync<TabBarClearBackStackTestDialog>(this, Qualifiers.Dialog);
89+
}
90+
catch (Exception ex)
91+
{
92+
StatusText.Text = $"Test 6 failed: {ex.Message}";
93+
}
94+
}
95+
8196
}

testing/TestHarness/TestHarness/Ext/Navigation/TabBar/TabBarClearBackStack/TabBarClearBackStackHostInit.cs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,9 @@ protected override void RegisterRoutes(IViewRegistry views, IRouteRegistry route
2424
new ViewMap<TabBarClearBackStackTabTwoPage, TabBarClearBackStackTabTwoModel>(),
2525
// Details uses ResultDataViewMap with data
2626
new ResultDataViewMap<TabBarClearBackStackDetailPage, TabBarClearBackStackDetailModel, string>(
27-
Data: new DataMap<string>())
27+
Data: new DataMap<string>()),
28+
// ContentDialog for testing dialog dismissal during root navigation
29+
new ViewMap<TabBarClearBackStackTestDialog>()
2830
);
2931

3032
// Route structure:

0 commit comments

Comments
 (0)