-
Notifications
You must be signed in to change notification settings - Fork 1.9k
[iOS/Win] Label will not unnecessarily expand #827
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,35 @@ | ||
using Xamarin.Forms.CustomAttributes; | ||
using Xamarin.Forms.Internals; | ||
|
||
namespace Xamarin.Forms.Controls.Issues | ||
{ | ||
[Preserve(AllMembers = true)] | ||
[Issue(IssueTracker.Bugzilla, 53362, "Layout regression in Grid on iOS: HorizontalOption = Center does not center", PlatformAffected.iOS)] | ||
public class Bugzilla53362 : TestContentPage | ||
{ | ||
protected override void Init() | ||
{ | ||
var label1 = new Label { Text = "auto sized row", TextColor = Color.Silver, HorizontalOptions = LayoutOptions.Center, BackgroundColor = Color.Purple }; | ||
var label2 = new Label { Text = "row size 20", TextColor = Color.Silver, HorizontalOptions = LayoutOptions.Center, BackgroundColor = Color.Purple }; | ||
var label3 = new Label { Text = "row size 25", TextColor = Color.Silver, HorizontalOptions = LayoutOptions.Center, BackgroundColor = Color.Purple }; | ||
|
||
var grid = new Grid | ||
{ | ||
RowDefinitions = | ||
{ | ||
new RowDefinition { Height = new GridLength(1, GridUnitType.Auto) }, | ||
new RowDefinition { Height = new GridLength(20, GridUnitType.Absolute) }, | ||
new RowDefinition { Height = new GridLength(25, GridUnitType.Absolute) }, | ||
new RowDefinition { Height = new GridLength(1, GridUnitType.Auto) }, | ||
} | ||
}; | ||
|
||
grid.Children.Add(label1, 0, 0); | ||
grid.Children.Add(label2, 0, 1); | ||
grid.Children.Add(label3, 0, 2); | ||
grid.Children.Add(new Label { Text = "If the three labels above are not all centered horizontally, this test has failed." }, 0, 3); | ||
|
||
Content = grid; | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -30,16 +30,31 @@ public override SizeRequest GetDesiredSize(double widthConstraint, double height | |
_perfectSizeValid = true; | ||
} | ||
|
||
if (widthConstraint >= _perfectSize.Request.Width && heightConstraint >= _perfectSize.Request.Height) | ||
var widthFits = widthConstraint >= _perfectSize.Request.Width; | ||
var heightFits = heightConstraint >= _perfectSize.Request.Height; | ||
|
||
if (widthFits && heightFits) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Refactored, but the logic is the same. |
||
return _perfectSize; | ||
|
||
var result = base.GetDesiredSize(widthConstraint, heightConstraint); | ||
result.Minimum = new Size(Math.Min(10, result.Request.Width), result.Request.Height); | ||
if (Element.LineBreakMode != LineBreakMode.NoWrap) | ||
var tinyWidth = Math.Min(10, result.Request.Width); | ||
result.Minimum = new Size(tinyWidth, result.Request.Height); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. More refactoring, no change here. |
||
|
||
if (widthFits || Element.LineBreakMode == LineBreakMode.NoWrap) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is the change that fixes this bug. Don't continue on to the "expand" logic if the text already fits the container. |
||
return result; | ||
|
||
bool containerIsNotInfinitelyWide = !double.IsInfinity(widthConstraint); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Simplification of |
||
|
||
if (containerIsNotInfinitelyWide) | ||
{ | ||
if (!double.IsInfinity(result.Request.Width) && !double.IsInfinity(widthConstraint)) | ||
if (result.Request.Width > widthConstraint || Element.LineBreakMode == LineBreakMode.WordWrap || Element.LineBreakMode == LineBreakMode.CharacterWrap) | ||
result.Request = new Size(Math.Max(result.Minimum.Width, widthConstraint), result.Request.Height); | ||
bool textCouldHaveWrapped = Element.LineBreakMode == LineBreakMode.WordWrap || Element.LineBreakMode == LineBreakMode.CharacterWrap; | ||
bool textExceedsContainer = result.Request.Width > widthConstraint; | ||
|
||
if (textExceedsContainer || textCouldHaveWrapped) | ||
{ | ||
var expandedWidth = Math.Max(tinyWidth, widthConstraint); | ||
result.Request = new Size(expandedWidth, result.Request.Height); | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The rest of these changes are all refactoring for readability. |
||
} | ||
|
||
return result; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This bug did not affect Windows, but the logic here was copied from iOS, so I copied the fix over for housekeeping purposes.