[Dashboard] add wallet user search #7182
Conversation
|
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
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. |
WalkthroughA search functionality was added to the in-app wallet users page. This includes a new Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant SearchInput
participant InAppWalletUsersPageContent
participant TWTable
User->>SearchInput: Types in search query
SearchInput->>InAppWalletUsersPageContent: onValueChange(searchValue)
InAppWalletUsersPageContent->>InAppWalletUsersPageContent: Update searchValue state
InAppWalletUsersPageContent->>InAppWalletUsersPageContent: Filter wallets (filteredWallets)
InAppWalletUsersPageContent->>TWTable: Pass filteredWallets as data prop
TWTable-->>User: Display filtered wallet users
✨ Finishing Touches
🪧 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 (
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #7182 +/- ##
=======================================
Coverage 55.66% 55.66%
=======================================
Files 904 904
Lines 58391 58391
Branches 4114 4114
=======================================
Hits 32504 32504
Misses 25781 25781
Partials 106 106
🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
apps/dashboard/src/components/embedded-wallets/Users/SearchInput.tsx (2)
6-10: Extract the props interface for better reusability and type safety.Consider extracting the props interface to improve reusability and make the component more maintainable.
+interface SearchInputProps { + placeholder: string; + value: string; + onValueChange: (value: string) => void; +} + -export function SearchInput(props: { - placeholder: string; - value: string; - onValueChange: (value: string) => void; -}) { +export function SearchInput(props: SearchInputProps) {
6-22: Component implementation looks good with minor accessibility enhancement needed.The SearchInput component is well-structured and follows React best practices. Consider adding accessibility attributes for better screen reader support.
export function SearchInput(props: { placeholder: string; value: string; onValueChange: (value: string) => void; }) { return ( <div className="relative"> <Input placeholder={props.placeholder} value={props.value} onChange={(e) => props.onValueChange(e.target.value)} className="bg-card pl-9" + aria-label="Search wallet users" /> <SearchIcon className="-translate-y-1/2 absolute top-1/2 left-3 size-4 text-muted-foreground" /> </div> ); }apps/dashboard/src/components/embedded-wallets/Users/index.tsx (1)
114-138: Consider performance optimizations for the filtering logic.The current filtering approach runs on every render and could be expensive for large datasets. Consider adding debouncing and memoization.
+import { useMemo } from "react"; +// Also consider adding a debouncing hook like useDebounce - const filteredWallets = searchValue - ? wallets.filter((wallet) => { - const term = searchValue.toLowerCase(); - if (wallet.id.toLowerCase().includes(term)) { - return true; - } - if ( - wallet.wallets?.some((w) => w.address?.toLowerCase().includes(term)) - ) { - return true; - } - if ( - wallet.linkedAccounts?.some((acc) => { - return Object.values(acc.details).some( - (detail) => - typeof detail === "string" && - detail.toLowerCase().includes(term), - ); - }) - ) { - return true; - } - return false; - }) - : wallets; + const filteredWallets = useMemo(() => { + if (!searchValue) return wallets; + + const term = searchValue.toLowerCase(); + return wallets.filter((wallet) => { + if (wallet.id.toLowerCase().includes(term)) { + return true; + } + if ( + wallet.wallets?.some((w) => w.address?.toLowerCase().includes(term)) + ) { + return true; + } + if ( + wallet.linkedAccounts?.some((acc) => { + return Object.values(acc.details).some( + (detail) => + typeof detail === "string" && + detail.toLowerCase().includes(term), + ); + }) + ) { + return true; + } + return false; + }); + }, [wallets, searchValue]);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/dashboard/src/components/embedded-wallets/Users/SearchInput.tsx(1 hunks)apps/dashboard/src/components/embedded-wallets/Users/index.tsx(4 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
apps/dashboard/src/components/embedded-wallets/Users/index.tsx (1)
apps/dashboard/src/components/embedded-wallets/Users/SearchInput.tsx (1)
SearchInput(6-22)
⏰ Context from checks skipped due to timeout of 90000ms (8)
- GitHub Check: Unit Tests
- GitHub Check: Size
- GitHub Check: E2E Tests (pnpm, esbuild)
- GitHub Check: E2E Tests (pnpm, webpack)
- GitHub Check: Build Packages
- GitHub Check: E2E Tests (pnpm, vite)
- GitHub Check: Lint Packages
- GitHub Check: Analyze (javascript)
🔇 Additional comments (11)
apps/dashboard/src/components/embedded-wallets/Users/SearchInput.tsx (1)
11-22: LGTM! Clean and well-structured search input component.The component implementation is clean with proper controlled component pattern, appropriate styling, and good icon positioning.
apps/dashboard/src/components/embedded-wallets/Users/index.tsx (10)
107-107: LGTM! Proper state management for search functionality.The search state is appropriately initialized and managed.
176-183: LGTM! Well-positioned search input in the UI.The search input is appropriately placed alongside the download button with proper responsive styling.
199-199: LGTM! Correct data flow with filtered results.The table correctly receives the filtered wallet data instead of the raw data.
23-23: LGTM! Clean import of the SearchInput component.The import statement is properly placed and follows the project's import conventions.
127-131:❓ Verification inconclusive
Verify type safety of the details object access.
The filtering logic uses
Object.values(acc.details)which might not be type-safe if the details structure is unexpected.
🏁 Script executed:
#!/bin/bash # Check the WalletUser type definition to understand the structure of linkedAccounts.details ast-grep --pattern 'type WalletUser = $$$' ast-grep --pattern 'interface WalletUser {$$$}' ast-grep --pattern 'linkedAccounts$$$details'Length of output: 240
🏁 Script executed:
#!/bin/bash # Search for the usage of acc.details in the Users component rg -n "acc\\.details" -A5 -B5 apps/dashboard/src/components/embedded-wallets/Users/index.tsx # Also locate the definition/type of acc to see where details comes from rg -n "users\\s*[:=]" -A5 -B5 apps/dashboard/src/components/embedded-wallets/Users/index.tsx rg -n "map\\(" -A5 -B5 apps/dashboard/src/components/embedded-wallets/Users/index.tsxLength of output: 1752
Verify type safety of
acc.detailsbefore filteringThe code at
apps/dashboard/src/components/embedded-wallets/Users/index.tsx(lines 127–131) does:return Object.values(acc.details).some( (detail) => typeof detail === "string" && detail.toLowerCase().includes(term), );This assumes that:
acc.detailsis always a non-null object- All values are (or safely coerced to) strings
Please confirm the actual type of
linkedAccountsand itsdetailsfield as returned bygetAllEmbeddedWallets(). Ifdetailscan be undefined, null, or contain non-string values, you should either:
- Tighten the TypeScript definition (e.g.
Record<string, string>)- Add a runtime guard, for example:
if (typeof acc.details !== "object" || acc.details == null) { return false; } return Object.values(acc.details).some(/* … */);- Or transform/validate
detailsat the API boundary to ensure it’s always in the expected shape.
23-23: Import looks good.Clean import of the SearchInput component.
107-107: State management is appropriate.Adding searchValue state for controlled input is the right approach.
176-183: UI integration looks clean.The SearchInput is well-positioned in the header section with appropriate responsive styling. The layout with the download button works well.
199-199: Correct data source for filtered results.Good integration - passing the filtered wallets to the table component instead of the original data.
114-138: Consider user experience with pagination and search interaction.The current implementation filters only the wallets on the current page. Users might expect the search to work across all their wallet users, not just the current page. Consider whether this behavior aligns with user expectations.
Verify if the current page-level filtering meets the product requirements or if a different approach (like server-side search or client-side search across all pages) would be more appropriate.
Also applies to: 199-199
| <Input | ||
| placeholder={props.placeholder} | ||
| value={props.value} | ||
| onChange={(e) => props.onValueChange(e.target.value)} | ||
| className="bg-card pl-9" | ||
| /> | ||
| <SearchIcon className="-translate-y-1/2 absolute top-1/2 left-3 size-4 text-muted-foreground" /> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add accessibility attributes for better screen reader support.
The input field lacks proper accessibility attributes which could impact users with assistive technologies.
<Input
placeholder={props.placeholder}
value={props.value}
onChange={(e) => props.onValueChange(e.target.value)}
className="bg-card pl-9"
+ aria-label={props.placeholder || "Search"}
+ type="search"
/>📝 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.
| <Input | |
| placeholder={props.placeholder} | |
| value={props.value} | |
| onChange={(e) => props.onValueChange(e.target.value)} | |
| className="bg-card pl-9" | |
| /> | |
| <SearchIcon className="-translate-y-1/2 absolute top-1/2 left-3 size-4 text-muted-foreground" /> | |
| <Input | |
| placeholder={props.placeholder} | |
| value={props.value} | |
| onChange={(e) => props.onValueChange(e.target.value)} | |
| className="bg-card pl-9" | |
| aria-label={props.placeholder || "Search"} | |
| type="search" | |
| /> | |
| <SearchIcon className="-translate-y-1/2 absolute top-1/2 left-3 size-4 text-muted-foreground" /> |
🤖 Prompt for AI Agents
In apps/dashboard/src/components/embedded-wallets/Users/SearchInput.tsx around
lines 13 to 19, the Input component is missing accessibility attributes. Add
appropriate attributes such as aria-label or aria-labelledby to the Input
element to provide descriptive text for screen readers, ensuring the input field
is accessible to users with assistive technologies.
| const filteredWallets = searchValue | ||
| ? wallets.filter((wallet) => { | ||
| const term = searchValue.toLowerCase(); | ||
| if (wallet.id.toLowerCase().includes(term)) { | ||
| return true; | ||
| } | ||
| if ( | ||
| wallet.wallets?.some((w) => w.address?.toLowerCase().includes(term)) | ||
| ) { | ||
| return true; | ||
| } | ||
| if ( | ||
| wallet.linkedAccounts?.some((acc) => { | ||
| return Object.values(acc.details).some( | ||
| (detail) => | ||
| typeof detail === "string" && | ||
| detail.toLowerCase().includes(term), | ||
| ); | ||
| }) | ||
| ) { | ||
| return true; | ||
| } | ||
| return false; | ||
| }) | ||
| : wallets; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add performance optimization and fix potential null access.
The filtering logic is comprehensive and handles multiple search criteria well. However, consider adding debouncing for better performance and fix a potential null access issue.
+import { useMemo } from "react";
+import { useDebounce } from "@/hooks/useDebounce"; // or implement debouncing
export function InAppWalletUsersPageContent(props: {
clientId: string;
trackingCategory: string;
authToken: string;
}) {
const [activePage, setActivePage] = useState(1);
const [searchValue, setSearchValue] = useState("");
+ const debouncedSearchValue = useDebounce(searchValue, 300);
- const filteredWallets = searchValue
+ const filteredWallets = useMemo(() => debouncedSearchValue
? wallets.filter((wallet) => {
- const term = searchValue.toLowerCase();
+ const term = debouncedSearchValue.toLowerCase();
if (wallet.id.toLowerCase().includes(term)) {
return true;
}
if (
wallet.wallets?.some((w) => w.address?.toLowerCase().includes(term))
) {
return true;
}
if (
wallet.linkedAccounts?.some((acc) => {
+ if (!acc.details) return false;
return Object.values(acc.details).some(
(detail) =>
typeof detail === "string" &&
detail.toLowerCase().includes(term),
);
})
) {
return true;
}
return false;
})
- : wallets;
+ : wallets, [debouncedSearchValue, wallets]);Note: The current implementation only searches within the current page of results. Consider if this meets the expected user behavior or if server-side search would be more appropriate for larger datasets.
📝 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.
| const filteredWallets = searchValue | |
| ? wallets.filter((wallet) => { | |
| const term = searchValue.toLowerCase(); | |
| if (wallet.id.toLowerCase().includes(term)) { | |
| return true; | |
| } | |
| if ( | |
| wallet.wallets?.some((w) => w.address?.toLowerCase().includes(term)) | |
| ) { | |
| return true; | |
| } | |
| if ( | |
| wallet.linkedAccounts?.some((acc) => { | |
| return Object.values(acc.details).some( | |
| (detail) => | |
| typeof detail === "string" && | |
| detail.toLowerCase().includes(term), | |
| ); | |
| }) | |
| ) { | |
| return true; | |
| } | |
| return false; | |
| }) | |
| : wallets; | |
| import { useMemo } from "react"; | |
| import { useDebounce } from "@/hooks/useDebounce"; | |
| export function InAppWalletUsersPageContent(props: { | |
| clientId: string; | |
| trackingCategory: string; | |
| authToken: string; | |
| }) { | |
| const [activePage, setActivePage] = useState(1); | |
| const [searchValue, setSearchValue] = useState(""); | |
| const debouncedSearchValue = useDebounce(searchValue, 300); | |
| const filteredWallets = useMemo( | |
| () => | |
| debouncedSearchValue | |
| ? wallets.filter((wallet) => { | |
| const term = debouncedSearchValue.toLowerCase(); | |
| if (wallet.id.toLowerCase().includes(term)) { | |
| return true; | |
| } | |
| if ( | |
| wallet.wallets?.some((w) => | |
| w.address?.toLowerCase().includes(term) | |
| ) | |
| ) { | |
| return true; | |
| } | |
| if ( | |
| wallet.linkedAccounts?.some((acc) => { | |
| if (!acc.details) return false; | |
| return Object.values(acc.details).some( | |
| (detail) => | |
| typeof detail === "string" && | |
| detail.toLowerCase().includes(term) | |
| ); | |
| }) | |
| ) { | |
| return true; | |
| } | |
| return false; | |
| }) | |
| : wallets, | |
| [debouncedSearchValue, wallets] | |
| ); | |
| // …rest of component | |
| } |
🤖 Prompt for AI Agents
In apps/dashboard/src/components/embedded-wallets/Users/index.tsx around lines
114 to 138, the filtering logic should be optimized by adding debouncing to the
search input to reduce the frequency of filter executions and improve
performance. Additionally, ensure all optional chaining is correctly applied to
avoid potential null or undefined access, especially when accessing nested
properties like wallet.wallets and wallet.linkedAccounts. Review whether the
current client-side filtering on the current page meets user expectations or if
a server-side search implementation is needed for larger datasets.
size-limit report 📦
|
joaquim-verges
deleted the
codex/add-text-search-to-inappwalletuserspagecontent
branch
Fixes TOOL-4581
Summary
SearchInputcomponent for the embedded wallet users pageTesting
npx biome check apps/dashboard/src/components/embedded-wallets/Users/SearchInput.tsx apps/dashboard/src/components/embedded-wallets/Users/index.tsxpnpm lint(fails: EHOSTUNREACH)pnpm test(fails: spawn anvil ENOENT)PR-Codex overview
This PR introduces a
SearchInputcomponent to enhance user experience by allowing users to filter wallet users based on search criteria. It integrates the search functionality into the existingInAppWalletUsersPageContentcomponent.Detailed summary
SearchInputcomponent inSearchInput.tsx.SearchInputintoInAppWalletUsersPageContentinindex.tsx.searchValuestate to manage input value.searchValue.Summary by CodeRabbit