Skip to content

Conversation

@cain4chain
Copy link
Contributor

@cain4chain cain4chain commented Mar 6, 2025

feat(pagination): implement server-side pagination for contacts
feat(pagination): add server-side pagination for paymails
feat(pagination): implement server-side pagination for access keys
feat(pagination): add server-side pagination for transactions
feat(pagination): implement server-side pagination for xpubs
feat(hooks): create usePagination hook for centralized pagination logic
feat(hooks): implement useTable hook for table state management
feat(utils): add query options with pagination support for API calls
fix(pagination): ensure consistent page index conversion between UI and API
feat(components): add pagination support to mobile data tables

Pull Request Checklist

  • 📖 I created my PR using provided : CODE_STANDARDS
  • 📖 I have read the short Code of Conduct: CODE_OF_CONDUCT
  • 🏠 I tested my changes locally.
  • ✅ I have provided tests for my changes.
  • 📝 I have used conventional commits.
  • 📗 I have updated any related documentation.
  • 💾 PR was issued based on the Github or Jira issue.

feat(pagination): implement server-side pagination for contacts
feat(pagination): add server-side pagination for paymails
feat(pagination): implement server-side pagination for access keys
feat(pagination): add server-side pagination for transactions
feat(pagination): implement server-side pagination for xpubs
feat(hooks): create usePagination hook for centralized pagination logic
feat(hooks): implement useTable hook for table state management
feat(utils): add query options with pagination support for API calls
fix(pagination): ensure consistent page index conversion between UI and API
feat(components): add pagination support to mobile data tables
@github-actions
Copy link

github-actions bot commented Mar 6, 2025

Manual Tests

ℹ️ Remember to ask team members to perform manual tests and to assign tested label after testing.

feat(Pagination) : Update pagination types and constants
feat(usePagination) : Common hook for useRoutePagination
@cain4chain cain4chain requested a review from chris-4chain March 7, 2025 12:23
original: accessKey,
getValue: (key: string) => (key === 'status' ? accessKey.status : undefined),
} as Row<AccessKeysColumns>;
} as unknown as Row<AccessKey>;
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this necessary ? (The unknown casting)

| ContactExtended
| PaymailExtended
| XpubExtended
| { id?: string; url?: string; status?: string };
Copy link
Contributor

Choose a reason for hiding this comment

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

Consider making a type for it to be consistent and to describe (by type name) what it is.

useEffect(() => {
setExpandedItems([]);
setIsAllExpanded(false);
}, [table.getState().pagination.pageIndex, table.getState().pagination.pageSize]);
Copy link
Contributor

Choose a reason for hiding this comment

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

You could make a local variable (constant) for the pagination state.

const total = manualPagination && totalRecords !== undefined ? totalRecords : table.getFilteredRowModel().rows.length;

// Track the current page size in local state to ensure the UI stays in sync
const [currentPageSize, setCurrentPageSize] = useState<number>(table.getState().pagination.pageSize);
Copy link
Contributor

Choose a reason for hiding this comment

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

You could also make a local const variable for table.getState().pagination

Comment on lines +33 to +46
// Initialize currentPageSize on mount and when table changes
useEffect(() => {
setCurrentPageSize(table.getState().pagination.pageSize);
}, [table]);

// Update local state when table pagination state changes
useEffect(() => {
// Always update the local state to match the table's page size
setCurrentPageSize(table.getState().pagination.pageSize);
}, [table.getState().pagination.pageSize]);

useEffect(() => {
setCurrentPageSize(table.getState().pagination.pageSize);
}, [table.getState().pagination]);
Copy link
Contributor

Choose a reason for hiding this comment

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

It can be one useEffect with those three in one dependency list.

Comment on lines +86 to +88
export function useRoutePagination<TRouter extends AnyRouter = RegisteredRouter, const TFrom extends string = string>(
routeID: ConstrainLiteral<TFrom, RouteIds<TRouter['routeTree']>>,
) {
Copy link
Contributor

Choose a reason for hiding this comment

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

🚀 👍

Comment on lines +89 to +107
// Local filter state for input field
const [filter, setFilter] = useState<string>(xpubId || '');
const [debouncedFilter] = useDebounce(filter, 500);

// Update URL when the debounced filter changes
useEffect(() => {
navigate({
search: (old) => ({
...old,
xpubId: debouncedFilter || undefined,
}),
replace: true,
});
}, [debouncedFilter, navigate]);

// Sync filter input when URL xpubId changes
useEffect(() => {
setFilter(xpubId || '');
}, [xpubId]);
Copy link
Contributor

Choose a reason for hiding this comment

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

Since my last PR, the Search component does "debouncing". I think, useSearchParam hook should be used here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants