Skip to content

Commit

Permalink
Trigger remeasure when constraints on CV changes (#22270)
Browse files Browse the repository at this point in the history
  • Loading branch information
PureWeen committed May 9, 2024
1 parent ddf9d7f commit f76ea52
Show file tree
Hide file tree
Showing 5 changed files with 282 additions and 21 deletions.
@@ -0,0 +1,58 @@
<?xml version="1.0" encoding="utf-8" ?>
<ContentPage xmlns="http://schemas.microsoft.com/dotnet/2021/maui"
xmlns:x="http://schemas.microsoft.com/winfx/2009/xaml"
x:Class="Maui.Controls.Sample.Issues.Issue21967"
Title="Issue21967">
<Grid
RowDefinitions="Auto,Auto,Auto,*"
RowSpacing="8"
Margin="8">

<Label Grid.Row="0" Text="Click the resize button and elements in the CV should resize.">
</Label>

<Button
Grid.Row="1"
Text="Set to Full Size"
AutomationId="FullSize"
x:Name="buttonFullSize"
HorizontalOptions="Center"
VerticalOptions="Center"/>

<Button
Grid.Row="2"
Text="Resize"
AutomationId="Resize"
x:Name="button"
HorizontalOptions="Center"
VerticalOptions="Center"/>

<CollectionView
Grid.Row="3"
x:Name="cv"
ItemSizingStrategy="MeasureFirstItem">

<CollectionView.ItemsLayout>
<GridItemsLayout
Orientation="Vertical"
HorizontalItemSpacing="8"
VerticalItemSpacing="8"
Span="2"/>
</CollectionView.ItemsLayout>

<CollectionView.ItemTemplate>
<DataTemplate>
<Grid BackgroundColor="LightGray">
<Label
Text="{Binding Text}"
AutomationId="{Binding AutomationId}"
HorizontalOptions="Fill"
VerticalOptions="Fill"/>
</Grid>
</DataTemplate>
</CollectionView.ItemTemplate>

</CollectionView>

</Grid>
</ContentPage>
@@ -0,0 +1,49 @@
using System;
using Microsoft.Maui;
using Microsoft.Maui.Controls;
using System.Collections.Generic;
using System.Linq;

namespace Maui.Controls.Sample.Issues;

[Issue(IssueTracker.Github, 21967, "CollectionView causes invalid measurements on resize", PlatformAffected.Android)]
public partial class Issue21967 : ContentPage
{
public readonly record struct Data(string Text, string AutomationId);
public Issue21967()
{
InitializeComponent();
cv.ItemsSource =
new List<string> { "Item1", "Item2", "Item3", "Item4", "Item5" }
.Select((x, i) =>
{
string text = x;
if (i == 1)
{
// Generate long text that would measure really big to ensure everything stays measured the
text = Enumerable.Range(0,1000).Select(x => x.ToString()).Aggregate((x, y) => x + " " + y);
}
return new Data(text, x);
})
.ToList();


button.Clicked += (_, _) =>
{
if (cv.WidthRequest == 200)
{
cv.WidthRequest = 100;
}
else
{
cv.WidthRequest = 200;
}
};

buttonFullSize.Clicked += (_, _) =>
{
cv.WidthRequest = Microsoft.Maui.Primitives.Dimension.Unset;
};
}
}
Expand Up @@ -14,9 +14,17 @@ public class StructuredItemsViewAdapter<TItemsView, TItemsViewSource> : ItemsVie
{
Size? _size;

// I'm storing this here because in the children I'm using a weakreference and
// I don't want this action to get GC'd
Action<Size> _reportMeasure;
Func<Size?> _retrieveStaticSize;

protected internal StructuredItemsViewAdapter(TItemsView itemsView,
Func<View, Context, ItemContentView> createItemContentView = null) : base(itemsView, createItemContentView)
{
_reportMeasure = SetStaticSize;
_retrieveStaticSize = () => _size ?? null;

UpdateHasHeader();
UpdateHasFooter();
}
Expand Down Expand Up @@ -98,7 +106,12 @@ protected override void BindTemplatedItemViewHolder(TemplatedItemViewHolder temp
{
if (ItemsView.ItemSizingStrategy == ItemSizingStrategy.MeasureFirstItem)
{
templatedItemViewHolder.Bind(context, ItemsView, SetStaticSize, _size);
templatedItemViewHolder.Bind(context, ItemsView, _reportMeasure, _size);

if (templatedItemViewHolder.ItemView is ItemContentView itemContentView)
{
itemContentView.RetrieveStaticSize = _retrieveStaticSize;
}
}
else
{
Expand Down
117 changes: 97 additions & 20 deletions src/Controls/src/Core/Handlers/Items/Android/ItemContentView.cs
Expand Up @@ -9,8 +9,16 @@ namespace Microsoft.Maui.Controls.Handlers.Items
{
public class ItemContentView : ViewGroup
{
Size? _size;
Action<Size> _reportMeasure;
Size? _pixelSize;
WeakReference _reportMeasure;
WeakReference _retrieveStaticSize;
int _previousPixelWidth = -1;
int _previousPixelHeight = -1;

Action<Size> ReportMeasure
{
get => _reportMeasure?.Target as Action<Size>;
}

protected IPlatformViewHandler Content;
internal IView View => Content?.VirtualView;
Expand All @@ -23,6 +31,12 @@ public ItemContentView(Context context) : base(context)

AView PlatformView => Content?.ContainerView ?? Content?.PlatformView;

internal Func<Size?> RetrieveStaticSize
{
get => _retrieveStaticSize?.Target as Func<Size?>;
set => _retrieveStaticSize = new WeakReference(value);
}

internal void RealizeContent(View view, ItemsView itemsView)
{
Content = CreateHandler(view, itemsView);
Expand Down Expand Up @@ -53,13 +67,16 @@ internal void Recycle()
}

Content = null;
_size = null;
_pixelSize = null;
_reportMeasure = null;
_previousPixelWidth = -1;
_previousPixelHeight = -1;
}

internal void HandleItemSizingStrategy(Action<Size> reportMeasure, Size? size)
{
_reportMeasure = reportMeasure;
_size = size;
_reportMeasure = new WeakReference(reportMeasure);
_pixelSize = size;
}

protected override void OnLayout(bool changed, int l, int t, int r, int b)
Expand All @@ -83,39 +100,100 @@ protected override void OnMeasure(int widthMeasureSpec, int heightMeasureSpec)
return;
}

int pixelWidth = MeasureSpec.GetSize(widthMeasureSpec);
int pixelHeight = MeasureSpec.GetSize(heightMeasureSpec);
var widthMode = MeasureSpec.GetMode(widthMeasureSpec);
var heightMode = MeasureSpec.GetMode(heightMeasureSpec);

// This checks to see if a static size has been set already on the adapter
// The problem is that HandleItemSizingStrategy(Action<Size> reportMeasure, Size? size)
// is called during "Bind" and then measure is called on all visible cells after that
// the result of this is that every single cell does an individual measure opposed to just using
// the first cell to set the size.

var possibleNewSize = RetrieveStaticSize?.Invoke();

// This means a different cell has already set our new size
// so let's just use that instead of perform our own speculative measure
if (possibleNewSize is not null &&
_pixelSize is not null &&
!_pixelSize.Equals(possibleNewSize))
{
_pixelSize = possibleNewSize;
}
else
{
_pixelSize = _pixelSize ?? possibleNewSize;

// If the measure changes significantly, we need to invalidate the pixel size
// This will happen if the user rotates the device or even just changes the height/width
// on the CollectionView itself.
// The Abs comparison I think is currently just a workaround for this
// https://github.com/dotnet/maui/issues/22271
// Once we fix 22271 I don't think we need this
// I'm also curious if we would still need this https://github.com/dotnet/maui/pull/21140
if (pixelWidth != 0 && Math.Abs(_previousPixelWidth - pixelWidth) > 1)
{
// We only need to worry about clearing pixel size if we've
// already made a first pass
if (_previousPixelWidth != -1)
{
_pixelSize = null;
}
}


if (pixelHeight != 0 && Math.Abs(_previousPixelHeight - pixelHeight) > 1)
{
// We only need to worry about clearing pixel size if we've
// already made a first pass
if (_previousPixelHeight != -1)
{
_pixelSize = null;
}

_pixelSize = null;
}
}

if (pixelWidth != 0)
{
_previousPixelWidth = pixelWidth;
}

if (pixelHeight != 0)
{
_previousPixelHeight = pixelHeight;
}

// If we're using ItemSizingStrategy.MeasureFirstItem and now we have a set size, use that
// Even though we already know the size we still need to pass the measure through to the children.
if (_size is not null)
if (_pixelSize is not null)
{
var w = (int)_size.Value.Width;
var h = (int)_size.Value.Height;
var w = (int)this.FromPixels(_pixelSize.Value.Width);
var h = (int)this.FromPixels(_pixelSize.Value.Height);

// If the platform childs measure has been invalidated, it's going to still want to
// participate in the measure lifecycle in order to update its internal
// book keeping.
_ = View.Measure
(
_ = (View.Handler as IPlatformViewHandler)?.MeasureVirtualView(
MeasureSpec.MakeMeasureSpec(w, MeasureSpecMode.Exactly),
MeasureSpec.MakeMeasureSpec(h, MeasureSpecMode.Exactly)
);

SetMeasuredDimension(w, h);
SetMeasuredDimension((int)_pixelSize.Value.Width, (int)_pixelSize.Value.Height);
return;
}

int pixelWidth = MeasureSpec.GetSize(widthMeasureSpec);
int pixelHeight = MeasureSpec.GetSize(heightMeasureSpec);

var widthSpec = MeasureSpec.GetMode(widthMeasureSpec) == MeasureSpecMode.Unspecified
var width = widthMode == MeasureSpecMode.Unspecified
? double.PositiveInfinity
: this.FromPixels(pixelWidth);

var heightSpec = MeasureSpec.GetMode(heightMeasureSpec) == MeasureSpecMode.Unspecified
var height = heightMode == MeasureSpecMode.Unspecified
? double.PositiveInfinity
: this.FromPixels(pixelHeight);


var measure = View.Measure(widthSpec, heightSpec);
var measure = View.Measure(width, height);

if (pixelWidth == 0)
{
Expand All @@ -127,8 +205,7 @@ protected override void OnMeasure(int widthMeasureSpec, int heightMeasureSpec)
pixelHeight = (int)this.ToPixels(measure.Height);
}

_reportMeasure?.Invoke(new Size(pixelWidth, pixelHeight));
_reportMeasure = null; // Make sure we only report back the measure once
ReportMeasure?.Invoke(new Size(pixelWidth, pixelHeight));

SetMeasuredDimension(pixelWidth, pixelHeight);
}
Expand Down
64 changes: 64 additions & 0 deletions src/Controls/tests/UITests/Tests/Issues/Issue21967.cs
@@ -0,0 +1,64 @@
using NUnit.Framework;
using OpenQA.Selenium;
using UITest.Appium;
using UITest.Core;

namespace Microsoft.Maui.AppiumTests.Issues
{
public class Issue21967 : _IssuesUITest
{
public Issue21967(TestDevice device) : base(device)
{
}

public override string Issue => "CollectionView causes invalid measurements on resize";

[Test]
[Category(UITestCategories.CollectionView)]
public void CollectionViewItemsResizeWhenContraintsOnCollectionViewChange()
{
var largestSize = App.WaitForElement("Item1").GetRect();
App.Tap("Resize");
var mediumSize = App.WaitForElement("Item1").GetRect();
App.Tap("Resize");
var smallSize = App.WaitForElement("Item1").GetRect();

Assert.Greater(largestSize.Width, mediumSize.Width);
Assert.Greater(mediumSize.Width, smallSize.Width);
}

[Test]
[Category(UITestCategories.CollectionView)]
public void CollectionViewFirstItemCorrectlySetsTheMeasure()
{
var itemSize = App.WaitForElement("Item1").GetRect();
Assert.Greater(200, itemSize.Height);
}

[Test]
[Category(UITestCategories.CollectionView)]
public void CollectionViewWorksWhenRotatingDevice()
{
this.IgnoreIfPlatforms(new TestDevice[] { TestDevice.Mac, TestDevice.Windows });

try
{
App.WaitForElement("FullSize");
App.Tap("FullSize");
App.SetOrientationPortrait();
var itemSizePortrait = App.WaitForElement("Item1").GetRect();
App.SetOrientationLandscape();
var itemSizeLandscape = App.WaitForElement("Item1").GetRect();
App.SetOrientationPortrait();
var itemSizePortrait2 = App.WaitForElement("Item1").GetRect();

Assert.Greater(itemSizeLandscape.Width, itemSizePortrait.Width);
Assert.AreEqual(itemSizePortrait2.Width, itemSizePortrait.Width);
}
finally
{
App.SetOrientationPortrait();
}
}
}
}

0 comments on commit f76ea52

Please sign in to comment.