Add repro of uncentered button image for Android (#176)

Fix vertical centering logic for button images in Android

4 files changed, 107 insertions, 25 deletions

diff --git a/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Bugzilla27417.cs b/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Bugzilla27417.cs
index 8edfb909..5ae2970f 100644
--- a/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Bugzilla27417.cs
+++ b/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Bugzilla27417.cs
@@ -10,20 +10,40 @@ namespace Xamarin.Forms.Controls.Issues
 	{
 		protected override void Init()
 		{
-			var instructions = new Label { Text = @"There should be 6 buttons below. 
-The first button should have the text 'Click Me' in the center.
-The second button should have an image in the center and no text.
-The third button should have the image on the left and the text on the right.
-The fourth button should have the image on the top and the text on the bottom.
-The fifth button should have the image on the right and the text on the left.
-The sixth button should have the image on the bottom and the text on the top." };
+			var instructions = new Label { Text = @"There should be 8 buttons below (the bottom 7 are in a ScrollView). 
+Buttons 1 and 2 have images which are horizontally and vertically centered.
+Button 3 should have the text 'Click Me' in the center.
+Button 4 should have an image in the center and no text.
+Button 5 should have the image on the left and the text on the right.
+Button 6 should have the image on the top and the text on the bottom.
+Button 7 should have the image on the bottom and the text on the top.
+Button 8 have the image on the right and the text on the left." };
+
+			var grid = new Grid
+			{
+				RowDefinitions = new RowDefinitionCollection
+				{
+					new RowDefinition { Height = 100 }
+				},
+				VerticalOptions = LayoutOptions.Start,
+				Children =
+				{
+					new Button
+					{
+						HeightRequest = 500, // Making sure that the image still gets centered vertically even if the HeightRequest won't be honored
+						Image = "coffee.png"
+					}
+				}
+			};
 
 			Content = new StackLayout
 			{
+				VerticalOptions = LayoutOptions.Fill,
 				Spacing = 10,
 				Children =
 				{
 					instructions,
+					grid,
 					new ScrollView
 					{
 						Content = new StackLayout
@@ -33,6 +53,7 @@ The sixth button should have the image on the bottom and the text on the top." }
 							HorizontalOptions = LayoutOptions.Center,
 							Children =
 							{
+								new Button { WidthRequest = 200, HeightRequest = 300, Image = "coffee.png" },
 								new Button { Text = "Click Me", BackgroundColor = Color.Gray },
 								new Button { Image = "coffee.png", BackgroundColor = Color.Gray },
 								CreateButton(new Button.ButtonContentLayout(Button.ButtonContentLayout.ImagePosition.Left, 10)),
diff --git a/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Bugzilla27417Xaml.xaml b/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Bugzilla27417Xaml.xaml
index d050524e..72442038 100644
--- a/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Bugzilla27417Xaml.xaml
+++ b/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Bugzilla27417Xaml.xaml
@@ -2,14 +2,16 @@
 <ContentPage xmlns="http://xamarin.com/schemas/2014/forms"
              xmlns:x="http://schemas.microsoft.com/winfx/2009/xaml"
              x:Class="Xamarin.Forms.Controls.Issues.Bugzilla27417Xaml">
-	<StackLayout Spacing="10" HorizontalOptions="Center" >
+  <StackLayout Spacing="10">
 
-		<Button BackgroundColor="Color.Gray" Text="Click Me"></Button>
-		<Button BackgroundColor="Color.Gray" Image="coffee.png"></Button>
-		<Button BackgroundColor="Color.Gray" Image="coffee.png" Text="Click Me"></Button>
-		<Button BackgroundColor="Color.Gray" Image="coffee.png" Text="Click Me" ContentLayout="Top,10"></Button>
-		<Button BackgroundColor="Color.Gray" Image="coffee.png" Text="Click Me" ContentLayout="Bottom,10"></Button>
-		<Button BackgroundColor="Color.Gray" Image="coffee.png" Text="Click Me" ContentLayout="Right"></Button>
+    <Button BackgroundColor="Color.Gray" Text="Click Me"></Button>
+    <Button BackgroundColor="Color.Gray" Image="coffee.png"></Button>
+    <Button BackgroundColor="Color.Gray" Image="coffee.png" Text="Click Me"></Button>
+    <Button BackgroundColor="Color.Gray" Image="coffee.png" Text="Click Me" ContentLayout="Top,10"></Button>
+    <Button BackgroundColor="Color.Gray" Image="coffee.png" Text="Click Me" ContentLayout="Bottom,10"></Button>
+    <Button BackgroundColor="Color.Gray" Image="coffee.png" Text="Click Me" ContentLayout="Right"></Button>
 
-	</StackLayout>
+    <Button HeightRequest="125" WidthRequest="200" BackgroundColor="Lime" Image="coffee.png" HorizontalOptions="Center" VerticalOptions="Center"/>
+    
+  </StackLayout>
 </ContentPage>
\ No newline at end of file
diff --git a/Xamarin.Forms.Platform.Android/AppCompat/ButtonRenderer.cs b/Xamarin.Forms.Platform.Android/AppCompat/ButtonRenderer.cs
index 8f4ef35e..2d175860 100644
--- a/Xamarin.Forms.Platform.Android/AppCompat/ButtonRenderer.cs
+++ b/Xamarin.Forms.Platform.Android/AppCompat/ButtonRenderer.cs
@@ -9,6 +9,7 @@ using Android.Support.V7.Widget;
 using Android.Util;
 using GlobalResource = Android.Resource;
 using Object = Java.Lang.Object;
+using static System.String;
 
 namespace Xamarin.Forms.Platform.Android.AppCompat
 {
@@ -18,6 +19,7 @@ namespace Xamarin.Forms.Platform.Android.AppCompat
 		float _defaultFontSize;
 		Typeface _defaultTypeface;
 		bool _isDisposed;
+		int _imageHeight = -1;
 
 		public ButtonRenderer()
 		{
@@ -41,6 +43,20 @@ namespace Xamarin.Forms.Platform.Android.AppCompat
 			return base.GetDesiredSize(widthConstraint, heightConstraint);
 		}
 
+		protected override void OnLayout(bool changed, int l, int t, int r, int b)
+		{
+			if (_imageHeight > -1)
+			{
+				// We've got an image (and no text); it's already centered horizontally,
+				// we just need to adjust the padding so it centers vertically
+				var diff = (b - t - _imageHeight) / 2;
+				diff = Math.Max(diff, 0);
+				Control?.SetPadding(0, diff, 0, -diff);
+			}
+
+			base.OnLayout(changed, l, t, r, b);
+		}
+
 		protected override AppCompatButton CreateNativeControl()
 		{
 			return new AppCompatButton(Context);
@@ -161,6 +177,7 @@ namespace Xamarin.Forms.Platform.Android.AppCompat
 		{
 			var elementImage = Element.Image;
 			var imageFile = elementImage?.File;
+			_imageHeight = -1;
 
 			if (elementImage == null || string.IsNullOrEmpty(imageFile))
 			{
@@ -170,14 +187,20 @@ namespace Xamarin.Forms.Platform.Android.AppCompat
 
 			var image = Context.Resources.GetDrawable(imageFile);
 
-			if (string.IsNullOrEmpty(Element.Text))
+			if (IsNullOrEmpty(Element.Text))
 			{
 				// No text, so no need for relative position; just center the image
 				// There's no option for just plain-old centering, so we'll use Top 
-				// (which handles the horizontal centering) and some tricksy padding 
-				// to handle the vertical centering
+				// (which handles the horizontal centering) and some tricksy padding (in OnLayout)
+				// to handle the vertical centering 
+
+				// Clear any previous padding and set the image as top/center
+				Control.SetPadding(0, 0, 0, 0);
 				Control.SetCompoundDrawablesWithIntrinsicBounds(null, image, null, null);
-				Control.SetPadding(0, Control.PaddingTop, 0, -Control.PaddingTop);
+
+				// Keep track of the image height so we can use it in OnLayout
+				_imageHeight = image.IntrinsicHeight;
+
 				image?.Dispose();
 				return;
 			}
@@ -239,7 +262,14 @@ namespace Xamarin.Forms.Platform.Android.AppCompat
 
 		void UpdateText()
 		{
+			var oldText = NativeButton.Text;
 			NativeButton.Text = Element.Text;
+
+			// If we went from or to having no text, we need to update the image position
+			if (IsNullOrEmpty(oldText) != IsNullOrEmpty(NativeButton.Text))
+			{
+				UpdateBitmap();
+			}
 		}
 
 		void UpdateTextColor()
diff --git a/Xamarin.Forms.Platform.Android/Renderers/ButtonRenderer.cs b/Xamarin.Forms.Platform.Android/Renderers/ButtonRenderer.cs
index 7247361a..5de429af 100644
--- a/Xamarin.Forms.Platform.Android/Renderers/ButtonRenderer.cs
+++ b/Xamarin.Forms.Platform.Android/Renderers/ButtonRenderer.cs
@@ -4,6 +4,7 @@ using Android.Content.Res;
 using Android.Graphics;
 using Android.Graphics.Drawables;
 using Android.Util;
+using static System.String;
 using AButton = Android.Widget.Button;
 using AView = Android.Views.View;
 using Object = Java.Lang.Object;
@@ -18,8 +19,8 @@ namespace Xamarin.Forms.Platform.Android
 		float _defaultFontSize;
 		Typeface _defaultTypeface;
 		bool _drawableEnabled;
-
 		bool _isDisposed;
+		int _imageHeight = -1;
 
 		public ButtonRenderer()
 		{
@@ -46,6 +47,20 @@ namespace Xamarin.Forms.Platform.Android
 			return base.GetDesiredSize(widthConstraint, heightConstraint);
 		}
 
+		protected override void OnLayout(bool changed, int l, int t, int r, int b)
+		{
+			if (_imageHeight > -1)
+			{
+				// We've got an image (and no text); it's already centered horizontally,
+				// we just need to adjust the padding so it centers vertically
+				var diff = (b - t - _imageHeight) / 2;
+				diff = Math.Max(diff, 0);
+				Control?.SetPadding(0, diff, 0, -diff);
+			}
+
+			base.OnLayout(changed, l, t, r, b);
+		}
+
 		protected override void Dispose(bool disposing)
 		{
 			if (_isDisposed)
@@ -142,8 +157,9 @@ namespace Xamarin.Forms.Platform.Android
 		{
 			var elementImage = Element.Image;
 			var imageFile = elementImage?.File;
+			_imageHeight = -1;
 
-			if (elementImage == null || string.IsNullOrEmpty(imageFile))
+			if (elementImage == null || IsNullOrEmpty(imageFile))
 			{
 				Control.SetCompoundDrawablesWithIntrinsicBounds(null, null, null, null);
 				return;
@@ -151,14 +167,20 @@ namespace Xamarin.Forms.Platform.Android
 
 			var image = Context.Resources.GetDrawable(imageFile);
 
-			if (string.IsNullOrEmpty(Element.Text))
+			if (IsNullOrEmpty(Element.Text))
 			{
 				// No text, so no need for relative position; just center the image
 				// There's no option for just plain-old centering, so we'll use Top 
-				// (which handles the horizontal centering) and some tricksy padding 
-				// to handle the vertical centering
+				// (which handles the horizontal centering) and some tricksy padding (in OnLayout)
+				// to handle the vertical centering 
+
+				// Clear any previous padding and set the image as top/center
+				Control.SetPadding(0, 0, 0, 0);
 				Control.SetCompoundDrawablesWithIntrinsicBounds(null, image, null, null);
-				Control.SetPadding(0, Control.PaddingTop, 0, -Control.PaddingTop);
+
+				// Keep track of the image height so we can use it in OnLayout
+				_imageHeight = image.IntrinsicHeight;
+
 				image?.Dispose();
 				return;
 			}
@@ -250,7 +272,14 @@ namespace Xamarin.Forms.Platform.Android
 
 		void UpdateText()
 		{
+			var oldText = NativeButton.Text;
 			NativeButton.Text = Element.Text;
+
+			// If we went from or to having no text, we need to update the image position
+			if (IsNullOrEmpty(oldText) != IsNullOrEmpty(NativeButton.Text))
+			{
+				UpdateBitmap();
+			}
 		}
 
 		void UpdateTextColor()