-
Notifications
You must be signed in to change notification settings - Fork 7
fix: Remove Pagination for Global Filtering & Sorting #341
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
WalkthroughThis pull request reintroduces and restructures pagination functionality in the Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant V as view-requests.svelte
U->>V: Click on next/previous page button
V->>V: Execute goToPage(newPage)
V->>V: Update currentPage variable
V->>V: Recalculate paginatedRequests (slice from processedRequests)
V->>U: Render updated request list for the new page
Possibly related PRs
Suggested Reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 1
🧹 Nitpick comments (2)
packages/invoice-dashboard/src/lib/view-requests.svelte (2)
212-216
: Unused parameter ingetRequestsQueryKey
The function signature includes
currentPage
as a parameter, but this parameter is not used in the returned array.- const getRequestsQueryKey = (address: string, currentPage: number) => [ + const getRequestsQueryKey = (address: string) => [ "requestsData", address, - currentPage, ];
473-474
: Consider more targeted query invalidation.The current implementation invalidates all queries when the decryption state changes, which might be broader than necessary.
if (localStorage?.getItem("isDecryptionEnabled") === "false") { - queryClient.invalidateQueries(); + queryClient.invalidateQueries({ queryKey: ["requestsData"] }); } if (localStorage?.getItem("isDecryptionEnabled") === "true") { - queryClient.invalidateQueries(); + queryClient.invalidateQueries({ queryKey: ["requestsData"] }); }Also applies to: 498-498
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/invoice-dashboard/src/lib/view-requests.svelte
(16 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build
🔇 Additional comments (8)
packages/invoice-dashboard/src/lib/view-requests.svelte (8)
136-139
: Good addition of pagination variables.The addition of these pagination variables is well-structured and follows best practices for implementation.
141-145
: Reactive pagination logic is well implemented.The reactive statements for calculating
totalPages
andpaginatedRequests
are elegantly implemented. Using Svelte's reactive declarations provides an efficient way to derive the pagination state from the processed requests.
148-152
: Pagination navigation function is robust.The
goToPage
function appropriately validates the requested page number before updating the state, preventing navigation to invalid pages.
218-226
: Function signature simplified correctly to fetch all data at once.The
fetchRequests
function has been correctly simplified to remove pagination parameters, which aligns with the PR objective of fetching all requests upfront to enable global filtering and sorting.
241-249
: Complete data fetching and sorting implementation.The query key has been simplified, and the full dataset is now sorted by timestamp after fetching. This approach properly enables global sorting and filtering as intended by the PR.
771-772
: Updated table to use paginated data.The table now correctly iterates over
paginatedRequests
instead ofprocessedRequests
, which ensures only the current page of data is rendered.
918-940
: Well-designed pagination controls with conditional rendering.The pagination controls are only displayed when there's more than one page, which is a good UX decision. The implementation includes intuitive navigation buttons with proper disabled states and clear page information.
1154-1186
: Clean and responsive pagination styling.The CSS for pagination is well-structured with appropriate transitions, hover states, and disabled styles. The design is clean and consistent with the overall UI.
Fixes: #335
Problem
The current implementation of paginated decryption is incompatible with filtering and sorting. Since only a subset of requests is decrypted at a time, users are unable to apply global filters and sorts across all their requests. This leads to an inconsistent user experience where expected results are not fully visible.
Changes
Summary by CodeRabbit