Grid row height is not calculated correctly with wrapping text

[eksperts]

Reported on community.

Fuse 1.4.0 build 14778, macOS and Windows 10. Tested on local preview and dotnet build, mobile targets likely affected as well.

Reproduction:

<App Background="#DDD" >
    <JavaScript>
        module.exports = {
            items1: [
                {desc: "aaaaa aaaaaaa"},
                {desc: "ccc ccc cccccccc cccccccccc cccc"},
                {desc: "bbbbbb"},
                {desc: "dddddddddd"}
            ],
            items2: [
                {desc: "aaaaa aaaaaaa"},
                {desc: "ccc ccc cccccccc cccccccccc cccc"},
                {desc: "bbbbbb"}
            ]
        }
    </JavaScript>
    <StackPanel Alignment="VerticalCenter">
        <Text Value="Problem:" Alignment="Center" />
        <Grid ColumnCount="3" CellSpacing="10" Margin="10">
            <Each Items="{items1}">
                <StackPanel Padding="5" Color="#FFF" Alignment="Top"  >
                    <Text Value="{desc}" Background="#faa" TextWrapping="Wrap" Opacity="0.8" />
                </StackPanel> 
            </Each>
        </Grid>
        <Text Value="No problem:" Alignment="Center" />
        <Grid ColumnCount="3" CellSpacing="10" Margin="10" Alignment="Top"  >
            <Each Items="{items2}">
                <StackPanel Padding="5" Color="#FFF" Alignment="Top"  >
                    <Text Value="{desc}" Background="#faa" TextWrapping="Wrap" Opacity="0.8" />
                </StackPanel> 
            </Each>
        </Grid>
    </StackPanel>
</App>

The middle cell on the first row in the upper Grid does not push the row to be high enough to accommodate the whole text. On the second grid (when there is no second row), it does push the row high enough just fine.

Screenshot:

[fusebuild]

The forum thread Grid Layout and TextWrapping not playing nice with each other posted by mircea@code11.com was linked to this issue.

[mortoray]

This does not appear to be the same issue fixed in https://github.com/fusetools/fuselibs-public/pull/1052 I was hoping it was.

[mortoray]

Adding DefaultRow="auto" to the Grid fixes the layout. (Update: See the below comment, it actually isn't fixing it, just improving it in the original test)

This indicates an issue in the detection of what the "default" row metric should be.

[mortoray]

The issue affects the size of the Grid as a whole, not just that one cell:

<App Background="#DDD" >
    <JavaScript>
        module.exports = {
            items1: [
                {desc: "ccc ccc cccccccc cccccccccc cccc"},
                {desc: "dddddddddd"}
            ],
        }
    </JavaScript>
    <StackPanel Alignment="VerticalCenter" MaxWidth="100" Color="#AAF" Padding="10">
        <Grid ColumnCount="1" CellSpacing="10" DefaultRow="auto">
            <Each Items="{items1}">
                <StackPanel Padding="5" Color="#FFF" Alignment="Top"  >
                    <Text Value="{desc}" Background="#faa" TextWrapping="Wrap" Opacity="0.8" />
                </StackPanel> 
            </Each>
        </Grid>
    </StackPanel>
</App>

[mortoray]

I have a fix that allows DefaultRow="auto" to work.

Looking further, there's no way to fix the original code. The decision for whether DefaultRow should be auto or proportional is not solvable in the current layout system.

I'm instead going to make it required that a Grid has both a specification for the Rows and Columns it contains. I'll produce a deprecation warning, but it will trigger on some seemingly valid grids:

<Grid ColumnCount="3" DefaultRow="auto">

That's because ColumnCount currently implies a default size, as opposed to proportional. Changing this behaviour could be backwards incompatible -- though it does appear to be a defect that a ..Count doesn't imply proportional unless otherwise stated.

Without making a backwards incompatible change the above code would have to be extended to not trigger a deprecation warning:

<Grid ColumnCount="3" DefaultColumn="Proportional" DefaultRow="auto">

[mortoray]

I think making ColumnCount or RowCount imply they have a proportional size may not be totally backwards incompatible. The current result of using this would be getting proportionally sized rows/columns. That would continue to be the case.

What changes is how the initial size is reported. It would previously add the automatic sizes, then divide into rows. That sounds like a defect on its own, as it doesn't produce predictable and/or usable results.

[mortoray]

I have a solution that emits a warning when a bad config is detected while retained support for good scenarios.