From ddcb71fd9f95f4e1dbf5788cce7e33059d44ee8f Mon Sep 17 00:00:00 2001 From: "E.Z. Hart" Date: Tue, 14 Feb 2023 16:21:52 -0700 Subject: [PATCH 1/8] Have Frame use MinimumHeight/Width for minimums instead of constraints Fixes #13074 --- .../Handlers/Android/FrameRenderer.cs | 28 +++-- .../DeviceTests/Elements/Frame/FrameTests.cs | 112 ++++++++++++++---- .../HandlerTests/HandlerTestBaseOfT.Tests.cs | 7 ++ 3 files changed, 115 insertions(+), 32 deletions(-) diff --git a/src/Controls/src/Core/Compatibility/Handlers/Android/FrameRenderer.cs b/src/Controls/src/Core/Compatibility/Handlers/Android/FrameRenderer.cs index 00575d85f19d..87627bb34ad6 100644 --- a/src/Controls/src/Core/Compatibility/Handlers/Android/FrameRenderer.cs +++ b/src/Controls/src/Core/Compatibility/Handlers/Android/FrameRenderer.cs @@ -6,6 +6,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; @@ -98,17 +99,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 == 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 = 20; // Legacy minimum value + } + + if (!Primitives.Dimension.IsExplicitSet(minHeight)) + { + minHeight = 20; // Legacy minimum value + } - return VisualElementRenderer.GetDesiredSize(this, widthMeasureSpec, heightMeasureSpec, + return VisualElementRenderer.GetDesiredSize(this, widthConstraint, heightConstraint, new Size(minWidth, minHeight)); } diff --git a/src/Controls/tests/DeviceTests/Elements/Frame/FrameTests.cs b/src/Controls/tests/DeviceTests/Elements/Frame/FrameTests.cs index 3c2794d3ee3b..6b6d48b2fdae 100644 --- 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 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 index 8e21d4bc0fbc..861a3b7677e7 100644 --- a/src/Core/tests/DeviceTests.Shared/HandlerTests/HandlerTestBaseOfT.Tests.cs +++ b/src/Core/tests/DeviceTests.Shared/HandlerTests/HandlerTestBaseOfT.Tests.cs @@ -287,6 +287,13 @@ public async Task ReturnsNonEmptyNativeBoundingBounds(int size) { // https://github.com/dotnet/maui/issues/11020 } + + // This type name check is gross, but this project doesn't know anything about Frame/FrameStub + else if(size == 1 && view.GetType().Name.EndsWith("FrameStub")) + { + // Frames have a legacy hard-coded minimum size of 20x20 + Assert.Equal(new Size(20, 20), nativeBoundingBox.Size); + } #endif else if (view is IProgress) { From 9441f3e3753101f739628fc7599f9ebc6d48d260 Mon Sep 17 00:00:00 2001 From: GitHub Actions Autoformatter Date: Tue, 14 Feb 2023 23:30:27 +0000 Subject: [PATCH 2/8] Auto-format source code --- .../src/Core/Compatibility/Handlers/Android/FrameRenderer.cs | 4 ++-- src/Controls/tests/DeviceTests/Elements/Frame/FrameTests.cs | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Controls/src/Core/Compatibility/Handlers/Android/FrameRenderer.cs b/src/Controls/src/Core/Compatibility/Handlers/Android/FrameRenderer.cs index 87627bb34ad6..5a77c48a223c 100644 --- a/src/Controls/src/Core/Compatibility/Handlers/Android/FrameRenderer.cs +++ b/src/Controls/src/Core/Compatibility/Handlers/Android/FrameRenderer.cs @@ -103,11 +103,11 @@ Size IViewHandler.GetDesiredSize(double widthConstraint, double heightConstraint { var virtualView = (this as IViewHandler).VirtualView; if (virtualView == null) - { + { return Size.Zero; } - var minWidth = virtualView.MinimumWidth; + var minWidth = virtualView.MinimumWidth; var minHeight = virtualView.MinimumHeight; if (!Primitives.Dimension.IsExplicitSet(minWidth)) diff --git a/src/Controls/tests/DeviceTests/Elements/Frame/FrameTests.cs b/src/Controls/tests/DeviceTests/Elements/Frame/FrameTests.cs index 6b6d48b2fdae..7a4a68e9c7c4 100644 --- a/src/Controls/tests/DeviceTests/Elements/Frame/FrameTests.cs +++ b/src/Controls/tests/DeviceTests/Elements/Frame/FrameTests.cs @@ -196,7 +196,7 @@ public async Task FrameRespectsMinimums() } [Fact] - public async Task FrameDoesNotInterpretConstraintsAsMinimums() + public async Task FrameDoesNotInterpretConstraintsAsMinimums() { SetupBuilder(); From 6ac076143a1e14de51b4e5d5ab4196201571cce7 Mon Sep 17 00:00:00 2001 From: "E.Z. Hart" Date: Wed, 15 Feb 2023 10:00:20 -0700 Subject: [PATCH 3/8] Adjust test for other screen sizes/densities --- .../HandlerTests/HandlerTestBaseOfT.Tests.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Core/tests/DeviceTests.Shared/HandlerTests/HandlerTestBaseOfT.Tests.cs b/src/Core/tests/DeviceTests.Shared/HandlerTests/HandlerTestBaseOfT.Tests.cs index 861a3b7677e7..1488bbe57d18 100644 --- a/src/Core/tests/DeviceTests.Shared/HandlerTests/HandlerTestBaseOfT.Tests.cs +++ b/src/Core/tests/DeviceTests.Shared/HandlerTests/HandlerTestBaseOfT.Tests.cs @@ -292,7 +292,8 @@ public async Task ReturnsNonEmptyNativeBoundingBounds(int size) else if(size == 1 && view.GetType().Name.EndsWith("FrameStub")) { // Frames have a legacy hard-coded minimum size of 20x20 - Assert.Equal(new Size(20, 20), nativeBoundingBox.Size); + Assert.True(CloseEnough(20, nativeBoundingBox.Size.Width)); + Assert.True(CloseEnough(20, nativeBoundingBox.Size.Height)); } #endif else if (view is IProgress) From 40ea114e97477d018c901d596dc03013b84df86b Mon Sep 17 00:00:00 2001 From: "E.Z. Hart" Date: Wed, 15 Feb 2023 10:40:36 -0700 Subject: [PATCH 4/8] Update src/Controls/src/Core/Compatibility/Handlers/Android/FrameRenderer.cs Co-authored-by: Manuel de la Pena --- .../src/Core/Compatibility/Handlers/Android/FrameRenderer.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Controls/src/Core/Compatibility/Handlers/Android/FrameRenderer.cs b/src/Controls/src/Core/Compatibility/Handlers/Android/FrameRenderer.cs index 5a77c48a223c..465986e764ac 100644 --- a/src/Controls/src/Core/Compatibility/Handlers/Android/FrameRenderer.cs +++ b/src/Controls/src/Core/Compatibility/Handlers/Android/FrameRenderer.cs @@ -101,8 +101,8 @@ protected Frame? Element Size IViewHandler.GetDesiredSize(double widthConstraint, double heightConstraint) { - var virtualView = (this as IViewHandler).VirtualView; - if (virtualView == null) + var virtualView = (this as IViewHandler)?.VirtualView; + if (virtualView is null) { return Size.Zero; } From 93163cdcfc8821d63291698dcfe52609c2cab95d Mon Sep 17 00:00:00 2001 From: "E.Z. Hart" Date: Thu, 16 Feb 2023 13:07:29 -0700 Subject: [PATCH 5/8] Make legacy min Frame size a constant --- .../Core/Compatibility/Handlers/Android/FrameRenderer.cs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/Controls/src/Core/Compatibility/Handlers/Android/FrameRenderer.cs b/src/Controls/src/Core/Compatibility/Handlers/Android/FrameRenderer.cs index 465986e764ac..5bb854079199 100644 --- a/src/Controls/src/Core/Compatibility/Handlers/Android/FrameRenderer.cs +++ b/src/Controls/src/Core/Compatibility/Handlers/Android/FrameRenderer.cs @@ -55,7 +55,6 @@ public static IPropertyMapper Mapper public static CommandMapper CommandMapper = new CommandMapper(ViewRenderer.VisualElementRendererCommandMapper); - float _defaultElevation = -1f; float _defaultCornerRadius = -1f; @@ -71,6 +70,8 @@ public static CommandMapper CommandMapper public event EventHandler? ElementChanged; public event EventHandler? ElementPropertyChanged; + const double LegacyMinimumFrameSize = 20; + public FrameRenderer(Context context) : this(context, Mapper) { } @@ -112,12 +113,12 @@ Size IViewHandler.GetDesiredSize(double widthConstraint, double heightConstraint if (!Primitives.Dimension.IsExplicitSet(minWidth)) { - minWidth = 20; // Legacy minimum value + minWidth = LegacyMinimumFrameSize; } if (!Primitives.Dimension.IsExplicitSet(minHeight)) { - minHeight = 20; // Legacy minimum value + minHeight = LegacyMinimumFrameSize; } return VisualElementRenderer.GetDesiredSize(this, widthConstraint, heightConstraint, From 2b4648bbc2da0637361d8adfc632eff03160a0d4 Mon Sep 17 00:00:00 2001 From: GitHub Actions Autoformatter Date: Thu, 16 Feb 2023 20:12:52 +0000 Subject: [PATCH 6/8] Auto-format source code --- .../src/Core/Compatibility/Handlers/Android/FrameRenderer.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Controls/src/Core/Compatibility/Handlers/Android/FrameRenderer.cs b/src/Controls/src/Core/Compatibility/Handlers/Android/FrameRenderer.cs index 5bb854079199..71efeca20f26 100644 --- a/src/Controls/src/Core/Compatibility/Handlers/Android/FrameRenderer.cs +++ b/src/Controls/src/Core/Compatibility/Handlers/Android/FrameRenderer.cs @@ -113,12 +113,12 @@ Size IViewHandler.GetDesiredSize(double widthConstraint, double heightConstraint if (!Primitives.Dimension.IsExplicitSet(minWidth)) { - minWidth = LegacyMinimumFrameSize; + minWidth = LegacyMinimumFrameSize; } if (!Primitives.Dimension.IsExplicitSet(minHeight)) { - minHeight = LegacyMinimumFrameSize; + minHeight = LegacyMinimumFrameSize; } return VisualElementRenderer.GetDesiredSize(this, widthConstraint, heightConstraint, From 9e8676c5e16274f7156733dba5d8f19d8224f018 Mon Sep 17 00:00:00 2001 From: "E.Z. Hart" Date: Fri, 17 Feb 2023 12:05:53 -0700 Subject: [PATCH 7/8] Make Frame test less awkward --- .../Frame/FrameHandlerTest.Android.cs | 22 ++++++++++++ .../HandlerTests/HandlerTestBaseOfT.Tests.cs | 36 +++++++++---------- 2 files changed, 40 insertions(+), 18 deletions(-) diff --git a/src/Controls/tests/DeviceTests/Elements/Frame/FrameHandlerTest.Android.cs b/src/Controls/tests/DeviceTests/Elements/Frame/FrameHandlerTest.Android.cs index 587891f74cf4..c5aa9938438f 100644 --- 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 Xunit.Sdk; +using Microsoft.Maui.Controls; +using Xunit; +using Java.Lang; 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/Core/tests/DeviceTests.Shared/HandlerTests/HandlerTestBaseOfT.Tests.cs b/src/Core/tests/DeviceTests.Shared/HandlerTests/HandlerTestBaseOfT.Tests.cs index 1488bbe57d18..2bd0060b9b3f 100644 --- 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,7 +245,7 @@ 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 @@ -252,7 +253,7 @@ public async Task ReturnsNonEmptyPlatformViewBounds(int size) [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 @@ -287,32 +287,32 @@ public async Task ReturnsNonEmptyNativeBoundingBounds(int size) { // https://github.com/dotnet/maui/issues/11020 } - - // This type name check is gross, but this project doesn't know anything about Frame/FrameStub - else if(size == 1 && view.GetType().Name.EndsWith("FrameStub")) - { - // Frames have a legacy hard-coded minimum size of 20x20 - Assert.True(CloseEnough(20, nativeBoundingBox.Size.Width)); - Assert.True(CloseEnough(20, nativeBoundingBox.Size.Height)); - } #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 From c5e2cece13300796bba5f71ee9071c4d8472374d Mon Sep 17 00:00:00 2001 From: GitHub Actions Autoformatter Date: Fri, 17 Feb 2023 19:10:07 +0000 Subject: [PATCH 8/8] Auto-format source code --- .../DeviceTests/Elements/Frame/FrameHandlerTest.Android.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Controls/tests/DeviceTests/Elements/Frame/FrameHandlerTest.Android.cs b/src/Controls/tests/DeviceTests/Elements/Frame/FrameHandlerTest.Android.cs index c5aa9938438f..f05babfdc875 100644 --- a/src/Controls/tests/DeviceTests/Elements/Frame/FrameHandlerTest.Android.cs +++ b/src/Controls/tests/DeviceTests/Elements/Frame/FrameHandlerTest.Android.cs @@ -1,8 +1,8 @@ using System.Threading.Tasks; -using Xunit.Sdk; +using Java.Lang; using Microsoft.Maui.Controls; using Xunit; -using Java.Lang; +using Xunit.Sdk; namespace Microsoft.Maui.DeviceTests { @@ -29,7 +29,7 @@ public override Task ContainerViewRemainsIfShadowMapperRunsAgain() public override async Task ReturnsNonEmptyNativeBoundingBox(int size) { // Frames have a legacy hard-coded minimum size of 20x20 - var expectedSize = Math.Max(20, size); + var expectedSize = Math.Max(20, size); var expectedBounds = new Graphics.Rect(0, 0, expectedSize, expectedSize); var view = new Frame()