Skip to content

Fix RemainingItemsThresholdReachedCommand not firing when CollectionVew has Header and Footer both defined #29618

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

Open
wants to merge 12 commits into
base: main
Choose a base branch
from
Open
Prev Previous commit
Next Next commit
Update RecyclerViewScrollListener.cs
  • Loading branch information
SuthiYuvaraj committed May 22, 2025
commit 7c90e92c6d5be8e84183b3ee9723c7f0ddfacafd
Original file line number Diff line number Diff line change
Expand Up @@ -57,14 +57,14 @@ public override void OnScrolled(RecyclerView recyclerView, int dx, int dy)

// Early returns for edge cases
if (Last == -1 || ItemsViewAdapter == null || _itemsView.RemainingItemsThreshold == -1)
{
return;
}

var itemsSourceCount = ItemsViewAdapter.ItemCount;
bool hasHeader = ItemsViewAdapter.ItemsSource.HasHeader;
bool hasFooter = ItemsViewAdapter.ItemsSource.HasFooter;

// Adjust the last visible item index to match the ItemsSource index (excluding header/footer)
// Range of actual data item indices in adapter
int firstDataItemIndex = (hasHeader && hasFooter) ? 1 : 0;
Copy link
Contributor

Choose a reason for hiding this comment

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

With the changes, firstDataItemIndex is set to 1 only when both Header and Footer exist. However, if only Header is present, the value should be 1 too, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes @jsuarezruiz , firstDataItemIndex should be set to 1 whenever only the header is present. Since the header and footer affect indexing independently, they should be evaluated separately.

I've updated the logic to reflect this and optimized the code for improved clarity and efficiency. Please review the modified fix and let me know if any further changes are required.

int lastDataItemIndex = itemsSourceCount - firstDataItemIndex;

Expand All @@ -76,13 +76,21 @@ public override void OnScrolled(RecyclerView recyclerView, int dx, int dy)
switch (_itemsView.RemainingItemsThreshold)
Copy link
Contributor

Choose a reason for hiding this comment

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

Here, could simplify the logic by consolidating checks:

bool isThresholdReached = (Last == lastDataItemIndex - 1) || (lastDataItemIndex - 1 - Last <= _itemsView.RemainingItemsThreshold);

if (isThresholdReached)
{
    _itemsView.SendRemainingItemsThresholdReached();
}

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've optimized the logic for improved clarity and efficiency. Please review the modified fix and let me know if any further changes are required.

{
case 0:
if (Last == lastDataItemIndex - 1)
_itemsView.SendRemainingItemsThresholdReached();
break;
{
if (Last == lastDataItemIndex - 1)
{
_itemsView.SendRemainingItemsThresholdReached();
}
break;
}
default:
if (lastDataItemIndex - 1 - Last <= _itemsView.RemainingItemsThreshold)
_itemsView.SendRemainingItemsThresholdReached();
break;
{
if (lastDataItemIndex - 1 - Last <= _itemsView.RemainingItemsThreshold)
{
_itemsView.SendRemainingItemsThresholdReached();
}
break;
}
}
}

Expand Down
Loading