Propagate cleared shell elements correctly (xamarin#9615)

* Propagate cleared shell elements correctly * - always propagate CollectionChanged
fabulous-dev · Feb 18, 2020 · f737dc7 · f737dc7
1 parent 0f75b4c
commit f737dc7
Show file tree

Hide file tree

Showing 5 changed files with 137 additions and 37 deletions.
diff --git a/Xamarin.Forms.Core.UnitTests/ShellTests.cs b/Xamarin.Forms.Core.UnitTests/ShellTests.cs
@@ -866,5 +866,68 @@ public async Task ShellItemNotVisible()
 			Assert.IsFalse(GetItems(shell).Contains(item1));
 			Assert.IsTrue(GetItems(shell).Contains(item2));
 		}
+
+		[Test]
+		public async Task ShellContentCollectionClear()
+		{
+			var shell = new Shell();
+			var item1 = CreateShellItem();
+			var section2 = CreateShellSection();
+
+			shell.Items.Add(item1);
+			item1.Items.Add(section2);
+
+			var mainTab = item1.Items[0];
+			var content1 = CreateShellContent();
+			var clearedContent = mainTab.Items[0];
+			mainTab.Items.Clear();
+			mainTab.Items.Add(content1);
+			mainTab.Items.Add(CreateShellContent());
+
+			Assert.IsNull(clearedContent.Parent);
+			Assert.AreEqual(2, mainTab.Items.Count);
+			Assert.AreEqual(content1, mainTab.CurrentItem);
+		}
+
+		[Test]
+		public async Task ShellItemCollectionClear()
+		{
+			var shell = new Shell();
+			var item1 = CreateShellItem();
+			shell.Items.Add(item1);
+
+
+			var item2 = CreateShellItem();
+			var item3 = CreateShellItem();
+
+			shell.Items.Clear();
+			shell.Items.Add(item2);
+			shell.Items.Add(item3);
+
+			Assert.IsNull(item1.Parent);
+			Assert.AreEqual(2, shell.Items.Count);
+			Assert.AreEqual(item2, shell.CurrentItem);
+		}
+
+		[Test]
+		public async Task ShellSectionCollectionClear()
+		{
+			var shell = new Shell();
+			var item1 = CreateShellItem();
+			shell.Items.Add(item1);
+
+			var section1 = CreateShellSection();
+			var section2 = CreateShellSection();
+			var clearedSection = item1.Items[0];
+
+			Assert.IsNotNull(clearedSection.Parent);
+			item1.Items.Clear();
+			item1.Items.Add(section1);
+			item1.Items.Add(section2);
+
+			Assert.IsNull(clearedSection.Parent);
+			Assert.AreEqual(2, item1.Items.Count);
+			Assert.AreEqual(section1, shell.CurrentItem.CurrentItem);
+		}
 	}
 }
diff --git a/Xamarin.Forms.Core/Shell/ShellContentCollection.cs b/Xamarin.Forms.Core/Shell/ShellContentCollection.cs
@@ -3,6 +3,7 @@
 using System.Collections.Generic;
 using System.Collections.ObjectModel;
 using System.Collections.Specialized;
+using System.Linq;
 
 namespace Xamarin.Forms
 {
@@ -38,11 +39,21 @@ void InnerCollectionChanged(object sender, NotifyCollectionChangedEventArgs e)
 
 			if (e.OldItems != null)
 			{
-				foreach (ShellContent element in e.OldItems)
-				{
-					if (element is IShellContentController controller)
-						controller.IsPageVisibleChanged -= OnIsPageVisibleChanged;
-				}
+				Removing(e.OldItems);
+			}
+
+			CollectionChanged?.Invoke(this, e);
+		}
+
+		void Removing(IEnumerable items)
+		{
+			foreach (ShellContent element in items)
+			{
+				if (_visibleContents.Contains(element))
+					_visibleContents.Remove(element);
+
+				if (element is IShellContentController controller)
+					controller.IsPageVisibleChanged -= OnIsPageVisibleChanged;
 			}
 		}
 
@@ -77,11 +88,7 @@ void CheckVisibility(ShellContent shellContent)
 			}
 		}
 
-		event NotifyCollectionChangedEventHandler INotifyCollectionChanged.CollectionChanged
-		{
-			add { ((INotifyCollectionChanged)_inner).CollectionChanged += value; }
-			remove { ((INotifyCollectionChanged)_inner).CollectionChanged -= value; }
-		}
+		public event NotifyCollectionChangedEventHandler CollectionChanged;
 
 		public int Count => _inner.Count;
 
@@ -95,7 +102,14 @@ public ShellContent this[int index]
 
 		public void Add(ShellContent item) => _inner.Add(item);
 
-		public void Clear() => _inner.Clear();
+		public void Clear()
+		{
+			var list = _inner.ToList();
+			Removing(_inner);
+			_inner.Clear();
+
+			CollectionChanged?.Invoke(this, new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Remove, list));
+		}
 
 		public bool Contains(ShellContent item) => _inner.Contains(item);
 

diff --git a/Xamarin.Forms.Core/Shell/ShellItemCollection.cs b/Xamarin.Forms.Core/Shell/ShellItemCollection.cs
@@ -2,6 +2,7 @@
 using System.Collections.Generic;
 using System.Collections.ObjectModel;
 using System.Collections.Specialized;
+using System.Linq;
 
 namespace Xamarin.Forms
 {
@@ -22,11 +23,7 @@ public ShellItemCollection()
 			};
 		}
 
-		event NotifyCollectionChangedEventHandler INotifyCollectionChanged.CollectionChanged
-		{
-			add { ((INotifyCollectionChanged)Inner).CollectionChanged += value; }
-			remove { ((INotifyCollectionChanged)Inner).CollectionChanged -= value; }
-		}
+		public event NotifyCollectionChangedEventHandler CollectionChanged;
 
 		public int Count => Inner.Count;
 		public bool IsReadOnly => ((IList<ShellItem>)Inner).IsReadOnly;
@@ -58,16 +55,25 @@ void InnerCollectionChanged(object sender, NotifyCollectionChangedEventArgs e)
 
 			if (e.OldItems != null)
 			{
-				foreach (ShellItem element in e.OldItems)
-				{
-					if (_visibleContents.Contains(element))
-						_visibleContents.Remove(element);
+				Removing(e.OldItems);
+			}
+
+			CollectionChanged?.Invoke(this, e);
+		}
+
+
+		void Removing(IEnumerable items)
+		{
+			foreach (ShellItem element in items)
+			{
+				if (_visibleContents.Contains(element))
+					_visibleContents.Remove(element);
 
-					if (element is IShellItemController controller)
-						controller.ItemsCollectionChanged -= OnShellItemControllerItemsCollectionChanged;
-				}
+				if (element is IShellSectionController controller)
+					controller.ItemsCollectionChanged -= OnShellItemControllerItemsCollectionChanged;
 			}
 		}
+
 		void OnShellItemControllerItemsCollectionChanged(object sender, NotifyCollectionChangedEventArgs e)
 		{
 			foreach (ShellSection section in (e.NewItems ?? e.OldItems ?? (IList)_inner))
@@ -154,7 +160,13 @@ item is TabBar
 			Inner.Add(item);
 		}
 
-		public void Clear() => Inner.Clear();
+		public void Clear()
+		{
+			var list = Inner.ToList();
+			Removing(Inner);
+			Inner.Clear();
+			CollectionChanged?.Invoke(this, new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Remove, list));
+		}
 
 		public bool Contains(ShellItem item) => Inner.Contains(item);
 

diff --git a/Xamarin.Forms.Core/Shell/ShellSectionCollection.cs b/Xamarin.Forms.Core/Shell/ShellSectionCollection.cs
@@ -2,6 +2,7 @@
 using System.Collections.Generic;
 using System.Collections.ObjectModel;
 using System.Collections.Specialized;
+using System.Linq;
 
 namespace Xamarin.Forms
 {
@@ -22,11 +23,7 @@ public ShellSectionCollection()
 
 		public ReadOnlyCollection<ShellSection> VisibleItems { get; }
 
-		event NotifyCollectionChangedEventHandler INotifyCollectionChanged.CollectionChanged
-		{
-			add { ((INotifyCollectionChanged)Inner).CollectionChanged += value; }
-			remove { ((INotifyCollectionChanged)Inner).CollectionChanged -= value; }
-		}
+		public event NotifyCollectionChangedEventHandler CollectionChanged;
 
 		public int Count => Inner.Count;
 		public bool IsReadOnly => Inner.IsReadOnly;
@@ -55,14 +52,21 @@ void InnerCollectionChanged(object sender, NotifyCollectionChangedEventArgs e)
 
 			if (e.OldItems != null)
 			{
-				foreach (ShellSection element in e.OldItems)
-				{
-					if (_visibleContents.Contains(element))
-						_visibleContents.Remove(element);
+				Removing(e.OldItems);
+			}
+
+			CollectionChanged?.Invoke(this, e);
+		}
 
-					if (element is IShellSectionController controller)
-						controller.ItemsCollectionChanged -= OnShellSectionControllerItemsCollectionChanged;
-				}
+		void Removing(IEnumerable items)
+		{
+			foreach (ShellSection element in items)
+			{
+				if (_visibleContents.Contains(element))
+					_visibleContents.Remove(element);
+
+				if (element is IShellSectionController controller)
+					controller.ItemsCollectionChanged -= OnShellSectionControllerItemsCollectionChanged;
 			}
 		}
 
@@ -119,7 +123,13 @@ public ShellSection this[int index]
 
 		public void Add(ShellSection item) => Inner.Add(item);
 
-		public void Clear() => Inner.Clear();
+		public void Clear()
+		{
+			var list = Inner.ToList();
+			Removing(Inner);
+			Inner.Clear();
+			CollectionChanged?.Invoke(this, new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Remove, list));
+		}
 
 		public bool Contains(ShellSection item) => Inner.Contains(item);
 

diff --git a/Xamarin.Forms.sln.DotSettings b/Xamarin.Forms.sln.DotSettings
@@ -669,6 +669,7 @@
 	<s:Double x:Key="/Default/Environment/InjectedLayers/InjectedLayerCustomization/=File64B48C7709A839499C5E49DBCE502A37/RelativePriority/@EntryValue">1</s:Double>
 	<s:Boolean x:Key="/Default/Environment/SettingsMigration/IsMigratorApplied/=JetBrains_002EReSharper_002EPsi_002ECSharp_002ECodeStyle_002ECSharpKeepExistingMigration/@EntryIndexedValue">True</s:Boolean>
 	<s:Boolean x:Key="/Default/Environment/SettingsMigration/IsMigratorApplied/=JetBrains_002EReSharper_002EPsi_002ECSharp_002ECodeStyle_002ECSharpPlaceEmbeddedOnSameLineMigration/@EntryIndexedValue">True</s:Boolean>
+	<s:Boolean x:Key="/Default/Environment/SettingsMigration/IsMigratorApplied/=JetBrains_002EReSharper_002EPsi_002ECSharp_002ECodeStyle_002ECSharpUseContinuousIndentInsideBracesMigration/@EntryIndexedValue">True</s:Boolean>
 	<s:Boolean x:Key="/Default/Environment/SettingsMigration/IsMigratorApplied/=JetBrains_002EReSharper_002EPsi_002ECSharp_002ECodeStyle_002ESettingsUpgrade_002EAddAccessorOwnerDeclarationBracesMigration/@EntryIndexedValue">True</s:Boolean>
 	<s:Boolean x:Key="/Default/Environment/SettingsMigration/IsMigratorApplied/=JetBrains_002EReSharper_002EPsi_002ECSharp_002ECodeStyle_002ESettingsUpgrade_002EAlwaysTreatStructAsNotReorderableMigration/@EntryIndexedValue">True</s:Boolean>
 	<s:Boolean x:Key="/Default/Environment/SettingsMigration/IsMigratorApplied/=JetBrains_002EReSharper_002EPsi_002ECSharp_002ECodeStyle_002ESettingsUpgrade_002EMigrateBlankLinesAroundFieldToBlankLinesAroundProperty/@EntryIndexedValue">True</s:Boolean>