[UWP] Improve toolbar consistency on MasterDetailPage (#744)

* Improve toolbar consistency on MasterDetailPage * Properly indent xaml page
xamarin · Mar 10, 2017 · 8ea3368 · 8ea3368
1 parent d455bc4
commit 8ea3368
Show file tree

Hide file tree

Showing 7 changed files with 124 additions and 37 deletions.
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
@@ -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." }
+					}
+				};
+			}
+		}
+	}
+}
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
@@ -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/...ssues/Xamarin.Forms.Controls.Issues.Shared/Xamarin.Forms.Controls.Issues.Shared.projitems b/...ssues/Xamarin.Forms.Controls.Issues.Shared/Xamarin.Forms.Controls.Issues.Shared.projitems
@@ -155,6 +155,9 @@
     <Compile Include="$(MSBuildThisFileDirectory)Bugzilla46630.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">
       <DependentUpon>Bugzilla51642.xaml</DependentUpon>

diff --git a/Xamarin.Forms.Platform.UAP/MasterDetailControl.cs b/Xamarin.Forms.Platform.UAP/MasterDetailControl.cs
@@ -294,6 +294,9 @@ void UpdateMode()
 			ContentTogglePaneButtonVisibility = _split.DisplayMode == SplitViewDisplayMode.Overlay 
 				? Visibility.Visible 
 				: Visibility.Collapsed;
+
+			if (ContentTogglePaneButtonVisibility == Visibility.Visible)
+				DetailTitleVisibility = Visibility.Visible;
 		}
 	}
 }
diff --git 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> 
+</ResourceDictionary>
diff --git a/Xamarin.Forms.Platform.UAP/MasterDetailPageRenderer.cs b/Xamarin.Forms.Platform.UAP/MasterDetailPageRenderer.cs
@@ -141,19 +141,17 @@ protected virtual void OnElementChanged(ElementChangedEventArgs<MasterDetailPage
 				}
 
 				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)
@@ -262,8 +260,7 @@ void UpdateDetail()
 				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;
@@ -301,24 +298,28 @@ void UpdateMaster()
 			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);
@@ -334,6 +335,5 @@ void SetAppBarForegroundBinding(FrameworkElement element)
 			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
 	}
 }
diff --git a/Xamarin.Forms.Platform.WinRT/NavigationPageRenderer.cs b/Xamarin.Forms.Platform.WinRT/NavigationPageRenderer.cs
@@ -177,11 +177,6 @@ public void SetElement(VisualElement element)
 
 					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;
@@ -194,7 +189,13 @@ public void SetElement(VisualElement element)
 				LookupRelevantParents();
 				UpdateTitleColor();
 				UpdateNavigationBarBackground();
-                UpdateToolbarPlacement();
+				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;