Skip to content
This repository has been archived by the owner on May 1, 2024. It is now read-only.

[Bug] Word wrapped Label *STILL* not measured correctly #13683

Open
JohnHDev opened this issue Feb 6, 2021 · 20 comments
Open

[Bug] Word wrapped Label *STILL* not measured correctly #13683

JohnHDev opened this issue Feb 6, 2021 · 20 comments
Labels
a/label a/layout i/high Completely doesn't work, crashes, or is unusably slow, has no obvious workaround; occurs less often t/bug 🐛

Comments

@JohnHDev
Copy link

JohnHDev commented Feb 6, 2021

Original bug that was 'fixed' after 8 months and we broke in 10 minutes: #8797

Original bug is being ignored as people go off and work on the new and shiny. But sure, I'll raise a new bug for this. I clearly have more free time than those working for MS, so why not.

Description

A Label in a grid that is contained in a StackLayout isn't measured correctly, with contents below the grid in the StackLayout overlapping the grid and label.

Steps to Reproduce

Create a new template XF solution.
Replace the MainPage xml with a new MainPage.cs
Install Xamarin Community Toolkit Markup nuget
Replace the code with the following and fix the namespace accordingly:

public class MainPage: ContentPage
{
    public MainPage()
    {
        BackgroundColor = Color.Gray;

        Content = new StackLayout()
        {
            Padding = new Thickness(10, 40, 10, 10),
            Children =
            {
                new Grid()
                {
                    ColumnSpacing = 8,
                    ColumnDefinitions =
                    {
                        new ColumnDefinition() { Width = GridLength.Auto },
                        new ColumnDefinition() { Width = GridLength.Star },
                    },

                    Children =
                    {
                        new Grid()
                        {
                            RowSpacing = 0,
                            Children =
                            {
                                new Label()
                                {
                                    Text = "ABC123"
                                }
                            }
                        }.Row(0).Column(0),
                        new Frame()
                        {
                            Margin = 0,
                            Padding = 0,
                            CornerRadius = 8,
                            HasShadow = false,
                            HorizontalOptions = LayoutOptions.FillAndExpand,
                            Content = new Grid()
                            {
                                ColumnSpacing = 0,
                                VerticalOptions = LayoutOptions.CenterAndExpand,
                                ColumnDefinitions =
                                {
                                    new ColumnDefinition() { Width = GridLength.Auto },
                                    new ColumnDefinition() { Width = GridLength.Star },
                                },
                                RowDefinitions =
                                {
                                    new RowDefinition() { Height = GridLength.Auto }
                                },
                                Children =
                                {
                                    new Label()
                                    {
                                        Margin = new Thickness(6, 3, 6, 3),
                                        VerticalOptions = LayoutOptions.CenterAndExpand,
                                        Text = "dfg fg df dfg dfg dfg f dfg dfg dfg dff gdf gdf gd fg dfg df gdf g df gd gdf gf dg ffd gdf gdf g df gdf gd fg df gdg df dfg a"
                                    }.Row(0).Column(1)
                                }
                            }
                        }.Row(0).Column(1)
                    }
        }
            }
        };
    }
}

Run it.

Expected Behavior

Label should display correctly, with all the text visible.

Actual Behavior

Label is wrapping out of the grid.

Basic Information

  • Version with issue: XF 5.0.0
  • Last known good version: Has there ever been a 'good' Xamarin Forms version?
  • Platform Target Frameworks:
    • iOS: 14.4

Workaround

Migrate all projects to Flutter

@JohnHDev JohnHDev added s/unverified New report that has yet to be verified t/bug 🐛 labels Feb 6, 2021
@jsuarezruiz jsuarezruiz added a/label a/layout i/high Completely doesn't work, crashes, or is unusably slow, has no obvious workaround; occurs less often labels Feb 8, 2021
@jsuarezruiz
Copy link
Contributor

jsuarezruiz commented Feb 8, 2021

Attach a reproduction sample:
Captura de pantalla 2021-02-08 a las 16 28 38

Issue136832.zip

Was fixed by #11336 but seems to fail with the latest changes in GridCalc.cs.

cc @hartez

@jsuarezruiz jsuarezruiz removed the s/unverified New report that has yet to be verified label Feb 8, 2021
@hartez hartez self-assigned this Feb 11, 2021
@hartez
Copy link
Contributor

hartez commented Feb 11, 2021

@JohnHDev There's certainly seems to be a bug here; we're taking a look at it.

If you're looking for a workaround in the meantime, I notice that your innermost Grid has an extra column declaration which doesn't do anything. If you remove the extra Star column from your layout (and put the Label in column 0), the problem goes away:

Content = new StackLayout()
{
	Padding = new Thickness(10, 40, 10, 10),
	Children =
{
	new Grid()
	{
		ColumnSpacing = 8,
		ColumnDefinitions =
		{
			new ColumnDefinition() { Width = GridLength.Auto },
			new ColumnDefinition() { Width = GridLength.Star },
		},

		Children =
		{
			new Grid()
			{
				RowSpacing = 0,
				Children =
				{
					new Label()
					{
						Text = "ABC123"
					}
				}
			}.Row(0).Column(0),
			new Frame()
			{
				Margin = 0,
				Padding = 0,
				CornerRadius = 8,
				HasShadow = false,
				HorizontalOptions = LayoutOptions.FillAndExpand,
				Content = new Grid()
				{
					ColumnSpacing = 0,
					VerticalOptions = LayoutOptions.CenterAndExpand,
					ColumnDefinitions =
					{
						new ColumnDefinition() { Width = GridLength.Auto },
						//new ColumnDefinition() { Width = GridLength.Star },
					},
					RowDefinitions =
					{
						new RowDefinition() { Height = GridLength.Auto }
					},
					Children =
					{
						new Label()
						{
							Margin = new Thickness(6, 3, 6, 3),
							VerticalOptions = LayoutOptions.CenterAndExpand,
							Text = "dfg fg df dfg dfg dfg f dfg dfg dfg dff gdf gdf gd fg dfg df gdf g df gd gdf gf dg ffd gdf gdf g df gdf gd fg df gdg df dfg a"
						}.Row(0)//.Column(1)
					}
				}
			}.Row(0).Column(1)
		}
}
}
};

All I did was comment out the extra column and the method which sets the Label's column. Tested on an iPhone 5s.

@JohnHDev
Copy link
Author

@hartez thank you for taking a look, the above is just an example, cut down from a much larger version we had in production where the first column was populated as well, but the IsVisible of that is bound to a property. The same problem occurs of course when IsVisible on the view in the first column is is bound to a property with a value of false.

@JohnHDev

This comment has been minimized.

@JohnHDev

This comment has been minimized.

@jeremy-bridges

This comment has been minimized.

@mcerqueira1509

This comment has been minimized.

@somoreingold
Copy link

Related:

#3754
#4950
#11270
#13448
#10077
#11713

@somoreingold
Copy link

Hi there, @jfversluis any hope in this one still being included in the next release https://github.com/xamarin/Xamarin.Forms/projects/89#card-71066115 ? Contemplating whether to go through all the labels in my projects and trying to figure out some type of workaround to each, or wait for an imminent fix.

@jfversluis
Copy link
Member

Depends on what you mean by next release. It won't be in SR7, I will see if I can prioritize for SR8. Any help is of course appreciated if you want to resolve this faster

@AlleSchonWeg
Copy link
Contributor

AlleSchonWeg commented Nov 12, 2021

Perhaps also releated: #12710
Sometimes, if you measure a text the result is wrong. A litte android (Pixel 3 Emu API29) demo:

<?xml version="1.0" encoding="utf-8" ?>
<ContentPage xmlns="http://xamarin.com/schemas/2014/forms"
			 xmlns:x="http://schemas.microsoft.com/winfx/2009/xaml"
			 xmlns:ios="clr-namespace:Xamarin.Forms.PlatformConfiguration.iOSSpecific;assembly=Xamarin.Forms.Core"
			 ios:Page.UseSafeArea="true"
			 Padding="0"
			 x:Class="XX.YY.Views.DemoPage">
	<ContentPage.Content>
		<StackLayout x:Name="stack"></StackLayout>
	</ContentPage.Content>
</ContentPage>
using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;

using Xamarin.Forms;
using Xamarin.Forms.Xaml;

namespace XX.YY.Views {
	[XamlCompilation(XamlCompilationOptions.Compile)]
	public partial class DemoPage : ContentPage {
		Label label;
		public DemoPage() {
			InitializeComponent();
			label = new Label {
				Text = "Yamaha XVS 650 A Drag Star",
				Padding = new Thickness(0, 2, 0, 2),
				HorizontalTextAlignment = TextAlignment.Start,
				VerticalTextAlignment = TextAlignment.Center,
				LineBreakMode = LineBreakMode.TailTruncation,
				MaxLines = 3,
			};
			stack.Children.Add(label);
		}

		protected override void OnAppearing() {
			base.OnAppearing();
			var size = label.Measure(123d, double.PositiveInfinity, MeasureFlags.IncludeMargins);
			Console.WriteLine("IncludeMargins: " + size);
		}
	}
}

The output is the following:

IncludeMargins: {Request={Width=123.272727272727 Height=40.3636363636364} Minimum={Width=10.1818181818182 Height=40.3636363636364}}

The Width 123.272727272727 is wrong. It should be 123, because i set widthConstraint to 123.

If you change the widthRequest to 100 for example:
var size = label.Measure(100, double.PositiveInfinity, MeasureFlags.IncludeMargins);

then the size is correct:

IncludeMargins: {Request={Width=100 Height=40.3636363636364} Minimum={Width=10.1818181818182 Height=40.3636363636364}}

@somoreingold
Copy link

@AlleSchonWeg I 100% agree, #12710 is related. Reading from the top of this ticket, I fear that this issue might be viewed only as a Grid/Label-specific edge-case bug, instead of the fundamental widespread Label bug it seems to be since XF 4.6.

@jfversluis Thanks for answering. I'd love to help the Microsoft team, but I haven't found any true solutions yet whenever I've spent time investigating this. I personally think this is critical (imagine legal or medical information being cut off in a label).

@AlleSchonWeg
Copy link
Contributor

I think i found a "bug" in the ToPixels() method, which is used in measureing.

return (float)Math.Ceiling(dp * s_displayDensity);

Xamarin use Math.Ceiling and this results sometimes in a larger widthconstraint or heightconstraint. So the measureing thinks its enough space to display the content. In my test i changed ToPixels:

	public static float ToPixels(this Context self, double dp)
		{
			SetupMetrics(self);
			return (float)dp * s_displayDensity;
		}

and the result for my demo above is now

IncludeMargins: {Request={Width=122.909090909091 Height=38.9090909090909} Minimum={Width=10 Height=38.9090909090909}}

@jfversluis I can create a PR, but i'm don't know if this change has side effect or why XF use Math.Ceiling.

@somoreingold
Copy link

Interesting @AlleSchonWeg . We're also getting this issue in iOS though. I wonder if Labels on iOS are taking into account their own padding (or parent padding) correctly in https://github.com/xamarin/Xamarin.Forms/blob/5.0.0/Xamarin.Forms.Platform.iOS/Renderers/LabelRenderer.cs (GetDesiredSize and/or LayoutSubviews)...

@somoreingold
Copy link

@hartez @jfversluis Will this or any of the other related issues be part of SR8, or is this shifting again?

Thank you

@lauxjpn
Copy link

lauxjpn commented Dec 28, 2021

In my case, I had the following layout, that results in the Label missing the last (wrapped) text line:

<StackLayout>
    <StackLayout Orientation="Horizontal">
        <Label>Some looong text that wraps across multiple lines</Label>
        <Image />
    </StackLayout>
    <ListView></ListView>
</StackLayout>

Removing the second StackLayout shows all text lines correctly:

<StackLayout>
    <Label>Some looong text that wraps across multiple lines</Label>
    <ListView></ListView>
</StackLayout>

It seems that as long as the outer layout is a StackLayout, a Label in an inner layout is missing text.
The following uses a Grid as the inner layout, resulting in the Label still missing text:

<StackLayout>
    <Grid>
        <Label>Some looong text that wraps across multiple lines</Label>
    </Grid>
    <ListView></ListView>
</StackLayout>

Changing the type of the outer layout to something else results in all text being displayed correctly:

<Grid>
    <Grid.RowDefinitions>
        <RowDefinition Height="Auto" />
        <RowDefinition Height="*" />
    </Grid.RowDefinitions>
    <StackLayout Grid.Row="0"
                 Orientation="Horizontal">
        <Label>Some looong text that wraps across multiple lines</Label>
        <Image />
    </StackLayout>
    <ListView Grid.Row="1"></ListView>
</Grid>

The LabelRenderer.GetDesiredSize() method seems to actually return the correct height request.
However, the height constraint from the outer elements is already (incorrectly) smaller (I believe by the margin) than the height requested by the Label, resulting in the smaller height constraint being used instead of the larger height request.

@jfversluis
Copy link
Member

@somoreingold we're working through the list of all things as you can hopefully see. We've been releasing monthly services releases with 20-30 fixes each month. Sorry we didn't get to the ones that are important to you yet. Thanks for your patience!

@somoreingold
Copy link

@jfversluis Yep, I've been seeing the service releases, thanks to you and your team for that. They have been helpful. And I understand MS resources/hartez are really focused on MAUI right now. So I've gone ahead and refactored Label usage across projects to prevent this from affecting displayed text, and warned my team not to use padding until this is resolved.

If anyone here is interested, wrapping the Label with a StackLayout and assigning the padding to the wrapper instead of the Label will often (but not always) be a workaround. The trouble comes when the Label is doing anything interesting, such as a background color etc. which also must be shifted to the wrapper. Furthermore, again, this doesn't always fix the issue.

@jfversluis
Copy link
Member

Thanks for the kind words and info @somoreingold of course if you would be willing and able to diagnose this issue more in our codebase that would also benefit the process in getting this resolved. Other than that, we're doing our best!

@somoreingold
Copy link

I wonder if this fix done in MAUI could be related/a solution for Xamarin: dotnet/maui#1870

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
a/label a/layout i/high Completely doesn't work, crashes, or is unusably slow, has no obvious workaround; occurs less often t/bug 🐛
Projects
None yet
Development

No branches or pull requests

9 participants