Have Frame use MinimumHeight/Width for minimums instead of constraints (

dotnet#13336) * Have Frame use MinimumHeight/Width for minimums instead of constraints Fixes dotnet#13074 * Auto-format source code * Adjust test for other screen sizes/densities * Update src/Controls/src/Core/Compatibility/Handlers/Android/FrameRenderer.cs Co-authored-by: Manuel de la Pena <[email protected]> * Make legacy min Frame size a constant * Auto-format source code * Make Frame test less awkward * Auto-format source code --------- Co-authored-by: GitHub Actions Autoformatter <[email protected]> Co-authored-by: Manuel de la Pena <[email protected]>
tj-devel709 · Feb 21, 2023 · 2183145 · 2183145
1 parent 945d5ba
commit 2183145
Show file tree

Hide file tree

Showing 4 changed files with 150 additions and 43 deletions.
diff --git a/src/Controls/src/Core/Compatibility/Handlers/Android/FrameRenderer.cs b/src/Controls/src/Core/Compatibility/Handlers/Android/FrameRenderer.cs
@@ -4,6 +4,7 @@
 using Android.Graphics;
 using Android.Graphics.Drawables;
 using Android.Views;
+using Android.Views.Animations;
 using AndroidX.CardView.Widget;
 using AndroidX.Core.View;
 using Microsoft.Maui.Controls.Platform;
@@ -52,7 +53,6 @@ public static IPropertyMapper<Frame, FrameRenderer> Mapper
 		public static CommandMapper<Frame, FrameRenderer> CommandMapper
 			= new CommandMapper<Frame, FrameRenderer>(ViewRenderer.VisualElementRendererCommandMapper);
 
-
 		float _defaultElevation = -1f;
 		float _defaultCornerRadius = -1f;
 
@@ -68,6 +68,8 @@ public static CommandMapper<Frame, FrameRenderer> CommandMapper
 		public event EventHandler<VisualElementChangedEventArgs>? ElementChanged;
 		public event EventHandler<PropertyChangedEventArgs>? ElementPropertyChanged;
 
+		const double LegacyMinimumFrameSize = 20;
+
 		public FrameRenderer(Context context) : this(context, Mapper)
 		{
 		}
@@ -96,17 +98,28 @@ protected Frame? Element
 			}
 		}
 
-		Size IViewHandler.GetDesiredSize(double widthMeasureSpec, double heightMeasureSpec)
+		Size IViewHandler.GetDesiredSize(double widthConstraint, double heightConstraint)
 		{
-			double minWidth = 20;
-			if (Primitives.Dimension.IsExplicitSet(widthMeasureSpec) && !double.IsInfinity(widthMeasureSpec))
-				minWidth = widthMeasureSpec;
+			var virtualView = (this as IViewHandler)?.VirtualView;
+			if (virtualView is null)
+			{
+				return Size.Zero;
+			}
+
+			var minWidth = virtualView.MinimumWidth;
+			var minHeight = virtualView.MinimumHeight;
 
-			double minHeight = 20;
-			if (Primitives.Dimension.IsExplicitSet(widthMeasureSpec) && !double.IsInfinity(heightMeasureSpec))
-				minHeight = heightMeasureSpec;
+			if (!Primitives.Dimension.IsExplicitSet(minWidth))
+			{
+				minWidth = LegacyMinimumFrameSize;
+			}
+
+			if (!Primitives.Dimension.IsExplicitSet(minHeight))
+			{
+				minHeight = LegacyMinimumFrameSize;
+			}
 
-			return VisualElementRenderer<Frame>.GetDesiredSize(this, widthMeasureSpec, heightMeasureSpec,
+			return VisualElementRenderer<Frame>.GetDesiredSize(this, widthConstraint, heightConstraint,
 				new Size(minWidth, minHeight));
 		}
 

diff --git a/src/Controls/tests/DeviceTests/Elements/Frame/FrameHandlerTest.Android.cs b/src/Controls/tests/DeviceTests/Elements/Frame/FrameHandlerTest.Android.cs
@@ -1,4 +1,8 @@
 using System.Threading.Tasks;
+using Java.Lang;
+using Microsoft.Maui.Controls;
+using Xunit;
+using Xunit.Sdk;
 
 namespace Microsoft.Maui.DeviceTests
 {
@@ -21,5 +25,23 @@ public override Task ContainerViewRemainsIfShadowMapperRunsAgain()
 			// https://github.com/dotnet/maui/pull/12218
 			return Task.CompletedTask;
 		}
+
+		public override async Task ReturnsNonEmptyNativeBoundingBox(int size)
+		{
+			// Frames have a legacy hard-coded minimum size of 20x20
+			var expectedSize = Math.Max(20, size);
+			var expectedBounds = new Graphics.Rect(0, 0, expectedSize, expectedSize);
+
+			var view = new Frame()
+			{
+				HeightRequest = size,
+				WidthRequest = size
+			};
+
+			var nativeBoundingBox = await GetValueAsync(view, handler => GetBoundingBox(handler));
+			Assert.NotEqual(nativeBoundingBox, Graphics.Rect.Zero);
+
+			AssertWithinTolerance(expectedBounds.Size, nativeBoundingBox.Size);
+		}
 	}
 }
diff --git a/src/Controls/tests/DeviceTests/Elements/Frame/FrameTests.cs b/src/Controls/tests/DeviceTests/Elements/Frame/FrameTests.cs
@@ -91,30 +91,7 @@ public async Task FrameWithEntryMeasuresCorrectly()
 				}
 			};
 
-			var layoutFrame =
-				await InvokeOnMainThreadAsync(() =>
-					layout.ToPlatform(MauiContext).AttachAndRun(async () =>
-					{
-						var size = (layout as IView).Measure(double.PositiveInfinity, double.PositiveInfinity);
-						(layout as IView).Arrange(new Graphics.Rect(0, 0, size.Width, size.Height));
-
-						await OnFrameSetToNotEmpty(layout);
-						await OnFrameSetToNotEmpty(frame);
-
-						// verify that the PlatformView was measured
-						var frameControlSize = (frame.Handler as IPlatformViewHandler).PlatformView.GetBoundingBox();
-						Assert.True(frameControlSize.Width > 0);
-						Assert.True(frameControlSize.Width > 0);
-
-						// if the control sits inside a container make sure that also measured
-						var containerControlSize = frame.ToPlatform().GetBoundingBox();
-						Assert.True(frameControlSize.Width > 0);
-						Assert.True(frameControlSize.Width > 0);
-
-						return layout.Frame;
-
-					})
-				);
+			var layoutFrame = await LayoutFrame(layout, frame, double.PositiveInfinity, double.PositiveInfinity);
 
 			Assert.True(entry.Width > 0);
 			Assert.True(entry.Height > 0);
@@ -187,5 +164,92 @@ await InvokeOnMainThreadAsync(() =>
 				platformView.AssertContainsColor(expectedColor);
 			});
 		}
+
+		[Fact(DisplayName = "Frame Respects minimum height/width")]
+		public async Task FrameRespectsMinimums()
+		{
+			SetupBuilder();
+
+			var content = new Button { Text = "Hey", WidthRequest = 50, HeightRequest = 50 };
+
+			var frame = new Frame()
+			{
+				Content = content,
+				MinimumHeightRequest = 100,
+				MinimumWidthRequest = 100,
+				VerticalOptions = LayoutOptions.Start,
+				HorizontalOptions = LayoutOptions.Start
+			};
+
+			var layout = new StackLayout()
+			{
+				Children =
+				{
+					frame
+				}
+			};
+
+			var layoutFrame = await LayoutFrame(layout, frame, 500, 500);
+
+			Assert.True(100 <= layoutFrame.Height);
+			Assert.True(100 <= layoutFrame.Width);
+		}
+
+		[Fact]
+		public async Task FrameDoesNotInterpretConstraintsAsMinimums()
+		{
+			SetupBuilder();
+
+			var content = new Button { Text = "Hey", WidthRequest = 50, HeightRequest = 50 };
+
+			var frame = new Frame()
+			{
+				Content = content,
+				MinimumHeightRequest = 100,
+				MinimumWidthRequest = 100,
+				VerticalOptions = LayoutOptions.Start,
+				HorizontalOptions = LayoutOptions.Start
+			};
+
+			var layout = new StackLayout()
+			{
+				Children =
+				{
+					frame
+				}
+			};
+
+			var layoutFrame = await LayoutFrame(layout, frame, 500, 500);
+
+			Assert.True(500 > layoutFrame.Width);
+			Assert.True(500 > layoutFrame.Height);
+		}
+
+		async Task<Rect> LayoutFrame(Layout layout, Frame frame, double measureWidth, double measureHeight)
+		{
+			return await InvokeOnMainThreadAsync(() =>
+					layout.ToPlatform(MauiContext).AttachAndRun(async () =>
+					{
+						var size = (layout as IView).Measure(measureWidth, measureHeight);
+						(layout as IView).Arrange(new Graphics.Rect(0, 0, size.Width, size.Height));
+
+						await OnFrameSetToNotEmpty(layout);
+						await OnFrameSetToNotEmpty(frame);
+
+						// verify that the PlatformView was measured
+						var frameControlSize = (frame.Handler as IPlatformViewHandler).PlatformView.GetBoundingBox();
+						Assert.True(frameControlSize.Width > 0);
+						Assert.True(frameControlSize.Width > 0);
+
+						// if the control sits inside a container make sure that also measured
+						var containerControlSize = frame.ToPlatform().GetBoundingBox();
+						Assert.True(frameControlSize.Width > 0);
+						Assert.True(frameControlSize.Width > 0);
+
+						return layout.Frame;
+
+					})
+				);
+		}
 	}
 }
diff --git a/src/Core/tests/DeviceTests.Shared/HandlerTests/HandlerTestBaseOfT.Tests.cs b/src/Core/tests/DeviceTests.Shared/HandlerTests/HandlerTestBaseOfT.Tests.cs
@@ -8,6 +8,7 @@
 using Microsoft.Maui.Hosting;
 using Microsoft.Maui.Media;
 using Xunit;
+using Xunit.Sdk;
 
 namespace Microsoft.Maui.DeviceTests
 {
@@ -244,15 +245,15 @@ public async Task ReturnsNonEmptyPlatformViewBounds(int size)
 			Assert.NotEqual(platformViewBounds, new Graphics.Rect());
 		}
 
-		[Theory(DisplayName = "Native View Bounding Box are not empty"
+		[Theory(DisplayName = "Native View Bounding Box is not empty"
 #if WINDOWS
 			, Skip = "https://github.com/dotnet/maui/issues/9054"
 #endif
 		)]
 		[InlineData(1)]
 		[InlineData(100)]
 		[InlineData(1000)]
-		public async Task ReturnsNonEmptyNativeBoundingBounds(int size)
+		public virtual async Task ReturnsNonEmptyNativeBoundingBox(int size)
 		{
 			var view = new TStub()
 			{
@@ -261,8 +262,7 @@ public async Task ReturnsNonEmptyNativeBoundingBounds(int size)
 			};
 
 			var nativeBoundingBox = await GetValueAsync(view, handler => GetBoundingBox(handler));
-			Assert.NotEqual(nativeBoundingBox, new Graphics.Rect());
-
+			Assert.NotEqual(nativeBoundingBox, Graphics.Rect.Zero);
 
 			// Currently there's an issue with label/progress where they don't set the frame size to
 			// the explicit Width and Height values set
@@ -290,21 +290,29 @@ public async Task ReturnsNonEmptyNativeBoundingBounds(int size)
 #endif
 			else if (view is IProgress)
 			{
-				if (!CloseEnough(size, nativeBoundingBox.Size.Width))
-					Assert.Equal(new Size(size, size), nativeBoundingBox.Size);
+				AssertWithinTolerance(size, nativeBoundingBox.Size.Width);
 			}
 			else
 			{
-				if (!CloseEnough(size, nativeBoundingBox.Size.Height) || !CloseEnough(size, nativeBoundingBox.Size.Width))
-					Assert.Equal(new Size(size, size), nativeBoundingBox.Size);
+				var expectedSize = new Size(size, size);
+				AssertWithinTolerance(expectedSize, nativeBoundingBox.Size);
 			}
+		}
 
-			bool CloseEnough(double value1, double value2)
+		protected void AssertWithinTolerance(double expected, double actual, double tolerance = 0.2, string message = "Value was not within tolerance.") 
+		{
+			var diff = System.Math.Abs(expected - actual);
+			if (diff > tolerance)
 			{
-				return System.Math.Abs(value2 - value1) < 0.2;
+				throw new XunitException($"{message} Expected: {expected}; Actual: {actual}; Tolerance {tolerance}");
 			}
 		}
 
+		protected void AssertWithinTolerance(Graphics.Size expected, Graphics.Size actual, double tolerance = 0.2) 
+		{
+			AssertWithinTolerance(expected.Height, actual.Height, tolerance, "Height was not within tolerance.");
+			AssertWithinTolerance(expected.Width, actual.Width, tolerance, "Width was not within tolerance.");
+		}
 
 		[Theory(DisplayName = "Native View Transforms are not empty"
 #if IOS