Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix VerticalOffset Update When Modifying CollectionView.ItemsSource While Scrolled #26782

Open
wants to merge 15 commits into
base: main
Choose a base branch
from
Open
Original file line number Diff line number Diff line change
Expand Up @@ -34,12 +34,9 @@ public override void OnScrolled(RecyclerView recyclerView, int dx, int dy)
{
base.OnScrolled(recyclerView, dx, dy);

// TODO: These offsets will be incorrect upon row size or count change.
// They are currently provided in place of LayoutManager's default offset calculation
// because it does not report accurate values in the presence of uneven rows.
// See https://stackoverflow.com/questions/27507715/android-how-to-get-the-current-x-offset-of-recyclerview
_horizontalOffset += dx;
_verticalOffset += dy;
var itemCount = recyclerView.GetAdapter()?.ItemCount;
_horizontalOffset = itemCount == 0 ? 0 : _horizontalOffset + dx;
_verticalOffset = itemCount == 0 ? 0 : _verticalOffset + dy;

var (First, Center, Last) = GetVisibleItemsIndex(recyclerView);
var itemsViewScrolledEventArgs = new ItemsViewScrolledEventArgs
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,12 +29,9 @@ public override void Scrolled(UIScrollView scrollView)
{
var (visibleItems, firstVisibleItemIndex, centerItemIndex, lastVisibleItemIndex) = GetVisibleItemsIndex();

if (!visibleItems)
return;

var contentInset = scrollView.ContentInset;
var contentOffsetX = scrollView.ContentOffset.X + contentInset.Left;
var contentOffsetY = scrollView.ContentOffset.Y + contentInset.Top;
var contentOffsetX = !visibleItems ? 0 : scrollView.ContentOffset.X + contentInset.Left;
var contentOffsetY = !visibleItems ? 0 : scrollView.ContentOffset.Y + contentInset.Top;

var itemsViewScrolledEventArgs = new ItemsViewScrolledEventArgs
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,12 +30,9 @@ public override void Scrolled(UIScrollView scrollView)
{
var (visibleItems, firstVisibleItemIndex, centerItemIndex, lastVisibleItemIndex) = GetVisibleItemsIndex();

if (!visibleItems)
return;

var contentInset = scrollView.ContentInset;
var contentOffsetX = scrollView.ContentOffset.X + contentInset.Left;
var contentOffsetY = scrollView.ContentOffset.Y + contentInset.Top;
var contentOffsetX = !visibleItems ? 0 : scrollView.ContentOffset.X + contentInset.Left;
var contentOffsetY = !visibleItems ? 0 : scrollView.ContentOffset.Y + contentInset.Top;

var itemsViewScrolledEventArgs = new ItemsViewScrolledEventArgs
{
Expand Down
69 changes: 69 additions & 0 deletions src/Controls/tests/TestCases.HostApp/Issues/Issue21708.xaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
<?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"
xmlns:controls="clr-namespace:Maui.Controls.Sample.Issues"
x:Class="Maui.Controls.Sample.Issues.Issue21708">

<Grid>
<Grid.RowDefinitions>
<RowDefinition Height="Auto" />
<RowDefinition Height="Auto" />
<RowDefinition />
</Grid.RowDefinitions>
<Grid.ColumnDefinitions>
<ColumnDefinition />
<ColumnDefinition />
<ColumnDefinition />
</Grid.ColumnDefinitions>

<Button
Grid.Row="0"
Grid.Column="0"
Margin="10"
AutomationId="Fill"
Clicked="FillButton_OnClicked"
Text="Fill" />

<Button
Grid.Row="0"
Grid.Column="2"
Margin="10"
AutomationId="Empty"
Clicked="EmptyButton_OnClicked"
Text="Empty" />

<Label
Grid.Row="1"
Grid.Column="0"
AutomationId="VerticalOffsetLabel"
Text="VerticalOffset: " />

<Label AutomationId="Label"
Grid.ColumnSpan="2"
Grid.Row="1"
HorizontalTextAlignment="Start"
Grid.Column="1"
Text="{Binding VerticalOffset}" />

<CollectionView AutomationId="CollectionView"
x:Name="CollectionView"
Grid.Row="2"
Grid.Column="0"
Grid.ColumnSpan="3"
ItemsSource="{Binding Items}"
Scrolled="CollectionView_OnScrolled">
<CollectionView.ItemTemplate>
<DataTemplate>
<Label
Margin="10"
FontSize="Default"
HeightRequest="20"
HorizontalTextAlignment="Center"
Text="{Binding .}"
TextColor="RoyalBlue" />
</DataTemplate>
</CollectionView.ItemTemplate>
</CollectionView>
</Grid>

</ContentPage>
52 changes: 52 additions & 0 deletions src/Controls/tests/TestCases.HostApp/Issues/Issue21708.xaml.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
using System.Collections.ObjectModel;

namespace Maui.Controls.Sample.Issues
{
[XamlCompilation(XamlCompilationOptions.Compile)]
[Issue(IssueTracker.Github, 21708, "CollectionView.Scrolled event offset isn't correctly reset when items change", PlatformAffected.All)]
public partial class Issue21708 : ContentPage
{
ObservableCollection<int> _items;
double _verticalOffset;

public Issue21708()
{
InitializeComponent();
_items = new ObservableCollection<int>();
AddItemsToCollectionView();
CollectionView.ItemsSource = _items;
BindingContext = this;
}

public double VerticalOffset
{
get => _verticalOffset;
set
{
_verticalOffset = value;
OnPropertyChanged(nameof(VerticalOffset));
}
}

void CollectionView_OnScrolled(object sender, ItemsViewScrolledEventArgs e)
{
VerticalOffset = e.VerticalOffset;
}

void EmptyButton_OnClicked(object sender, EventArgs e)
{
_items.Clear();
}

void FillButton_OnClicked(object sender, EventArgs e)
{
AddItemsToCollectionView();
}

void AddItemsToCollectionView()
{
foreach (var i in Enumerable.Range(0, 50))
_items.Add(i);
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
using NUnit.Framework;
using UITest.Appium;
using UITest.Core;

namespace Microsoft.Maui.TestCases.Tests.Issues;

public class Issue21708 : _IssuesUITest
{
public Issue21708(TestDevice device) : base(device) { }

public override string Issue => "CollectionView.Scrolled event offset isn't correctly reset when items change";

[Test]
[Category(UITestCategories.CollectionView)]
public void VerifyCollectionViewVerticalOffset()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test is failing on Android:

OneTimeSetUp: System.TimeoutException : CollectionView.Scrolled event offset isn't correctly reset when items change

   at UITest.Appium.HelperExtensions.Wait(Func`1 query, Func`2 satisfactory, String timeoutMessage, Nullable`1 timeout, Nullable`1 retryFrequency) in /_/src/TestUtils/src/UITest.Appium/HelperExtensions.cs:line 2149
   at UITest.Appium.HelperExtensions.WaitForAtLeastOne(Func`1 query, String timeoutMessage, Nullable`1 timeout, Nullable`1 retryFrequency) in /_/src/TestUtils/src/UITest.Appium/HelperExtensions.cs:line 2166
   at Microsoft.Maui.TestCases.Tests._IssuesUITest.NavigateToIssue(String issue) in /_/src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/_IssuesUITest.cs:line 52
   at Microsoft.Maui.TestCases.Tests._IssuesUITest.FixtureSetup() in /_/src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/_IssuesUITest.cs:line 30
   at UITest.Appium.NUnit.UITestBase.OneTimeSetup() in /_/src/TestUtils/src/UITest.NUnit/UITestBase.cs:line 111
   at System.RuntimeMethodHandle.InvokeMethod(Object target, Void** arguments, Signature sig, Boolean isConstructor)
   at System.Reflection.MethodBaseInvoker.InvokeWithNoArgs(Object obj, BindingFlags invokeAttr)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jsuarezruiz, I ran the test on my local machine, and it passed successfully.

image

{
App.WaitForElement("Fill");
App.ScrollDown("CollectionView");
Assert.That(App.FindElement("Label").GetText(), Is.GreaterThan("0"));
App.Tap("Empty");
Assert.That(App.FindElement("Label").GetText(), Is.EqualTo("0"));
}
}