When navigating between Shell Items disconnect the renderers from the xplat elements (#11791)

* Disconnect renderer before disposing of it

* - fix ui test

* - remove any processing animations

* - cleanup code

* - fix ui test and add null checks

* - include android

fixes #11784
fixes #11777

Author: PureWeen

Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Issue11723.cs

@@ -0,0 +1,108 @@
+using System;
+using System.Collections.Generic;
+using System.Collections.ObjectModel;
+using System.Text;
+using Xamarin.Forms.CustomAttributes;
+using Xamarin.Forms.Internals;
+using System.Threading.Tasks;
+using System.Runtime.CompilerServices;
+using System.Linq;
+
+
+#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, 11723, "[Bug] ContentPage in NavigationStack misplaced initially",
+		PlatformAffected.iOS)]
+#if UITEST
+	[NUnit.Framework.Category(Core.UITests.UITestCategories.Github10000)]
+	[NUnit.Framework.Category(UITestCategories.Shell)]
+#endif
+	public class Issue11723 : TestShell
+	{
+		int labelIndex = 0;
+		ContentPage CreateContentPage()
+		{
+			var page = new ContentPage()
+			{
+				Content = new StackLayout()
+				{
+					Children =
+					{
+						new Label()
+						{
+							Text = "As you navigate this text should show up in the correct spot. If it's hidden and then shows up this test has failed.",
+							AutomationId = $"InitialText{labelIndex}"
+						},
+						new Button()
+						{
+							Text = "Push Page",
+							AutomationId = "PushPage",
+							Command = new Command(async () =>
+							{
+								labelIndex++;
+								await Navigation.PushAsync(CreateContentPage());
+							})
+						},
+						new Button()
+						{
+							Text = "Pop Page",
+							AutomationId = "PopPage",
+							Command = new Command(async () =>
+							{
+								labelIndex--;
+								await Navigation.PopAsync();
+							})
+						}
+					}
+				}
+			};
+
+			SetNavBarIsVisible(page, false);
+			PlatformConfiguration.iOSSpecific.Page.SetUseSafeArea(page, true);
+
+			return page;
+		}
+
+		protected override void Init()
+		{
+			AddContentPage(CreateContentPage());
+		}
+
+
+#if UITEST
+		[Test]
+		public void PaddingIsSetOnPageBeforeItsVisible()
+		{
+			var initialTextPosition = RunningApp.WaitForFirstElement($"InitialText0").Rect;
+			RunningApp.Tap("PushPage");
+			CompareTextLocation(initialTextPosition, 1);
+			RunningApp.Tap("PushPage");
+			CompareTextLocation(initialTextPosition, 2);
+			RunningApp.Tap("PushPage");
+			CompareTextLocation(initialTextPosition, 3);
+			RunningApp.Tap("PopPage");
+			CompareTextLocation(initialTextPosition, 2);
+			RunningApp.Tap("PopPage");
+			CompareTextLocation(initialTextPosition, 1);
+
+		}
+
+		void CompareTextLocation(UITest.Queries.AppRect initialRect, int i)
+		{
+			var newRect = RunningApp.WaitForFirstElement($"InitialText{i}").Rect;
+
+			Assert.AreEqual(newRect.X, initialRect.X, $"Error With Test :{i}");
+			Assert.AreEqual(newRect.Y, initialRect.Y, $"Error With Test :{i}");
+			Assert.AreEqual(newRect.CenterX, initialRect.CenterX, $"Error With Test :{i}");
+			Assert.AreEqual(newRect.CenterY, initialRect.CenterY, $"Error With Test :{i}");
+		}
+#endif
+	}
+}

Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Xamarin.Forms.Controls.Issues.Shared.projitems

@@ -1445,6 +1445,7 @@
     <Compile Include="$(MSBuildThisFileDirectory)Issue11430.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)Issue11247.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)Issue10608.cs" />
+    <Compile Include="$(MSBuildThisFileDirectory)Issue11723.cs" />
   </ItemGroup>
   <ItemGroup>
     <EmbeddedResource Include="$(MSBuildThisFileDirectory)Bugzilla22229.xaml">

Xamarin.Forms.Platform.iOS/EventTracker.cs

@@ -57,6 +57,12 @@ ObservableCollection<IGestureRecognizer> ElementGestureRecognizers
 			}
 		}
 
+		internal void Disconnect()
+		{
+			if (ElementGestureRecognizers != null)
+				ElementGestureRecognizers.CollectionChanged -= _collectionChangedHandler;
+		}
+
 		public void Dispose()
 		{
 			if (_disposed)
@@ -75,8 +81,7 @@ public void Dispose()
 
 			_gestureRecognizers.Clear();
 
-			if (ElementGestureRecognizers != null)
-				ElementGestureRecognizers.CollectionChanged -= _collectionChangedHandler;
+			Disconnect();
 
 			_handler = null;
 		}

Xamarin.Forms.Platform.iOS/Renderers/IDisconnectable.cs

@@ -0,0 +1,8 @@
+using System;
+namespace Xamarin.Forms.Platform.iOS
+{
+	internal interface IDisconnectable
+	{
+		void Disconnect();
+	}
+}

Xamarin.Forms.Platform.iOS/Renderers/PageRenderer.cs

@@ -9,7 +9,7 @@
 
 namespace Xamarin.Forms.Platform.iOS
 {
-	public class PageRenderer : UIViewController, IVisualElementRenderer, IEffectControlProvider, IAccessibilityElementsController, IShellContentInsetObserver
+	public class PageRenderer : UIViewController, IVisualElementRenderer, IEffectControlProvider, IAccessibilityElementsController, IShellContentInsetObserver, IDisconnectable
 	{
 		bool _appeared;
 		bool _disposed;
@@ -110,7 +110,7 @@ public UIView NativeView
 		{
 			get { return _disposed ? null : View; }
 		}
-
+		
 		public void SetElement(VisualElement element)
 		{
 			VisualElement oldElement = Element;
@@ -172,7 +172,7 @@ public override void ViewDidLayoutSubviews()
 		{
 			base.ViewDidLayoutSubviews();
 
-			if (_disposed)
+			if (_disposed || Element == null)
 				return;
 
 			if (Element.Parent is BaseShellItem)
@@ -195,7 +195,7 @@ public override void ViewDidAppear(bool animated)
 		{
 			base.ViewDidAppear(animated);
 
-			if (_appeared || _disposed)
+			if (_appeared || _disposed || Element == null)
 				return;
 
 			_appeared = true;
@@ -213,15 +213,15 @@ public override void ViewDidDisappear(bool animated)
 		{
 			base.ViewDidDisappear(animated);
 
-			if (!_appeared || _disposed)
+			if (!_appeared || _disposed || Element == null)
 				return;
 
 			_appeared = false;
 
 			if (Element.Parent is CarouselPage)
 				return;
 
-			Page.SendDisappearing();
+			Page?.SendDisappearing();
 		}
 
 		public override void ViewDidLoad()
@@ -260,6 +260,25 @@ public override void ViewWillDisappear(bool animated)
 			NativeView?.Window?.EndEditing(true);
 		}
 
+		void IDisconnectable.Disconnect()
+		{
+			if (_shellSection != null)
+			{
+				((IShellSectionController)_shellSection).RemoveContentInsetObserver(this);
+				_shellSection = null;
+			}
+
+			if (Element != null)
+			{
+				Element.PropertyChanged -= OnHandlePropertyChanged;
+				Platform.SetRenderer(Element, null);
+				Element = null;
+			}
+
+			_events?.Disconnect();
+			_packager?.Disconnect();
+			_tracker?.Disconnect();
+		}
 
 		protected override void Dispose(bool disposing)
 		{
@@ -268,36 +287,19 @@ protected override void Dispose(bool disposing)
 
 			if (disposing)
 			{
-				if (_shellSection != null)
-				{
-					((IShellSectionController)_shellSection).RemoveContentInsetObserver(this);
-					_shellSection = null;
-				}
+				var page = Page;
+				(this as IDisconnectable).Disconnect();
 
-				Element.PropertyChanged -= OnHandlePropertyChanged;
-				Platform.SetRenderer(Element, null);
 				if (_appeared)
-					Page.SendDisappearing();
+					page?.SendDisappearing();
 
 				_appeared = false;
-
-				if (_events != null)
-				{
-					_events.Dispose();
-					_events = null;
-				}
-
-				if (_packager != null)
-				{
-					_packager.Dispose();
-					_packager = null;
-				}
-
-				if (_tracker != null)
-				{
-					_tracker.Dispose();
-					_tracker = null;
-				}
+				_events?.Dispose();
+				_packager?.Dispose();
+				_tracker?.Dispose();
+				_events = null;
+				_packager = null;
+				_tracker = null;
 
 				Element = null;
 				Container?.Dispose();
@@ -396,9 +398,6 @@ void UpdateUseSafeArea()
 			if (!IsPartOfShell && !Forms.IsiOS11OrNewer)
 				return;
 
-			if (IsPartOfShell && !_appeared)
-				return;
-
 			var tabThickness = _tabThickness;
 			if (!_isInItems)
 				tabThickness = 0;

Xamarin.Forms.Platform.iOS/Renderers/ShellItemRenderer.cs

@@ -10,7 +10,7 @@
 
 namespace Xamarin.Forms.Platform.iOS
 {
-	public class ShellItemRenderer : UITabBarController, IShellItemRenderer, IAppearanceObserver, IUINavigationControllerDelegate
+	public class ShellItemRenderer : UITabBarController, IShellItemRenderer, IAppearanceObserver, IUINavigationControllerDelegate, IDisconnectable
 	{
 		#region IShellItemRenderer
 
@@ -113,36 +113,60 @@ public override void ViewDidLoad()
 			};
 		}
 
-		protected override void Dispose(bool disposing)
+		void IDisconnectable.Disconnect()
 		{
-			base.Dispose(disposing);
-
-			if (disposing && !_disposed)
+			if (_sectionRenderers != null)
 			{
-				_disposed = true;
 				foreach (var kvp in _sectionRenderers.ToList())
 				{
-					var renderer = kvp.Value;
-					RemoveRenderer(renderer);
+					var renderer = kvp.Value as IDisconnectable;
+					renderer?.Disconnect();
+					kvp.Value.ShellSection.PropertyChanged -= OnShellSectionPropertyChanged;
 				}
+			}
 
-				if (_displayedPage != null)
-					_displayedPage.PropertyChanged -= OnDisplayedPagePropertyChanged;
+			if (_displayedPage != null)
+				_displayedPage.PropertyChanged -= OnDisplayedPagePropertyChanged;
 
-				if (_currentSection != null)
-					((IShellSectionController)_currentSection).RemoveDisplayedPageObserver(this);
+			if (_currentSection != null)
+				((IShellSectionController)_currentSection).RemoveDisplayedPageObserver(this);
 
 
-				_sectionRenderers.Clear();
+			if(ShellItem != null)
 				ShellItem.PropertyChanged -= OnElementPropertyChanged;
-				((IShellController)_context.Shell).RemoveAppearanceObserver(this);
+
+			if(_context?.Shell is IShellController shellController)
+				shellController.RemoveAppearanceObserver(this);
+
+			if(ShellItemController != null)
 				ShellItemController.ItemsCollectionChanged -= OnItemsCollectionChanged;
+		}
+
+		protected override void Dispose(bool disposing)
+		{
+			if (_disposed)
+				return;
+
+			_disposed = true;
 
+			if (disposing)
+			{
+				(this as IDisconnectable).Disconnect();
+
+				foreach (var kvp in _sectionRenderers.ToList())
+				{
+					var renderer = kvp.Value;
+					RemoveRenderer(renderer);
+				}
+
+				_sectionRenderers.Clear();
 				CurrentRenderer = null;
 				_shellItem = null;
 				_currentSection = null;
 				_displayedPage = null;
 			}
+
+			base.Dispose(disposing);
 		}
 
 		protected virtual void OnElementPropertyChanged(object sender, PropertyChangedEventArgs e)

Xamarin.Forms.Platform.iOS/Renderers/ShellItemTransition.cs

@@ -10,6 +10,7 @@ public Task Transition(IShellItemRenderer oldRenderer, IShellItemRenderer newRen
 			TaskCompletionSource<bool> task = new TaskCompletionSource<bool>();
 			var oldView = oldRenderer.ViewController.View;
 			var newView = newRenderer.ViewController.View;
+			oldView.Layer.RemoveAllAnimations();
 			newView.Alpha = 0;
 
 			newView.Superview.InsertSubviewAbove(newView, oldView);

Xamarin.Forms.Platform.iOS/Renderers/ShellRenderer.cs

@@ -249,6 +249,7 @@ protected virtual void OnElementSet(Shell element)
 		protected async void SetCurrentShellItemController(IShellItemRenderer value)
 		{
 			var oldRenderer = _currentShellItemRenderer;
+			(oldRenderer as IDisconnectable)?.Disconnect();
 			var newRenderer = value;
 
 			_currentShellItemRenderer = value;
@@ -258,7 +259,7 @@ protected async void SetCurrentShellItemController(IShellItemRenderer value)
 			View.SendSubviewToBack(newRenderer.ViewController.View);
 
 			newRenderer.ViewController.View.Frame = View.Bounds;
-
+			
 			if (oldRenderer != null)
 			{
 				var transition = CreateShellItemTransition();