[Android] Don't mess with the Button.Padding if the Padding prope…

…rty isn't set (xamarin#3174) * [Android] Don't mess with the Button.Padding if the Padding property isn't set * Update 1720 test case to clear padding value * [Android] Can clear Padding value on Button
Greenliff · Jun 28, 2018 · 3090843 · 3090843
1 parent 317f153
commit 3090843
Show file tree

Hide file tree

Showing 2 changed files with 34 additions and 12 deletions.
diff --git a/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/GitHub1702.cs b/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/GitHub1702.cs
@@ -81,7 +81,7 @@ Grid ActionGrid(List<Button> buttons)
 				Text = "Reset padding",
 				Command = new Command(() =>
 				{
-					buttons.ForEach(b => b.Padding = new Thickness(0, 0, 0, 0));
+					buttons.ForEach(b => b.ClearValue(Button.PaddingProperty));
 				})
 			}, 0, 1);
 			actionGrid.AddChild(new Button()

diff --git a/Xamarin.Forms.Platform.Android/AppCompat/ButtonRenderer.cs b/Xamarin.Forms.Platform.Android/AppCompat/ButtonRenderer.cs
@@ -15,11 +15,12 @@
 
 namespace Xamarin.Forms.Platform.Android.AppCompat
 {
-    public class ButtonRenderer : ViewRenderer<Button, AppCompatButton>, AView.IOnAttachStateChangeListener
+	public class ButtonRenderer : ViewRenderer<Button, AppCompatButton>, AView.IOnAttachStateChangeListener
 	{
 		ButtonBackgroundTracker _backgroundTracker;
 		TextColorSwitcher _textColorSwitcher;
 		float _defaultFontSize;
+		Thickness? _defaultPadding;
 		Typeface _defaultTypeface;
 		bool _isDisposed;
 		int _imageHeight = -1;
@@ -31,7 +32,7 @@ public ButtonRenderer(Context context) : base(context)
 		}
 
 		[Obsolete("This constructor is obsolete as of version 2.5. Please use ButtonRenderer(Context) instead.")]
-		public ButtonRenderer() 
+		public ButtonRenderer()
 		{
 			AutoPackage = false;
 		}
@@ -118,7 +119,7 @@ protected override void OnElementChanged(ElementChangedEventArgs<Button> e)
 					button.Tag = this;
 
 					var useLegacyColorManagement = e.NewElement.UseLegacyColorManagement();
-					_textColorSwitcher = new TextColorSwitcher(button.TextColors, useLegacyColorManagement);  
+					_textColorSwitcher = new TextColorSwitcher(button.TextColors, useLegacyColorManagement);
 
 					SetNativeControl(button);
 					button.AddOnAttachStateChangeListener(this);
@@ -129,6 +130,9 @@ protected override void OnElementChanged(ElementChangedEventArgs<Button> e)
 				else
 					_backgroundTracker.Button = e.NewElement;
 
+				_defaultFontSize = 0f;
+				_defaultPadding = null;
+
 				UpdateAll();
 			}
 		}
@@ -287,17 +291,35 @@ void UpdateTextColor()
 
 		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)
-			);
+			if (Control == null)
+				return;
+
+			if (!_defaultPadding.HasValue)
+				_defaultPadding = new Thickness(Control.PaddingLeft, Control.PaddingTop, Control.PaddingRight, Control.PaddingBottom);
+
+			if (Element.IsSet(Button.PaddingProperty))
+			{
+				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)
+				);
+			}
+			else
+			{
+				Control.SetPadding(
+						(int)_defaultPadding.Value.Left,
+						(int)_defaultPadding.Value.Top,
+						(int)_defaultPadding.Value.Right,
+						(int)_defaultPadding.Value.Bottom
+					);
+			}
 		}
 
-		void UpdateContentEdge (Thickness? delta = null)
+		void UpdateContentEdge(Thickness? delta = null)
 		{
-			_paddingDeltaPix = delta ?? new Thickness ();
+			_paddingDeltaPix = delta ?? new Thickness();
 			UpdatePadding();
 		}