Make UWP toolbar display rules consistent with other platforms (#638)

6 files changed, 111 insertions, 18 deletions

diff --git a/Xamarin.Forms.Platform.UAP/FormsCommandBar.cs b/Xamarin.Forms.Platform.UAP/FormsCommandBar.cs
index 41451864..bfa146c4 100644
--- a/Xamarin.Forms.Platform.UAP/FormsCommandBar.cs
+++ b/Xamarin.Forms.Platform.UAP/FormsCommandBar.cs
@@ -3,6 +3,7 @@ using Windows.Foundation.Collections;
 using Windows.UI.Xaml;
 using Windows.UI.Xaml.Controls;
 using Windows.UI.Xaml.Data;
+using Windows.UI.Xaml.Media;
 
 namespace Xamarin.Forms.Platform.UWP
 {
@@ -11,7 +12,9 @@ namespace Xamarin.Forms.Platform.UWP
 		// TODO Once 10.0.14393.0 is available (and we don't have to support lower versions), enable dynamic overflow: https://msdn.microsoft.com/en-us/library/windows/apps/windows.ui.xaml.controls.commandbar.isdynamicoverflowenabled.aspx 
 
 		Windows.UI.Xaml.Controls.Button _moreButton;
-		
+		Windows.UI.Xaml.Controls.ItemsControl _primaryItemsControl;
+		bool _isInValidLocation;
+
 		public FormsCommandBar()
 		{
 			PrimaryCommands.VectorChanged += OnCommandsChanged;
@@ -20,11 +23,26 @@ namespace Xamarin.Forms.Platform.UWP
 			WatchForContentChanges();
 		}
 
+		// Set by the container if the container is a valid place to show a toolbar.
+		// This exists to provide consistency with the other platforms; we've got 
+		// rules in place that limit toolbars to Navigation Page and to Tabbed 
+		// and Master-Detail Pages when they're currently displaying a Navigation Page
+		public bool IsInValidLocation
+		{
+			get { return _isInValidLocation; }
+			set
+			{
+				_isInValidLocation = value;
+				UpdateVisibility();
+			}
+		}
+
 		protected override void OnApplyTemplate()
 		{
 			base.OnApplyTemplate();
 
 			_moreButton = GetTemplateChild("MoreButton") as Windows.UI.Xaml.Controls.Button;
+			_primaryItemsControl = GetTemplateChild("PrimaryItemsControl") as Windows.UI.Xaml.Controls.ItemsControl;
 		}
 
 		void OnCommandsChanged(IObservableVector<ICommandBarElement> sender, IVectorChangedEventArgs args)
@@ -34,8 +52,52 @@ namespace Xamarin.Forms.Platform.UWP
 
 		void UpdateVisibility()
 		{
+			// Determine whether we have a title (or some other content) inside this command bar
+			var frameworkElement = Content as FrameworkElement;
+
+			// Apply the rules for consistency with other platforms
+
+			// Not in one of the acceptable toolbar locations from the other platforms
+			if (!IsInValidLocation)
+			{
+				// If there's no title to display (e.g., toolbarplacement is set to bottom)
+				// or the title is collapsed (e.g., because it's empty)
+				if (frameworkElement == null || frameworkElement.Visibility != Visibility.Visible)
+				{
+					// Just collapse the whole thing
+					Visibility = Visibility.Collapsed;
+					return;
+				}
+			
+				// The title needs to be visible, but we're not allowed to show a toolbar
+				// So we need to hide the toolbar items
+
+				Visibility = Visibility.Visible;
+
+				if (_moreButton != null)
+				{
+					_moreButton.Visibility = Visibility.Collapsed;
+				}
+
+				if (_primaryItemsControl != null)
+				{
+					_primaryItemsControl.Visibility = Visibility.Collapsed;
+				}
+
+				return;
+			}
+
+			// We're in one of the acceptable toolbar locations from the other platforms so the normal rules apply
+
+			if (_primaryItemsControl != null)
+			{
+				// This is normally visible by default, but it might have been collapsed by the toolbar consistency rules above
+				_primaryItemsControl.Visibility = Visibility.Visible;
+			}
+
+			// Are there any commands to display?
 			var visibility = PrimaryCommands.Count + SecondaryCommands.Count > 0 ? Visibility.Visible : Visibility.Collapsed;
-		
+
 			if (_moreButton != null)
 			{
 				// The "..." button should only be visible if we have commands to display
@@ -44,26 +106,15 @@ namespace Xamarin.Forms.Platform.UWP
 				// There *is* an OverflowButtonVisibility property that does more or less the same thing, 
 				// but it became available in 10.0.14393.0 and we have to support 10.0.10240
 			}
-
-			// If we have a title (or some other content) inside this command bar
-			// and that content is not collapsed
-			var frameworkElement = Content as FrameworkElement;
-
-			// Temporarily tie the visibility of the toolbar to the visibility of the Title
-			// to be consistent with the old style / other platforms
-			if (frameworkElement != null && frameworkElement.Visibility == Visibility.Collapsed)
-			{
-				Visibility = Visibility.Collapsed;
-				return;
-			}
-
+			
 			if (frameworkElement != null && frameworkElement.Visibility != Visibility.Collapsed)
 			{
+				// If there's a title to display, we have to be visible whether or not we have commands
 				Visibility = Visibility.Visible;
 			}
 			else
 			{
-				// Otherwise, collapse it if there are no commands
+				// Otherwise, visibility depends on whether we have commands
 				Visibility = visibility;
 			}
 		}
diff --git a/Xamarin.Forms.Platform.UAP/MasterDetailControl.cs b/Xamarin.Forms.Platform.UAP/MasterDetailControl.cs
index 2357232e..79da59cb 100644
--- a/Xamarin.Forms.Platform.UAP/MasterDetailControl.cs
+++ b/Xamarin.Forms.Platform.UAP/MasterDetailControl.cs
@@ -51,6 +51,12 @@ namespace Xamarin.Forms.Platform.UWP
 		CommandBar _commandBar;
 		readonly ToolbarPlacementHelper _toolbarPlacementHelper = new ToolbarPlacementHelper();
 
+		public bool ShouldShowToolbar
+		{
+			get { return _toolbarPlacementHelper.ShouldShowToolBar; }
+			set { _toolbarPlacementHelper.ShouldShowToolBar = value; }
+		}
+
 		TaskCompletionSource<CommandBar> _commandBarTcs;
 		FrameworkElement _masterPresenter;
 		FrameworkElement _detailPresenter;
diff --git a/Xamarin.Forms.Platform.UAP/MasterDetailPageRenderer.cs b/Xamarin.Forms.Platform.UAP/MasterDetailPageRenderer.cs
index f0048154..e024b235 100644
--- a/Xamarin.Forms.Platform.UAP/MasterDetailPageRenderer.cs
+++ b/Xamarin.Forms.Platform.UAP/MasterDetailPageRenderer.cs
@@ -261,6 +261,9 @@ 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; 
 			}
 
 			Control.Detail = element;
@@ -297,6 +300,9 @@ 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;
 		}
 
 		void UpdateMode()
diff --git a/Xamarin.Forms.Platform.UAP/TabbedPageRenderer.cs b/Xamarin.Forms.Platform.UAP/TabbedPageRenderer.cs
index 568267db..02b73c94 100644
--- a/Xamarin.Forms.Platform.UAP/TabbedPageRenderer.cs
+++ b/Xamarin.Forms.Platform.UAP/TabbedPageRenderer.cs
@@ -317,6 +317,9 @@ namespace Xamarin.Forms.Platform.UWP
             var nav = page as NavigationPage;
             TitleProvider.ShowTitle = nav != null;
 
+			// Enforce consistency rules on toolbar (show toolbar if visible Tab is Navigation Page)
+			Control.ShouldShowToolbar = nav != null;
+
             if (page == null)
                 return;
 
diff --git a/Xamarin.Forms.Platform.UAP/TabbedPageStyle.xaml b/Xamarin.Forms.Platform.UAP/TabbedPageStyle.xaml
index ef7467c4..cac1045d 100644
--- a/Xamarin.Forms.Platform.UAP/TabbedPageStyle.xaml
+++ b/Xamarin.Forms.Platform.UAP/TabbedPageStyle.xaml
@@ -68,7 +68,7 @@
                             <uwp:FormsCommandBar x:Name="CommandBar" Background="{TemplateBinding ToolbarBackground}" MinHeight="{ThemeResource TitleBarHeight}">
                                 <uwp:FormsCommandBar.Content>
                                     <Border x:Name="TitleArea" Visibility="{TemplateBinding TitleVisibility}" Height="{ThemeResource TitleBarHeight}">
-                                        <TextBlock Text="{Binding Title}" VerticalAlignment="Center" Padding="10,0,0,0" Foreground="Red" Style="{ThemeResource TitleTextBlockStyle}" />
+                                        <TextBlock Text="{TemplateBinding Title}" TextWrapping="NoWrap" VerticalAlignment="Center" Margin="10,0,0,0" Foreground="{TemplateBinding ToolbarForeground}" Style="{ThemeResource TitleTextBlockStyle}" />
                                     </Border>
                                 </uwp:FormsCommandBar.Content>
                             </uwp:FormsCommandBar>
diff --git a/Xamarin.Forms.Platform.UAP/ToolbarPlacementHelper.cs b/Xamarin.Forms.Platform.UAP/ToolbarPlacementHelper.cs
index 1b36010a..0985cc3f 100644
--- a/Xamarin.Forms.Platform.UAP/ToolbarPlacementHelper.cs
+++ b/Xamarin.Forms.Platform.UAP/ToolbarPlacementHelper.cs
@@ -26,7 +26,11 @@ namespace Xamarin.Forms.Platform.UWP
 			{
 				// We have to wait for the command bar to load so that it'll be in the control hierarchy
 				// otherwise we can't properly move it to wherever the toolbar is supposed to be
-				_commandBar.Loaded += (sender, args) => UpdateToolbarPlacement();
+				_commandBar.Loaded += (sender, args) =>
+				{
+					UpdateToolbarPlacement();
+					UpdateIsInValidLocation();
+				};
 			}
 		}
 
@@ -90,5 +94,28 @@ namespace Xamarin.Forms.Platform.UWP
 				}
 			}
 		}
+
+		// For the time being, keeping this logic for dealing with consistency between the platforms
+		// re: toolbar visibility here; at some point we should be separating toolbars from navigation bars,
+		// and this won't be necessary
+		bool _shouldShowToolBar;
+		public bool ShouldShowToolBar
+		{
+			get { return _shouldShowToolBar; }
+			set
+			{
+				_shouldShowToolBar = value;
+				UpdateIsInValidLocation();
+			}
+		}
+
+		void UpdateIsInValidLocation()
+		{
+			var formsCommandBar = _commandBar as FormsCommandBar;
+			if (formsCommandBar != null)
+			{
+				formsCommandBar.IsInValidLocation = ShouldShowToolBar;
+			}
+		}
 	}
 }
\ No newline at end of file