diff --git a/Xamarin.Forms.ControlGallery.iOS/Xamarin.Forms.ControlGallery.iOS.csproj b/Xamarin.Forms.ControlGallery.iOS/Xamarin.Forms.ControlGallery.iOS.csproj index b39758230f4..e69d017231c 100644 --- a/Xamarin.Forms.ControlGallery.iOS/Xamarin.Forms.ControlGallery.iOS.csproj +++ b/Xamarin.Forms.ControlGallery.iOS/Xamarin.Forms.ControlGallery.iOS.csproj @@ -390,7 +390,7 @@ - + diff --git a/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Issue13916.cs b/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Issue13916.cs new file mode 100644 index 00000000000..b03dbe4b984 --- /dev/null +++ b/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Issue13916.cs @@ -0,0 +1,90 @@ +using System; +using System.Collections.Generic; +using System.Collections.ObjectModel; +using System.Text; +using Xamarin.Forms.CustomAttributes; +using Xamarin.Forms.Internals; + + +#if UITEST +using Xamarin.UITest; +using NUnit.Framework; +using Xamarin.Forms.Core.UITests; +#endif + +namespace Xamarin.Forms.Controls.Issues +{ + [Preserve(AllMembers = true)] + [Issue(IssueTracker.Github, 13916, "[iOS] iOS Application crashes on Back press when navigated to using GoToAsync with \"//\" or \"///\" route if 2 or more things are removed from the navigation stack", + PlatformAffected.iOS)] +#if UITEST + [NUnit.Framework.Category(Core.UITests.UITestCategories.Github5000)] + [NUnit.Framework.Category(UITestCategories.Shell)] +#endif + public class Issue13916 : TestShell + { + static int pageCount = 1; + protected override void Init() + { + Routing.RegisterRoute(nameof(Issue13916SuccessPage), typeof(Issue13916SuccessPage)); + + AddFlyoutItem(CreateContentPage(), "Push Me"); + } + + + public class Issue13916SuccessPage : ContentPage + { + public Issue13916SuccessPage() + { + StackLayout layout = new StackLayout(); + Label label = new Label() + { + Text = "Success", + AutomationId = "Success" + }; + layout.Children.Add(label); + Content = layout; + } + } + + ContentPage CreateContentPage() + { + StackLayout layout = new StackLayout(); + Button button = new Button() + { + Text = "Click Me", + AutomationId = $"ClickMe{pageCount}", + Command = new Command(async () => + { + if (Navigation.NavigationStack.Count >= 3) + { + await GoToAsync($"../../{nameof(Issue13916SuccessPage)}"); + } + else + { + await Navigation.PushAsync(CreateContentPage()); + } + }) + }; + pageCount++; + + layout.Children.Add(button); + + return new ContentPage() + { + Content = layout + }; + } + +#if UITEST + [Test] + public void RemovingMoreThanOneInnerPageAndThenPushingAPageCrashes() + { + RunningApp.Tap("ClickMe1"); + RunningApp.Tap("ClickMe2"); + RunningApp.Tap("ClickMe3"); + RunningApp.WaitForElement("Success"); + } +#endif + } +} diff --git a/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Xamarin.Forms.Controls.Issues.Shared.projitems b/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Xamarin.Forms.Controls.Issues.Shared.projitems index 309f493248b..f32dce8a3fe 100644 --- a/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Xamarin.Forms.Controls.Issues.Shared.projitems +++ b/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Xamarin.Forms.Controls.Issues.Shared.projitems @@ -21,6 +21,7 @@ + diff --git a/Xamarin.Forms.Core.Android.UITests/Xamarin.Forms.Core.Android.UITests.csproj b/Xamarin.Forms.Core.Android.UITests/Xamarin.Forms.Core.Android.UITests.csproj index 853774bc28b..3310d04552d 100644 --- a/Xamarin.Forms.Core.Android.UITests/Xamarin.Forms.Core.Android.UITests.csproj +++ b/Xamarin.Forms.Core.Android.UITests/Xamarin.Forms.Core.Android.UITests.csproj @@ -48,7 +48,7 @@ - + 3.17.0 diff --git a/Xamarin.Forms.Core.UnitTests/ShellNavigatedArgsTests.cs b/Xamarin.Forms.Core.UnitTests/ShellNavigatedArgsTests.cs new file mode 100644 index 00000000000..538e48b2248 --- /dev/null +++ b/Xamarin.Forms.Core.UnitTests/ShellNavigatedArgsTests.cs @@ -0,0 +1,211 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using System.Runtime.InteropServices; +using System.Text; +using System.Threading.Tasks; +using NUnit.Framework; +using Xamarin.Forms.Internals; + +namespace Xamarin.Forms.Core.UnitTests +{ + [TestFixture] + public class ShellNavigatedArgsTests : ShellTestBase + { + [TearDown] + public override void TearDown() + { + base.TearDown(); + Routing.Clear(); + } + + [Test] + public async Task RemoveInnerPagesNavigatingArgs() + { + Routing.RegisterRoute("SecondPageView", typeof(ContentPage)); + Routing.RegisterRoute("ThirdPageView", typeof(ContentPage)); + Routing.RegisterRoute("FourthPage", typeof(ContentPage)); + + var shell = new TestShell(CreateShellItem(shellContentRoute: "HomePageView")); + + await shell.GoToAsync("//HomePageView/SecondPageView/ThirdPageView"); + await shell.GoToAsync("//HomePageView/FourthPage"); + + shell.TestNavigatedArgs(ShellNavigationSource.Pop, "//HomePageView/SecondPageView/ThirdPageView", "//HomePageView/FourthPage"); + Assert.AreEqual(3, shell.NavigatedCount); + } + + [Test] + public async Task PopToRootSetsCorrectNavigationSource() + { + var shell = new TestShell(CreateShellItem()); + await shell.Navigation.PushAsync(new ContentPage()); + await shell.Navigation.PushAsync(new ContentPage()); + await shell.Navigation.PopToRootAsync(); + Assert.AreEqual(ShellNavigationSource.PopToRoot, shell.LastShellNavigatingEventArgs.Source); + + await shell.Navigation.PushAsync(new ContentPage()); + await shell.Navigation.PushAsync(new ContentPage()); + + await shell.Navigation.PopAsync(); + Assert.AreEqual(ShellNavigationSource.Pop, shell.LastShellNavigatingEventArgs.Source); + + await shell.Navigation.PopAsync(); + Assert.AreEqual(ShellNavigationSource.PopToRoot, shell.LastShellNavigatingEventArgs.Source); + } + + [Test] + public async Task PushingSetsCorrectNavigationSource() + { + var shell = new TestShell(CreateShellItem(shellItemRoute: "item1")); + shell.RegisterPage(nameof(PushingSetsCorrectNavigationSource)); + await shell.GoToAsync(nameof(PushingSetsCorrectNavigationSource)); + + shell.TestNavigatingArgs(ShellNavigationSource.Push, + "//item1", $"{nameof(PushingSetsCorrectNavigationSource)}"); + + shell.TestNavigatedArgs(ShellNavigationSource.Push, + "//item1", $"//item1/{nameof(PushingSetsCorrectNavigationSource)}"); + } + + [Test] + public async Task ChangingShellItemSetsCorrectNavigationSource() + { + var shell = new TestShell( + CreateShellItem(shellItemRoute: "item1"), + CreateShellItem(shellItemRoute: "item2") + ); + + await shell.GoToAsync("//item2"); + + shell.TestNavigationArgs(ShellNavigationSource.ShellItemChanged, + "//item1", "//item2"); + } + + [Test] + public async Task ChangingShellSectionSetsCorrectNavigationSource() + { + var shell = new TestShell( + CreateShellItem(shellSectionRoute: "item1") + ); + + shell.Items[0].Items.Add(CreateShellSection(shellContentRoute: "item2")); + + await shell.GoToAsync("//item2"); + + shell.TestNavigationArgs(ShellNavigationSource.ShellSectionChanged, + "//item1", "//item2"); + } + + [Test] + public async Task PoppingSamePageSetsCorrectNavigationSource() + { + Routing.RegisterRoute("detailspage", typeof(ContentPage)); + var shell = new TestShell(CreateShellItem(shellItemRoute: "item1")); + await shell.GoToAsync("detailspage/detailspage"); + await shell.Navigation.PopAsync(); + + + shell.TestNavigatingArgs(ShellNavigationSource.Pop, + "//item1/detailspage/detailspage", $".."); + + shell.TestNavigatedArgs(ShellNavigationSource.Pop, + "//item1/detailspage/detailspage", $"//item1/detailspage"); + } + + [Test] + public async Task ChangingShellContentSetsCorrectNavigationSource() + { + var shell = new TestShell( + CreateShellItem(shellContentRoute: "item1") + ); + + shell.Items[0].Items[0].Items.Add(CreateShellContent(shellContentRoute: "item2")); + + await shell.GoToAsync("//item2"); + + shell.TestNavigationArgs(ShellNavigationSource.ShellContentChanged, + "//item1", "//item2"); + } + + [Test] + public async Task InsertPageSetsCorrectNavigationSource() + { + Routing.RegisterRoute("pagemiddle", typeof(ContentPage)); + Routing.RegisterRoute("page", typeof(ContentPage)); + var shell = new TestShell( + CreateShellItem(shellItemRoute: "item") + ); + + await shell.GoToAsync("//item/page"); + await shell.GoToAsync("//item/pagemiddle/page"); + + shell.TestNavigationArgs(ShellNavigationSource.Insert, + "//item/page", "//item/pagemiddle/page"); + } + + + [Test] + public async Task InsertPageFromINavigationSetsCorrectNavigationSource() + { + Routing.RegisterRoute("pagemiddle", typeof(ContentPage)); + Routing.RegisterRoute("page", typeof(ContentPage)); + var shell = new TestShell( + CreateShellItem(shellItemRoute: "item") + ); + + await shell.GoToAsync("//item/page"); + ContentPage contentPage = new ContentPage(); + Routing.SetRoute(contentPage, "pagemiddle"); + shell.Navigation.InsertPageBefore(contentPage, shell.Navigation.NavigationStack.Last()); + + shell.TestNavigationArgs(ShellNavigationSource.Insert, + "//item/page", "//item/pagemiddle/page"); + } + + + [Test] + public async Task RemovePageFromINavigationSetsCorrectNavigationSource() + { + Routing.RegisterRoute("pagemiddle", typeof(ContentPage)); + Routing.RegisterRoute("page", typeof(ContentPage)); + var shell = new TestShell( + CreateShellItem(shellItemRoute: "item") + ); + + await shell.GoToAsync("//item/pagemiddle/page"); + shell.Navigation.RemovePage(shell.Navigation.NavigationStack[1]); + + shell.TestNavigationArgs(ShellNavigationSource.Remove, + "//item/pagemiddle/page", "//item/page"); + } + + [Test] + public async Task RemovePageSetsCorrectNavigationSource() + { + Routing.RegisterRoute("pagemiddle", typeof(ContentPage)); + Routing.RegisterRoute("page", typeof(ContentPage)); + var shell = new TestShell( + CreateShellItem(shellItemRoute: "item") + ); + + await shell.GoToAsync("//item/pagemiddle/page"); + await shell.GoToAsync("//item/page"); + + + shell.TestNavigationArgs(ShellNavigationSource.Remove, + "//item/pagemiddle/page", "//item/page"); + } + + [Test] + public async Task InitialNavigatingArgs() + { + var shell = new TestShell( + CreateShellItem(shellItemRoute: "item") + ); + + shell.TestNavigationArgs(ShellNavigationSource.ShellItemChanged, + null, "//item"); + } + } +} diff --git a/Xamarin.Forms.Core.UnitTests/ShellNavigatingTests.cs b/Xamarin.Forms.Core.UnitTests/ShellNavigatingTests.cs index dcfff1c3ea4..bcd802b0918 100644 --- a/Xamarin.Forms.Core.UnitTests/ShellNavigatingTests.cs +++ b/Xamarin.Forms.Core.UnitTests/ShellNavigatingTests.cs @@ -488,22 +488,6 @@ public async Task MiddleRoutesAreRemovedWithoutPoppingStack() Assert.AreEqual(3, tab.NavigationsFired.Count); } - [Test] - public async Task PoppingSamePageSetsCorrectNavigationSource() - { - Routing.RegisterRoute("detailspage", typeof(ContentPage)); - var shell = new TestShell(CreateShellItem(shellItemRoute: "item1")); - await shell.GoToAsync("detailspage/detailspage"); - await shell.Navigation.PopAsync(); - - - shell.TestNavigatingArgs(ShellNavigationSource.Pop, - "//item1/detailspage/detailspage", $".."); - - shell.TestNavigatedArgs(ShellNavigationSource.Pop, - "//item1/detailspage/detailspage", $"//item1/detailspage"); - } - [Test] public async Task PoppingSetsCorrectNavigationSource() { @@ -522,127 +506,6 @@ public async Task PoppingSetsCorrectNavigationSource() "//item1/page1/page2", $"//item1/page1"); } - [Test] - public async Task PopToRootSetsCorrectNavigationSource() - { - var shell = new TestShell(CreateShellItem()); - await shell.Navigation.PushAsync(new ContentPage()); - await shell.Navigation.PushAsync(new ContentPage()); - await shell.Navigation.PopToRootAsync(); - Assert.AreEqual(ShellNavigationSource.PopToRoot, shell.LastShellNavigatingEventArgs.Source); - - await shell.Navigation.PushAsync(new ContentPage()); - await shell.Navigation.PushAsync(new ContentPage()); - - await shell.Navigation.PopAsync(); - Assert.AreEqual(ShellNavigationSource.Pop, shell.LastShellNavigatingEventArgs.Source); - - await shell.Navigation.PopAsync(); - Assert.AreEqual(ShellNavigationSource.PopToRoot, shell.LastShellNavigatingEventArgs.Source); - } - - [Test] - public async Task PushingSetsCorrectNavigationSource() - { - var shell = new TestShell(CreateShellItem(shellItemRoute: "item1")); - shell.RegisterPage(nameof(PushingSetsCorrectNavigationSource)); - await shell.GoToAsync(nameof(PushingSetsCorrectNavigationSource)); - - shell.TestNavigatingArgs(ShellNavigationSource.Push, - "//item1", $"{nameof(PushingSetsCorrectNavigationSource)}"); - - shell.TestNavigatedArgs(ShellNavigationSource.Push, - "//item1", $"//item1/{nameof(PushingSetsCorrectNavigationSource)}"); - } - - [Test] - public async Task ChangingShellItemSetsCorrectNavigationSource() - { - var shell = new TestShell( - CreateShellItem(shellItemRoute: "item1"), - CreateShellItem(shellItemRoute: "item2") - ); - - await shell.GoToAsync("//item2"); - - shell.TestNavigationArgs(ShellNavigationSource.ShellItemChanged, - "//item1", "//item2"); - } - - [Test] - public async Task ChangingShellSectionSetsCorrectNavigationSource() - { - var shell = new TestShell( - CreateShellItem(shellSectionRoute: "item1") - ); - - shell.Items[0].Items.Add(CreateShellSection(shellContentRoute: "item2")); - - await shell.GoToAsync("//item2"); - - shell.TestNavigationArgs(ShellNavigationSource.ShellSectionChanged, - "//item1", "//item2"); - } - - [Test] - public async Task ChangingShellContentSetsCorrectNavigationSource() - { - var shell = new TestShell( - CreateShellItem(shellContentRoute: "item1") - ); - - shell.Items[0].Items[0].Items.Add(CreateShellContent(shellContentRoute: "item2")); - - await shell.GoToAsync("//item2"); - - shell.TestNavigationArgs(ShellNavigationSource.ShellContentChanged, - "//item1", "//item2"); - } - - [Test] - public async Task InsertPageSetsCorrectNavigationSource() - { - Routing.RegisterRoute("pagemiddle", typeof(ContentPage)); - Routing.RegisterRoute("page", typeof(ContentPage)); - var shell = new TestShell( - CreateShellItem(shellItemRoute: "item") - ); - - await shell.GoToAsync("//item/page"); - await shell.GoToAsync("//item/pagemiddle/page"); - - shell.TestNavigationArgs(ShellNavigationSource.Insert, - "//item/page", "//item/pagemiddle/page"); - } - - [Test] - public async Task RemovePageSetsCorrectNavigationSource() - { - Routing.RegisterRoute("pagemiddle", typeof(ContentPage)); - Routing.RegisterRoute("page", typeof(ContentPage)); - var shell = new TestShell( - CreateShellItem(shellItemRoute: "item") - ); - - await shell.GoToAsync("//item/pagemiddle/page"); - await shell.GoToAsync("//item/page"); - - - shell.TestNavigationArgs(ShellNavigationSource.Remove, - "//item/pagemiddle/page", "//item/page"); - } - - [Test] - public async Task InitialNavigatingArgs() - { - var shell = new TestShell( - CreateShellItem(shellItemRoute: "item") - ); - - shell.TestNavigationArgs(ShellNavigationSource.ShellItemChanged, - null, "//item"); - } - [TestCase(true, 2)] [TestCase(false, 2)] diff --git a/Xamarin.Forms.Core.UnitTests/ShellTestBase.cs b/Xamarin.Forms.Core.UnitTests/ShellTestBase.cs index d863ae3ce60..23ca3742fc7 100644 --- a/Xamarin.Forms.Core.UnitTests/ShellTestBase.cs +++ b/Xamarin.Forms.Core.UnitTests/ShellTestBase.cs @@ -374,8 +374,6 @@ protected override void OnNavigating(ShellNavigatingEventArgs args) OnNavigatingCount++; } - - public void TestNavigationArgs(ShellNavigationSource source, string from, string to) { TestNavigatingArgs(source, from, to); @@ -392,6 +390,7 @@ public void TestNavigatedArgs(ShellNavigationSource source, string from, string Assert.AreEqual(from, this.LastShellNavigatedEventArgs.Previous.Location.ToString()); Assert.AreEqual(to, this.LastShellNavigatedEventArgs.Current.Location.ToString()); + Assert.AreEqual(to, this.CurrentState.Location.ToString()); } public void TestNavigatingArgs(ShellNavigationSource source, string from, string to) diff --git a/Xamarin.Forms.Core.UnitTests/Xamarin.Forms.Core.UnitTests.csproj b/Xamarin.Forms.Core.UnitTests/Xamarin.Forms.Core.UnitTests.csproj index 77570a2b975..be859cdf495 100644 --- a/Xamarin.Forms.Core.UnitTests/Xamarin.Forms.Core.UnitTests.csproj +++ b/Xamarin.Forms.Core.UnitTests/Xamarin.Forms.Core.UnitTests.csproj @@ -94,6 +94,7 @@ + diff --git a/Xamarin.Forms.Core.Windows.UITests/Xamarin.Forms.Core.Windows.UITests.csproj b/Xamarin.Forms.Core.Windows.UITests/Xamarin.Forms.Core.Windows.UITests.csproj index 49d17671b46..46b7a665b5f 100644 --- a/Xamarin.Forms.Core.Windows.UITests/Xamarin.Forms.Core.Windows.UITests.csproj +++ b/Xamarin.Forms.Core.Windows.UITests/Xamarin.Forms.Core.Windows.UITests.csproj @@ -53,7 +53,7 @@ - + 3.17.0 diff --git a/Xamarin.Forms.Core.iOS.UITests/Xamarin.Forms.Core.iOS.UITests.csproj b/Xamarin.Forms.Core.iOS.UITests/Xamarin.Forms.Core.iOS.UITests.csproj index dd98f39c088..f1b8c74abe3 100644 --- a/Xamarin.Forms.Core.iOS.UITests/Xamarin.Forms.Core.iOS.UITests.csproj +++ b/Xamarin.Forms.Core.iOS.UITests/Xamarin.Forms.Core.iOS.UITests.csproj @@ -49,7 +49,7 @@ - + 3.17.0 diff --git a/Xamarin.Forms.Core.macOS.UITests/Xamarin.Forms.Core.macOS.UITests.csproj b/Xamarin.Forms.Core.macOS.UITests/Xamarin.Forms.Core.macOS.UITests.csproj index 91658911124..f1a8f4277f2 100644 --- a/Xamarin.Forms.Core.macOS.UITests/Xamarin.Forms.Core.macOS.UITests.csproj +++ b/Xamarin.Forms.Core.macOS.UITests/Xamarin.Forms.Core.macOS.UITests.csproj @@ -53,7 +53,7 @@ - + diff --git a/Xamarin.Forms.Core/Shell/Shell.cs b/Xamarin.Forms.Core/Shell/Shell.cs index 789e27994af..2e8671db5e8 100644 --- a/Xamarin.Forms.Core/Shell/Shell.cs +++ b/Xamarin.Forms.Core/Shell/Shell.cs @@ -468,10 +468,13 @@ void IShellController.UpdateCurrentState(ShellNavigationSource source) var modalStack = shellSection?.Navigation?.ModalStack; var result = ShellNavigationManager.GetNavigationState(shellItem, shellSection, shellContent, stack, modalStack); - SetValueFromRenderer(CurrentStatePropertyKey, result); - - _navigationManager.HandleNavigated(new ShellNavigatedEventArgs(oldState, CurrentState, source)); + if (result?.Location != oldState?.Location) + { + SetValueFromRenderer(CurrentStatePropertyKey, result); + _navigationManager.HandleNavigated(new ShellNavigatedEventArgs(oldState, CurrentState, source)); + } } + ReadOnlyCollection IShellController.GetItems() => new ReadOnlyCollection(((ShellItemCollection)Items).VisibleItemsReadOnly.ToList()); diff --git a/Xamarin.Forms.Core/Shell/ShellNavigationManager.cs b/Xamarin.Forms.Core/Shell/ShellNavigationManager.cs index 172f0558ea5..530c014bbfb 100644 --- a/Xamarin.Forms.Core/Shell/ShellNavigationManager.cs +++ b/Xamarin.Forms.Core/Shell/ShellNavigationManager.cs @@ -11,7 +11,7 @@ internal class ShellNavigationManager readonly Shell _shell; ShellNavigatedEventArgs _accumulatedEvent; bool _accumulateNavigatedEvents; - + public bool AccumulateNavigatedEvents => _accumulateNavigatedEvents; public event EventHandler Navigated; public event EventHandler Navigating; @@ -166,6 +166,7 @@ await Device.InvokeOnMainThreadAsync(() => await _shell.CurrentItem.CurrentItem.GoToAsync(navigationRequest, queryData, animate, isRelativePopping); } + (_shell as IShellController).UpdateCurrentState(source); _accumulateNavigatedEvents = false; // this can be null in the event that no navigation actually took place! diff --git a/Xamarin.Forms.Core/Shell/ShellSection.cs b/Xamarin.Forms.Core/Shell/ShellSection.cs index 382ddd0862d..73431187e5a 100644 --- a/Xamarin.Forms.Core/Shell/ShellSection.cs +++ b/Xamarin.Forms.Core/Shell/ShellSection.cs @@ -116,7 +116,6 @@ async void IShellSectionController.SendPopping(Task poppingCompleted) await poppingCompleted; RemovePage(page); - SendUpdateCurrentState(ShellNavigationSource.Pop); } async void IShellSectionController.SendPoppingToRoot(Task finishedPopping) @@ -135,8 +134,6 @@ async void IShellSectionController.SendPoppingToRoot(Task finishedPopping) for (int i = 1; i < oldStack.Count; i++) RemovePage(oldStack[i]); - - SendUpdateCurrentState(ShellNavigationSource.PopToRoot); } [Obsolete] @@ -151,8 +148,6 @@ void IShellSectionController.SendPopped() _navStack.Remove(last); RemovePage(last); - - SendUpdateCurrentState(ShellNavigationSource.Pop); } // we want the list returned from here to remain point in time accurate @@ -178,7 +173,6 @@ void IShellSectionController.SendPopped(Page page) _navStack.Remove(page); RemovePage(page); - SendUpdateCurrentState(ShellNavigationSource.Pop); } @@ -572,7 +566,7 @@ internal async Task GoToAsync(NavigationRequest request, IDictionary OnPopAsync(bool animated) @@ -762,8 +754,6 @@ protected async virtual Task OnPopAsync(bool animated) await args.Task; RemovePage(page); - SendUpdateCurrentState(ShellNavigationSource.Pop); - return page; } @@ -804,7 +794,6 @@ protected virtual async Task OnPopToRootAsync(bool animated) } PresentedPageAppearing(); - SendUpdateCurrentState(ShellNavigationSource.PopToRoot); } protected virtual Task OnPushAsync(Page page, bool animated) @@ -834,8 +823,6 @@ protected virtual Task OnPushAsync(Page page, bool animated) AddPage(page); _navigationRequested?.Invoke(this, args); - SendUpdateCurrentState(ShellNavigationSource.Push); - if (args.Task == null) return Task.FromResult(true); return args.Task; @@ -867,8 +854,6 @@ internal async Task PopModalStackToPage(Page page, bool? animated) bool isAnimated = animated ?? (Shell.GetPresentationMode(pageToPop) & PresentationMode.NotAnimated) != PresentationMode.NotAnimated; await Navigation.PopModalAsync(isAnimated); } - - ((IShellController)Shell).UpdateCurrentState(ShellNavigationSource.ShellSectionChanged); } finally { @@ -911,8 +896,6 @@ protected virtual void OnRemovePage(Page page) RequestType = NavigationRequestType.Remove }; _navigationRequested?.Invoke(this, args); - - SendUpdateCurrentState(ShellNavigationSource.Remove); } internal bool IsVisibleSection => Parent?.Parent is Shell shell && shell.CurrentItem?.CurrentItem == this; @@ -1013,14 +996,6 @@ void RemovePage(Page page) void SendAppearanceChanged() => ((IShellController)Parent?.Parent)?.AppearanceChanged(this, false); - void SendUpdateCurrentState(ShellNavigationSource source) - { - if (Parent?.Parent is IShellController shell) - { - shell.UpdateCurrentState(source); - } - } - protected override void OnBindingContextChanged() { base.OnBindingContextChanged(); @@ -1051,8 +1026,6 @@ class NavigationImpl : NavigationProxy protected override IReadOnlyList GetNavigationStack() => _owner.GetNavigationStack(); - protected override void OnInsertPageBefore(Page page, Page before) => _owner.OnInsertPageBefore(page, before); - protected override async Task OnPopAsync(bool animated) { if (!_owner.IsVisibleSection) @@ -1116,7 +1089,64 @@ protected override Task OnPushAsync(Page page, bool animated) return _owner.Shell.NavigationManager.GoToAsync(navigationParameters); } - protected override void OnRemovePage(Page page) => _owner.OnRemovePage(page); + protected override void OnRemovePage(Page page) + { + if (!_owner.IsVisibleSection || _owner.Shell.NavigationManager.AccumulateNavigatedEvents) + { + _owner.OnRemovePage(page); + return; + } + + var stack = _owner.Stack.ToList(); + stack.Remove(page); + var navigationState = GetUpdatedStatus(stack); + + ShellNavigatingEventArgs shellNavigatingEventArgs = new ShellNavigatingEventArgs( + _owner.Shell.CurrentState, + navigationState.Location, + ShellNavigationSource.Remove, + false); + + _owner.Shell.NavigationManager.HandleNavigating(shellNavigatingEventArgs); + _owner.OnRemovePage(page); + (_owner.Shell as IShellController).UpdateCurrentState(ShellNavigationSource.Remove); + } + + protected override void OnInsertPageBefore(Page page, Page before) + { + if (!_owner.IsVisibleSection || _owner.Shell.NavigationManager.AccumulateNavigatedEvents) + { + _owner.OnInsertPageBefore(page, before); + return; + } + + var stack = _owner.Stack.ToList(); + var index = stack.IndexOf(before); + if (index == -1) + throw new ArgumentException("Page not found in nav stack"); + + stack.Insert(index, page); + var navigationState = GetUpdatedStatus(stack); + + ShellNavigatingEventArgs shellNavigatingEventArgs = new ShellNavigatingEventArgs( + _owner.Shell.CurrentState, + navigationState.Location, + ShellNavigationSource.Insert, + false); + + _owner.Shell.NavigationManager.HandleNavigating(shellNavigatingEventArgs); + _owner.OnInsertPageBefore(page, before); + (_owner.Shell as IShellController).UpdateCurrentState(ShellNavigationSource.Insert); + } + + ShellNavigationState GetUpdatedStatus(IReadOnlyList stack) + { + var shellItem = _owner.Shell.CurrentItem; + var shellSection = shellItem?.CurrentItem; + var shellContent = shellSection?.CurrentItem; + var modalStack = shellSection?.Navigation?.ModalStack; + return ShellNavigationManager.GetNavigationState(shellItem, shellSection, shellContent, stack, modalStack); + } } } } diff --git a/Xamarin.Forms.Platform.iOS/Renderers/ShellSectionRenderer.cs b/Xamarin.Forms.Platform.iOS/Renderers/ShellSectionRenderer.cs index 0b5b67e7014..6c99404782c 100644 --- a/Xamarin.Forms.Platform.iOS/Renderers/ShellSectionRenderer.cs +++ b/Xamarin.Forms.Platform.iOS/Renderers/ShellSectionRenderer.cs @@ -68,19 +68,33 @@ void IAppearanceObserver.OnAppearanceChanged(ShellAppearance appearance) ShellSection _shellSection; bool _ignorePopCall; + // When setting base.ViewControllers iOS doesn't modify the property right away. + // if you set base.ViewControllers to a new array and then retrieve base.ViewControllers + // iOS will return the previous array until the new array has been processed + // This means if you try to remove one VC and then try to remove a second VC before the first one is processed + // you'll end up re-adding back the first VC + // ViewControllers = ViewControllers.Remove(vc1) + // ViewControllers = ViewControllers.Remove(vc2) + // You've now added vc1 back because the second call to ViewControllers will still return a ViewControllers list with vc1 in it + UIViewController[] _pendingViewControllers; + public ShellSectionRenderer(IShellContext context) : base() { Delegate = new NavDelegate(this); _context = context; _context.Shell.PropertyChanged += HandleShellPropertyChanged; + _context.Shell.Navigated += OnNavigated; + _context.Shell.Navigating += OnNavigating; } - public ShellSectionRenderer(IShellContext context, Type navigationBarType, Type toolbarType) + public ShellSectionRenderer(IShellContext context, Type navigationBarType, Type toolbarType) : base(navigationBarType, toolbarType) { Delegate = new NavDelegate(this); _context = context; _context.Shell.PropertyChanged += HandleShellPropertyChanged; + _context.Shell.Navigated += OnNavigated; + _context.Shell.Navigating += OnNavigating; } [Export("navigationBar:shouldPopItem:")] @@ -89,14 +103,14 @@ public bool ShouldPopItem(UINavigationBar navigationBar, UINavigationItem item) SendPop(); internal bool SendPop() - { + { // this means the pop is already done, nothing we can do - if (ViewControllers.Length < NavigationBar.Items.Length) + if (ActiveViewControllers().Length < NavigationBar.Items.Length) return true; - foreach(var tracker in _trackers) + foreach (var tracker in _trackers) { - if(tracker.Value.ViewController == TopViewController) + if (tracker.Value.ViewController == TopViewController) { var behavior = Shell.GetBackButtonBehavior(tracker.Value.Page); var command = behavior.GetPropertyIfSet(BackButtonBehavior.CommandProperty, null); @@ -104,7 +118,7 @@ internal bool SendPop() if (command != null) { - if(command.CanExecute(commandParameter)) + if (command.CanExecute(commandParameter)) { command.Execute(commandParameter); } @@ -202,6 +216,8 @@ void IDisconnectable.Disconnect() if (_context.Shell != null) { _context.Shell.PropertyChanged -= HandleShellPropertyChanged; + _context.Shell.Navigated -= OnNavigated; + _context.Shell.Navigating -= OnNavigating; ((IShellController)_context.Shell).RemoveAppearanceObserver(this); } @@ -263,7 +279,7 @@ protected virtual void LoadPages() _renderer = CreateShellSectionRootRenderer(ShellSection, _context); PushViewController(_renderer.ViewController, false); - + var stack = ShellSection.Stack; for (int i = 1; i < stack.Count; i++) { @@ -307,7 +323,7 @@ protected virtual void OnInsertRequested(NavigationRequestedEventArgs e) _trackers[page] = tracker; - ViewControllers.Insert(ViewControllers.IndexOf(beforeRenderer.ViewController), renderer.ViewController); + InsertViewController(ActiveViewControllers().IndexOf(beforeRenderer.ViewController), renderer.ViewController); } protected virtual void OnNavigationRequested(object sender, NavigationRequestedEventArgs e) @@ -353,7 +369,7 @@ protected virtual async void OnPopRequested(NavigationRequestedEventArgs e) public override UIViewController[] PopToRootViewController(bool animated) { - if (!_ignorePopCall && ViewControllers.Length > 1) + if (!_ignorePopCall && ActiveViewControllers().Length > 1) { ProcessPopToRoot(); } @@ -432,9 +448,7 @@ protected virtual void OnRemoveRequested(NavigationRequestedEventArgs e) OnPopRequested(e); } - if(ViewControllers.Contains(viewController)) - ViewControllers = ViewControllers.Remove(viewController); - + RemoveViewController(viewController); DisposePage(page); } } @@ -461,8 +475,11 @@ void DisposePage(Page page, bool calledFromDispose = false) { if (_trackers.TryGetValue(page, out var tracker)) { - if(!calledFromDispose && tracker.ViewController != null && ViewControllers.Contains(tracker.ViewController)) - ViewControllers = ViewControllers.Remove(_trackers[page].ViewController); + if (!calledFromDispose && tracker.ViewController != null && ActiveViewControllers().Contains(tracker.ViewController)) + { + System.Diagnostics.Debug.Write($"Disposing {_trackers[page].ViewController.GetHashCode()}"); + RemoveViewController(_trackers[page].ViewController); + } tracker.Dispose(); _trackers.Remove(page); @@ -502,6 +519,70 @@ void OnDisplayedPagePropertyChanged(object sender, PropertyChangedEventArgs e) UpdateNavigationBarHasShadow(); } + // We only care about using pendingViewControllers when we are setting the ViewControllers array directly + // So, once navigation starts again (or ends) we can just clear the pendingViewControllers + void OnNavigating(object sender, ShellNavigatingEventArgs e) + { + _pendingViewControllers = null; + } + + void OnNavigated(object sender, ShellNavigatedEventArgs e) + { + _pendingViewControllers = null; + } + + // These are all just safety nets to ensure that _pendingViewControllers doesn't for some reason get out of sync + // and start causing issues. In theory we could just override ViewControllers here to make sure _pendingViewControllers + // stays in sync but I don't trust that `ViewControllers.set` is reliably called with every modification + public override UIViewController[] ViewControllers + { + get => base.ViewControllers; + set + { + if (_pendingViewControllers != null) + _pendingViewControllers = value; + + base.ViewControllers = value; + } + } + + public override UIViewController[] PopToViewController(UIViewController viewController, bool animated) + { + _pendingViewControllers = null; + return base.PopToViewController(viewController, animated); + } + + public override void PushViewController(UIViewController viewController, bool animated) + { + _pendingViewControllers = null; + base.PushViewController(viewController, animated); + } + + public override UIViewController PopViewController(bool animated) + { + _pendingViewControllers = null; + return base.PopViewController(animated); + } + + UIViewController[] ActiveViewControllers() => + _pendingViewControllers ?? base.ViewControllers; + + void RemoveViewController(UIViewController viewController) + { + _pendingViewControllers = _pendingViewControllers ?? base.ViewControllers; + if (_pendingViewControllers.Contains(viewController)) + _pendingViewControllers = _pendingViewControllers.Remove(viewController); + + ViewControllers = _pendingViewControllers; + } + + void InsertViewController(int index, UIViewController viewController) + { + _pendingViewControllers = _pendingViewControllers ?? base.ViewControllers; + _pendingViewControllers = _pendingViewControllers.Insert(index, viewController); + ViewControllers = _pendingViewControllers; + } + void PushPage(Page page, bool animated, TaskCompletionSource completionSource = null) { var renderer = Platform.CreateRenderer(page); @@ -576,7 +657,7 @@ public GestureDelegate(UINavigationController parent, Func shouldPop) public override bool ShouldBegin(UIGestureRecognizer recognizer) { - if (_parent.ViewControllers.Length == 1) + if ((_parent as ShellSectionRenderer).ActiveViewControllers().Length == 1) return false; return _shouldPop(); } @@ -619,6 +700,7 @@ public override void DidShowViewController(UINavigationController navigationCont public override void WillShowViewController(UINavigationController navigationController, [Transient] UIViewController viewController, bool animated) { + System.Diagnostics.Debug.Write($"WillShowViewController {viewController.GetHashCode()}"); var element = _self.ElementForViewController(viewController); bool navBarVisible;