Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Have Frame use MinimumHeight/Width for minimums instead of constraints #13336

Merged
merged 9 commits into from
Feb 17, 2023
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;

Expand All @@ -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)
{
}
Expand Down Expand Up @@ -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)
{
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we have a way to test this also?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's a test in this PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unless you mean the null scenario?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah when would it get Size zero?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If someone disconnect the handler in a non-UI thread, or something weird like that. The point is that if the VirtualView is null, then having any size at all doesn't make sense.

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));
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,8 @@
using System.Threading.Tasks;
using Java.Lang;
using Microsoft.Maui.Controls;
using Xunit;
using Xunit.Sdk;

namespace Microsoft.Maui.DeviceTests
{
Expand All @@ -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);
}
}
}
112 changes: 88 additions & 24 deletions src/Controls/tests/DeviceTests/Elements/Frame/FrameTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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;

})
);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
using Microsoft.Maui.Hosting;
using Microsoft.Maui.Media;
using Xunit;
using Xunit.Sdk;

namespace Microsoft.Maui.DeviceTests
{
Expand Down Expand Up @@ -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()
{
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down