[iOS, Android, UWP] Added padding to Button control (#2426) Fixes #1702

Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/GitHub1702.cs

@@ -0,0 +1,165 @@
+using Xamarin.Forms.CustomAttributes;
+using Xamarin.Forms.Internals;
+using System;
+using System.Linq;
+using System.Threading;
+using System.Collections.Generic;
+
+namespace Xamarin.Forms.Controls.Issues
+{
+	[Preserve(AllMembers = true)]
+	[Issue(IssueTracker.Github, 1702, "[Enhancement] Padding on Buttons", PlatformAffected.All)]
+	public class GitHub1702 : TestContentPage
+	{
+		bool animation = false;
+
+		protected override void Init()
+		{
+			StackLayout layout = new StackLayout()
+			{
+				Children =
+				{
+					new Button()
+					{
+						Image = "coffee.png",
+						BackgroundColor = Color.GreenYellow,
+						Text = "No padding? Height 100",
+						HeightRequest = 100,
+					},
+					new Button()
+					{
+						Image = "coffee.png",
+						BackgroundColor = Color.Green,
+						Padding = new Thickness(100, 0, 0, 0),
+						Text = "Do I have left padding? I should have left padding.",
+					},
+					new Button()
+					{
+						Image = "coffee.png",
+						BackgroundColor = Color.LawnGreen,
+						Padding = new Thickness(0, 30, 0, 0),
+						Text = "Do I have top padding? I should have top padding."
+					},
+					new Button()
+					{
+						Image = "coffee.png",
+						BackgroundColor = Color.LightGreen,
+						Padding = new Thickness(0, 0, 100, 0),
+						Text = "Do I have right padding? I should have right padding."
+					},
+					new Button()
+					{
+						Image = "coffee.png",
+						BackgroundColor = Color.ForestGreen,
+						Padding = new Thickness(0, 0, 0, 30),
+						Text = "Do I have bottom padding? I should have bottom padding."
+					}
+				}
+			};
+
+			var buttons = layout.Children.OfType<Button>();
+			layout.Children.Insert(0, ActionGrid(buttons.ToList()));
+			PaddingAnimation(buttons).Start();
+
+			Content = layout;
+		}
+
+		Grid ActionGrid(List<Button> buttons)
+		{
+			Button firstButton = buttons.FirstOrDefault();
+			Grid actionGrid = new Grid();
+			actionGrid.AddChild(new Button()
+			{
+				Text = "Reset text",
+				Command = new Command(() =>
+				{
+					buttons.ForEach(b => b.Text = string.Empty);
+				})
+			}, 0, 0);
+			actionGrid.AddChild(new Button()
+			{
+				Text = "Reset padding",
+				Command = new Command(() =>
+				{
+					buttons.ForEach(b => b.Padding = new Thickness(0, 0, 0, 0));
+				})
+			}, 0, 1);
+			actionGrid.AddChild(new Button()
+			{
+				Text = "Set text",
+				Command = new Command(() =>
+				{
+					buttons.ForEach(b => b.Text = "Some text");
+				})
+			}, 1, 0);
+			actionGrid.AddChild(new Button()
+			{
+				Text = "Animation",
+				Command = new Command(() => animation = !animation)
+			}, 1, 1);
+			actionGrid.AddChild(new Button()
+			{
+				Text = "Add Top",
+				Command = new Command(() =>
+				{
+					var button = firstButton;
+					button.Padding = new Thickness(0, button.Padding.Top + 10, 0, button.Padding.Bottom);
+					if (!String.IsNullOrWhiteSpace(button.Text))
+						button.Text = $"Top: {button.Padding.Top} Bottom: {button.Padding.Bottom}";
+				})
+			}, 2, 0);
+			actionGrid.AddChild(new Button()
+			{
+				Text = "Add Bottom",
+				Command = new Command(() =>
+				{
+					var button = firstButton;
+					button.Padding = new Thickness(0, button.Padding.Top, 0, button.Padding.Bottom + 10);
+					if (!String.IsNullOrWhiteSpace(button.Text))
+						button.Text = $"Top: {button.Padding.Top} Bottom: {button.Padding.Bottom}";
+				})
+			}, 2, 1);
+			return actionGrid;
+		}
+
+		Thread PaddingAnimation(IEnumerable<Button> buttons)
+		{
+			return new Thread(() =>
+			{
+				int increment = 1;
+				int current = 0;
+				int max = 15;
+				int FPS = 30;
+				int sleep = 1000 / FPS;
+
+				while (true)
+				{
+					Thread.Sleep(sleep);
+
+					if (!animation)
+						continue;
+
+					current += increment;
+					if (current > max || current < 0)
+					{
+						increment *= -1;
+						current += increment * 2;
+					}
+
+					Device.BeginInvokeOnMainThread(() =>
+					{
+						foreach (var button in buttons)
+						{
+							var padding = button.Padding;
+							button.Padding = padding = new Thickness(
+								padding.Left + increment,
+								padding.Top + increment,
+								padding.Right + increment,
+								padding.Bottom + increment);
+						}
+					});
+				}
+			});
+		}
+	}
+}

Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Xamarin.Forms.Controls.Issues.Shared.projitems

@@ -239,6 +239,7 @@
     <Compile Include="$(MSBuildThisFileDirectory)Bugzilla59580.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)Effects\AttachedStateEffect.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)Effects\AttachedStateEffectList.cs" />
+    <Compile Include="$(MSBuildThisFileDirectory)GitHub1702.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)GitHub2598.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)GitHub1878.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)Issue1677.cs" />

Xamarin.Forms.Controls/CoreGalleryPages/ButtonCoreGalleryPage.cs

@@ -93,6 +93,12 @@ namespace Xamarin.Forms.Controls
 				}
 			);
 
+			var paddingContainer = new ViewContainer<Button> (Test.Button.Padding,
+				new Button {
+					Text = "Padding", BackgroundColor = Color.Red, Padding = new Thickness (20, 30, 60, 15)
+				}
+			);
+
 			Add (borderButtonContainer);
 			Add (borderRadiusContainer);
 			Add (borderWidthContainer);
@@ -102,6 +108,7 @@ namespace Xamarin.Forms.Controls
 			Add (imageContainer);
 			Add (textContainer);
 			Add (textColorContainer);
+			Add (paddingContainer);
 			//stackLayout.Children.Add (textColorContainer);
 		}
 	}

Xamarin.Forms.Core/Button.cs

@@ -8,7 +8,7 @@ using Xamarin.Forms.Platform;
 namespace Xamarin.Forms
 {
 	[RenderWith(typeof(_ButtonRenderer))]
-	public class Button : View, IFontElement, ITextElement, IBorderElement, IButtonController, IElementConfiguration<Button>
+	public class Button : View, IFontElement, ITextElement, IBorderElement, IButtonController, IElementConfiguration<Button>, IPaddingElement
 	{
 		const double DefaultSpacing = 10;
 		const int DefaultBorderRadius = 5;
@@ -50,6 +50,24 @@ namespace Xamarin.Forms
 			propertyChanging: (bindable, oldvalue, newvalue) => ((Button)bindable).OnSourcePropertyChanging((ImageSource)oldvalue, (ImageSource)newvalue),
 			propertyChanged: (bindable, oldvalue, newvalue) => ((Button)bindable).OnSourcePropertyChanged((ImageSource)oldvalue, (ImageSource)newvalue));
 
+		public static readonly BindableProperty PaddingProperty = PaddingElement.PaddingProperty;
+
+		public Thickness Padding
+		{
+			get { return (Thickness)GetValue (PaddingElement.PaddingProperty); }
+			set { SetValue (PaddingElement.PaddingProperty, value); }
+		}
+
+		Thickness IPaddingElement.PaddingDefaultValueCreator ()
+		{
+			return default (Thickness);
+		}
+
+		void IPaddingElement.OnPaddingPropertyChanged (Thickness oldValue, Thickness newValue)
+		{
+			InvalidateMeasureInternal(InvalidationTrigger.MeasureChanged);
+		}
+
 		readonly Lazy<PlatformConfigurationRegistry<Button>> _platformConfigurationRegistry;
 
 		public Color BorderColor

Xamarin.Forms.CustomAttributes/TestAttributes.cs

@@ -227,6 +227,7 @@ namespace Xamarin.Forms.CustomAttributes
 			BorderColor,
 			BorderRadius,
 			Image,
+			Padding
 		}
 
 		public enum VisualElement

Xamarin.Forms.Platform.Android/AppCompat/ButtonRenderer.cs

@@ -23,6 +23,7 @@ namespace Xamarin.Forms.Platform.Android.AppCompat
 		Typeface _defaultTypeface;
 		bool _isDisposed;
 		int _imageHeight = -1;
+		Thickness _paddingDeltaPix = new Thickness();
 
 		public ButtonRenderer(Context context) : base(context)
 		{
@@ -58,9 +59,13 @@ namespace Xamarin.Forms.Platform.Android.AppCompat
 			{
 				// 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;
+				var diff = ((b - Context.ToPixels(Element.Padding.Bottom + Element.Padding.Top)) - t - _imageHeight) / 2;
 				diff = Math.Max(diff, 0);
-				Control?.SetPadding(0, diff, 0, -diff);
+				UpdateContentEdge(new Thickness(0, diff, 0, -diff));
+			}
+			else
+			{
+				UpdateContentEdge();
 			}
 
 			base.OnLayout(changed, l, t, r, b);
@@ -142,6 +147,8 @@ namespace Xamarin.Forms.Platform.Android.AppCompat
 				UpdateBitmap();
 			else if (e.PropertyName == VisualElement.IsVisibleProperty.PropertyName)
 				UpdateText();
+			else if (e.PropertyName == Button.PaddingProperty.PropertyName)
+				UpdatePadding();
 
 			base.OnElementPropertyChanged(sender, e);
 		}
@@ -163,6 +170,7 @@ namespace Xamarin.Forms.Platform.Android.AppCompat
 			UpdateEnabled();
 			UpdateBackgroundColor();
 			UpdateDrawable();
+			UpdatePadding();
 		}
 
 		void UpdateDrawable()
@@ -195,7 +203,7 @@ namespace Xamarin.Forms.Platform.Android.AppCompat
 				// to handle the vertical centering 
 
 				// Clear any previous padding and set the image as top/center
-				Control.SetPadding(0, 0, 0, 0);
+				UpdateContentEdge();
 				Control.SetCompoundDrawablesWithIntrinsicBounds(null, image, null, null);
 
 				// Keep track of the image height so we can use it in OnLayout
@@ -277,6 +285,22 @@ namespace Xamarin.Forms.Platform.Android.AppCompat
 			_textColorSwitcher?.UpdateTextColor(Control, Element.TextColor);
 		}
 
+		void UpdatePadding()
+		{
+			Control?.SetPadding(
+				(int)(Context.ToPixels(Element.Padding.Left) + _paddingDeltaPix.Left),
+				(int)(Context.ToPixels(Element.Padding.Top) + _paddingDeltaPix.Top),
+				(int)(Context.ToPixels(Element.Padding.Right) + _paddingDeltaPix.Right),
+				(int)(Context.ToPixels(Element.Padding.Bottom) + _paddingDeltaPix.Bottom)
+			);
+		}
+
+		void UpdateContentEdge (Thickness? delta = null)
+		{
+			_paddingDeltaPix = delta ?? new Thickness ();
+			UpdatePadding();
+		}
+
 		class ButtonClickListener : Object, AView.IOnClickListener
 		{
 			#region Statics

Xamarin.Forms.Platform.Android/FastRenderers/ButtonRenderer.cs

@@ -27,6 +27,7 @@ namespace Xamarin.Forms.Platform.Android.FastRenderers
 		readonly EffectControlProvider _effectControlProvider;
 		VisualElementTracker _tracker;
 		ButtonBackgroundTracker _backgroundTracker;
+		Thickness _paddingDeltaPix = new Thickness();
 
 		public event EventHandler<VisualElementChangedEventArgs> ElementChanged;
 		public event EventHandler<PropertyChangedEventArgs> ElementPropertyChanged;
@@ -245,6 +246,7 @@ namespace Xamarin.Forms.Platform.Android.FastRenderers
 				UpdateInputTransparent();
 				UpdateBackgroundColor();
 				UpdateDrawable();
+				UpdatePadding();
 
 				ElevationHelper.SetElevation(this, e.NewElement);
 			}
@@ -282,6 +284,10 @@ namespace Xamarin.Forms.Platform.Android.FastRenderers
 			{
 				UpdateInputTransparent();
 			}
+			else if (e.PropertyName == Button.PaddingProperty.PropertyName)
+			{
+				UpdatePadding();
+			}
 
 			ElementPropertyChanged?.Invoke(this, e);
 		}
@@ -297,9 +303,13 @@ namespace Xamarin.Forms.Platform.Android.FastRenderers
 			{
 				// We've got an image (and no text); it's already centered horizontally,
 				// we just need to adjust the padding so it centers vertically
-				int diff = (b - t - _imageHeight) / 2;
+				var diff = ((b - Context.ToPixels(Button.Padding.Bottom + Button.Padding.Top)) - t - _imageHeight) / 2;
 				diff = Math.Max(diff, 0);
-				SetPadding(0, diff, 0, -diff);
+				UpdateContentEdge(new Thickness(0, diff, 0, -diff));
+			}
+			else
+			{
+				UpdateContentEdge();
 			}
 
 			base.OnLayout(changed, l, t, r, b);
@@ -362,7 +372,7 @@ namespace Xamarin.Forms.Platform.Android.FastRenderers
 				// to handle the vertical centering 
 
 				// Clear any previous padding and set the image as top/center
-				SetPadding(0, 0, 0, 0);
+				UpdateContentEdge();
 				SetCompoundDrawablesWithIntrinsicBounds(null, image, null, null);
 
 				// Keep track of the image height so we can use it in OnLayout
@@ -475,6 +485,22 @@ namespace Xamarin.Forms.Platform.Android.FastRenderers
 			_textColorSwitcher.Value.UpdateTextColor(this, Button.TextColor);
 		}
 
+		void UpdatePadding ()
+		{
+			SetPadding(
+				(int)(Context.ToPixels(Button.Padding.Left) + _paddingDeltaPix.Left),
+				(int)(Context.ToPixels(Button.Padding.Top) + _paddingDeltaPix.Top),
+				(int)(Context.ToPixels(Button.Padding.Right) + _paddingDeltaPix.Right),
+				(int)(Context.ToPixels(Button.Padding.Bottom) + _paddingDeltaPix.Bottom)
+			);
+		}
+
+		void UpdateContentEdge (Thickness? delta = null)
+		{
+			_paddingDeltaPix = delta ?? new Thickness ();
+			UpdatePadding ();
+		}
+
 		void UpdateDrawable()
 		{
 			_backgroundTracker?.UpdateDrawable();

Xamarin.Forms.Platform.Android/Renderers/ButtonRenderer.cs

@@ -21,6 +21,7 @@ namespace Xamarin.Forms.Platform.Android
 		Typeface _defaultTypeface;
 		bool _isDisposed;
 		int _imageHeight = -1;
+		Thickness _paddingDeltaPix = new Thickness();
 
 		public ButtonRenderer(Context context) : base(context)
 		{
@@ -59,9 +60,13 @@ namespace Xamarin.Forms.Platform.Android
 			{
 				// 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;
+				var diff = ((b - Context.ToPixels(Element.Padding.Bottom + Element.Padding.Top)) - t - _imageHeight) / 2;
 				diff = Math.Max(diff, 0);
-				Control?.SetPadding(0, diff, 0, -diff);
+				UpdateContentEdge(new Thickness(0, diff, 0, -diff));
+			}
+			else
+			{
+				UpdateContentEdge();
 			}
 
 			base.OnLayout(changed, l, t, r, b);
@@ -131,6 +136,8 @@ namespace Xamarin.Forms.Platform.Android
 				UpdateBitmap();
 			else if (e.PropertyName == VisualElement.IsVisibleProperty.PropertyName)
 				UpdateText();
+			else if (e.PropertyName == Button.PaddingProperty.PropertyName)
+				UpdatePadding();
 
 			base.OnElementPropertyChanged(sender, e);
 		}
@@ -151,6 +158,7 @@ namespace Xamarin.Forms.Platform.Android
 			UpdateTextColor();
 			UpdateEnabled();
 			UpdateDrawable();
+			UpdatePadding();
 		}
 
 		void UpdateBitmap()
@@ -175,7 +183,7 @@ namespace Xamarin.Forms.Platform.Android
 				// to handle the vertical centering 
 
 				// Clear any previous padding and set the image as top/center
-				Control.SetPadding(0, 0, 0, 0);
+				UpdateContentEdge();
 				Control.SetCompoundDrawablesWithIntrinsicBounds(null, image, null, null);
 
 				// Keep track of the image height so we can use it in OnLayout
@@ -260,6 +268,22 @@ namespace Xamarin.Forms.Platform.Android
 			_textColorSwitcher?.UpdateTextColor(Control, Element.TextColor);
 		}
 
+		void UpdatePadding()
+		{
+			Control?.SetPadding (
+				(int)(Context.ToPixels(Element.Padding.Left) + _paddingDeltaPix.Left),
+				(int)(Context.ToPixels(Element.Padding.Top) + _paddingDeltaPix.Top),
+				(int)(Context.ToPixels(Element.Padding.Right) + _paddingDeltaPix.Right),
+				(int)(Context.ToPixels(Element.Padding.Bottom) + _paddingDeltaPix.Bottom)
+			);
+		}
+
+		void UpdateContentEdge (Thickness? delta = null)
+		{
+			_paddingDeltaPix = delta ?? new Thickness ();
+			UpdatePadding();
+		}
+
 		class ButtonClickListener : Object, IOnClickListener
 		{
 			public static readonly Lazy<ButtonClickListener> Instance = new Lazy<ButtonClickListener>(() => new ButtonClickListener());

Xamarin.Forms.Platform.UAP/ButtonRenderer.cs

@@ -55,6 +55,9 @@ namespace Xamarin.Forms.Platform.UWP
 				if (Element.IsSet(Button.CornerRadiusProperty) && Element.CornerRadius != (int)Button.CornerRadiusProperty.DefaultValue)
 					UpdateBorderRadius();
 
+				if (Element.IsSet(Button.PaddingProperty) && Element.Padding != (Thickness)Button.PaddingProperty.DefaultValue)
+					UpdatePadding();
+
 				UpdateFont();
 			}
 		}
@@ -104,6 +107,10 @@ namespace Xamarin.Forms.Platform.UWP
 			{
 				UpdateBorderRadius();
 			}
+			else if (e.PropertyName == Button.PaddingProperty.PropertyName)
+			{
+				UpdatePadding();
+			}
 		}
 
 		protected override void UpdateBackgroundColor()
@@ -250,5 +257,15 @@ namespace Xamarin.Forms.Platform.UWP
 		{
 			Control.Foreground = Element.TextColor != Color.Default ? Element.TextColor.ToBrush() : (Brush)Windows.UI.Xaml.Application.Current.Resources["DefaultTextForegroundThemeBrush"];
 		}
+
+		void UpdatePadding()
+		{
+			Control.Padding = new WThickness(
+				Element.Padding.Left,
+				Element.Padding.Top,
+				Element.Padding.Right,
+				Element.Padding.Bottom
+			);
+		}
 	}
 }

Xamarin.Forms.Platform.iOS/Renderers/ButtonRenderer.cs

@@ -17,6 +17,7 @@ namespace Xamarin.Forms.Platform.iOS
 		bool _useLegacyColorManagement;
 		bool _titleChanged;
 		SizeF _titleSize;
+		UIEdgeInsets _paddingDelta = new UIEdgeInsets();
 
 		// This looks like it should be a const under iOS Classic,
 		// but that doesn't work under iOS 
@@ -80,6 +81,7 @@ namespace Xamarin.Forms.Platform.iOS
 				UpdateBorder();
 				UpdateImage();
 				UpdateTextColor();
+				UpdatePadding();
 			}
 		}
 
@@ -102,6 +104,8 @@ namespace Xamarin.Forms.Platform.iOS
 				UpdateBorder();
 			else if (e.PropertyName == Button.ImageProperty.PropertyName)
 				UpdateImage();
+			else if (e.PropertyName == Button.PaddingProperty.PropertyName)
+				UpdatePadding();
 		}
     
 		protected override void SetAccessibilityLabel()
@@ -227,6 +231,25 @@ namespace Xamarin.Forms.Platform.iOS
 			}
 		}
 
+		void UpdatePadding(UIButton button = null)
+		{
+			var uiElement = button ?? Control;
+			if (uiElement == null)
+				return;
+			uiElement.ContentEdgeInsets = new UIEdgeInsets(
+				(float)(Element.Padding.Top + _paddingDelta.Top),
+				(float)(Element.Padding.Left + _paddingDelta.Left),
+				(float)(Element.Padding.Bottom + _paddingDelta.Bottom),
+				(float)(Element.Padding.Right + _paddingDelta.Right)
+			);
+		}
+
+		void UpdateContentEdge(UIButton button, UIEdgeInsets? delta = null)
+		{
+			_paddingDelta = delta ?? new UIEdgeInsets ();
+			UpdatePadding(button);
+		}
+
 		void ClearEdgeInsets(UIButton button)
 		{
 			if (button == null)
@@ -234,7 +257,7 @@ namespace Xamarin.Forms.Platform.iOS
 
 			Control.ImageEdgeInsets = new UIEdgeInsets(0, 0, 0, 0);
 			Control.TitleEdgeInsets = new UIEdgeInsets(0, 0, 0, 0);
-			Control.ContentEdgeInsets = new UIEdgeInsets(0, 0, 0, 0);
+			UpdateContentEdge (Control);
 		}
 
 		void ComputeEdgeInsets(UIButton button, Button.ButtonContentLayout layout)
@@ -249,7 +272,7 @@ namespace Xamarin.Forms.Platform.iOS
 			{
 				button.ImageEdgeInsets = new UIEdgeInsets(0, -spacing, 0, spacing);
 				button.TitleEdgeInsets = new UIEdgeInsets(0, spacing, 0, -spacing);
-				button.ContentEdgeInsets = new UIEdgeInsets(0, 2 * spacing, 0, 2 * spacing);
+				UpdateContentEdge (button, new UIEdgeInsets(0, 2 * spacing, 0, 2 * spacing));
 				return;
 			}
 
@@ -268,7 +291,7 @@ namespace Xamarin.Forms.Platform.iOS
 			{
 				button.ImageEdgeInsets = new UIEdgeInsets(0, labelWidth + spacing, 0, -labelWidth - spacing);
 				button.TitleEdgeInsets = new UIEdgeInsets(0, -imageWidth - spacing, 0, imageWidth + spacing);
-				button.ContentEdgeInsets = new UIEdgeInsets(0, 2 * spacing, 0, 2 * spacing);
+				UpdateContentEdge (button, new UIEdgeInsets(0, 2 * spacing, 0, 2 * spacing));
 				return;
 			}
 
@@ -277,7 +300,7 @@ namespace Xamarin.Forms.Platform.iOS
 
 			var edgeOffset = (float)Math.Min(imageVertOffset, titleVertOffset);
 
-			button.ContentEdgeInsets = new UIEdgeInsets(edgeOffset, 0, edgeOffset, 0);
+			UpdateContentEdge (button, new UIEdgeInsets (edgeOffset, 0, edgeOffset, 0));
 
 			var horizontalImageOffset = labelWidth / 2;
 			var horizontalTitleOffset = imageWidth / 2;