[UWP] Fixed double set margins in Layouts (#3570) fixes #3398 fixes #…

…3616

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

@@ -1,4 +1,6 @@
-using Xamarin.Forms.CustomAttributes;
+using System;
+using System.Threading;
+using Xamarin.Forms.CustomAttributes;
 using Xamarin.Forms.Internals;
 
 #if UITEST
@@ -14,17 +16,54 @@ public class Bugzilla41418 : TestContentPage
 	{
 		protected override void Init()
 		{
-			Content = new ScrollView()
+			var box = new BoxView
 			{
-				BackgroundColor = Color.Yellow,
-				Content = new BoxView
-				{
-					Margin = 100,
-					WidthRequest = 500,
-					HeightRequest = 800,
-					BackgroundColor = Color.Red
+				Margin = 100,
+				WidthRequest = 500,
+				HeightRequest = 800,
+				BackgroundColor = Color.Red
+			};
+			var description = "The red rectangle with margins is nested in the yellow rectangle. " +
+				$"This margins should be visible as yellow Indents and will change in separate thread until the test is closed.{Environment.NewLine}" +
+				"Margins = ";
+			var desc = new Label
+			{
+				BackgroundColor = Color.Azure,
+				Text = $"{description}{box.Margin.Top}"
+			};
+			Content = new StackLayout
+			{
+				Children = {
+					desc,
+					new ScrollView
+					{
+						BackgroundColor = Color.Yellow,
+						Content = box
+					}
 				}
 			};
+
+			var disappeared = false;
+
+			// change margin of box after the first rendering
+			new Thread(() => {
+				while (true)
+				{
+					for (int margin = 20; margin < 160; margin += 20)
+					{
+						Thread.Sleep(1000);
+						if (disappeared)
+							return;
+						Device.BeginInvokeOnMainThread(() =>
+						{
+							box.Margin = margin;
+							desc.Text = $"{description}{margin}";
+						});
+					}
+				};
+			}).Start();
+
+			Disappearing += (_, __) => disappeared = true;
 		}
 	}
 }

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

@@ -0,0 +1,39 @@
+using Xamarin.Forms.CustomAttributes;
+using Xamarin.Forms.Internals;
+
+namespace Xamarin.Forms.Controls.Issues
+{
+	[Preserve(AllMembers = true)]
+	[Issue(IssueTracker.Github, 3398, "Labels not always rendering in a StackLayout", PlatformAffected.UWP)]
+	public class Issue3398 : TestContentPage
+	{
+		protected override void Init()
+		{
+			Content = new StackLayout
+			{
+				Margin = new Thickness(20),
+				Children = {
+					new Label {
+						Margin = new Thickness(0, 10),
+						FontSize = 20,
+						Text = "Should be seen 2 labels. Above and below the page." ,
+						BackgroundColor = Color.OrangeRed
+					},
+					new BoxView
+					{
+						BackgroundColor = Color.Teal,
+						WidthRequest = 300,
+						HeightRequest = 300,
+						HorizontalOptions = LayoutOptions.Center,
+						VerticalOptions = LayoutOptions.CenterAndExpand
+					},
+					new Label {
+						Text = "Label 2",
+						BackgroundColor = Color.Aqua,
+						FontSize = 20
+					}
+				}
+			};
+		}
+	}
+}

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

@@ -366,6 +366,7 @@
     <Compile Include="$(MSBuildThisFileDirectory)Issue3019.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)Issue2993.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)Issue3507.cs" />
+    <Compile Include="$(MSBuildThisFileDirectory)Issue3398.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)LegacyComponents\NonAppCompatSwitch.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)MapsModalCrash.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)ModalActivityIndicatorTest.cs" />
@@ -953,4 +954,4 @@
       <Generator>MSBuild:UpdateDesignTimeXaml</Generator>
     </EmbeddedResource>
   </ItemGroup>
-</Project>
+</Project>

Xamarin.Forms.Platform.UAP/ScrollViewRenderer.cs

@@ -107,7 +107,7 @@ protected override void OnElementPropertyChanged(object sender, PropertyChangedE
 			if (e.PropertyName == "Content")
 				UpdateContent();
 			else if (e.PropertyName == Layout.PaddingProperty.PropertyName)
-				UpdateMargins();
+				UpdateContentMargins();
 			else if (e.PropertyName == ScrollView.OrientationProperty.PropertyName)
 				UpdateOrientation();
 			else if (e.PropertyName == ScrollView.VerticalScrollBarVisibilityProperty.PropertyName)
@@ -116,13 +116,25 @@ protected override void OnElementPropertyChanged(object sender, PropertyChangedE
 				UpdateHorizontalScrollBarVisibility();
 		}
 
+		protected void OnContentElementPropertyChanged(object sender, PropertyChangedEventArgs e)
+		{
+			if (e.PropertyName == View.MarginProperty.PropertyName)
+				UpdateContentMargins();
+		}
+
 		void UpdateContent()
 		{
 			if (_currentView != null)
 				_currentView.Cleanup();
 
-			if (Control?.Content is FrameworkElement frameworkElement)
-				frameworkElement.LayoutUpdated -= SetInitialRtlPosition;
+			if (Control?.Content is FrameworkElement oldElement)
+			{
+				oldElement.LayoutUpdated -= SetInitialRtlPosition;
+
+				if (oldElement is IVisualElementRenderer oldRenderer
+					&& oldRenderer.Element is View oldContentView)
+					oldContentView.PropertyChanged -= OnContentElementPropertyChanged;
+			}
 
 			_currentView = Element.Content;
 
@@ -132,7 +144,10 @@ void UpdateContent()
 
 			Control.Content = renderer != null ? renderer.ContainerElement : null;
 
-			UpdateMargins();
+			UpdateContentMargins();
+			if (renderer?.Element != null)
+				renderer.Element.PropertyChanged += OnContentElementPropertyChanged;
+
 			if (renderer?.ContainerElement != null)
 				renderer.ContainerElement.LayoutUpdated += SetInitialRtlPosition;
 		}
@@ -213,30 +228,39 @@ void OnViewChanged(object sender, ScrollViewerViewChangedEventArgs e)
 				Element.SendScrollFinished();
 		}
 
-		Windows.UI.Xaml.Thickness AddMargin(Windows.UI.Xaml.Thickness original, double left, double top, double right, double bottom)
+		Windows.UI.Xaml.Thickness AddMargin(Thickness original, double left, double top, double right, double bottom)
 		{
 			return new Windows.UI.Xaml.Thickness(original.Left + left, original.Top + top, original.Right + right, original.Bottom + bottom);
 		}
 
-		void UpdateMargins()
+		// UAP ScrollView forces Content origin to be the same as the ScrollView origin.
+		// This prevents Forms layout from emulating Padding and Margin by offsetting the origin. 
+		// So we must actually set the UAP Margin property instead of emulating it with an origin offset. 
+		// Not only that, but in UAP Padding and Margin are aliases with
+		// the former living on the parent and the latter on the child. 
+		// So that's why the UAP Margin is set to the sum of the Forms Padding and Forms Margin.
+		void UpdateContentMargins()
 		{
-			var element = Control.Content as FrameworkElement;
-			if (element == null)
+			if (!(Control.Content is FrameworkElement element
+				&& element is IVisualElementRenderer renderer
+				&& renderer.Element is View contentView))
 				return;
 
+			var margin = contentView.Margin;
+			var padding = Element.Padding;
 			switch (Element.Orientation)
 			{
 				case ScrollOrientation.Horizontal:
 					// need to add left/right margins
-					element.Margin = AddMargin(element.Margin, Element.Padding.Left, 0, Element.Padding.Right, 0);
+					element.Margin = AddMargin(margin, padding.Left, 0, padding.Right, 0);
 					break;
 				case ScrollOrientation.Vertical:
 					// need to add top/bottom margins
-					element.Margin = AddMargin(element.Margin, 0, Element.Padding.Top, 0, Element.Padding.Bottom);
+					element.Margin = AddMargin(margin, 0, padding.Top, 0, padding.Bottom);
 					break;
 				case ScrollOrientation.Both:
 					// need to add all margins
-					element.Margin = AddMargin(element.Margin, Element.Padding.Left, Element.Padding.Top, Element.Padding.Right, Element.Padding.Bottom);
+					element.Margin = AddMargin(margin, padding.Left, padding.Top, padding.Right, padding.Bottom);
 					break;
 			}
 		}

Xamarin.Forms.Platform.UAP/ViewRenderer.cs

@@ -18,7 +18,6 @@ protected override void OnElementChanged(ElementChangedEventArgs<TElement> e)
 			{
 				UpdateBackgroundColor();
 				UpdateFlowDirection();
-				UpdateMargins();
 			}
 		}
 
@@ -131,10 +130,5 @@ void UpdateFlowDirection()
 		{
 			Control.UpdateFlowDirection(Element);
 		}
-
-		void UpdateMargins()
-		{
-			Margin = new Windows.UI.Xaml.Thickness(Element.Margin.Left, Element.Margin.Top, Element.Margin.Right, Element.Margin.Bottom);
-		}
 	}
 }