Clean up NavigationBar code in Platform (#753)

4 files changed, 63 insertions, 28 deletions

diff --git a/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Bugzilla30166.cs b/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Bugzilla30166.cs
new file mode 100644
index 00000000..9f4ed7c7
--- /dev/null
+++ b/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Bugzilla30166.cs
@@ -0,0 +1,49 @@
+using System;
+using Xamarin.Forms.CustomAttributes;
+using Xamarin.Forms.Internals;
+
+#if UITEST
+using Xamarin.UITest;
+using NUnit.Framework;
+#endif
+
+namespace Xamarin.Forms.Controls.Issues
+{
+	[Preserve(AllMembers = true)]
+	[Issue(IssueTracker.Bugzilla, 30166, "NavigationBar.BarBackgroundColor resets on Lollipop after popping modal page", PlatformAffected.Android)]
+	public class Bugzilla30166 : TestNavigationPage
+	{
+		protected override void Init()
+		{
+			BarBackgroundColor = Color.Red;
+
+			Navigation.PushAsync(new ContentPage
+			{
+				Content = new Button
+				{
+					Text = "Push Modal",
+					Command = new Command(async () => await Navigation.PushModalAsync(new ContentPage
+					{
+						Content = new Button
+						{
+							Text = "Back",
+							Command = new Command(async () => await Navigation.PopModalAsync()),
+						},
+					})),
+				},
+			});
+		}
+
+#if UITEST
+		[Test]
+		public void Bugzilla30166Test()
+		{
+			RunningApp.WaitForElement(q => q.Marked("Push Modal"));
+			RunningApp.Tap(q => q.Marked("Push Modal"));
+			RunningApp.WaitForElement(q => q.Marked("Back"));
+			RunningApp.Tap(q => q.Marked("Back"));
+			RunningApp.Screenshot("Navigation bar should be red");
+		}
+#endif
+	}
+}
\ No newline at end of file
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 2eb55495..bb7d0f29 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
@@ -44,6 +44,7 @@
     <Compile Include="$(MSBuildThisFileDirectory)Bugzilla29158.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)Bugzilla29363.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)Bugzilla29229.cs" />
+    <Compile Include="$(MSBuildThisFileDirectory)Bugzilla30166.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)Bugzilla31141.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)Bugzilla31145.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)Bugzilla31333.cs" />
diff --git a/Xamarin.Forms.Platform.Android/Platform.cs b/Xamarin.Forms.Platform.Android/Platform.cs
index 3f969091..72a6d0c9 100644
--- a/Xamarin.Forms.Platform.Android/Platform.cs
+++ b/Xamarin.Forms.Platform.Android/Platform.cs
@@ -109,11 +109,6 @@ namespace Xamarin.Forms.Platform.Android
 					_currentNavigationPage.PropertyChanged += CurrentNavigationPageOnPropertyChanged;
 					RegisterNavPageCurrent(_currentNavigationPage.CurrentPage);
 				}
-
-				UpdateActionBarBackgroundColor();
-				UpdateActionBarTextColor();
-				UpdateActionBarUpImageColor();
-				UpdateActionBarTitle();
 			}
 		}
 
@@ -304,6 +299,7 @@ namespace Xamarin.Forms.Platform.Android
 		public void UpdateActionBarTextColor()
 		{
 			SetActionBarTextColor();
+			UpdateActionBarUpImageColor();
 		}
 
 		protected override void OnBindingContextChanged()
@@ -429,9 +425,8 @@ namespace Xamarin.Forms.Platform.Android
 		internal void UpdateActionBar()
 		{
 			if (ActionBar == null) //Fullscreen theme doesn't have action bar
-			{
 				return;
-			}
+
 			List<Page> relevantAncestors = AncestorPagesOfPage(_navModel.CurrentPage);
 
 			IEnumerable<NavigationPage> navPages = relevantAncestors.OfType<NavigationPage>();
@@ -453,19 +448,19 @@ namespace Xamarin.Forms.Platform.Android
 				throw new InvalidOperationException("NavigationPage must have a root Page before being used. Either call PushAsync with a valid Page, or pass a Page to the constructor before usage.");
 			}
 
-			UpdateActionBarTitle();
-
-			if (ShouldShowActionBarTitleArea() || tabbedPage != null)
+			if (ShouldShowActionBarTitleArea() || CurrentTabbedPage != null)
 				ShowActionBar();
 			else
 				HideActionBar();
+
 			UpdateMasterDetailToggle();
 		}
 
 		internal void UpdateActionBarBackgroundColor()
 		{
-			if (!((Activity)_context).ActionBar.IsShowing)
+			if (!ShouldShowActionBarTitleArea())
 				return;
+
 			Color colorToUse = Color.Default;
 			if (CurrentNavigationPage != null)
 			{
@@ -515,13 +510,6 @@ namespace Xamarin.Forms.Platform.Android
 			MasterDetailPageToggle.SyncState();
 		}
 
-		internal void UpdateNavigationTitleBar()
-		{
-			UpdateActionBarTitle();
-			UpdateActionBar();
-			UpdateActionBarUpImageColor();
-		}
-
 		void AddChild(VisualElement view, bool layout = false)
 		{
 			if (GetRenderer(view) != null)
@@ -593,12 +581,12 @@ namespace Xamarin.Forms.Platform.Android
 
 		void CurrentNavigationPageOnPopped(object sender, NavigationEventArgs eventArg)
 		{
-			UpdateNavigationTitleBar();
+			UpdateActionBar();
 		}
 
 		void CurrentNavigationPageOnPoppedToRoot(object sender, EventArgs eventArgs)
 		{
-			UpdateNavigationTitleBar();
+			UpdateActionBar();
 		}
 
 		void CurrentNavigationPageOnPropertyChanged(object sender, PropertyChangedEventArgs e)
@@ -610,17 +598,14 @@ namespace Xamarin.Forms.Platform.Android
 			else if (e.PropertyName == NavigationPage.BarBackgroundColorProperty.PropertyName)
 				UpdateActionBarBackgroundColor();
 			else if (e.PropertyName == NavigationPage.BarTextColorProperty.PropertyName)
-			{
 				UpdateActionBarTextColor();
-				UpdateActionBarUpImageColor();
-			}
 			else if (e.PropertyName == NavigationPage.CurrentPageProperty.PropertyName)
 				RegisterNavPageCurrent(CurrentNavigationPage.CurrentPage);
 		}
 
 		void CurrentNavigationPageOnPushed(object sender, NavigationEventArgs eventArg)
 		{
-			UpdateNavigationTitleBar();
+			UpdateActionBar();
 		}
 
 		void CurrentTabbedPageChildrenChanged(object sender, NotifyCollectionChangedEventArgs e)
@@ -918,10 +903,10 @@ namespace Xamarin.Forms.Platform.Android
 		void ShowActionBar()
 		{
 			ReloadToolbarItems();
-			UpdateActionBarHomeAsUp(ActionBar);
-			ActionBar.Show();
+			UpdateActionBarTitle();
 			UpdateActionBarBackgroundColor();
 			UpdateActionBarTextColor();
+			ActionBar.Show();
 		}
 
 		void ToolbarTrackerOnCollectionChanged(object sender, EventArgs eventArgs)
diff --git a/Xamarin.Forms.Platform.Android/Renderers/NavigationRenderer.cs b/Xamarin.Forms.Platform.Android/Renderers/NavigationRenderer.cs
index 5d678287..bcacee6b 100644
--- a/Xamarin.Forms.Platform.Android/Renderers/NavigationRenderer.cs
+++ b/Xamarin.Forms.Platform.Android/Renderers/NavigationRenderer.cs
@@ -150,7 +150,7 @@ namespace Xamarin.Forms.Platform.Android
 				throw new InvalidOperationException("This should never happen, please file a bug");
 
 			Device.StartTimer(TimeSpan.FromMilliseconds(0), () => {
-				((Platform)Element.Platform).UpdateNavigationTitleBar();
+				((Platform)Element.Platform).UpdateActionBar();
 				return false;
 			});
 		}
@@ -197,7 +197,7 @@ namespace Xamarin.Forms.Platform.Android
 
 			Device.StartTimer(TimeSpan.FromMilliseconds(0), () =>
 			{
-				((Platform)Element.Platform).UpdateNavigationTitleBar();
+				((Platform)Element.Platform).UpdateActionBar();
 				return false;
 			});
 		}