Keep page in NavigationPage's logical tree until animation is complete (#21751)

* Keep page in NavigationPage's logical tree until animation is complete

* Share templates in NavigationPageTests

src/Avalonia.Controls/Page/NavigationPage.cs

@@ -860,7 +860,9 @@ namespace Avalonia.Controls
             if (old != null)
                 _pageSet.Remove(old);
 
-            if (old is ILogical oldLogical)
+            // Remove the page from the logical tree only if it isn't in a presenter.
+            // If it is, it's animating out and will be removed once the animation completes.
+            if (old is ILogical oldLogical && !IsPageInPresenter(old))
                 LogicalChildren.Remove(oldLogical);
 
             InvalidateNavigationStackCache();
@@ -1035,20 +1037,13 @@ namespace Avalonia.Controls
 
                 var poppedPages = new List<Page>();
 
-                void TearDownPopped(Page popped)
+                while (_navigationStack.Count > 1)
                 {
-                    _pageSet.Remove(popped);
+                    var popped = _navigationStack.Pop();
-                    if (popped is ILogical poppedLogical)
+                    TearDownPoppedPage(popped);
-                        LogicalChildren.Remove(poppedLogical);
-                    popped.Navigation = null;
-                    popped.SetInNavigationPage(false);
-                    popped.SafeAreaPadding = default;
                     poppedPages.Add(popped);
                 }
 
-                while (_navigationStack.Count > 1)
-                    TearDownPopped(_navigationStack.Pop());
-
                 InvalidateNavigationStackCache();
                 _isPop = true;
                 UpdateActivePage();
@@ -1075,6 +1070,20 @@ namespace Avalonia.Controls
             }
         }
 
+        private void TearDownPoppedPage(Page popped)
+        {
+            _pageSet.Remove(popped);
+
+            // Remove the page from the logical tree only if it isn't in a presenter.
+            // If it is, it will be removed once the animation is complete.
+            if (popped is ILogical poppedLogical && !IsPageInPresenter(popped))
+                LogicalChildren.Remove(poppedLogical);
+
+            popped.Navigation = null;
+            popped.SetInNavigationPage(false);
+            popped.SafeAreaPadding = default;
+        }
+
         /// <summary>
         /// Pops all pages to the root page using <paramref name="transition"/>.
         /// </summary>
@@ -1137,20 +1146,13 @@ namespace Avalonia.Controls
 
                 var poppedPages = new List<Page>();
 
-                void TearDownPopped(Page popped)
+                while (_navigationStack.Count > 1 && _navigationStack.Peek() != page)
                 {
-                    _pageSet.Remove(popped);
+                    var popped = _navigationStack.Pop();
-                    if (popped is ILogical poppedLogical)
+                    TearDownPoppedPage(popped);
-                        LogicalChildren.Remove(poppedLogical);
-                    popped.Navigation = null;
-                    popped.SetInNavigationPage(false);
-                    popped.SafeAreaPadding = default;
                     poppedPages.Add(popped);
                 }
 
-                while (_navigationStack.Count > 1 && _navigationStack.Peek() != page)
-                    TearDownPopped(_navigationStack.Pop());
-
                 InvalidateNavigationStackCache();
                 _isPop = true;
                 UpdateActivePage();
@@ -1800,6 +1802,9 @@ namespace Avalonia.Controls
                 _currentTransition?.Dispose();
                 _currentTransition = null;
 
+                // A previous transition may have been canceled above before its teardown ran, leaving its outgoing page
+                // in the back presenter; reconcile the logical tree.
+                DetachOrphanedPageFromLogicalTree(_pageBackPresenter.Content);
                 _pageBackPresenter.IsVisible = false;
                 _pageBackPresenter.Content = null;
                 _pageBackPresenter.RenderTransform = null;
@@ -1839,6 +1844,7 @@ namespace Avalonia.Controls
                 }
                 else
                 {
+                    var oldPageContent = _pagePresenter.Content;
                     _lastPageTransitionTask = Task.CompletedTask;
 
                     _pagePresenter.Content = page;
@@ -1848,6 +1854,8 @@ namespace Avalonia.Controls
                     _pageBackPresenter.Content = null;
                     _pageBackPresenter.IsVisible = false;
                     _pageBackPresenter.ZIndex = 0;
+
+                    DetachOrphanedPageFromLogicalTree(oldPageContent);
                 }
 
                 if (page != null)
@@ -1932,10 +1940,12 @@ namespace Avalonia.Controls
             if (ct.IsCancellationRequested)
                 return;
 
+            var fromContent = from.Content;
             from.IsVisible = false;
             from.Content = null;
             from.RenderTransform = null;
             from.Opacity = 1;
+            DetachOrphanedPageFromLogicalTree(fromContent);
         }
 
         private Task AwaitPageTransitionAsync()
@@ -1974,7 +1984,9 @@ namespace Avalonia.Controls
             _pageSet.Add(page);
             InvalidateNavigationStackCache();
 
-            if (removed is ILogical removedLogical)
+            // Remove the replaced page from the logical tree only if it isn't in a presenter.
+            // If it is, it's animating out and will be removed once the animation completes.
+            if (removed is ILogical removedLogical && !IsPageInPresenter(removed))
                 LogicalChildren.Remove(removedLogical);
             if (page is ILogical addedLogical)
                 LogicalChildren.Add(addedLogical);
@@ -1985,6 +1997,20 @@ namespace Avalonia.Controls
             UpdateActivePage();
         }
 
+        private void DetachOrphanedPageFromLogicalTree(object? content)
+        {
+            if (content is ILogical logical
+                && content is Page page
+                && !_pageSet.Contains(page)
+                && logical.LogicalParent == this)
+            {
+                LogicalChildren.Remove(logical);
+            }
+        }
+
+        private bool IsPageInPresenter(Page page)
+            => ReferenceEquals(page, _pagePresenter?.Content) || ReferenceEquals(page, _pageBackPresenter?.Content);
+
         private void SwapModalPresenters()
         {
             if (_modalPresenter == null || _modalBackPresenter == null)

tests/Avalonia.Controls.UnitTests/NavigationPageTests.cs

@@ -22,6 +22,60 @@ namespace Avalonia.Controls.UnitTests;
 
 public class NavigationPageTests
 {
+    private static NavigationPage CreateNavigationPage(IPageTransition? transition = null)
+    {
+        var nav = new NavigationPage
+        {
+            PageTransition = transition,
+            Template = CreateNavigationPageTemplate()
+        };
+        var root = new TestRoot { Child = nav };
+        root.LayoutManager.ExecuteInitialLayoutPass();
+        return nav;
+    }
+
+    private static IControlTemplate CreateNavigationPageTemplate()
+    {
+        return new FuncControlTemplate<NavigationPage>((_, ns) =>
+        {
+            var contentHost = new Panel
+            {
+                Name = "PART_ContentHost",
+                Children =
+                {
+                    new ContentPresenter { Name = "PART_PageBackPresenter" }.RegisterInNameScope(ns),
+                    new ContentPresenter { Name = "PART_PagePresenter" }.RegisterInNameScope(ns),
+                }
+            }.RegisterInNameScope(ns);
+
+            return new Panel
+            {
+                Children =
+                {
+                    new Border
+                    {
+                        Name = "PART_NavigationBar",
+                        Child = new Button { Name = "PART_BackButton" }.RegisterInNameScope(ns)
+                    }.RegisterInNameScope(ns),
+                    contentHost,
+                    new ContentPresenter { Name = "PART_TopCommandBar" }.RegisterInNameScope(ns),
+                    new ContentPresenter { Name = "PART_ModalBackPresenter" }.RegisterInNameScope(ns),
+                    new ContentPresenter { Name = "PART_ModalPresenter" }.RegisterInNameScope(ns),
+                }
+            };
+        });
+    }
+
+    private sealed class ControllableTransition(Task gate) : IPageTransition
+    {
+        public async Task Start(Visual? from, Visual? to, bool forward, CancellationToken cancellationToken)
+        {
+            to?.IsVisible = true;
+            await gate;
+            from?.IsVisible = false;
+        }
+    }
+
     public class PushTests : ScopedTestBase
     {
         [Fact]
@@ -1784,6 +1838,86 @@ public class NavigationPageTests
             Assert.Null(modal.Navigation);
             Assert.False(modal.IsInNavigationPage);
         }
+
+        [Fact]
+        public async Task Pop_KeepsOutgoingPageInLogicalTree_UntilTransitionCompletes()
+        {
+            var gate = new TaskCompletionSource();
+            var nav = CreateNavigationPage();
+
+            var root = new ContentPage();
+            var top = new ContentPage();
+            await nav.PushAsync(root);
+            await nav.PushAsync(top);
+
+            nav.PageTransition = new ControllableTransition(gate.Task);
+
+            var popTask = nav.PopAsync();
+
+            // The outgoing page is still animating out, so it must remain in the logical tree.
+            Assert.Contains(top, nav.GetLogicalChildren());
+
+            gate.SetResult();
+            await popTask;
+
+            // Once the transition completes, the page is detached.
+            Assert.DoesNotContain(top, nav.GetLogicalChildren());
+            Assert.Contains(root, nav.GetLogicalChildren());
+        }
+
+        [Fact]
+        public async Task Replace_KeepsOutgoingPageInLogicalTree_UntilTransitionCompletes()
+        {
+            var gate = new TaskCompletionSource();
+            var nav = CreateNavigationPage();
+
+            var original = new ContentPage();
+            await nav.PushAsync(original);
+
+            nav.PageTransition = new ControllableTransition(gate.Task);
+
+            var replacement = new ContentPage();
+            var replaceTask = nav.ReplaceAsync(replacement);
+
+            // The replaced page is still animating out, so both pages are in the logical tree.
+            Assert.Contains(original, nav.GetLogicalChildren());
+            Assert.Contains(replacement, nav.GetLogicalChildren());
+
+            gate.SetResult();
+            await replaceTask;
+
+            Assert.DoesNotContain(original, nav.GetLogicalChildren());
+            Assert.Contains(replacement, nav.GetLogicalChildren());
+        }
+
+        [Fact]
+        public async Task PopToRoot_KeepsVisibleTopPageInLogicalTree_UntilTransitionCompletes()
+        {
+            var gate = new TaskCompletionSource();
+            var nav = CreateNavigationPage();
+
+            var root = new ContentPage();
+            var middle = new ContentPage();
+            var top = new ContentPage();
+            await nav.PushAsync(root);
+            await nav.PushAsync(middle);
+            await nav.PushAsync(top);
+
+            nav.PageTransition = new ControllableTransition(gate.Task);
+
+            var popTask = nav.PopToRootAsync();
+
+            // The hidden intermediate page is detached eagerly, but the visible top page is
+            // animating out and must stay attached until the transition completes.
+            Assert.DoesNotContain(middle, nav.GetLogicalChildren());
+            Assert.Contains(top, nav.GetLogicalChildren());
+
+            gate.SetResult();
+            await popTask;
+
+            Assert.DoesNotContain(top, nav.GetLogicalChildren());
+            Assert.Contains(root, nav.GetLogicalChildren());
+        }
     }
 
     public class PopAllModalsTests : ScopedTestBase
@@ -2440,69 +2574,6 @@ public class NavigationPageTests
             Assert.False(navigatedFromDuringTransition);
             Assert.False(navigatedToDuringTransition);
         }
-
-        private static NavigationPage CreateNavigationPage(IPageTransition? transition)
-        {
-            var nav = new NavigationPage
-            {
-                PageTransition = transition,
-                Template = NavigationPageTemplate()
-            };
-            var root = new TestRoot { Child = nav };
-            root.LayoutManager.ExecuteInitialLayoutPass();
-            return nav;
-        }
-
-        private static IControlTemplate NavigationPageTemplate()
-        {
-            return new FuncControlTemplate<NavigationPage>((parent, ns) =>
-            {
-                var contentHost = new Panel
-                {
-                    Name = "PART_ContentHost",
-                    Children =
-                    {
-                        new ContentPresenter { Name = "PART_PageBackPresenter" }.RegisterInNameScope(ns),
-                        new ContentPresenter { Name = "PART_PagePresenter" }.RegisterInNameScope(ns),
-                    }
-                }.RegisterInNameScope(ns);
-
-                return new Panel
-                {
-                    Children =
-                    {
-                        new Border
-                        {
-                            Name = "PART_NavigationBar",
-                            Child = new Button { Name = "PART_BackButton" }.RegisterInNameScope(ns)
-                        }.RegisterInNameScope(ns),
-                        contentHost,
-                        new ContentPresenter { Name = "PART_TopCommandBar" }.RegisterInNameScope(ns),
-                        new ContentPresenter { Name = "PART_ModalBackPresenter" }.RegisterInNameScope(ns),
-                        new ContentPresenter { Name = "PART_ModalPresenter" }.RegisterInNameScope(ns),
-                    }
-                };
-            });
-        }
-
-        private class ControllableTransition : IPageTransition
-        {
-            private readonly Task _gate;
-
-            public ControllableTransition(Task gate)
-            {
-                _gate = gate;
-            }
-
-            public async Task Start(Visual? from, Visual? to, bool forward, CancellationToken cancellationToken)
-            {
-                if (to != null)
-                    to.IsVisible = true;
-                await _gate;
-                if (from != null)
-                    from.IsVisible = false;
-            }
-        }
     }
 
     public class SwipeGestureTests : ScopedTestBase
@@ -2654,59 +2725,6 @@ public class NavigationPageTests
 
             Assert.False(nav.IsNavigating);
         }
-
-        private static NavigationPage CreateNavigationPage(IPageTransition? transition)
-        {
-            var nav = new NavigationPage
-            {
-                PageTransition = transition,
-                Template = new FuncControlTemplate<NavigationPage>((parent, ns) =>
-                {
-                    return new Panel
-                    {
-                        Children =
-                        {
-                            new Panel
-                            {
-                                Name = "PART_ContentHost",
-                                Children =
-                                {
-                                    new ContentPresenter { Name = "PART_PageBackPresenter" }.RegisterInNameScope(ns),
-                                    new ContentPresenter { Name = "PART_PagePresenter" }.RegisterInNameScope(ns),
-                                }
-                            }.RegisterInNameScope(ns),
-                            new Border { Name = "PART_NavigationBar",
-                                Child = new Button { Name = "PART_BackButton" }.RegisterInNameScope(ns) }.RegisterInNameScope(ns),
-                            new ContentPresenter { Name = "PART_TopCommandBar" }.RegisterInNameScope(ns),
-                            new ContentPresenter { Name = "PART_ModalBackPresenter" }.RegisterInNameScope(ns),
-                            new ContentPresenter { Name = "PART_ModalPresenter" }.RegisterInNameScope(ns),
-                        }
-                    };
-                })
-            };
-            var root = new TestRoot { Child = nav };
-            root.LayoutManager.ExecuteInitialLayoutPass();
-            return nav;
-        }
-
-        private class ControllableTransition : IPageTransition
-        {
-            private readonly Task _gate;
-
-            public ControllableTransition(Task gate)
-            {
-                _gate = gate;
-            }
-
-            public async Task Start(Visual? from, Visual? to, bool forward, CancellationToken cancellationToken)
-            {
-                if (to != null)
-                    to.IsVisible = true;
-                await _gate;
-                if (from != null)
-                    from.IsVisible = false;
-            }
-        }
     }
 
     public class VisualTreeLifecycleTests : ScopedTestBase
@@ -2714,35 +2732,7 @@ public class NavigationPageTests
         [Fact]
         public async Task Detach_And_Reattach_PreservesModalStack()
         {
-            var nav = new NavigationPage
+            var nav = new NavigationPage { Template = CreateNavigationPageTemplate() };
-            {
-                Template = new FuncControlTemplate<NavigationPage>((parent, ns) =>
-                {
-                    return new Panel
-                    {
-                        Children =
-                        {
-                            new Panel
-                            {
-                                Name = "PART_ContentHost",
-                                Children =
-                                {
-                                    new ContentPresenter { Name = "PART_PageBackPresenter" }.RegisterInNameScope(ns),
-                                    new ContentPresenter { Name = "PART_PagePresenter" }.RegisterInNameScope(ns),
-                                }
-                            }.RegisterInNameScope(ns),
-                            new Border
-                            {
-                                Name = "PART_NavigationBar",
-                                Child = new Button { Name = "PART_BackButton" }.RegisterInNameScope(ns)
-                            }.RegisterInNameScope(ns),
-                            new ContentPresenter { Name = "PART_TopCommandBar" }.RegisterInNameScope(ns),
-                            new ContentPresenter { Name = "PART_ModalBackPresenter" }.RegisterInNameScope(ns),
-                            new ContentPresenter { Name = "PART_ModalPresenter" }.RegisterInNameScope(ns),
-                        }
-                    };
-                })
-            };
 
             var root = new TestRoot { Child = nav };
             root.LayoutManager.ExecuteInitialLayoutPass();