From e0b0008996408a305632e6e1d2310b37b20dae6a Mon Sep 17 00:00:00 2001 From: Rachel Kang Date: Tue, 17 May 2022 11:26:55 -0400 Subject: [PATCH 1/7] Add AbsoluteLayoutManager RTL tests --- .../Layouts/AbsoluteLayoutManagerTests.cs | 48 +++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/src/Core/tests/UnitTests/Layouts/AbsoluteLayoutManagerTests.cs b/src/Core/tests/UnitTests/Layouts/AbsoluteLayoutManagerTests.cs index 88ff4a176b47..731994d278b6 100644 --- a/src/Core/tests/UnitTests/Layouts/AbsoluteLayoutManagerTests.cs +++ b/src/Core/tests/UnitTests/Layouts/AbsoluteLayoutManagerTests.cs @@ -445,5 +445,53 @@ public void ChildMeasureRespectsProportionalBounds() child.Received().Measure(Arg.Is(expectedWidth * widthConstraint), Arg.Is(expectedHeight * heightConstraint)); } + + [Fact(DisplayName = "First View in LTR Absolute Layout is on the left")] + public void LtrShouldHaveFirstItemOnTheLeft() + { + var abs = CreateTestLayout(); + var child = CreateTestView(); + SubstituteChildren(abs, child); + var childBounds = new Rect(0, 0, 100, 100); + SetLayoutBounds(abs, child, childBounds); + + abs.FlowDirection.Returns(FlowDirection.LeftToRight); + + var manager = new AbsoluteLayoutManager(abs); + var measuredSize = manager.Measure(double.PositiveInfinity, double.PositiveInfinity); + manager.ArrangeChildren(new Rect(Point.Zero, measuredSize)); + + // We expect that the starting view (0) should be arranged on the left, + // and the next rectangle (1) should be on the right + var expectedRectangle0 = new Rect(0, 0, 100, 100); + var expectedRectangle1 = new Rect(100, 0, 100, 100); + + abs[0].Received().Arrange(Arg.Is(expectedRectangle0)); + abs[1].Received().Arrange(Arg.Is(expectedRectangle1)); + } + + [Fact(DisplayName = "First View in RTL Absolute Layout is on the right")] + public void RtlShouldHaveFirstItemOnTheRight() + { + var abs = CreateTestLayout(); + var child = CreateTestView(); + SubstituteChildren(abs, child); + var childBounds = new Rect(0, 0, 100, 100); + SetLayoutBounds(abs, child, childBounds); + + abs.FlowDirection.Returns(FlowDirection.LeftToRight); + + var manager = new AbsoluteLayoutManager(abs); + var measuredSize = manager.Measure(double.PositiveInfinity, double.PositiveInfinity); + manager.ArrangeChildren(new Rect(Point.Zero, measuredSize)); + + // We expect that the starting view (0) should be arranged on the right, + // and the next rectangle (1) should be on the left + var expectedRectangle0 = new Rect(100, 0, 100, 100); + var expectedRectangle1 = new Rect(0, 0, 100, 100); + + abs[0].Received().Arrange(Arg.Is(expectedRectangle0)); + abs[1].Received().Arrange(Arg.Is(expectedRectangle1)); + } } } From 5cf34ac8e636451dac936d544a423c82e799edf6 Mon Sep 17 00:00:00 2001 From: Rachel Kang Date: Tue, 17 May 2022 11:28:40 -0400 Subject: [PATCH 2/7] First attempt at fix --- src/Core/src/Layouts/AbsoluteLayoutManager.cs | 58 ++++++++++++++++++- 1 file changed, 57 insertions(+), 1 deletion(-) diff --git a/src/Core/src/Layouts/AbsoluteLayoutManager.cs b/src/Core/src/Layouts/AbsoluteLayoutManager.cs index 2608f279dc28..b10266c495d2 100644 --- a/src/Core/src/Layouts/AbsoluteLayoutManager.cs +++ b/src/Core/src/Layouts/AbsoluteLayoutManager.cs @@ -1,5 +1,6 @@ using System; using Microsoft.Maui.Graphics; +using Microsoft.Maui.Primitives; namespace Microsoft.Maui.Layouts { @@ -62,9 +63,15 @@ public override Size ArrangeChildren(Rect bounds) double top = padding.Top + bounds.Y; double left = padding.Left + bounds.X; + double right = padding.Right + bounds.X; double availableWidth = bounds.Width - padding.HorizontalThickness; double availableHeight = bounds.Height - padding.VerticalThickness; + bool leftToRight = AbsoluteLayout.ShouldArrangeLeftToRight(); + + // Figure out where we're starting from (the left edge of the padded area, or the right edge) + double xPosition = leftToRight ? padding.Left + bounds.Left : bounds.Right - padding.Right; + for (int n = 0; n < AbsoluteLayout.Count; n++) { var child = AbsoluteLayout[n]; @@ -93,15 +100,64 @@ public override Size ArrangeChildren(Rect bounds) destination.Y = (availableHeight - destination.Height) * destination.Y; } - destination.X += left; + if (leftToRight) + destination.X += left; + else + destination.X -= right; + destination.Y += top; child.Arrange(destination); + + if (leftToRight) + xPosition += destination.Width; + else + xPosition -= destination.Width; + + //if (child.HorizontalLayoutAlignment != LayoutAlignment.Fill) + //{ + // SizeRequest request = child.Measure(bounds.Width, bounds.Height); + // double diff = Math.Max(0, bounds.Width - request.Request.Width); + // double horizontalAlign = (double)child.HorizontalLayoutAlignment; + // if (!leftToRight) + // horizontalAlign = 1 - horizontalAlign; + // bounds.X += (int)(diff * horizontalAlign); + // bounds.Width -= diff; + //} } + // If we started from the left, the total width is the current x position; + // If we started from the right, it's the difference between the right edge and the current x position + availableWidth = leftToRight ? xPosition : bounds.Right - xPosition; + return new Size(availableWidth, availableHeight); } + //internal static IView UpdateFlowDirection(this IView child, Rect bounds) + //{ + // bool isRightToLeft = false; + // if (child.Parent is IFlowDirectionController parent && + // (isRightToLeft = parent.ApplyEffectiveFlowDirectionToChildContainer && + // parent.EffectiveFlowDirection.IsRightToLeft()) && + // (parent.Width - bounds.Right) != bounds.X) + // { + // bounds = new Rect(parent.Width - bounds.Right, bounds.Y, bounds.Width, bounds.Height); + // } + + // if (child.HorizontalLayoutAlignment != LayoutAlignment.Fill) + // { + // SizeRequest request = child.Measure(bounds.Width, bounds.Height); + // double diff = Math.Max(0, bounds.Width - request.Request.Width); + // double horizontalAlign = child.HorizontalLayoutAlignment.ToDouble(); + // if (isRightToLeft) + // horizontalAlign = 1 - horizontalAlign; + // bounds.X += (int)(diff * horizontalAlign); + // bounds.Width -= diff; + // } + + // return child; + //} + static bool HasFlag(AbsoluteLayoutFlags a, AbsoluteLayoutFlags b) { // Avoiding Enum.HasFlag here for performance reasons; we don't need the type check From c11ba3ac945d7b2d0a4bc66d9026aa42d8f991bd Mon Sep 17 00:00:00 2001 From: Rachel Kang Date: Wed, 18 May 2022 17:16:48 -0400 Subject: [PATCH 3/7] Fix up tests and RTL math --- src/Core/src/Layouts/AbsoluteLayoutManager.cs | 51 ++----------------- .../Layouts/AbsoluteLayoutManagerTests.cs | 28 ++++------ 2 files changed, 16 insertions(+), 63 deletions(-) diff --git a/src/Core/src/Layouts/AbsoluteLayoutManager.cs b/src/Core/src/Layouts/AbsoluteLayoutManager.cs index b10266c495d2..5a340e7e4722 100644 --- a/src/Core/src/Layouts/AbsoluteLayoutManager.cs +++ b/src/Core/src/Layouts/AbsoluteLayoutManager.cs @@ -61,9 +61,9 @@ public override Size ArrangeChildren(Rect bounds) { var padding = AbsoluteLayout.Padding; - double top = padding.Top + bounds.Y; - double left = padding.Left + bounds.X; - double right = padding.Right + bounds.X; + double top = padding.Top + bounds.Top; + double left = padding.Left + bounds.Left; + double right = bounds.Right - padding.Right; double availableWidth = bounds.Width - padding.HorizontalThickness; double availableHeight = bounds.Height - padding.VerticalThickness; @@ -103,27 +103,11 @@ public override Size ArrangeChildren(Rect bounds) if (leftToRight) destination.X += left; else - destination.X -= right; - + destination.X = right - destination.X - destination.Width; + destination.Y += top; child.Arrange(destination); - - if (leftToRight) - xPosition += destination.Width; - else - xPosition -= destination.Width; - - //if (child.HorizontalLayoutAlignment != LayoutAlignment.Fill) - //{ - // SizeRequest request = child.Measure(bounds.Width, bounds.Height); - // double diff = Math.Max(0, bounds.Width - request.Request.Width); - // double horizontalAlign = (double)child.HorizontalLayoutAlignment; - // if (!leftToRight) - // horizontalAlign = 1 - horizontalAlign; - // bounds.X += (int)(diff * horizontalAlign); - // bounds.Width -= diff; - //} } // If we started from the left, the total width is the current x position; @@ -133,31 +117,6 @@ public override Size ArrangeChildren(Rect bounds) return new Size(availableWidth, availableHeight); } - //internal static IView UpdateFlowDirection(this IView child, Rect bounds) - //{ - // bool isRightToLeft = false; - // if (child.Parent is IFlowDirectionController parent && - // (isRightToLeft = parent.ApplyEffectiveFlowDirectionToChildContainer && - // parent.EffectiveFlowDirection.IsRightToLeft()) && - // (parent.Width - bounds.Right) != bounds.X) - // { - // bounds = new Rect(parent.Width - bounds.Right, bounds.Y, bounds.Width, bounds.Height); - // } - - // if (child.HorizontalLayoutAlignment != LayoutAlignment.Fill) - // { - // SizeRequest request = child.Measure(bounds.Width, bounds.Height); - // double diff = Math.Max(0, bounds.Width - request.Request.Width); - // double horizontalAlign = child.HorizontalLayoutAlignment.ToDouble(); - // if (isRightToLeft) - // horizontalAlign = 1 - horizontalAlign; - // bounds.X += (int)(diff * horizontalAlign); - // bounds.Width -= diff; - // } - - // return child; - //} - static bool HasFlag(AbsoluteLayoutFlags a, AbsoluteLayoutFlags b) { // Avoiding Enum.HasFlag here for performance reasons; we don't need the type check diff --git a/src/Core/tests/UnitTests/Layouts/AbsoluteLayoutManagerTests.cs b/src/Core/tests/UnitTests/Layouts/AbsoluteLayoutManagerTests.cs index 731994d278b6..3bc14bff8a92 100644 --- a/src/Core/tests/UnitTests/Layouts/AbsoluteLayoutManagerTests.cs +++ b/src/Core/tests/UnitTests/Layouts/AbsoluteLayoutManagerTests.cs @@ -452,22 +452,19 @@ public void LtrShouldHaveFirstItemOnTheLeft() var abs = CreateTestLayout(); var child = CreateTestView(); SubstituteChildren(abs, child); - var childBounds = new Rect(0, 0, 100, 100); + var childBounds = new Rect(10, 0, 100, 100); SetLayoutBounds(abs, child, childBounds); abs.FlowDirection.Returns(FlowDirection.LeftToRight); var manager = new AbsoluteLayoutManager(abs); - var measuredSize = manager.Measure(double.PositiveInfinity, double.PositiveInfinity); + var measuredSize = manager.Measure(double.PositiveInfinity, 100); manager.ArrangeChildren(new Rect(Point.Zero, measuredSize)); - // We expect that the starting view (0) should be arranged on the left, - // and the next rectangle (1) should be on the right - var expectedRectangle0 = new Rect(0, 0, 100, 100); - var expectedRectangle1 = new Rect(100, 0, 100, 100); + // We expect that the view should be arranged on the left + var expectedRectangle = new Rect(10, 0, 100, 100); - abs[0].Received().Arrange(Arg.Is(expectedRectangle0)); - abs[1].Received().Arrange(Arg.Is(expectedRectangle1)); + abs[0].Received().Arrange(Arg.Is(expectedRectangle)); } [Fact(DisplayName = "First View in RTL Absolute Layout is on the right")] @@ -476,22 +473,19 @@ public void RtlShouldHaveFirstItemOnTheRight() var abs = CreateTestLayout(); var child = CreateTestView(); SubstituteChildren(abs, child); - var childBounds = new Rect(0, 0, 100, 100); + var childBounds = new Rect(10, 0, 100, 100); SetLayoutBounds(abs, child, childBounds); - abs.FlowDirection.Returns(FlowDirection.LeftToRight); + abs.FlowDirection.Returns(FlowDirection.RightToLeft); var manager = new AbsoluteLayoutManager(abs); - var measuredSize = manager.Measure(double.PositiveInfinity, double.PositiveInfinity); + var measuredSize = manager.Measure(double.PositiveInfinity, 100); manager.ArrangeChildren(new Rect(Point.Zero, measuredSize)); - // We expect that the starting view (0) should be arranged on the right, - // and the next rectangle (1) should be on the left - var expectedRectangle0 = new Rect(100, 0, 100, 100); - var expectedRectangle1 = new Rect(0, 0, 100, 100); + // We expect that the view hould be arranged on the right + var expectedRectangle = new Rect(0, 0, 100, 100); - abs[0].Received().Arrange(Arg.Is(expectedRectangle0)); - abs[1].Received().Arrange(Arg.Is(expectedRectangle1)); + abs[0].Received().Arrange(Arg.Is(expectedRectangle)); } } } From 83a6c3fc73281ec3c28cfd65e68b3f256cdec453 Mon Sep 17 00:00:00 2001 From: Rachel Kang Date: Wed, 18 May 2022 17:26:22 -0400 Subject: [PATCH 4/7] Remove unnecessary using --- src/Core/src/Layouts/AbsoluteLayoutManager.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Core/src/Layouts/AbsoluteLayoutManager.cs b/src/Core/src/Layouts/AbsoluteLayoutManager.cs index 5a340e7e4722..8173c24483a2 100644 --- a/src/Core/src/Layouts/AbsoluteLayoutManager.cs +++ b/src/Core/src/Layouts/AbsoluteLayoutManager.cs @@ -1,6 +1,5 @@ using System; using Microsoft.Maui.Graphics; -using Microsoft.Maui.Primitives; namespace Microsoft.Maui.Layouts { From 71d10e01317e1c107d5e986bfe94cddd9f05787f Mon Sep 17 00:00:00 2001 From: Rachel Kang Date: Thu, 19 May 2022 12:09:30 -0400 Subject: [PATCH 5/7] Remove unneeded and wrong code --- src/Core/src/Layouts/AbsoluteLayoutManager.cs | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/Core/src/Layouts/AbsoluteLayoutManager.cs b/src/Core/src/Layouts/AbsoluteLayoutManager.cs index 8173c24483a2..2eb234ae129b 100644 --- a/src/Core/src/Layouts/AbsoluteLayoutManager.cs +++ b/src/Core/src/Layouts/AbsoluteLayoutManager.cs @@ -109,10 +109,6 @@ public override Size ArrangeChildren(Rect bounds) child.Arrange(destination); } - // If we started from the left, the total width is the current x position; - // If we started from the right, it's the difference between the right edge and the current x position - availableWidth = leftToRight ? xPosition : bounds.Right - xPosition; - return new Size(availableWidth, availableHeight); } From f7b327b0bb42edd493dc553924c4e41e6569e838 Mon Sep 17 00:00:00 2001 From: Rachel Kang Date: Fri, 20 May 2022 11:11:34 -0400 Subject: [PATCH 6/7] Update based on feedback --- src/Core/src/Layouts/AbsoluteLayoutManager.cs | 2 +- src/Core/tests/UnitTests/Layouts/AbsoluteLayoutManagerTests.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Core/src/Layouts/AbsoluteLayoutManager.cs b/src/Core/src/Layouts/AbsoluteLayoutManager.cs index 2eb234ae129b..ae212a24f745 100644 --- a/src/Core/src/Layouts/AbsoluteLayoutManager.cs +++ b/src/Core/src/Layouts/AbsoluteLayoutManager.cs @@ -69,7 +69,7 @@ public override Size ArrangeChildren(Rect bounds) bool leftToRight = AbsoluteLayout.ShouldArrangeLeftToRight(); // Figure out where we're starting from (the left edge of the padded area, or the right edge) - double xPosition = leftToRight ? padding.Left + bounds.Left : bounds.Right - padding.Right; + double xPosition = leftToRight ? left : right; for (int n = 0; n < AbsoluteLayout.Count; n++) { diff --git a/src/Core/tests/UnitTests/Layouts/AbsoluteLayoutManagerTests.cs b/src/Core/tests/UnitTests/Layouts/AbsoluteLayoutManagerTests.cs index 3bc14bff8a92..d8f5ce463a6a 100644 --- a/src/Core/tests/UnitTests/Layouts/AbsoluteLayoutManagerTests.cs +++ b/src/Core/tests/UnitTests/Layouts/AbsoluteLayoutManagerTests.cs @@ -482,7 +482,7 @@ public void RtlShouldHaveFirstItemOnTheRight() var measuredSize = manager.Measure(double.PositiveInfinity, 100); manager.ArrangeChildren(new Rect(Point.Zero, measuredSize)); - // We expect that the view hould be arranged on the right + // We expect that the view should be arranged on the right var expectedRectangle = new Rect(0, 0, 100, 100); abs[0].Received().Arrange(Arg.Is(expectedRectangle)); From 76198592c5db6fdcc0d6909f4b1719f8e214c704 Mon Sep 17 00:00:00 2001 From: Rachel Kang Date: Mon, 23 May 2022 09:58:28 -0400 Subject: [PATCH 7/7] Remove unused variable --- src/Core/src/Layouts/AbsoluteLayoutManager.cs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/Core/src/Layouts/AbsoluteLayoutManager.cs b/src/Core/src/Layouts/AbsoluteLayoutManager.cs index ae212a24f745..72bee22c2bfd 100644 --- a/src/Core/src/Layouts/AbsoluteLayoutManager.cs +++ b/src/Core/src/Layouts/AbsoluteLayoutManager.cs @@ -68,9 +68,6 @@ public override Size ArrangeChildren(Rect bounds) bool leftToRight = AbsoluteLayout.ShouldArrangeLeftToRight(); - // Figure out where we're starting from (the left edge of the padded area, or the right edge) - double xPosition = leftToRight ? left : right; - for (int n = 0; n < AbsoluteLayout.Count; n++) { var child = AbsoluteLayout[n]; @@ -99,6 +96,7 @@ public override Size ArrangeChildren(Rect bounds) destination.Y = (availableHeight - destination.Height) * destination.Y; } + // Figure out where we're starting from (the left edge of the padded area, or the right edge) if (leftToRight) destination.X += left; else