IncrementalLoadingCollection.LoadMoreItemsAsync(unit count) ignores count.

[Rosuavio]

Describe the bug

IncrementalLoadingCollection.LoadMoreItemsAsync(unit count) ignores count in its implementation. This breaks the contract of the ISupportIncrementalLoading.LoadMoreItemsAsync(unit count) interface

Instead of loading the requested amount of items, we load a single group of items starting at IncrementalLoadingCollection.CurrentPageIndex of size IncrementalLoadingCollection.ItemsPerPage

• [ ] Is this bug a regression in the toolkit? If so, what toolkit version did you last see it work:

Steps to Reproduce

• [ ] Can this be reproduced in the Sample App? (Either in a sample as-is or with new XAML pasted in the editor.) If so, please provide custom XAML or steps to reproduce. If not, let us know why it can't be reproduced (e.g. more complex setup, environment, dependencies, etc...)

Steps to reproduce the behavior:

1. Make a uwp project with these files.

   <Page
   x:Class="Sample.MainPage"
   xmlns="http://schemas.microsoft.com/winfx/2006/xaml/presentation"
   xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml"
   xmlns:d="http://schemas.microsoft.com/expression/blend/2008"
   xmlns:mc="http://schemas.openxmlformats.org/markup-compatibility/2006"
   Background="{ThemeResource ApplicationPageBackgroundThemeBrush}"
   mc:Ignorable="d">
   <Grid>
       <Grid.RowDefinitions>
           <RowDefinition Height="Auto" />
           <RowDefinition Height="900" />
       </Grid.RowDefinitions>
       <Button Click="Button_Click" Content="Test" />
       <ListView
           x:Name="ListView"
           Grid.Row="1"
           DataFetchSize="3"
           IncrementalLoadingTrigger="None">
           <ListView.ItemTemplate>
               <DataTemplate>
                   <TextBlock Text="{Binding}" />
               </DataTemplate>
           </ListView.ItemTemplate>
       </ListView>
   </Grid>
   </Page>

using Microsoft.Toolkit.Uwp;
using System;
using Windows.UI.Xaml.Controls;

// The Blank Page item template is documented at https://go.microsoft.com/fwlink/?LinkId=402352&clcid=0x409

namespace Sample
{
    /// <summary>
    /// An empty page that can be used on its own or navigated to within a Frame.
    /// </summary>
    public sealed partial class MainPage : Page
    {
        public MainPage()
        {
            this.InitializeComponent();

            var collection = new IncrementalLoadingCollection<Source, int>(new Source());

            ListView.ItemsSource = collection;
        }

        private async void Button_Click(object sender, Windows.UI.Xaml.RoutedEventArgs e)
        {
            await ListView.LoadMoreItemsAsync();
        }
    }
}

using Microsoft.Toolkit.Collections;
using System.Collections.Generic;
using System.Linq;
using System.Threading;
using System.Threading.Tasks;

namespace Sample
{
    public class Source : IIncrementalSource<int>
    {
        private readonly IEnumerable<int> numbers = Enumerable.Range(0, int.MaxValue);
        public Task<IEnumerable<int>> GetPagedItemsAsync(int pageIndex, int pageSize, CancellationToken cancellationToken = default)
        {
            var result = (from p in numbers
                          select p).Skip(pageIndex * pageSize).Take(pageSize);

            return Task.FromResult(result);
        }
    }
}

2. Click the "Test" button.
3. See not three items load.

Expected behavior

Three items load into the collection every time the "Test" button is clicked

Screenshots

Environment

NuGet Package(s): 

Package Version(s): 

Windows 10 Build Number:
- [ ] Fall Creators Update (16299)
- [ ] April 2018 Update (17134)
- [ ] October 2018 Update (17763)
- [ ] May 2019 Update (18362)
- [ ] May 2020 Update (19041)
- [ ] Insider Build (build number: )

App min and target version:
- [ ] Fall Creators Update (16299)
- [ ] April 2018 Update (17134)
- [ ] October 2018 Update (17763)
- [ ] May 2019 Update (18362)
- [ ] May 2020 Update (19041)
- [ ] Insider Build (xxxxx)

Device form factor:
- [ ] Desktop
- [ ] Xbox
- [ ] Surface Hub
- [ ] IoT

Visual Studio 
- [ ] 2017 (version: )
- [ ] 2019 (version: ) 
- [ ] 2019 Preview (version: )

Additional context

Add any other context about the problem here.

[ghost]

Hello RosarioPulella, thank you for opening an issue with us!

I have automatically added a "needs triage" label to help get things started. Our team will analyze and investigate the issue, and escalate it to the relevant team if possible. Other community members may also look into the issue and provide feedback 🙌

[Rosuavio]

Wired issue, we take a pageSize in the ctor, but also we should be respecting DataFetchSize from the ListView that we get threw IncrementalLoadingCollection.LoadMoreItemsAsync(unit count).

I can identify three options to resolve this.

1. Drop the pageSize in the ctor only use the requested count per request.
2. Continue to ignore the the count per request. Document it and make the current behavior clearer in Docs or where we can.
3. Use the pageSize as a max size for each request and fetch the smaller amount of items, count or pageSize , per request.

[michael-hawker]

@RosarioPulella when we see it being called from the ListView, do we know what the count value normally seems to come through as?

[Rosuavio]

I seem to have made the incorrect assumption that the DataFetchSize on the ListView is directly used. I modified the sample app to have a DataFetchSize and set up a button to trigger loading more items and I was getting very unexpected numbers coming threw for the count on IAsyncOperation<LoadMoreItemsResult> LoadMoreItemsAsync(uint count).

With DataFetchSize set to 5 I the count would be 1 on the first request for more items and 57 for every consecutive one. With DataFetchSize set to 1 I the count would be 1 on the first request for more items and 11 for every consecutive one. With DataFetchSize set to 20 I the count would be 1 on the first request for more items and 229 for every consecutive one.

After reading the docs more closely I see I misunderstood DataFetchSize "The amount of data to fetch per interval, in pages"

[Rosuavio]

It seems like any notion of a "Page Size" is actually calculated from the amount of the items that can be visible on the list at a time. So that count might come from "Page Size" * DataFetchSize. I am not sure why the first request comes threw with 1 regardless.

[michael-hawker]

Yeah, the first request always being 1 seems a bit odd. Maybe something we should file an issue about on the WinUI repo?

https://github.com/windows-toolkit/WindowsCommunityToolkit/blob/d1f334a946396d97e7e17b793d589b8bc75f706c/Microsoft.Toolkit/IncrementalLoadingCollection/IIncrementalSource.cs#L32

So the pageSize is from IIncrementalSource, but all we're really doing is passing the ItemsPerPage (default 20 from our constructor) of the IncrementalLoadingCollection to the source. So ItemsPerPage is really just a configuration value that the implementer can provide to their own data source.

https://github.com/windows-toolkit/WindowsCommunityToolkit/blob/d1f334a946396d97e7e17b793d589b8bc75f706c/Microsoft.Toolkit.Uwp/IncrementalLoadingCollection/IncrementalLoadingCollection.cs#L164-L177

We just also use that value to pass-thru in LoadDataAsync:

https://github.com/windows-toolkit/WindowsCommunityToolkit/blob/d1f334a946396d97e7e17b793d589b8bc75f706c/Microsoft.Toolkit.Uwp/IncrementalLoadingCollection/IncrementalLoadingCollection.cs#L229-L231

So, I almost feel like we need to define if we take the greater (or lesser) of count vs ItemsPerPage. Maybe that's another setting? Right, do you want to limit your collection to only load a certain amount of items regardless of what the OS is requesting OR do you want to ensure there's a minimum loaded at least but potentially load all the things being requested...

Right?? @azchohfi @vgromfeld @Sergio0694 any thoughts on these scenarios here?