fix: pagination visibility based on totalCount #929
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jeffredodd
force-pushed
the
jdj/payroll-total-count-fix
branch
from
c59a8a5 to
4ef041d
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes pagination control visibility by basing it on the total item count rather than the number of pages. When users set a page size larger than the total number of items, pagination controls now appropriately hide. The PR also removes the unused usePagination hook and its tests.
Key changes:
- Implemented new visibility logic for
PaginationControlbased ontotalCount - Updated list components to pass
totalCountto pagination controls - Removed unused
usePaginationhook and associated tests
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/components/Common/PaginationControl/PaginationControl.tsx | Added visibility logic using totalCount instead of totalPages |
| src/components/Common/PaginationControl/PaginationControlTypes.ts | Added optional totalCount prop to PaginationControlProps |
| src/components/Common/PaginationControl/PaginationControl.test.tsx | Added comprehensive tests for pagination visibility scenarios |
| src/components/Employee/EmployeeList/EmployeeList.tsx | Extracted and passed totalCount from response headers |
| src/components/Employee/EmployeeList/List.tsx | Forwarded totalCount to pagination control |
| src/components/Employee/EmployeeList/useEmployeeList.ts | Added totalCount to context type |
| src/components/Contractor/ContractorList/index.tsx | Added totalCount to pagination props |
| src/components/Company/Locations/LocationsList/LocationsList.tsx | Extracted and passed totalCount from response headers |
| src/components/Company/Locations/LocationsList/List.tsx | Forwarded totalCount to pagination control |
| src/components/Company/Locations/LocationsList/useLocationsList.ts | Added totalCount to context type |
| src/components/Payroll/PayrollConfiguration/PayrollConfiguration.tsx | Extracted and passed totalCount from response headers |
| src/hooks/usePagination/usePagination.ts | Removed unused hook |
| src/hooks/usePagination/usePagination.test.ts | Removed tests for unused hook |
| src/hooks/usePagination/index.ts | Removed export file for unused hook |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
jeffredodd
requested review from
dmortal,
krisxcrash,
serikjensen and
shermango
serikjensen
approved these changes
Jan 8, 2026
Comment on lines
+21
to
+27
| return render( | ||
| <ThemeProvider> | ||
| <ComponentsProvider value={defaultComponents}> | ||
| <PaginationControl {...basePaginationProps} {...props} /> | ||
| </ComponentsProvider> | ||
| </ThemeProvider>, | ||
| ) |
Member
There was a problem hiding this comment.
Recommend to update this to renderWithProviders instead, that'll give you the needed providers out of the box
- Add totalCount prop to PaginationControlProps - Update PaginationControl to show/hide based on total items: - Hide when totalCount is 0 (empty state) - Hide when totalCount <= 5 (minimum page size) - Show when totalCount > 5 (user can adjust page size) - Show when totalCount undefined (fallback) - Pass totalCount from x-total-count header in all list components - Remove unused usePagination hook
jeffredodd
force-pushed
the
jdj/payroll-total-count-fix
branch
from
4ef041d to
e2482d5
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Fixes payroll pagination behavior based on test fest feedback where pagination options would disappear when setting a per-page amount larger than the number of people on the payroll.
Changes
PaginationControl visibility logic:
totalCountis 0 (empty state)totalCount<= 5 (minimum page size - no pagination needed)totalCount> 5 (user may want to adjust page size)totalCountis undefined (fallback to always showing when server doesn't provide count)Updates to list components:
totalCountfromx-total-countheader to PaginationControl in:Cleanup:
usePaginationhookTesting
Added unit tests for PaginationControl visibility logic covering all scenarios.