[Navigation] Fire Page lifecycle events after transitions complete, not before (#20826)

* Send Page lifecycle events after page transitions

* Cleanup

* Moved the push lifecycle calls out of ExecutePushCore

---------

Co-authored-by: Javier Suárez Ruiz <javiersuarezruiz@hotmail.com>

src/Avalonia.Controls/Page/NavigationPage.cs

@@ -43,6 +43,7 @@ namespace Avalonia.Controls
         private ContentPresenter? _pagePresenter;
         private ContentPresenter? _pageBackPresenter;
         private CancellationTokenSource? _currentTransition;
+        private Task _lastPageTransitionTask = Task.CompletedTask;
         private CancellationTokenSource? _currentModalTransition;
         private Border? _navBar;
         private Border? _navBarShadow;
@@ -769,15 +770,13 @@ namespace Avalonia.Controls
             page.SetInNavigationPage(true);
 
             UpdateActivePage();
-
-            previousPage?.SendNavigatedFrom(new NavigatedFromEventArgs(page, NavigationType.Push));
-            page.SendNavigatedTo(new NavigatedToEventArgs(previousPage, NavigationType.Push));
-            Pushed?.Invoke(this, new NavigationEventArgs(page, NavigationType.Push));
         }
 
         /// <summary>
-        /// Performs the stack mutation and lifecycle events for a pop. The visual transition runs
+        /// Performs the stack mutation for a pop. The visual transition runs
-        /// subsequently via <see cref="UpdateActivePage"/>.
+        /// subsequently via <see cref="UpdateActivePage"/>. Callers are responsible
+        /// for firing lifecycle events via <see cref="SendPopLifecycleEvents"/>
+        /// after awaiting the page transition where possible.
         /// </summary>
         private Page? ExecutePopCore()
         {
@@ -810,11 +809,6 @@ namespace Avalonia.Controls
             {
                 old.Navigation = null;
                 old.SetInNavigationPage(false);
-
-                var newCurrentPage = CurrentPage;
-                old.SendNavigatedFrom(new NavigatedFromEventArgs(newCurrentPage, NavigationType.Pop));
-                newCurrentPage?.SendNavigatedTo(new NavigatedToEventArgs(old, NavigationType.Pop));
-                Popped?.Invoke(this, new NavigationEventArgs(old, NavigationType.Pop));
             }
 
             return old;
@@ -844,6 +838,12 @@ namespace Avalonia.Controls
                 }
 
                 ExecutePushCore(page, previousPage);
+
+                await AwaitPageTransitionAsync();
+
+                previousPage?.SendNavigatedFrom(new NavigatedFromEventArgs(page, NavigationType.Push));
+                page.SendNavigatedTo(new NavigatedToEventArgs(previousPage, NavigationType.Push));
+                Pushed?.Invoke(this, new NavigationEventArgs(page, NavigationType.Push));
             }
             finally
             {
@@ -886,7 +886,14 @@ namespace Avalonia.Controls
                         return null;
                 }
 
-                return ExecutePopCore();
+                var old = ExecutePopCore();
+
+                await AwaitPageTransitionAsync();
+
+                if (old != null)
+                    SendPopLifecycleEvents(old, NavigationType.Pop);
+
+                return old;
             }
             finally
             {
@@ -931,6 +938,7 @@ namespace Avalonia.Controls
                 }
 
                 bool isIncc = Pages is INotifyCollectionChanged;
+                var poppedPages = new List<Page>();
 
                 void TearDownPopped(Page popped)
                 {
@@ -939,8 +947,7 @@ namespace Avalonia.Controls
                         LogicalChildren.Remove(poppedLogical);
                     popped.Navigation = null;
                     popped.SetInNavigationPage(false);
-                    popped.SendNavigatedFrom(new NavigatedFromEventArgs(rootPage, NavigationType.PopToRoot));
+                    poppedPages.Add(popped);
-                    Popped?.Invoke(this, new NavigationEventArgs(popped, NavigationType.PopToRoot));
                 }
 
                 if (Pages is Stack<Page> stack)
@@ -962,6 +969,14 @@ namespace Avalonia.Controls
                 _isPop = true;
                 UpdateActivePage();
 
+                await AwaitPageTransitionAsync();
+
+                foreach (var popped in poppedPages)
+                {
+                    popped.SendNavigatedFrom(new NavigatedFromEventArgs(rootPage, NavigationType.PopToRoot));
+                    Popped?.Invoke(this, new NavigationEventArgs(popped, NavigationType.PopToRoot));
+                }
+
                 var newCurrentPage = CurrentPage;
 
                 if (newCurrentPage != null)
@@ -1013,6 +1028,7 @@ namespace Avalonia.Controls
                 }
 
                 bool isIncc = Pages is INotifyCollectionChanged;
+                var poppedPages = new List<Page>();
 
                 void TearDownPopped(Page popped)
                 {
@@ -1021,8 +1037,7 @@ namespace Avalonia.Controls
                         LogicalChildren.Remove(poppedLogical);
                     popped.Navigation = null;
                     popped.SetInNavigationPage(false);
-                    popped.SendNavigatedFrom(new NavigatedFromEventArgs(page, NavigationType.Pop));
+                    poppedPages.Add(popped);
-                    Popped?.Invoke(this, new NavigationEventArgs(popped, NavigationType.Pop));
                 }
 
                 if (Pages is Stack<Page> stack)
@@ -1044,6 +1059,14 @@ namespace Avalonia.Controls
                 _isPop = true;
                 UpdateActivePage();
 
+                await AwaitPageTransitionAsync();
+
+                foreach (var popped in poppedPages)
+                {
+                    popped.SendNavigatedFrom(new NavigatedFromEventArgs(page, NavigationType.Pop));
+                    Popped?.Invoke(this, new NavigationEventArgs(popped, NavigationType.Pop));
+                }
+
                 var newCurrentPage = CurrentPage;
                 if (newCurrentPage != null)
                 {
@@ -1356,7 +1379,9 @@ namespace Avalonia.Controls
             {
                 if (stack.Count > 0 && ReferenceEquals(stack.Peek(), page))
                 {
-                    ExecutePopCore();
+                    var old = ExecutePopCore();
+                    if (old != null)
+                        SendPopLifecycleEvents(old, NavigationType.Pop);
                     PageRemoved?.Invoke(this, new PageRemovedEventArgs(page));
                     return;
                 }
@@ -1387,7 +1412,9 @@ namespace Avalonia.Controls
 
                 if (idx == list.Count - 1)
                 {
-                    ExecutePopCore();
+                    var old = ExecutePopCore();
+                    if (old != null)
+                        SendPopLifecycleEvents(old, NavigationType.Pop);
                     PageRemoved?.Invoke(this, new PageRemovedEventArgs(page));
                     return;
                 }
@@ -1595,12 +1622,14 @@ namespace Avalonia.Controls
                         oldPresenter.ZIndex = 0;
                     }
 
-                    _ = RunPageTransitionAsync(resolvedTransition, oldPresenter, newPresenter, !isPop, cancel.Token);
+                    _lastPageTransitionTask = RunPageTransitionAsync(resolvedTransition, oldPresenter, newPresenter, !isPop, cancel.Token);
 
                     (_pagePresenter, _pageBackPresenter) = (newPresenter, oldPresenter);
                 }
                 else
                 {
+                    _lastPageTransitionTask = Task.CompletedTask;
+
                     _pagePresenter.Content = page;
                     _pagePresenter.IsVisible = page != null;
                     _pagePresenter.ZIndex = 0;
@@ -1686,6 +1715,25 @@ namespace Avalonia.Controls
             from.Opacity = 1;
         }
 
+        private Task AwaitPageTransitionAsync()
+        {
+            var task = _lastPageTransitionTask;
+            _lastPageTransitionTask = Task.CompletedTask;
+            return task;
+        }
+
+        /// <summary>
+        /// Fires lifecycle events after a pop: SendNavigatedFrom on the old page,
+        /// SendNavigatedTo on the new current page, and raises the Popped event.
+        /// </summary>
+        private void SendPopLifecycleEvents(Page oldPage, NavigationType navigationType)
+        {
+            var newCurrentPage = CurrentPage;
+            oldPage.SendNavigatedFrom(new NavigatedFromEventArgs(newCurrentPage, navigationType));
+            newCurrentPage?.SendNavigatedTo(new NavigatedToEventArgs(oldPage, navigationType));
+            Popped?.Invoke(this, new NavigationEventArgs(oldPage, navigationType));
+        }
+
         /// <summary>
         /// Swaps the top of the navigation stack with <paramref name="page"/>.
         /// </summary>

tests/Avalonia.Controls.UnitTests/NavigationPageTests.cs

@@ -1,7 +1,11 @@
 using System;
 using System.Collections.Generic;
+using System.Threading;
 using System.Threading.Tasks;
 using Avalonia.Animation;
+using Avalonia.Controls.Presenters;
+using Avalonia.Controls.Primitives;
+using Avalonia.Controls.Templates;
 using Avalonia.Interactivity;
 using Avalonia.LogicalTree;
 using Avalonia.UnitTests;
@@ -1574,4 +1578,164 @@ public class NavigationPageTests
         }
     }
 
+    public class LifecycleAfterTransitionTests : ScopedTestBase
+    {
+        [Fact]
+        public async Task PushAsync_LifecycleEvents_FireAfterTransition()
+        {
+            var tcs = new TaskCompletionSource();
+            var transition = new ControllableTransition(tcs.Task);
+            var nav = CreateNavigationPage(transition);
+
+            var root = new ContentPage { Header = "Root" };
+            await nav.PushAsync(root);
+
+            bool navigatedFromDuringTransition = false;
+            bool navigatedToDuringTransition = false;
+            bool pushedDuringTransition = false;
+
+            var second = new ContentPage { Header = "Second" };
+            root.NavigatedFrom += (_, _) => navigatedFromDuringTransition = !tcs.Task.IsCompleted;
+            second.NavigatedTo += (_, _) => navigatedToDuringTransition = !tcs.Task.IsCompleted;
+            nav.Pushed += (_, _) => pushedDuringTransition = !tcs.Task.IsCompleted;
+
+            var pushTask = nav.PushAsync(second);
+
+            tcs.SetResult();
+            await pushTask;
+
+            Assert.False(navigatedFromDuringTransition);
+            Assert.False(navigatedToDuringTransition);
+            Assert.False(pushedDuringTransition);
+        }
+
+        [Fact]
+        public async Task PopAsync_LifecycleEvents_FireAfterTransition()
+        {
+            var tcs = new TaskCompletionSource();
+            var nav = CreateNavigationPage(null);
+
+            var root = new ContentPage { Header = "Root" };
+            var top = new ContentPage { Header = "Top" };
+            await nav.PushAsync(root);
+            await nav.PushAsync(top);
+
+            nav.PageTransition = new ControllableTransition(tcs.Task);
+
+            bool navigatedFromDuringTransition = false;
+            bool navigatedToDuringTransition = false;
+            bool poppedDuringTransition = false;
+
+            top.NavigatedFrom += (_, _) => navigatedFromDuringTransition = !tcs.Task.IsCompleted;
+            root.NavigatedTo += (_, _) => navigatedToDuringTransition = !tcs.Task.IsCompleted;
+            nav.Popped += (_, _) => poppedDuringTransition = !tcs.Task.IsCompleted;
+
+            var popTask = nav.PopAsync();
+
+            tcs.SetResult();
+            await popTask;
+
+            Assert.False(navigatedFromDuringTransition);
+            Assert.False(navigatedToDuringTransition);
+            Assert.False(poppedDuringTransition);
+        }
+
+        [Fact]
+        public async Task PopToRootAsync_LifecycleEvents_FireAfterTransition()
+        {
+            var tcs = new TaskCompletionSource();
+            var nav = CreateNavigationPage(null);
+
+            var root = new ContentPage { Header = "Root" };
+            var second = new ContentPage { Header = "Second" };
+            var third = new ContentPage { Header = "Third" };
+            await nav.PushAsync(root);
+            await nav.PushAsync(second);
+            await nav.PushAsync(third);
+
+            nav.PageTransition = new ControllableTransition(tcs.Task);
+
+            bool navigatedFromDuringTransition = false;
+            bool navigatedToDuringTransition = false;
+            bool poppedToRootDuringTransition = false;
+
+            second.NavigatedFrom += (_, _) => navigatedFromDuringTransition = !tcs.Task.IsCompleted;
+            third.NavigatedFrom += (_, _) => navigatedFromDuringTransition = !tcs.Task.IsCompleted;
+            root.NavigatedTo += (_, _) => navigatedToDuringTransition = !tcs.Task.IsCompleted;
+            nav.PoppedToRoot += (_, _) => poppedToRootDuringTransition = !tcs.Task.IsCompleted;
+
+            var popTask = nav.PopToRootAsync();
+
+            tcs.SetResult();
+            await popTask;
+
+            Assert.False(navigatedFromDuringTransition);
+            Assert.False(navigatedToDuringTransition);
+            Assert.False(poppedToRootDuringTransition);
+        }
+
+        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;
+            }
+        }
+    }
+
 }