-
Notifications
You must be signed in to change notification settings - Fork 10.5k
Fix PaginationState UI not refreshing on filtered count reduction (#58487) #64735
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 PaginationState UI not refreshing on filtered count reduction (#58487) #64735
Conversation
…tnet#58487) The PaginationState component failed to raise TotalItemCountChanged when the total item count was reduced in a way that also required a page index reset (e.g., filtering 100 items down to 5 items while on page 10). The internal logic exited early because the count technically remained unchanged during the recursive call, causing the UI to remain stale. This fix introduces a _hasPendingTotalItemCountChangedEvent flag to track this state and ensure the event is raised after the recursive page reset. Credit to @htmlsplash for the original solution proposal in the issue. Co-authored-by: Anna Eklund <[email protected]> Co-authored-by: Diana Todorova <[email protected]> Co-authored-by: Nina Eriksson <[email protected]>
@dotnet-policy-service agree |
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.
Pull request overview
This PR fixes a race condition in PaginationState where the TotalItemCountChanged event was not raised when filtered data caused a page index reset. The bug occurred when users were on a high page number (e.g., page 10) and a filter reduced the total item count, requiring a page reset. The recursive call to SetTotalItemCountAsync would see the count was already set and exit early without raising the event, leaving the UI displaying a phantom page state.
Key Changes:
- Added a
_hasPendingTotalItemCountChangedEventflag to track when a page reset triggers a recursive call - Extracted event raising logic into a new
RaiseTotalItemCountChangedAsynchelper method - Added comprehensive test suite to verify the fix and related pagination behaviors
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
src/Components/QuickGrid/Microsoft.AspNetCore.Components.QuickGrid/src/Pagination/PaginationState.cs |
Implements the fix using a flag-based approach to ensure TotalItemCountChanged fires even when recursive calls have the same total count; refactors event raising into a helper method |
src/Components/QuickGrid/Microsoft.AspNetCore.Components.QuickGrid/test/PaginationStateTest.cs |
Adds new test suite with regression test for the bug and additional tests for page reset behavior, LastPageIndex calculation, event firing, and page preservation scenarios |
src/Components/QuickGrid/Microsoft.AspNetCore.Components.QuickGrid/test/PaginationStateTest.cs
Outdated
Show resolved
Hide resolved
…Grid/test/PaginationStateTest.cs Co-authored-by: Copilot <[email protected]>
|
Thanks for submitting a PR. For us to be able to consider taking this change, it should include E2E test coverage in addition to unit tests. |
Description
Fixes #58487
Addresses a race condition in
PaginationStatewhereTotalItemCountChangedwas not raised when the item count reduction triggered a page index reset.Bug Logic
SetTotalItemCountAsync(5)detects invalid page -> callsSetCurrentPageIndexAsync(0).SetCurrentPageIndexAsynctriggers a data refresh, callingSetTotalItemCountAsync(5)again.TotalItemCountis already 5, so it exits early without raising the event.Fix
Added a
_hasPendingTotalItemCountChangedEventflag.Acknowledgements
Special thanks to @htmlsplash for identifying the root cause and proposing the solution in the issue discussion.
Verification
Added PaginationTests suite with regression test for the bug where TotalItemCountChanged fails to fire after page reset when the subsequent re-query returns the same total count. Additional tests cover page reset behavior, LastPageIndex calculation, event firing, and page preservation scenarios.