-
Notifications
You must be signed in to change notification settings - Fork 620
[Docs] Add search and pagination to wallet list #7703
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
[Docs] Add search and pagination to wallet list #7703
Conversation
…onent Co-authored-by: joaquim.verges <[email protected]>
|
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThe Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant AllSupportedWallets (Client)
participant WalletAPI
User->>AllSupportedWallets (Client): Mount component
AllSupportedWallets (Client)->>WalletAPI: Fetch wallet list (useQuery)
WalletAPI-->>AllSupportedWallets (Client): Return wallet list
AllSupportedWallets (Client)->>AllSupportedWallets (Client): Store wallets in state
User->>AllSupportedWallets (Client): Enter search query
AllSupportedWallets (Client)->>AllSupportedWallets (Client): Filter wallets, reset pagination
User->>AllSupportedWallets (Client): Change page
AllSupportedWallets (Client)->>AllSupportedWallets (Client): Update current page
AllSupportedWallets (Client)->>WalletAPI: Fetch wallet image (useQuery)
WalletAPI-->>AllSupportedWallets (Client): Return wallet image
AllSupportedWallets (Client)-->>User: Display filtered, paginated wallets with images
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (1)
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
How to use the Graphite Merge QueueAdd either label to this PR to merge it via the merge queue:
You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has enabled the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #7703 +/- ##
==========================================
- Coverage 56.31% 56.26% -0.06%
==========================================
Files 908 908
Lines 58177 58177
Branches 4207 4208 +1
==========================================
- Hits 32765 32731 -34
- Misses 25302 25337 +35
+ Partials 110 109 -1
🚀 New features to boost your workflow:
|
size-limit report 📦
|
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: 0
🧹 Nitpick comments (1)
apps/portal/src/components/others/AllSupportedWallets.tsx (1)
141-141: Consider improving type safety.The type assertion
as WalletIdcould be avoided ifWalletInfo.idwas typed asWalletIdinstead ofstring. This would provide better compile-time type safety.interface WalletInfo { - id: string; + id: WalletId; name: string; }Then remove the type assertion:
-<WalletImage id={wallet.id as WalletId} /> +<WalletImage id={wallet.id} />
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/portal/src/components/others/AllSupportedWallets.tsx(2 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (CLAUDE.md)
**/*.{ts,tsx}: Write idiomatic TypeScript with explicit function declarations and return types
Limit each file to one stateless, single-responsibility function for clarity
Re-use shared types from@/typesor localtypes.tsbarrels
Prefer type aliases over interface except for nominal shapes
Avoidanyandunknownunless unavoidable; narrow generics when possible
Choose composition over inheritance; leverage utility types (Partial,Pick, etc.)
Comment only ambiguous logic; avoid restating TypeScript in prose
Files:
apps/portal/src/components/others/AllSupportedWallets.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit Inference Engine (CLAUDE.md)
Load heavy dependencies inside async paths to keep initial bundle lean (lazy loading)
Files:
apps/portal/src/components/others/AllSupportedWallets.tsx
🧠 Learnings (2)
📓 Common learnings
Learnt from: CR
PR: thirdweb-dev/js#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-18T19:19:55.613Z
Learning: Applies to packages/thirdweb/src/wallets/** : EIP-1193, EIP-5792, EIP-7702 standard support in wallet modules
Learnt from: CR
PR: thirdweb-dev/js#0
File: .cursor/rules/dashboard.mdc:0-0
Timestamp: 2025-07-18T19:20:32.530Z
Learning: Applies to dashboard/**/*client.tsx : Interactive UI that relies on hooks (`useState`, `useEffect`, React Query, wallet hooks).
Learnt from: CR
PR: thirdweb-dev/js#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-18T19:19:55.613Z
Learning: Applies to packages/thirdweb/src/wallets/** : Support for in-app wallets (social/email login)
Learnt from: CR
PR: thirdweb-dev/js#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-18T19:19:55.613Z
Learning: Applies to packages/thirdweb/src/wallets/** : Unified `Wallet` and `Account` interfaces in wallet architecture
Learnt from: CR
PR: thirdweb-dev/js#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-18T19:19:55.613Z
Learning: Surface breaking changes prominently in PR descriptions
Learnt from: CR
PR: thirdweb-dev/js#0
File: .cursor/rules/dashboard.mdc:0-0
Timestamp: 2025-07-18T19:20:32.530Z
Learning: Applies to dashboard/**/*.{tsx,jsx} : Local state or effects live inside; data fetching happens in hooks.
Learnt from: CR
PR: thirdweb-dev/js#0
File: .cursor/rules/dashboard.mdc:0-0
Timestamp: 2025-07-18T19:20:32.530Z
Learning: Applies to dashboard/**/*client.tsx : Use React Query (`@tanstack/react-query`) for all client data fetching.
Learnt from: CR
PR: thirdweb-dev/js#0
File: .cursor/rules/dashboard.mdc:0-0
Timestamp: 2025-07-18T19:20:32.530Z
Learning: Applies to dashboard/**/*client.tsx : Pages requiring fast transitions where data is prefetched on the client.
Learnt from: CR
PR: thirdweb-dev/js#0
File: .cursor/rules/dashboard.mdc:0-0
Timestamp: 2025-07-18T19:20:32.530Z
Learning: Applies to dashboard/**/*.{tsx,jsx} : Keep components pure; fetch data outside (server component or hook) and pass it down via props.
Learnt from: CR
PR: thirdweb-dev/js#0
File: .cursor/rules/dashboard.mdc:0-0
Timestamp: 2025-07-18T19:20:32.530Z
Learning: Applies to dashboard/**/*client.tsx : Anything that consumes hooks from `@tanstack/react-query` or thirdweb SDKs.
Learnt from: CR
PR: thirdweb-dev/js#0
File: .cursor/rules/dashboard.mdc:0-0
Timestamp: 2025-07-18T19:20:32.530Z
Learning: Applies to dashboard/**/*client.tsx : Components that listen to user events, animations or live updates.
Learnt from: CR
PR: thirdweb-dev/js#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-18T19:19:55.613Z
Learning: Applies to apps/{dashboard,playground-web}/**/*.{ts,tsx} : Wrap client-side data fetching calls in React Query (`@tanstack/react-query`)
Learnt from: CR
PR: thirdweb-dev/js#0
File: .cursor/rules/dashboard.mdc:0-0
Timestamp: 2025-07-18T19:20:32.530Z
Learning: Applies to dashboard/**/*client.tsx : Create light wrappers (e.g. `fetchJson`) that automatically attach the JWT from cookies/session when calling internal API routes.
apps/portal/src/components/others/AllSupportedWallets.tsx (16)
Learnt from: CR
PR: thirdweb-dev/js#0
File: .cursor/rules/dashboard.mdc:0-0
Timestamp: 2025-07-18T19:20:32.530Z
Learning: Applies to dashboard/**/*client.tsx : Interactive UI that relies on hooks (useState, useEffect, React Query, wallet hooks).
Learnt from: CR
PR: thirdweb-dev/js#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-18T19:19:55.613Z
Learning: Applies to packages/thirdweb/src/wallets/** : Unified Wallet and Account interfaces in wallet architecture
Learnt from: CR
PR: thirdweb-dev/js#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-18T19:19:55.613Z
Learning: Applies to packages/thirdweb/src/wallets/** : EIP-1193, EIP-5792, EIP-7702 standard support in wallet modules
Learnt from: CR
PR: thirdweb-dev/js#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-18T19:19:55.613Z
Learning: Applies to packages/thirdweb/src/wallets/** : Support for in-app wallets (social/email login)
Learnt from: CR
PR: thirdweb-dev/js#0
File: .cursor/rules/dashboard.mdc:0-0
Timestamp: 2025-07-18T19:20:32.530Z
Learning: Applies to dashboard/**/*client.tsx : Pages requiring fast transitions where data is prefetched on the client.
Learnt from: CR
PR: thirdweb-dev/js#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-18T19:19:55.613Z
Learning: Applies to test/src/test-wallets.ts : Predefined test accounts are in test/src/test-wallets.ts
Learnt from: CR
PR: thirdweb-dev/js#0
File: .cursor/rules/dashboard.mdc:0-0
Timestamp: 2025-07-18T19:20:32.530Z
Learning: Applies to dashboard/**/*.{tsx,jsx} : Local state or effects live inside; data fetching happens in hooks.
Learnt from: CR
PR: thirdweb-dev/js#0
File: .cursor/rules/dashboard.mdc:0-0
Timestamp: 2025-07-18T19:20:32.530Z
Learning: Applies to dashboard/**/*client.tsx : Anything that consumes hooks from @tanstack/react-query or thirdweb SDKs.
Learnt from: CR
PR: thirdweb-dev/js#0
File: .cursor/rules/dashboard.mdc:0-0
Timestamp: 2025-07-18T19:20:32.530Z
Learning: Applies to dashboard/**/*client.tsx : Use React Query (@tanstack/react-query) for all client data fetching.
Learnt from: CR
PR: thirdweb-dev/js#0
File: .cursor/rules/dashboard.mdc:0-0
Timestamp: 2025-07-18T19:20:32.530Z
Learning: Applies to dashboard/**/*.{tsx,jsx} : Keep components pure; fetch data outside (server component or hook) and pass it down via props.
Learnt from: CR
PR: thirdweb-dev/js#0
File: .cursor/rules/dashboard.mdc:0-0
Timestamp: 2025-07-18T19:20:32.530Z
Learning: Applies to dashboard/**/*client.tsx : When you need access to browser APIs (localStorage, window, IntersectionObserver etc.).
Learnt from: CR
PR: thirdweb-dev/js#0
File: .cursor/rules/dashboard.mdc:0-0
Timestamp: 2025-07-18T19:20:32.530Z
Learning: Applies to dashboard/**/components/*.client.tsx : Client components must start with 'use client'; before imports.
Learnt from: CR
PR: thirdweb-dev/js#0
File: .cursor/rules/dashboard.mdc:0-0
Timestamp: 2025-07-18T19:20:32.530Z
Learning: Applies to dashboard/**/*client.tsx : Components that listen to user events, animations or live updates.
Learnt from: CR
PR: thirdweb-dev/js#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-18T19:19:55.613Z
Learning: Applies to apps/{dashboard,playground-web}/**/*.{ts,tsx} : Client Components (browser): Begin files with 'use client';
Learnt from: MananTank
PR: #7227
File: apps/dashboard/src/app/(app)/(dashboard)/(chain)/[chain_id]/[contractAddress]/modules/components/OpenEditionMetadata.tsx:26-26
Timestamp: 2025-05-30T17:14:25.332Z
Learning: The ModuleCardUIProps interface already includes a client prop of type ThirdwebClient, so when components use Omit<ModuleCardUIProps, "children" | "updateButton">, they inherit the client prop without needing to add it explicitly.
Learnt from: CR
PR: thirdweb-dev/js#0
File: .cursor/rules/dashboard.mdc:0-0
Timestamp: 2025-07-18T19:20:32.530Z
Learning: Applies to dashboard/**/*client.tsx : Create light wrappers (e.g. fetchJson) that automatically attach the JWT from cookies/session when calling internal API routes.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Size
- GitHub Check: Unit Tests
- GitHub Check: Build Packages
- GitHub Check: Analyze (javascript)
🔇 Additional comments (7)
apps/portal/src/components/others/AllSupportedWallets.tsx (7)
1-14: LGTM! Client component setup is correct.The conversion to a client component with appropriate imports for state management and UI components aligns with the learnings about client components requiring the
'use client'directive and using hooks for interactive UI.
23-28: Good type definition and constant naming.The
WalletInfointerface andITEMS_PER_PAGEconstant are well-defined and follow TypeScript best practices.
57-66: Excellent use of useMemo for search filtering.The filtering logic is well-implemented with proper memoization and case-insensitive search across both wallet name and ID.
68-88: Well-implemented pagination logic.The pagination calculations are correct, and the UX consideration to reset to the first page when search changes is a nice touch. The boundary checks in the navigation handlers prevent invalid page states.
90-96: Loading state handling looks good.The loading UI provides clear feedback. Note that if you adopt React Query as suggested earlier, you'd use
isLoadinginstead of theloadingstate.
179-203: Sophisticated pagination controls with good UX.The page number calculation logic elegantly handles showing 5 page buttons while keeping the current page centered when possible. The implementation correctly handles edge cases near the start and end of the page range.
36-55: Use React Query for data fetching instead of useEffect.According to the learnings, client components should use React Query (
@tanstack/react-query) for all data fetching. The current implementation uses a plainuseEffectwhich doesn't provide features like caching, background refetching, and error states.Additionally, consider showing an error state to users instead of only logging to console.
Here's a suggested refactor using React Query:
- const [wallets, setWallets] = useState<WalletInfo[]>([]); - const [loading, setLoading] = useState(true); + const { data: wallets = [], isLoading, error } = useQuery({ + queryKey: ['wallets'], + queryFn: async () => { + const allWallets = await getAllWalletsList(); + return allWallets + .filter((w) => !(w.id in specialWallets)) + .map((w) => ({ + id: w.id, + name: w.name, + })); + }, + }); - useEffect(() => { - async function loadWallets() { - try { - const allWallets = await getAllWalletsList(); - const filteredWallets = allWallets - .filter((w) => !(w.id in specialWallets)) - .map((w) => ({ - id: w.id, - name: w.name, - })); - setWallets(filteredWallets); - } catch (error) { - console.error("Failed to load wallets:", error); - } finally { - setLoading(false); - } - } - - loadWallets(); - }, []);Don't forget to wrap your component tree with
QueryClientProviderif not already done.⛔ Skipped due to learnings
Learnt from: CR PR: thirdweb-dev/js#0 File: .cursor/rules/dashboard.mdc:0-0 Timestamp: 2025-07-18T19:20:32.530Z Learning: Applies to dashboard/**/*client.tsx : Use React Query (`@tanstack/react-query`) for all client data fetching.Learnt from: CR PR: thirdweb-dev/js#0 File: .cursor/rules/dashboard.mdc:0-0 Timestamp: 2025-07-18T19:20:32.530Z Learning: Applies to dashboard/**/*client.tsx : Interactive UI that relies on hooks (`useState`, `useEffect`, React Query, wallet hooks).Learnt from: CR PR: thirdweb-dev/js#0 File: CLAUDE.md:0-0 Timestamp: 2025-07-18T19:19:55.613Z Learning: Applies to apps/{dashboard,playground-web}/**/*.{ts,tsx} : Wrap client-side data fetching calls in React Query (`@tanstack/react-query`)Learnt from: CR PR: thirdweb-dev/js#0 File: .cursor/rules/dashboard.mdc:0-0 Timestamp: 2025-07-18T19:20:32.530Z Learning: Applies to dashboard/**/*.{tsx,jsx} : Local state or effects live inside; data fetching happens in hooks.Learnt from: CR PR: thirdweb-dev/js#0 File: .cursor/rules/dashboard.mdc:0-0 Timestamp: 2025-07-18T19:20:32.530Z Learning: Applies to dashboard/**/*client.tsx : Anything that consumes hooks from `@tanstack/react-query` or thirdweb SDKs.Learnt from: CR PR: thirdweb-dev/js#0 File: .cursor/rules/dashboard.mdc:0-0 Timestamp: 2025-07-18T19:20:32.530Z Learning: Applies to dashboard/**/*.{tsx,jsx} : Keep components pure; fetch data outside (server component or hook) and pass it down via props.Learnt from: CR PR: thirdweb-dev/js#0 File: CLAUDE.md:0-0 Timestamp: 2025-07-18T19:19:55.613Z Learning: Applies to apps/{dashboard,playground-web}/**/*.{ts,tsx} : Use descriptive, stable `queryKeys` for React Query cache hitsLearnt from: CR PR: thirdweb-dev/js#0 File: .cursor/rules/dashboard.mdc:0-0 Timestamp: 2025-07-18T19:20:32.530Z Learning: Applies to dashboard/**/*client.tsx : Keep `queryKey` stable and descriptive for cache hits.Learnt from: CR PR: thirdweb-dev/js#0 File: .cursor/rules/dashboard.mdc:0-0 Timestamp: 2025-07-18T19:20:32.530Z Learning: Applies to dashboard/**/*client.tsx : Pages requiring fast transitions where data is prefetched on the client.Learnt from: CR PR: thirdweb-dev/js#0 File: CLAUDE.md:0-0 Timestamp: 2025-07-18T19:19:55.613Z Learning: Applies to apps/{dashboard,playground-web}/**/*.{ts,tsx} : Configure `staleTime`/`cacheTime` in React Query based on freshness (default ≥ 60s)Learnt from: CR PR: thirdweb-dev/js#0 File: CLAUDE.md:0-0 Timestamp: 2025-07-18T19:19:55.613Z Learning: Applies to test/src/test-wallets.ts : Predefined test accounts are in `test/src/test-wallets.ts`
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: 5
🧹 Nitpick comments (3)
apps/portal/src/components/others/AllSupportedWallets.tsx (3)
185-209: Extract complex pagination logic into a helper function.The page number calculation logic is complex and reduces readability. Consider extracting it for better maintainability.
Add this helper function before the component:
function getPageNumbers(currentPage: number, totalPages: number): number[] { if (totalPages <= 5) { return Array.from({ length: totalPages }, (_, i) => i + 1); } if (currentPage <= 3) { return [1, 2, 3, 4, 5]; } if (currentPage >= totalPages - 2) { return Array.from({ length: 5 }, (_, i) => totalPages - 4 + i); } return Array.from({ length: 5 }, (_, i) => currentPage - 2 + i); }Then simplify the render logic:
- {Array.from({ length: Math.min(5, totalPages) }, (_, i) => { - let pageNumber: number; - - if (totalPages <= 5) { - pageNumber = i + 1; - } else if (currentPage <= 3) { - pageNumber = i + 1; - } else if (currentPage >= totalPages - 2) { - pageNumber = totalPages - 4 + i; - } else { - pageNumber = currentPage - 2 + i; - } - - return ( + {getPageNumbers(currentPage, totalPages).map((pageNumber) => ( <Button key={pageNumber} variant={currentPage === pageNumber ? "default" : "outline"} size="sm" onClick={() => handlePageClick(pageNumber)} className="min-w-[32px]" > {pageNumber} </Button> - ); - })} + ))}
173-182: Add accessibility attributes to pagination buttons.The pagination buttons lack proper accessibility attributes for screen readers.
<Button variant="outline" size="sm" onClick={handlePreviousPage} disabled={currentPage === 1} + aria-label="Go to previous page" > <ChevronLeftIcon className="size-4" /> Previous </Button><Button variant="outline" size="sm" onClick={handleNextPage} disabled={currentPage === totalPages} + aria-label="Go to next page" > Next <ChevronRightIcon className="size-4" /> </Button>Also applies to: 212-221
231-231: Document the boolean parameter in getWalletInfo call.The
trueparameter passed togetWalletInfois not self-documenting. Consider adding a comment to clarify its purpose.- queryFn: () => getWalletInfo(props.id, true), + queryFn: () => getWalletInfo(props.id, true /* includeImage */),
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/portal/src/components/others/AllSupportedWallets.tsx(2 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (CLAUDE.md)
**/*.{ts,tsx}: Write idiomatic TypeScript with explicit function declarations and return types
Limit each file to one stateless, single-responsibility function for clarity
Re-use shared types from@/typesor localtypes.tsbarrels
Prefer type aliases over interface except for nominal shapes
Avoidanyandunknownunless unavoidable; narrow generics when possible
Choose composition over inheritance; leverage utility types (Partial,Pick, etc.)
Comment only ambiguous logic; avoid restating TypeScript in prose
Files:
apps/portal/src/components/others/AllSupportedWallets.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit Inference Engine (CLAUDE.md)
Load heavy dependencies inside async paths to keep initial bundle lean (lazy loading)
Files:
apps/portal/src/components/others/AllSupportedWallets.tsx
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: E2E Tests (pnpm, esbuild)
- GitHub Check: Unit Tests
- GitHub Check: Analyze (javascript)
| const wallets = await getAllWalletsList(); | ||
| const ITEMS_PER_PAGE = 20; | ||
|
|
||
| const queryClient = new QueryClient(); |
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.
Move QueryClient instantiation inside the component to avoid SSR issues.
Creating QueryClient at the module level can cause hydration mismatches and memory leaks in SSR environments. Initialize it inside the component or use a factory function.
Apply this diff:
-const queryClient = new QueryClient();
-
export function AllSupportedWallets() {
+ const [queryClient] = useState(() => new QueryClient());
return (Also add the missing import:
-import { useMemo, useState } from "react";
+import { useMemo, useState } from "react";Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In apps/portal/src/components/others/AllSupportedWallets.tsx at line 31, move
the instantiation of QueryClient from the module level into the component
function to prevent SSR hydration mismatches and memory leaks. Also, add the
missing import statement for QueryClient from the appropriate library at the top
of the file.
| export function AllSupportedWallets() { | ||
| return ( | ||
| <QueryClientProvider client={queryClient}> | ||
| <AllSupportedWalletsContent /> | ||
| </QueryClientProvider> | ||
| ); | ||
| } |
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.
🛠️ Refactor suggestion
Add explicit return type to the component function.
Per the coding guidelines, TypeScript functions should have explicit return types.
-export function AllSupportedWallets() {
+export function AllSupportedWallets(): JSX.Element {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export function AllSupportedWallets() { | |
| return ( | |
| <QueryClientProvider client={queryClient}> | |
| <AllSupportedWalletsContent /> | |
| </QueryClientProvider> | |
| ); | |
| } | |
| export function AllSupportedWallets(): JSX.Element { | |
| return ( | |
| <QueryClientProvider client={queryClient}> | |
| <AllSupportedWalletsContent /> | |
| </QueryClientProvider> | |
| ); | |
| } |
🤖 Prompt for AI Agents
In apps/portal/src/components/others/AllSupportedWallets.tsx around lines 33 to
39, the AllSupportedWallets function component lacks an explicit return type.
Add the React.FC (or React.FunctionComponent) return type annotation to the
function declaration to comply with TypeScript coding guidelines for explicit
return types in components.
| ); | ||
| } | ||
|
|
||
| function AllSupportedWalletsContent() { |
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.
🛠️ Refactor suggestion
Add explicit return type to the component function.
-function AllSupportedWalletsContent() {
+function AllSupportedWalletsContent(): JSX.Element {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| function AllSupportedWalletsContent() { | |
| -function AllSupportedWalletsContent() { | |
| +function AllSupportedWalletsContent(): JSX.Element { |
🤖 Prompt for AI Agents
In apps/portal/src/components/others/AllSupportedWallets.tsx at line 41, the
function AllSupportedWalletsContent lacks an explicit return type. Add the
appropriate React component return type, such as React.ReactElement or
JSX.Element, to the function signature to explicitly specify the return type of
the component.
| const filteredWallets = useMemo(() => { | ||
| if (!searchQuery) return wallets || []; | ||
| if (!wallets) return []; | ||
|
|
||
| setCurrentPage(1); | ||
| const query = searchQuery.toLowerCase(); | ||
| return wallets.filter( | ||
| (wallet) => | ||
| wallet.name.toLowerCase().includes(query) || | ||
| wallet.id.toLowerCase().includes(query), | ||
| ); | ||
| }, [wallets, searchQuery]); | ||
|
|
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.
Remove side effect from useMemo and fix inefficient re-renders.
The setCurrentPage(1) call inside useMemo is a side effect that violates React best practices and will cause the component to re-render unnecessarily on every search query change.
Apply this diff to fix the issue:
+ // Reset page when search query changes
+ useEffect(() => {
+ setCurrentPage(1);
+ }, [searchQuery]);
+
const filteredWallets = useMemo(() => {
if (!searchQuery) return wallets || [];
if (!wallets) return [];
- setCurrentPage(1);
const query = searchQuery.toLowerCase();
return wallets.filter(
(wallet) =>
wallet.name.toLowerCase().includes(query) ||
wallet.id.toLowerCase().includes(query),
);
- }, [wallets, searchQuery]);
+ }, [wallets, searchQuery]);Don't forget to import useEffect:
-import { useMemo, useState } from "react";
+import { useEffect, useMemo, useState } from "react";🤖 Prompt for AI Agents
In apps/portal/src/components/others/AllSupportedWallets.tsx around lines 59 to
71, remove the side effect call setCurrentPage(1) from inside the useMemo hook
because useMemo should be pure and not cause side effects. Instead, move
setCurrentPage(1) into a separate useEffect hook that runs whenever searchQuery
changes. This will prevent unnecessary re-renders and adhere to React best
practices. Also, ensure useEffect is imported from React.
|
|
||
| async function WalletImage(props: { id: WalletId }) { | ||
| const img = await getWalletInfo(props.id, true); | ||
| function WalletImage(props: { id: WalletId }) { |
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.
🛠️ Refactor suggestion
Add explicit return type and consider adding error handling.
-function WalletImage(props: { id: WalletId }) {
+function WalletImage(props: { id: WalletId }): JSX.Element {Also consider handling the error state from useQuery:
- const { data: img } = useQuery({
+ const { data: img, error } = useQuery({
queryKey: ["wallet-image", props.id],
queryFn: () => getWalletInfo(props.id, true),
staleTime: 1000 * 60 * 60 * 24, // 24 hours
});
- if (!img) {
+ if (!img || error) {
return (
<div className="rounded-lg bg-muted" style={{ width: 44, height: 44 }} />
);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| function WalletImage(props: { id: WalletId }) { | |
| function WalletImage(props: { id: WalletId }): JSX.Element { | |
| const { data: img, error } = useQuery({ | |
| queryKey: ["wallet-image", props.id], | |
| queryFn: () => getWalletInfo(props.id, true), | |
| staleTime: 1000 * 60 * 60 * 24, // 24 hours | |
| }); | |
| if (!img || error) { | |
| return ( | |
| <div className="rounded-lg bg-muted" style={{ width: 44, height: 44 }} /> | |
| ); | |
| } | |
| // ...rest of rendering logic | |
| } |
🤖 Prompt for AI Agents
In apps/portal/src/components/others/AllSupportedWallets.tsx at line 228, the
WalletImage function lacks an explicit return type and does not handle potential
errors from the useQuery hook. Add an explicit return type to the WalletImage
function, such as React.ReactElement or JSX.Element, and include error handling
logic for the useQuery result, for example by checking for an error state and
rendering an appropriate fallback UI or message.
Slack Thread • Open in Web • Open in Cursor
PR-Codex overview
This PR introduces a new component,
AllSupportedWallets, that utilizesreact-queryfor fetching and displaying a paginated list of wallets. It includes a search feature, improved loading states, and pagination controls.Detailed summary
react-queryfor data fetching.AllSupportedWalletsandAllSupportedWalletsContentcomponents.WalletImagefunction to usereact-queryfor fetching wallet images.Summary by CodeRabbit
New Features
Refactor