-
Notifications
You must be signed in to change notification settings - Fork 13
[TASK-7466] : home page profile section #603
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
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThe pull request introduces significant changes to the mobile UI and profile-related components. The modifications focus on enhancing the user interface for wallet management, profile display, and transaction history. New components like Changes
Suggested reviewers
Poem
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: 4
🧹 Nitpick comments (12)
src/components/Profile/Components/ProfileHeader.tsx (1)
11-20
: Remove redundant navigation logicThe back button uses both
Link
andonClick
for navigation, which is redundant. Since you're already usingLink
, theonClick
handler is unnecessary.<Link href={'/home'}> <Button variant="stroke" - onClick={() => router.push('/home')} className="h-8 w-8 p-0" aria-label="Go back" > <Icon name="arrow-prev" className="h-7 w-7" /> </Button> </Link>
src/components/Profile/Components/ProfileSection.tsx (2)
6-8
: Remove unnecessary empty interface and destructuringThe empty interface and destructuring pattern are unnecessary since the component doesn't accept any props.
-interface ProfileSectionProps {} - -const ProfileSection = ({}: ProfileSectionProps) => { +const ProfileSection = () => {🧰 Tools
🪛 Biome (1.9.4)
[error] 8-8: Unexpected empty object pattern.
(lint/correctness/noEmptyPattern)
14-15
: Track the TODO commentThe TODO comment about implementing boost logic should be tracked for future implementation.
Would you like me to create a GitHub issue to track the implementation of the boost logic?
src/components/Profile/Components/ProfileWalletBalance.tsx (2)
25-26
: Track the chain image TODO commentThe TODO comment about dynamic chain image loading should be tracked for implementation.
Would you like me to create a GitHub issue to track the implementation of dynamic chain image loading?
48-50
: Add comment explaining the number conversion logicThe conversion of token value to USDC is complex and would benefit from a comment explaining the calculation.
+ {/* Convert token value to USDC by multiplying by 10^6 (USDC decimals) */} <div className="text-right text-sm font-bold text-black"> $ {printableUsdc(BigInt(Math.floor(Number(getMaxBalanceToken?.value || 0) * 10 ** 6)))} </div>
src/components/Profile/Components/PeanutWalletHistory.tsx (1)
5-54
: Consider extracting mock data to a separate file.The mock data takes up a significant portion of the component file. Consider moving it to a separate mock data file for better maintainability and cleaner component structure.
Create a new file
src/mocks/walletHistory.ts
:import { IProfileTableData } from '@/interfaces' export const mockWalletHistory: IProfileTableData[] = [ // ... existing mock data ]Then import and use it in the component:
import { IProfileTableData } from '@/interfaces' import { MobileTableComponent } from './MobileTableComponent' +import { mockWalletHistory } from '@/mocks/walletHistory' -const mockData: IProfileTableData[] = [ - // ... remove mock data -] const PeanutWalletHistory = () => { return ( <div className="space-y-3 pb-4"> <div>History</div> <div className="border-b border-n-1"> - {mockData.map((data) => ( + {mockWalletHistory.map((data) => (src/components/Profile/index.tsx (1)
114-118
: Consider extracting the profile content into a separate component.The profile content section could be extracted into a dedicated component to improve maintainability and reduce the complexity of the Profile component.
Create a new component
ProfileContent
:// src/components/Profile/Components/ProfileContent.tsx import ProfileHeader from './ProfileHeader' import ProfileSection from './ProfileSection' import ProfileWalletBalance from './ProfileWalletBalance' import PeanutWalletHistory from './PeanutWalletHistory' export const ProfileContent = () => { return ( <div className="col w-full gap-5"> <ProfileHeader /> <ProfileSection /> <ProfileWalletBalance /> <PeanutWalletHistory /> </div> ) }Then use it in the Profile component:
- <div className="col w-full gap-5"> - <ProfileHeader /> - <ProfileSection /> - <ProfileWalletBalance /> - <PeanutWalletHistory /> - </div> + <ProfileContent />src/components/Profile/Components/MobileTableComponent.tsx (4)
Line range hint
12-19
: Enhance JSDoc documentationConsider adding
@param
descriptions for each prop to improve code documentation and maintainability./** * MobileTableComponent renders a mobile-friendly table for user profile data, such as accounts, contacts, or transaction history. * It handles different types of items (e.g., history, contacts, accounts), provides UI for avatars, statuses, and actions. * The component also includes a modal for additional actions like copying links, refunding, or viewing transactions in explorers. + * + * @param {string} itemKey - Unique identifier for the table row + * @param {string} primaryText - Main text content + * @param {string} secondaryText - Additional text content + * @param {string} tertiaryText - Third level text content + * @param {string} quaternaryText - Fourth level text content + * @param {'history' | 'contacts' | 'accounts'} type - Type of the table row + * @param {object} avatar - Avatar configuration object + * @param {object} dashboardItem - Dashboard item data + * @param {string} address - User address */
69-95
: Simplify status rendering logicThe nested conditional rendering can be simplified by:
- Extracting the repeated div structure into a reusable component
- Using a switch statement or lookup object for status mapping
- <div className="flex flex-col items-end justify-end gap-2 text-end"> - <div className="text-xs"> - {type === 'history' && dashboardItem ? ( - dashboardItem.status === 'claimed' ? ( - <div className="border border-teal-3 p-0.5 text-center text-teal-3"> - claimed - </div> - ) : dashboardItem.status === 'transfer' ? ( - <div className="border border-teal-3 p-0.5 text-center text-teal-3">sent</div> - ) : dashboardItem.status === 'paid' ? ( - <div className="border border-teal-3 p-0.5 text-center text-teal-3">paid</div> - ) : dashboardItem.status ? ( - <div className="border border-gray-1 p-0.5 text-center text-gray-1"> - {dashboardItem.status.toLowerCase().replaceAll('_', ' ')} - </div> - ) : ( - <div className="border border-gray-1 p-0.5 text-center text-gray-1"> - pending - </div> - ) - ) : type === 'contacts' ? ( - <label className="font-bold">txs: {quaternaryText}</label> - ) : ( - type === 'accounts' && '' - )} - </div> - </div> + <div className="flex flex-col items-end justify-end gap-2 text-end"> + <div className="text-xs"> + {type === 'history' && dashboardItem ? ( + <StatusBadge status={dashboardItem.status} /> + ) : type === 'contacts' ? ( + <label className="font-bold">txs: {quaternaryText}</label> + ) : null} + </div> + </div> // New component const StatusBadge = ({ status }: { status?: string }) => { const statusConfig = { claimed: { text: 'claimed', variant: 'success' }, transfer: { text: 'sent', variant: 'success' }, paid: { text: 'paid', variant: 'success' }, default: { text: status?.toLowerCase().replaceAll('_', ' ') || 'pending', variant: 'default' } } const config = statusConfig[status as keyof typeof statusConfig] || statusConfig.default const colorClass = config.variant === 'success' ? 'border-teal-3 text-teal-3' : 'border-gray-1 text-gray-1' return ( <div className={`border ${colorClass} p-0.5 text-center`}> {config.text} </div> ) }
103-106
: Optimize date formatting performanceMemoize the formatted date to prevent unnecessary re-renders.
+ const formattedDate = useMemo(() => { + if (!dashboardItem?.date) return '' + return utils.formatDate(new Date(dashboardItem.date)) + }, [dashboardItem?.date]) - <div> - <label className="text-xs font-normal"> - {dashboardItem?.date ? utils.formatDate(new Date(dashboardItem.date)) : ''} - </label> - </div> + <div> + <label className="text-xs font-normal"> + {formattedDate} + </label> + </div>
Line range hint
169-183
: Improve modal actions organizationExtract URL manipulation logic and simplify conditional rendering of actions.
+ const getOfframpStatusUrl = (link?: string) => { + if (!link) return '' + const url = new URL(link) + url.pathname = '/cashout/status' + return url.toString() + } + const renderModalActions = () => { + if (type !== 'history') { + return type === 'contacts' && ( + <Link href={`/send?recipientAddress=${encodeURIComponent(address as string)}`}> + <div className="modal-action-button">Send to this address</div> + </Link> + ) + } + return ( + <> + {dashboardItem?.type !== 'Link Received' && + dashboardItem?.type !== 'Request Link' && + dashboardItem?.status === 'pending' && ( + <div + onClick={() => dashboardItem.link && window.open(dashboardItem.link, '_blank')} + className="modal-action-button" + > + Refund + </div> + )} + {/* ... other actions ... */} + </> + ) + } - {type === 'history' ? ( - <> - {dashboardItem?.type !== 'Link Received' && - dashboardItem?.type !== 'Request Link' && - dashboardItem?.status === 'pending' && ( - <div - onClick={() => { - dashboardItem.link && window.open(dashboardItem?.link ?? '', '_blank') - }} - className="flex h-12 w-full items-center gap-2 px-4 text-h8 text-sm font-bold transition-colors last:mb-0 hover:bg-n-3/10 disabled:cursor-not-allowed disabled:bg-n-4 disabled:hover:bg-n-4/90 dark:hover:bg-white/20 " - > - Refund - </div> - )} - {/* ... other actions ... */} - </> - ) : ( - type === 'contacts' && ( - <Link href={`/send?recipientAddress=${encodeURIComponent(address as string)}`}> - <div className="flex h-12 w-full items-center gap-2 px-4 text-h8 text-sm font-bold transition-colors last:mb-0 hover:bg-n-3/10 disabled:cursor-not-allowed disabled:bg-n-4 disabled:hover:bg-n-4/90 dark:hover:bg-white/20 "> - Send to this address - </div> - </Link> - ) - )} + {renderModalActions()}src/app/(mobile-ui)/history/page.tsx (1)
142-143
: Consider internationalizing currency displayThe hardcoded
$
symbol might not be appropriate for all users. Consider using proper currency formatting based on the user's locale.- secondaryText: `$ ${data.amount}`, + secondaryText: new Intl.NumberFormat(locale, { + style: 'currency', + currency: 'USD' + }).format(data.amount),
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
src/assets/icons/share.svg
is excluded by!**/*.svg
📒 Files selected for processing (12)
src/app/(mobile-ui)/history/page.tsx
(3 hunks)src/app/(mobile-ui)/home/page.tsx
(2 hunks)src/app/(mobile-ui)/layout.tsx
(1 hunks)src/assets/icons/index.ts
(1 hunks)src/components/Home/WalletCard.tsx
(0 hunks)src/components/Profile/Components/MobileTableComponent.tsx
(2 hunks)src/components/Profile/Components/PeanutWalletHistory.tsx
(1 hunks)src/components/Profile/Components/ProfileHeader.tsx
(1 hunks)src/components/Profile/Components/ProfileSection.tsx
(1 hunks)src/components/Profile/Components/ProfileWalletBalance.tsx
(1 hunks)src/components/Profile/index.tsx
(2 hunks)src/components/Setup/Views/AddWallets.tsx
(0 hunks)
💤 Files with no reviewable changes (2)
- src/components/Setup/Views/AddWallets.tsx
- src/components/Home/WalletCard.tsx
🧰 Additional context used
🪛 Biome (1.9.4)
src/components/Profile/Components/ProfileSection.tsx
[error] 5-6: An empty interface is equivalent to {}.
Safe fix: Use a type alias instead.
(lint/suspicious/noEmptyInterface)
[error] 8-8: Unexpected empty object pattern.
(lint/correctness/noEmptyPattern)
🔇 Additional comments (8)
src/components/Profile/Components/ProfileHeader.tsx (1)
10-27
: LGTM! Clean and accessible header implementationThe header layout is well-structured with proper spacing and accessibility attributes.
src/assets/icons/index.ts (1)
11-11
: LGTM! Clean icon export additionThe SHARE_ICON export is properly added while maintaining the file's alphabetical ordering.
src/components/Profile/Components/ProfileSection.tsx (1)
19-48
: LGTM! Well-structured profile sectionThe component has a clean layout with proper spacing and organization. The points display is implemented efficiently using Object.entries.
src/components/Profile/Components/ProfileWalletBalance.tsx (2)
11-17
: LGTM! Efficient balance calculationThe use of useMemo for calculating the maximum balance token is well-implemented with proper null checks.
19-59
: LGTM! Clean wallet balance displayThe component has a well-structured layout with proper error handling and fallbacks for missing data.
src/components/Profile/Components/PeanutWalletHistory.tsx (1)
4-4
: Address the TODO comment with a clear timeline.The comment indicates a pending migration to Redux for wallet data. This technical debt should be tracked and addressed to ensure the component uses real data.
Would you like me to help create a GitHub issue to track this Redux migration task?
src/app/(mobile-ui)/layout.tsx (1)
45-45
: Verify the removal of responsive padding.The change removes responsive padding (
md:p-6
) for non-home pages. This might affect the layout on different screen sizes.Run this script to check for consistent padding usage across the app:
src/app/(mobile-ui)/home/page.tsx (1)
59-63
: LGTM! Good routing logic improvement.The conditional routing based on wallet provider type improves the user experience by directing users to the appropriate section based on their wallet type.
<div className="text-gray-1">peanut.me/</div> | ||
<div className="">{user?.user.username}</div> |
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 null check for username display
The username access could potentially cause issues if the user object is null.
- <div className="text-gray-1">peanut.me/</div>
- <div className="">{user?.user.username}</div>
+ {user?.user.username && (
+ <>
+ <div className="text-gray-1">peanut.me/</div>
+ <div className="">{user.user.username}</div>
+ </>
+ )}
📝 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.
<div className="text-gray-1">peanut.me/</div> | |
<div className="">{user?.user.username}</div> | |
{user?.user.username && ( | |
<> | |
<div className="text-gray-1">peanut.me/</div> | |
<div className="">{user.user.username}</div> | |
</> | |
)} |
{mockData.map((data) => ( | ||
<div key={(data.itemKey ?? '') + Math.random()}> | ||
<MobileTableComponent | ||
itemKey={(data.itemKey ?? '') + Math.random()} | ||
primaryText={data.primaryText} | ||
secondaryText={data.secondaryText} | ||
tertiaryText={data.tertiaryText} | ||
quaternaryText={data.quaternaryText} | ||
address={data.address} | ||
type={data.type} | ||
avatar={data.avatar} | ||
dashboardItem={data.dashboardItem} | ||
/> | ||
</div> | ||
))} |
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.
Fix the key generation in the map function.
Using Math.random()
in keys can cause unnecessary re-renders and potential performance issues as React relies on stable keys for reconciliation.
Apply this diff to fix the key generation:
- <div key={(data.itemKey ?? '') + Math.random()}>
+ <div key={data.itemKey}>
<MobileTableComponent
- itemKey={(data.itemKey ?? '') + Math.random()}
+ itemKey={data.itemKey}
primaryText={data.primaryText}
secondaryText={data.secondaryText}
tertiaryText={data.tertiaryText}
quaternaryText={data.quaternaryText}
address={data.address}
type={data.type}
avatar={data.avatar}
dashboardItem={data.dashboardItem}
/>
</div>
📝 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.
{mockData.map((data) => ( | |
<div key={(data.itemKey ?? '') + Math.random()}> | |
<MobileTableComponent | |
itemKey={(data.itemKey ?? '') + Math.random()} | |
primaryText={data.primaryText} | |
secondaryText={data.secondaryText} | |
tertiaryText={data.tertiaryText} | |
quaternaryText={data.quaternaryText} | |
address={data.address} | |
type={data.type} | |
avatar={data.avatar} | |
dashboardItem={data.dashboardItem} | |
/> | |
</div> | |
))} | |
{mockData.map((data) => ( | |
<div key={data.itemKey}> | |
<MobileTableComponent | |
itemKey={data.itemKey} | |
primaryText={data.primaryText} | |
secondaryText={data.secondaryText} | |
tertiaryText={data.tertiaryText} | |
quaternaryText={data.quaternaryText} | |
address={data.address} | |
type={data.type} | |
avatar={data.avatar} | |
dashboardItem={data.dashboardItem} | |
/> | |
</div> | |
))} |
<div className="relative mr-2 min-w-fit"> | ||
{dashboardItem?.tokenSymbol === 'USDC' && ( | ||
<Image | ||
src={'https://cryptologos.cc/logos/usd-coin-usdc-logo.png?v=040'} | ||
alt="token logo" | ||
width={30} | ||
height={30} | ||
className="rounded-full" | ||
/> | ||
)} | ||
|
||
{dashboardItem?.tokenSymbol === 'USDC' && dashboardItem.chain === 'Arbitrum One' && ( | ||
<Image | ||
src={ARBITRUM_ICON} | ||
alt="token logo" | ||
width={16} | ||
height={16} | ||
className="absolute -right-2 bottom-0 size-4 rounded-full" | ||
/> | ||
)} | ||
</div> |
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
Improve image handling and accessibility
Several improvements can be made to the image handling logic:
- Move the USDC logo URL to constants to avoid hardcoding
- Add error handling for image loading
- Improve alt text for better accessibility
- <div className="relative mr-2 min-w-fit">
- {dashboardItem?.tokenSymbol === 'USDC' && (
- <Image
- src={'https://cryptologos.cc/logos/usd-coin-usdc-logo.png?v=040'}
- alt="token logo"
- width={30}
- height={30}
- className="rounded-full"
- />
- )}
-
- {dashboardItem?.tokenSymbol === 'USDC' && dashboardItem.chain === 'Arbitrum One' && (
- <Image
- src={ARBITRUM_ICON}
- alt="token logo"
- width={16}
- height={16}
- className="absolute -right-2 bottom-0 size-4 rounded-full"
- />
- )}
- </div>
+ <div className="relative mr-2 min-w-fit">
+ {dashboardItem?.tokenSymbol === 'USDC' && (
+ <Image
+ src={consts.TOKEN_LOGOS.USDC}
+ alt={`USDC token logo`}
+ width={30}
+ height={30}
+ className="rounded-full"
+ onError={(e) => {
+ e.currentTarget.src = consts.FALLBACK_TOKEN_LOGO
+ }}
+ />
+ )}
+
+ {dashboardItem?.tokenSymbol === 'USDC' && dashboardItem.chain === 'Arbitrum One' && (
+ <Image
+ src={ARBITRUM_ICON}
+ alt={`Arbitrum One chain icon`}
+ width={16}
+ height={16}
+ className="absolute -right-2 bottom-0 size-4 rounded-full"
+ onError={(e) => {
+ e.currentTarget.src = consts.FALLBACK_CHAIN_ICON
+ }}
+ />
+ )}
+ </div>
Committable suggestion skipped: line range outside the PR's diff.
<div className="block w-full sm:hidden"> | ||
{tableData.slice((currentPage - 1) * itemsPerPage, currentPage * itemsPerPage).map((data) => ( | ||
<div key={(data.itemKey ?? '') + Math.random()}> | ||
<div key={(data.itemKey ?? '') + Math.random()} className="border-b border-n-1"> |
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.
Avoid using Math.random() in React keys
Using Math.random()
for React keys can lead to unnecessary re-renders and potential key collisions. Consider using a more stable unique identifier.
- <div key={(data.itemKey ?? '') + Math.random()} className="border-b border-n-1">
+ <div key={`history-${data.itemKey ?? ''}`} className="border-b border-n-1">
📝 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.
<div key={(data.itemKey ?? '') + Math.random()} className="border-b border-n-1"> | |
<div key={`history-${data.itemKey ?? ''}`} className="border-b border-n-1"> |
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.
Collisions are unlikely but agreed on potential re-renders if we're calling math.random each time
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.
@Hugo0 Thanks for the clarification! Yes, the re-rendering is the primary concern here.
⟳ thoughtfully considering your feedback
✏️ Learnings added
Learnt from: Hugo0
PR: peanutprotocol/peanut-ui#603
File: src/app/(mobile-ui)/history/page.tsx:262-262
Timestamp: 2025-01-09T23:14:55.309Z
Learning: When reviewing React key usage, focus on potential re-rendering issues over collision risks. While using Math.random() as React keys may not necessarily lead to collisions, it can cause unnecessary re-renders as new random values are generated on each render.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
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 (2)
src/app/(mobile-ui)/home/page.tsx (2)
78-102
: Consider extracting the sign-in button logic to a separate component.While the implementation is thorough with proper state handling and error management, the conditional rendering logic is complex enough to warrant its own component. This would improve maintainability and make the code more modular.
Consider creating a new component like this:
// components/Auth/SignInButton.tsx export const SignInButton = () => { const { isPeanutWallet, isConnected } = useWallet() const { handleLogin, isLoggingIn } = useZeroDev() const toast = useToast() if (!hasWallets || (!isPeanutWallet && !isConnected)) return null return ( <Button loading={isLoggingIn} disabled={isLoggingIn} shadowSize={!isConnected ? '4' : undefined} variant={isConnected ? 'green' : 'purple'} size="small" onClick={() => { if (isConnected) return handleLogin().catch((_error) => { toast.error('Error logging in') }) }} > {isConnected ? 'Connected' : 'Sign In'} </Button> ) }
103-103
: Consider adding error boundary and loading states for ProfileSection.The ProfileSection component might benefit from:
- Conditional rendering based on authentication state
- Error boundary to handle potential rendering failures
- Loading state while user data is being fetched
Example implementation:
<ErrorBoundary fallback={<ProfileErrorState />}> <Suspense fallback={<ProfileLoadingState />}> {isAuthenticated && <ProfileSection />} </Suspense> </ErrorBoundary>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/app/(mobile-ui)/home/page.tsx
(4 hunks)
🔇 Additional comments (3)
src/app/(mobile-ui)/home/page.tsx (3)
4-4
: LGTM! Well-structured imports and hook usage.The new imports and hook usage changes are well-organized and support the profile page functionality effectively.
Also applies to: 7-7, 10-11, 33-36
63-67
: LGTM! Clean routing logic based on wallet type.The conditional navigation based on wallet provider type is well-implemented and aligns with the profile page integration.
76-77
: LGTM! Well-implemented responsive layout.The flex container with percentage-based widths for different breakpoints provides good responsive behavior.
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.
Please add a PR comment / summary for bigger PRs like this one. Especially if there's soemthing to watch out for or complex logic to explain
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.
Please add a PR comment / summary for bigger PRs like this one. Especially if there's soemthing to watch out for or complex logic to explain
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.
Code itself looks good but I'm a bit confused by some of the choices - let's chat tomorrow during sync
<div className="block w-full sm:hidden"> | ||
{tableData.slice((currentPage - 1) * itemsPerPage, currentPage * itemsPerPage).map((data) => ( | ||
<div key={(data.itemKey ?? '') + Math.random()}> | ||
<div key={(data.itemKey ?? '') + Math.random()} className="border-b border-n-1"> |
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.
Collisions are unlikely but agreed on potential re-renders if we're calling math.random each time
}) | ||
}} | ||
> | ||
{isConnected ? 'Connected' : 'Sign In'} |
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.
Thought : The whole connected / sign in thing needs to be revisited
<div className="relative mr-2 min-w-fit"> | ||
{dashboardItem?.tokenSymbol === 'USDC' && ( | ||
<Image | ||
src={'https://cryptologos.cc/logos/usd-coin-usdc-logo.png?v=040'} |
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.
nit: not ideal to hardcode here
/> | ||
)} | ||
|
||
{dashboardItem?.tokenSymbol === 'USDC' && dashboardItem.chain === 'Arbitrum One' && ( |
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.
nit: same, what if we change chains / token?
// todo: use acutal data after context to redux migration is done for walletContext | ||
const mockData: IProfileTableData[] = [ |
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.
Q: is there a task for this?
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.
idg why we have both /history and /profile
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.
Q: is there a task for this?
not a separate task, but have mentioned this under context migration task description
<Card> | ||
<Card.Content className="flex items-center justify-between border-b-0 px-5 py-4"> | ||
<div className="flex items-center gap-3"> | ||
{/* todo: figure out a way to get chain image dynamically here */} |
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.
thought: should prob be in consts, it won't change often
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.
the name of this file is a bit confusing - if its just for peanut wallet
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.
it's only relevant in profile section, and followed the old structure so kept it like this 👀
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.
Code itself looks good but I'm a bit confused by some of the choices - let's chat tomorrow during sync
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
♻️ Duplicate comments (1)
src/components/Profile/Components/ProfileSection.tsx (1)
29-37
:⚠️ Potential issueFix username null check and KYC status logic
The username access needs null safety (previous comment still applies)
The KYC badge logic seems incorrect - it shows "No KYC" when status is not null
- {user?.user.kycStatus !== null && ( + {!user?.user.kycStatus && ( <Badge color="purple" className="bg-white text-purple-1"> No KYC </Badge> )}
🧹 Nitpick comments (3)
src/components/Profile/Components/ProfileSection.tsx (3)
7-9
: Remove unnecessary empty interface and destructuringThe empty interface and destructuring pattern provide no value. Since the component doesn't accept props currently, we can simplify it.
-interface ProfileSectionProps {} - -const ProfileSection = ({}: ProfileSectionProps) => { +const ProfileSection = () => {🧰 Tools
🪛 Biome (1.9.4)
[error] 9-9: Unexpected empty object pattern.
(lint/correctness/noEmptyPattern)
12-17
: Add type definitions and track TODOThe points object lacks type safety, and there's an unimplemented boost logic.
- Add type definitions:
+interface Points { + Points: number | undefined; + Invites: number | undefined; + Boost: number; +} + const points = {
- Would you like me to create a GitHub issue to track the implementation of the boost logic mentioned in the TODO comment?
47-52
: Add fallback for undefined point valuesThe points display should handle undefined values gracefully.
{Object.entries(points).map(([key, value]) => ( <div key={key} className="space-y-0.5"> - <div className="text-base font-bold">{value}</div> + <div className="text-base font-bold">{value ?? 0}</div> <div className="text-xs text-gray-500">{key}</div> </div> ))}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/app/(mobile-ui)/history/page.tsx
(3 hunks)src/components/Profile/Components/ProfileSection.tsx
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/app/(mobile-ui)/history/page.tsx
🧰 Additional context used
🪛 Biome (1.9.4)
src/components/Profile/Components/ProfileSection.tsx
[error] 6-7: An empty interface is equivalent to {}.
Safe fix: Use a type alias instead.
(lint/suspicious/noEmptyInterface)
[error] 9-9: Unexpected empty object pattern.
(lint/correctness/noEmptyPattern)
</div> | ||
</div> | ||
<div className="flex size-8 items-center justify-center rounded-full bg-white p-2"> | ||
<CopyToClipboard fill="black" textToCopy={`https://peanut.me/${user?.user.username}`} /> |
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
Use environment variable for base URL
The hardcoded URL 'peanut.me' should be configurable for different environments.
- <CopyToClipboard fill="black" textToCopy={`https://peanut.me/${user?.user.username}`} />
+ <CopyToClipboard fill="black" textToCopy={`${process.env.NEXT_PUBLIC_BASE_URL}/${user?.user.username}`} />
📝 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.
<CopyToClipboard fill="black" textToCopy={`https://peanut.me/${user?.user.username}`} /> | |
<CopyToClipboard fill="black" textToCopy={`${process.env.NEXT_PUBLIC_BASE_URL}/${user?.user.username}`} /> |
Summary by CodeRabbit
Release Notes
New Features
ProfileSection
to display user profile information.ProfileWalletBalance
component for showcasing wallet balances.PeanutWalletHistory
to display wallet transaction history.UI/UX Improvements
Home
page with new profile section and conditional rendering for wallet connection status.MobileTableComponent
for improved transaction detail presentation.ProfileHeader
to focus on navigation.Minor Changes
AddWallets
component.