From 65d848051912e1522d692ba647945625f271b23c Mon Sep 17 00:00:00 2001
From: Jimmy Garrido <jimmygarrido@outlook.com>
Date: Mon, 6 Mar 2017 14:19:36 -0800
Subject: [PATCH] [UWP] Improve toolbar consistency on MasterDetailPage (#744)

* Improve toolbar consistency on MasterDetailPage

* Properly indent xaml page
---
 .../Bugzilla51553.cs                               | 44 ++++++++++++++++++++++
 .../Bugzilla51802.cs                               | 36 ++++++++++++++++++
 .../Xamarin.Forms.Controls.Issues.Shared.projitems |  2 +
 Xamarin.Forms.Platform.UAP/MasterDetailControl.cs  |  3 ++
 .../MasterDetailControlStyle.xaml                  | 34 ++++++++---------
 .../MasterDetailPageRenderer.cs                    | 28 +++++++-------
 .../NavigationPageRenderer.cs                      | 10 ++---
 7 files changed, 121 insertions(+), 36 deletions(-)
 create mode 100644 Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Bugzilla51553.cs
 create mode 100644 Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Bugzilla51802.cs

diff --git a/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Bugzilla51553.cs b/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Bugzilla51553.cs
new file mode 100644
index 0000000..1362ac0
--- /dev/null
+++ b/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Bugzilla51553.cs
@@ -0,0 +1,44 @@
+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, 51553, "[UWP] Toolbar not shown on first Detail page", PlatformAffected.WinRT)]
+	public class Bugzilla51553 : TestMasterDetailPage
+	{
+		protected override void Init()
+		{
+			Master = new ContentPage
+			{
+				Title = "Master",
+				BackgroundColor = Color.Red
+			};
+
+			Detail = new NavigationPage(new TestPage());
+		}
+
+		class TestPage : ContentPage
+		{
+			public TestPage()
+			{
+				Title = "Test Page";
+
+				ToolbarItems.Add(new ToolbarItem("Test", "coffee.png", () => System.Diagnostics.Debug.WriteLine("ToolbarItem pressed")));
+
+				Content = new StackLayout
+				{
+					Children = {
+						new Label { Text = "If the ToolbarItem is not visible then this test has failed." }
+					}
+				};
+			}
+		}
+	}
+}
\ No newline at end of file
diff --git a/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Bugzilla51802.cs b/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Bugzilla51802.cs
new file mode 100644
index 0000000..f73653e
--- /dev/null
+++ b/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Bugzilla51802.cs
@@ -0,0 +1,36 @@
+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, 51802, "[UWP] Detail Page Has Navigation Bar Even When Not Inside a NavigationPage", PlatformAffected.WinRT)]
+	public class Bugzilla51802 : TestMasterDetailPage
+	{
+		protected override void Init()
+		{
+			Master = new ContentPage
+			{
+				Title = "Master",
+				BackgroundColor = Color.Red
+			};
+
+			Detail = new ContentPage
+			{
+				Content = new StackLayout
+				{
+					VerticalOptions = LayoutOptions.Center,
+					Children = {
+						new Label { Text = "If a navigation bar is present on this page then this test has failed." }
+					}
+				}
+			};
+		}
+	}
+}
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 4490cf0..2876319 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
@@ -172,6 +172,8 @@
     <Compile Include="$(MSBuildThisFileDirectory)Bugzilla47923.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)Bugzilla48236.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)Bugzilla47971.cs" />
+    <Compile Include="$(MSBuildThisFileDirectory)Bugzilla51553.cs" />
+    <Compile Include="$(MSBuildThisFileDirectory)Bugzilla51802.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)Bugzilla51236.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)Bugzilla51238.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)Bugzilla51642.xaml.cs">
diff --git a/Xamarin.Forms.Platform.UAP/MasterDetailControl.cs b/Xamarin.Forms.Platform.UAP/MasterDetailControl.cs
index 79da59c..56dd4d9 100644
--- a/Xamarin.Forms.Platform.UAP/MasterDetailControl.cs
+++ b/Xamarin.Forms.Platform.UAP/MasterDetailControl.cs
@@ -294,6 +294,9 @@ namespace Xamarin.Forms.Platform.UWP
 			ContentTogglePaneButtonVisibility = _split.DisplayMode == SplitViewDisplayMode.Overlay 
 				? Visibility.Visible 
 				: Visibility.Collapsed;
+
+			if (ContentTogglePaneButtonVisibility == Visibility.Visible)
+				DetailTitleVisibility = Visibility.Visible;
 		}
 	}
 }
\ No newline at end of file
diff --git a/Xamarin.Forms.Platform.UAP/MasterDetailControlStyle.xaml b/Xamarin.Forms.Platform.UAP/MasterDetailControlStyle.xaml
index 3f13f76..ee51204 100644
--- a/Xamarin.Forms.Platform.UAP/MasterDetailControlStyle.xaml
+++ b/Xamarin.Forms.Platform.UAP/MasterDetailControlStyle.xaml
@@ -29,26 +29,26 @@
 									<RowDefinition Height="Auto" />
 								</Grid.RowDefinitions>
 
-                                <Border x:Name="TopCommandBarArea" HorizontalAlignment="Stretch" Background="{TemplateBinding ToolbarBackground}">
-                                    <uwp:FormsCommandBar x:Name="CommandBar" Background="{TemplateBinding ToolbarBackground}" MinHeight="{ThemeResource TitleBarHeight}">
-                                        <uwp:FormsCommandBar.Content>
-                                            <Border x:Name="TitleArea" Height="{ThemeResource TitleBarHeight}">
-                                                <StackPanel Orientation="Horizontal" VerticalAlignment="Center" Background="{TemplateBinding ToolbarBackground}" >
+								<Border x:Name="TopCommandBarArea" HorizontalAlignment="Stretch" Background="{TemplateBinding ToolbarBackground}">
+									<uwp:FormsCommandBar x:Name="CommandBar" Background="{TemplateBinding ToolbarBackground}" MinHeight="{ThemeResource TitleBarHeight}">
+										<uwp:FormsCommandBar.Content>
+											<Border x:Name="TitleArea" Height="{ThemeResource TitleBarHeight}" Visibility="{TemplateBinding DetailTitleVisibility}">
+												<StackPanel Orientation="Horizontal" VerticalAlignment="Center" Background="{TemplateBinding ToolbarBackground}" >
 
-                                                    <Button Name="ContentTogglePane" Style="{ThemeResource PaneButton}" Foreground="{TemplateBinding ToolbarForeground}"  
-											Visibility="{TemplateBinding ContentTogglePaneButtonVisibility}" />
+													<Button Name="ContentTogglePane" Style="{ThemeResource PaneButton}" Foreground="{TemplateBinding ToolbarForeground}"  
+															Visibility="{TemplateBinding ContentTogglePaneButtonVisibility}" />
 
-                                                    <Border Height="{ThemeResource TitleBarHeight}" Visibility="{TemplateBinding DetailTitleVisibility}">
-                                                        <TextBlock Text="{TemplateBinding DetailTitle}" VerticalAlignment="Center" Margin="10,0,0,0" Foreground="{TemplateBinding ToolbarForeground}" Style="{ThemeResource TitleTextBlockStyle}" />
-                                                    </Border>
+													<Border Height="{ThemeResource TitleBarHeight}" Visibility="{TemplateBinding DetailTitleVisibility}">
+														<TextBlock Text="{TemplateBinding DetailTitle}" VerticalAlignment="Center" Margin="10,0,0,0" Foreground="{TemplateBinding ToolbarForeground}" Style="{ThemeResource TitleTextBlockStyle}" />
+													</Border>
 
-                                                </StackPanel>
-                                            </Border>
-                                        </uwp:FormsCommandBar.Content>
-                                    </uwp:FormsCommandBar>
-                                </Border>
+												</StackPanel>
+											</Border>
+										</uwp:FormsCommandBar.Content>
+									</uwp:FormsCommandBar>
+								</Border>
 
-                                <ContentPresenter x:Name="DetailPresenter" Grid.Row="1" Content="{Binding RelativeSource={RelativeSource TemplatedParent}, Path=Detail}" />
+								<ContentPresenter x:Name="DetailPresenter" Grid.Row="1" Content="{Binding RelativeSource={RelativeSource TemplatedParent}, Path=Detail}" />
 
 								<Border x:Name="BottomCommandBarArea" Grid.Row="2" HorizontalAlignment="Stretch"></Border>
 							</Grid>
@@ -58,4 +58,4 @@
 			</Setter.Value>
 		</Setter>
 	</Style>
-</ResourceDictionary> 
\ No newline at end of file
+</ResourceDictionary>
\ No newline at end of file
diff --git a/Xamarin.Forms.Platform.UAP/MasterDetailPageRenderer.cs b/Xamarin.Forms.Platform.UAP/MasterDetailPageRenderer.cs
index b2b0d8b..d6518d2 100644
--- a/Xamarin.Forms.Platform.UAP/MasterDetailPageRenderer.cs
+++ b/Xamarin.Forms.Platform.UAP/MasterDetailPageRenderer.cs
@@ -146,19 +146,17 @@ namespace Xamarin.Forms.Platform.UWP
 				}
 
 				e.NewElement.PropertyChanged += OnElementPropertyChanged;
+				UpdateMode();
 				UpdateDetail();
 				UpdateMaster();
-				UpdateMode();
 				UpdateIsPresented();
 
 				if (!string.IsNullOrEmpty(e.NewElement.AutomationId))
 					Control.SetValue(AutomationProperties.AutomationIdProperty, e.NewElement.AutomationId);
 
-#if WINDOWS_UWP
-                UpdateToolbarPlacement();
-#endif
-
-            }
+				((ITitleProvider)this).BarBackgroundBrush = (Brush)Windows.UI.Xaml.Application.Current.Resources["SystemControlBackgroundChromeMediumLowBrush"];
+				UpdateToolbarPlacement();
+			}
 		}
 
 		protected virtual void OnElementPropertyChanged(object sender, PropertyChangedEventArgs e)
@@ -267,8 +265,7 @@ namespace Xamarin.Forms.Platform.UWP
 				IVisualElementRenderer renderer = _detail.GetOrCreateRenderer();
 				element = renderer.ContainerElement;
 
-				// Enforce consistency rules on toolbar (show toolbar if Detail is Navigation Page)
-				Control.ShouldShowToolbar = _detail is NavigationPage; 
+				UpdateToolbarVisibilty();
 			}
 
 			Control.Detail = element;
@@ -311,24 +308,28 @@ namespace Xamarin.Forms.Platform.UWP
 			Control.Master = element;
 			Control.MasterTitle = _master?.Title;
 
-			// Enforce consistency rules on toolbar (show toolbar if Master is Navigation Page)
-			Control.ShouldShowToolbar = _master is NavigationPage;
+			UpdateToolbarVisibilty();
 		}
 
 		void UpdateMode()
 		{
+			UpdateDetailTitle();
 			Control.CollapseStyle = Element.OnThisPlatform().GetCollapseStyle();
 			Control.CollapsedPaneWidth = Element.OnThisPlatform().CollapsedPaneWidth();
 			Control.ShouldShowSplitMode = MasterDetailPageController.ShouldShowSplitMode;
 		}
 
-#if WINDOWS_UWP
-
-        void UpdateToolbarPlacement()
+		void UpdateToolbarPlacement()
 		{
 			Control.ToolbarPlacement = Element.OnThisPlatform().GetToolbarPlacement();
 		}
 
+		void UpdateToolbarVisibilty()
+		{
+			// Enforce consistency rules on toolbar
+			Control.ShouldShowToolbar = _detail is NavigationPage || _master is NavigationPage;
+		}
+
 		public void BindForegroundColor(AppBar appBar)
 		{
 			SetAppBarForegroundBinding(appBar);
@@ -344,6 +345,5 @@ namespace Xamarin.Forms.Platform.UWP
 			element.SetBinding(Windows.UI.Xaml.Controls.Control.ForegroundProperty,
 				new Windows.UI.Xaml.Data.Binding { Path = new PropertyPath("Control.ToolbarForeground"), Source = this, RelativeSource = new RelativeSource { Mode = RelativeSourceMode.TemplatedParent } });
 		}
-#endif
 	}
 }
\ No newline at end of file
diff --git a/Xamarin.Forms.Platform.WinRT/NavigationPageRenderer.cs b/Xamarin.Forms.Platform.WinRT/NavigationPageRenderer.cs
index d0b5727..9f14150 100644
--- a/Xamarin.Forms.Platform.WinRT/NavigationPageRenderer.cs
+++ b/Xamarin.Forms.Platform.WinRT/NavigationPageRenderer.cs
@@ -170,11 +170,6 @@ namespace Xamarin.Forms.Platform.WinRT
 
 					Tracker = new BackgroundTracker<PageControl>(Control.BackgroundProperty) { Element = (Page)element, Container = _container };
 
-#if WINDOWS_UWP
-					// Enforce consistency rules on toolbar (show toolbar if top-level page is Navigation Page)
-					_container.ShouldShowToolbar = _parentMasterDetailPage == null && _parentMasterDetailPage == null;
-#endif
-
 					SetPage(Element.CurrentPage, false, false);
 
 					_container.Loaded += OnLoaded;
@@ -189,6 +184,11 @@ namespace Xamarin.Forms.Platform.WinRT
 				UpdateNavigationBarBackground();
 				UpdateToolbarPlacement();
 
+#if WINDOWS_UWP
+				// Enforce consistency rules on toolbar (show toolbar if top-level page is Navigation Page)
+				_container.ShouldShowToolbar = _parentMasterDetailPage == null && _parentTabbedPage == null;
+#endif
+
 				Element.PropertyChanged += OnElementPropertyChanged;
 				((INavigationPageController)Element).PushRequested += OnPushRequested;
 				((INavigationPageController)Element).PopRequested += OnPopRequested;
-- 
2.7.4