-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
base: main
Are you sure you want to change the base?
Conversation
/azp run MAUI-UITests-public |
Azure Pipelines successfully started running 1 pipeline(s). |
bool hasHeader = ItemsViewAdapter.ItemsSource.HasHeader; | ||
bool hasFooter = ItemsViewAdapter.ItemsSource.HasFooter; | ||
|
||
int firstDataItemIndex = (hasHeader && hasFooter) ? 1 : 0; |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
return; | ||
} | ||
|
||
switch (_itemsView.RemainingItemsThreshold) |
There was a problem hiding this comment.
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();
}
There was a problem hiding this comment.
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.
src/Controls/src/Core/Handlers/Items/Android/RecyclerViewScrollListener.cs
Outdated
Show resolved
Hide resolved
using System.Windows.Input; | ||
using Microsoft.Maui.Controls; | ||
|
||
namespace Microsoft.Maui.Controls.Issues |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
super nit: File-scope namespace can be used here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@MartyIX , Thanks for the feedback , I have updated the code changes, let me know if any further changes are required.
…lListener.cs Co-authored-by: MartyIX <[email protected]>
/azp run MAUI-UITests-public |
Azure Pipelines successfully started running 1 pipeline(s). |
Note
Are you waiting for the changes in this PR to be merged?
It would be very helpful if you could test the resulting artifacts from this PR and let us know in a comment if this change resolves your issue. Thank you!
Issue Description
In Android, the
RemainingThresholdReached
command is not triggered when both aHeader
and aFooter
are defined for the collection.RootCause
The
SendRemainingThresholdReached
method is triggered based on theLastVisibleIndex
and theItemCount
provided by the ItemAdapter. However, on Android, when both a Header and a Footer are defined, the ItemCount consider it, leading to incorrect triggering logic.Description of Change
Updated the logic to ensure that the ItemCount correctly includes both the Header and Footer. This change ensures that the Footer becomes visible as expected and that the SendRemainingThresholdReached command is properly executed.
Issues Fixed
Fixes #29588
Tested the behaviour in the following platforms
Output Screenshot
RemainingThresholdBefore.mov
RemainingThresholdAfter.mov