When popping multiple pages remove middle pages first (#12331)

* When popping multiple pages remove middle pages first * - additional remove page scenarios * - clean up code and fix iOS crash * - remove comment * - fix android when removing last page * Update ShellSectionRenderer.cs * Update ShellSectionRenderer.cs
xamarin · Oct 9, 2020 · a695fab · a695fab
1 parent 31e7552
commit a695fab
Show file tree

Hide file tree

Showing 7 changed files with 475 additions and 77 deletions.
diff --git a/Xamarin.Forms.Core.UnitTests/ShellNavigatingTests.cs b/Xamarin.Forms.Core.UnitTests/ShellNavigatingTests.cs
@@ -5,6 +5,7 @@
 using System.Text;
 using System.Threading.Tasks;
 using NUnit.Framework;
+using Xamarin.Forms.Internals;
 
 namespace Xamarin.Forms.Core.UnitTests
 {
@@ -237,6 +238,203 @@ public async Task PopToRootWithMultipleFlyoutItems()
 			await testShell.Navigation.PopToRootAsync();
 		}
 
+		[Test]
+		public async Task MultiplePopsRemoveMiddlePagesBeforeFinalPop()
+		{
+			TestShell testShell = new TestShell(
+				CreateShellSection<NavigationMonitoringTab>(shellContentRoute: "rootpage")
+			);
+
+			var pageLeftOnStack = new ContentPage();
+			var tab = (NavigationMonitoringTab)testShell.CurrentItem.CurrentItem;
+			await testShell.Navigation.PushAsync(pageLeftOnStack);
+			await testShell.Navigation.PushAsync(new ContentPage());
+			await testShell.Navigation.PushAsync(new ContentPage());
+			tab.NavigationsFired.Clear();
+			await testShell.GoToAsync("../..");
+			Assert.That(testShell.CurrentState.Location.ToString(),
+				Is.EqualTo($"//rootpage/{Routing.GetRoute(pageLeftOnStack)}"));
+
+			Assert.AreEqual("OnRemovePage", tab.NavigationsFired[0]);
+			Assert.AreEqual("OnPopAsync", tab.NavigationsFired[1]);
+			Assert.AreEqual(2, tab.NavigationsFired.Count);
+		}
+
+		[Test]
+		public async Task PopToRootRemovesMiddlePagesBeforePoppingVisibleModalPages()
+		{
+			Routing.RegisterRoute("ModalTestPage", typeof(ShellModalTests.ModalTestPage));
+			TestShell testShell = new TestShell(
+				CreateShellSection<NavigationMonitoringTab>(shellContentRoute: "rootpage")
+			);
+
+			var middlePage = new ContentPage();
+			var tab = (NavigationMonitoringTab)testShell.CurrentItem.CurrentItem;
+			await testShell.Navigation.PushAsync(middlePage);
+			await testShell.GoToAsync("ModalTestPage");			
+			tab.NavigationsFired.Clear();
+
+			await testShell.GoToAsync("../..");
+			Assert.That(testShell.CurrentState.Location.ToString(),
+				Is.EqualTo($"//rootpage"));
+
+			Assert.AreEqual("OnRemovePage", tab.NavigationsFired[0]);
+			Assert.AreEqual("OnPopModal", tab.NavigationsFired[1]);
+			Assert.AreEqual(2, tab.NavigationsFired.Count);
+		}
+
+
+
+		[Test]
+		public async Task MultiplePopsRemoveMiddlePagesBeforeFinalPopWhenUsingModal()
+		{
+			Routing.RegisterRoute("ModalTestPage", typeof(ShellModalTests.ModalTestPage));
+			TestShell testShell = new TestShell(
+				CreateShellSection<NavigationMonitoringTab>(shellContentRoute: "rootpage")
+			);
+
+			var pageLeftOnStack = new ContentPage();
+			var middlePage = new ContentPage();
+			var tab = (NavigationMonitoringTab)testShell.CurrentItem.CurrentItem;
+			await testShell.Navigation.PushAsync(pageLeftOnStack);
+			await testShell.Navigation.PushAsync(middlePage);
+			await testShell.GoToAsync("ModalTestPage");
+			tab.NavigationsFired.Clear();
+
+			await testShell.GoToAsync("../..");
+
+			Assert.That(testShell.CurrentState.Location.ToString(),
+				Is.EqualTo($"//rootpage/{Routing.GetRoute(pageLeftOnStack)}"));
+
+			Assert.AreEqual("OnRemovePage", tab.NavigationsFired[0]);
+			Assert.AreEqual("OnPopModal", tab.NavigationsFired[1]);
+			Assert.AreEqual(2, tab.NavigationsFired.Count);
+		}
+
+
+		[Test]
+		public async Task SwappingOutVisiblePageDoesntRevealPreviousPage()
+		{
+			TestShell testShell = new TestShell(
+				CreateShellSection<NavigationMonitoringTab>(shellContentRoute: "rootpage")
+			);
+
+
+			testShell.RegisterPage("firstPage");
+			testShell.RegisterPage("pageToSwapIn");
+
+			var tab = (NavigationMonitoringTab)testShell.CurrentItem.CurrentItem;
+			await testShell.GoToAsync("firstPage");
+			tab.NavigationsFired.Clear();
+
+			await testShell.GoToAsync($"../pageToSwapIn");
+			Assert.That(testShell.CurrentState.Location.ToString(),
+				Is.EqualTo($"//rootpage/pageToSwapIn"));
+
+			Assert.AreEqual("OnPushAsync", tab.NavigationsFired[0]);
+			Assert.AreEqual("OnRemovePage", tab.NavigationsFired[1]);
+			Assert.AreEqual(2, tab.NavigationsFired.Count);
+		}
+
+
+		[Test]
+		public async Task MiddleRoutesAreRemovedWithoutPoppingStack()
+		{
+			TestShell testShell = new TestShell(
+				CreateShellSection<NavigationMonitoringTab>(shellContentRoute: "rootpage")
+			);
+
+			testShell.RegisterPage("firstPage");
+			testShell.RegisterPage("secondPage");
+			testShell.RegisterPage("thirdPage");
+			testShell.RegisterPage("fourthPage");
+			testShell.RegisterPage("fifthPage");
+
+			var tab = (NavigationMonitoringTab)testShell.CurrentItem.CurrentItem;
+			await testShell.GoToAsync("firstPage/secondPage/thirdPage/fourthPage/fifthPage");
+			tab.NavigationsFired.Clear();
+
+			Assert.That(testShell.CurrentState.Location.ToString(),
+				Is.EqualTo($"//rootpage/firstPage/secondPage/thirdPage/fourthPage/fifthPage"));
+
+			await testShell.GoToAsync($"//rootpage/thirdPage/fifthPage");
+			Assert.That(testShell.CurrentState.Location.ToString(),
+				Is.EqualTo($"//rootpage/thirdPage/fifthPage"));
+
+			Assert.AreEqual("OnRemovePage", tab.NavigationsFired[0]);
+			Assert.AreEqual("OnRemovePage", tab.NavigationsFired[1]);
+			Assert.AreEqual("OnRemovePage", tab.NavigationsFired[2]);
+			Assert.AreEqual(3, tab.NavigationsFired.Count);
+		}
+
+		public class NavigationMonitoringTab : Tab
+		{
+			public List<string> NavigationsFired = new List<string>();
+
+			public NavigationMonitoringTab()
+			{
+				Navigation = new NavigationImpl(this, Navigation);
+			}
+
+			protected override Task OnPushAsync(Page page, bool animated)
+			{
+				NavigationsFired.Add(nameof(OnPushAsync));
+				return base.OnPushAsync(page, animated);
+			}
+
+			protected override void OnRemovePage(Page page)
+			{
+				base.OnRemovePage(page);
+				NavigationsFired.Add(nameof(OnRemovePage));
+			}
+
+			protected override Task<Page> OnPopAsync(bool animated)
+			{
+				NavigationsFired.Add(nameof(OnPopAsync));
+				return base.OnPopAsync(animated);
+			}
+
+			public class NavigationImpl : NavigationProxy
+			{
+				readonly NavigationMonitoringTab _navigationMonitoringTab;
+				readonly INavigation _navigation;
+
+				public NavigationImpl(
+					NavigationMonitoringTab navigationMonitoringTab, 
+					INavigation navigation)
+				{
+					_navigationMonitoringTab = navigationMonitoringTab;
+					_navigation = navigation;
+				}
+
+				protected override IReadOnlyList<Page> GetModalStack() => _navigation.ModalStack;
+
+				protected override IReadOnlyList<Page> GetNavigationStack() => _navigation.NavigationStack;
+
+				protected override void OnInsertPageBefore(Page page, Page before) => _navigation.InsertPageBefore(page, before);
+
+				protected override Task<Page> OnPopAsync(bool animated) => _navigation.PopAsync(animated);
+
+				protected override Task<Page> OnPopModal(bool animated)
+				{
+					_navigationMonitoringTab.NavigationsFired.Add(nameof(OnPopModal));
+					return _navigation.PopModalAsync(animated);
+				}
+
+				protected override Task OnPopToRootAsync(bool animated) => _navigation.PopToRootAsync(animated);
+
+				protected override Task OnPushAsync(Page page, bool animated) => _navigation.PushAsync(page, animated);
+
+				protected override Task OnPushModal(Page modal, bool animated)
+				{
+					_navigationMonitoringTab.NavigationsFired.Add(nameof(OnPushModal));
+					return _navigation.PushModalAsync(modal, animated);
+				}
+
+				protected override void OnRemovePage(Page page) => _navigation.RemovePage(page);
+			}
+		}
+
 		ShellNavigatingEventArgs CreateShellNavigatedEventArgs() =>
 			new ShellNavigatingEventArgs("..", "../newstate", ShellNavigationSource.Push, true);
 	}

diff --git a/Xamarin.Forms.Core.UnitTests/ShellTestBase.cs b/Xamarin.Forms.Core.UnitTests/ShellTestBase.cs
@@ -311,6 +311,31 @@ public TestShell(params ShellItem[] shellItems) : this()
 				shellItems.ForEach(x => Items.Add(x));
 			}
 
+			public ContentPage RegisterPage(string route)
+			{
+				ContentPage page = new ContentPage();
+				RegisterPage(route, page);
+				return page;
+			}
+
+			public void RegisterPage(string route, ContentPage contentPage)
+			{
+				Routing.SetRoute(contentPage, route);
+				Routing.RegisterRoute(route, new ConcretePageFactory(contentPage));
+			}
+
+			public class ConcretePageFactory : RouteFactory
+			{
+				ContentPage _contentPage;
+
+				public ConcretePageFactory(ContentPage contentPage)
+				{
+					_contentPage = contentPage;
+				}
+
+				public override Element GetOrCreate() => _contentPage;
+			}
+
 			public Action<ShellNavigatedEventArgs> OnNavigatedHandler { get; set; }
 			protected override void OnNavigated(ShellNavigatedEventArgs args)
 			{

diff --git a/Xamarin.Forms.Core/Shell/Shell.cs b/Xamarin.Forms.Core/Shell/Shell.cs
@@ -603,6 +603,14 @@ internal async Task GoToAsync(ShellNavigationParameters shellNavigationParameter
 					// - or route contains no global route requests
 					if (navigatedToNewShellElement || navigationRequest.Request.GlobalRoutes.Count == 0)
 					{
+						// remove all non visible pages first so the transition just smoothly goes from
+						// currently visible modal to base page
+						if(navigationRequest.Request.GlobalRoutes.Count == 0)
+						{
+							for (int i = currentShellSection.Stack.Count - 1; i >= 1; i--)
+								currentShellSection.Navigation.RemovePage(currentShellSection.Stack[i]);
+						}
+
 						await currentShellSection.PopModalStackToPage(null, animate);
 					}
 				}