[TASK-7461] feat: add wallet header component#593
[TASK-7461] feat: add wallet header component#593kushagrasarathe merged 2 commits intopeanut-walletfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThis pull request introduces several enhancements to the application's user interface and component structure. The changes primarily focus on wallet management, adding a new Changes
Sequence DiagramsequenceDiagram
participant User
participant WalletHeader
participant WalletModal
participant WalletEntryCard
User->>WalletHeader: Click wallet button
WalletHeader->>WalletModal: Open wallet selection
WalletModal->>WalletEntryCard: Display wallet list
User->>WalletEntryCard: Select wallet
WalletEntryCard->>WalletHeader: Update selected wallet
WalletModal->>WalletHeader: Close modal
Suggested labels
Suggested reviewers
Poem
Tip CodeRabbit's docstrings feature is now available as part of our Early Access Program! Simply use the command 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (6)
src/components/Global/CopyToClipboard/index.tsx (2)
5-9: Consider renaming “fill” prop for clarity.
The prop name “fill” may be ambiguous, especially for newcomers who might expect a “color” prop. Renaming this prop to something more explicit like “iconColor” or “iconFillColor” can help clarify its purpose.
22-30: Provide an accessible label or aria attribute for the Icon.
Because Icon usage here relies on theonClickevent, some users relying on screen readers might not understand what this icon does. Adding anaria-labelor an accessible tooltip ensures that all users can recognize the behavior of this component.src/assets/icons/index.ts (1)
5-6: Unify or confirm naming conventions for icon exports.
Both uppercase and mixed-case identifiers (e.g.,Wallet_ICON) are present. Consistently using an uppercase or PascalCase scheme for all icon exports could improve readability and maintainability.Also applies to: 8-9, 17-17
src/components/Global/WalletHeader/index.tsx (3)
31-38: Efficient sorting approach
UsinguseMemoand a simple comparator to place the selected wallet at the top is a clean solution. Ensure large wallet lists won't degrade performance—check for any potential scaling issues if the user might have numerous wallets.
49-67: Yellow variant usage in Button
Leveraging the new'yellow'variant is consistent with the updated Tailwind class.btn-yellow. This approach keeps styling consistent across the UI.Optional: If the project style guide requires CTA alignment or descriptive text for accessibility, consider adding ARIA labels or informative text on the button if it’s critical for screen readers.
109-159: WalletEntryCard
- The dynamic background for the active wallet is a nice visual cue.
- The
CopyToClipboardintegration is consistent with the rest of the code.- The usage of
usernamefor Peanut addresses is straightforward.Potential improvement:
- If the list grows large, consider virtualization or lazy rendering.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
src/assets/icons/wallet.svgis excluded by!**/*.svg
📒 Files selected for processing (8)
src/app/(mobile-ui)/home/page.tsx(2 hunks)src/assets/icons/index.ts(1 hunks)src/components/0_Bruddle/Button.tsx(2 hunks)src/components/Global/CopyToClipboard/index.tsx(1 hunks)src/components/Global/Icon/index.tsx(2 hunks)src/components/Global/Modal/index.tsx(4 hunks)src/components/Global/WalletHeader/index.tsx(1 hunks)tailwind.config.js(2 hunks)
🔇 Additional comments (16)
src/assets/icons/index.ts (1)
5-6: Verify that references to the re-exported icons remain consistent.
The reintroduction or reorganization of ETHEREUM_ICON, GAS_ICON, and Wallet_ICON could affect components referencing these icons. Please confirm that all relevant imports and usage throughout the codebase are up to date and do not rely on removed exports.
Also applies to: 8-9, 17-17
✅ Verification successful
All icon references are consistent with the exports
Based on the search results, all references to the icons in question (ETHEREUM_ICON, GAS_ICON, Wallet_ICON, GITHUB_ICON, and GITHUB_INVERTED_ICON) are properly aligned with their exports in src/assets/icons/index.ts. Here's where they are used:
ETHEREUM_ICONis used insrc/components/Profile/Components/SkeletonPage.tsxWallet_ICONis used insrc/components/Global/WalletHeader/index.tsxGITHUB_ICONandGITHUB_INVERTED_ICONare used insrc/components/Global/Footer/consts.tsGAS_ICONis exported but not currently used, which is fine
All imports and usages are consistent with the exports, and there are no broken references or imports relying on removed exports.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Searching for references to old icon exports to ensure they are replaced or removed.
rg "GITHUB_ICON|GITHUB_INVERTED_ICON|ETHEREUM_ICON|GAS_ICON|Wallet_ICON" -A 5
Length of output: 3349
src/components/0_Bruddle/Button.tsx (1)
5-5: Confirm color contrast of the new 'yellow' variant.
The newly introduced 'yellow' variant uses a 'bg-yellow-1' class. Make sure it meets minimum accessibility guidelines and works well with existing text colors, especially on hover states.
Also applies to: 27-27
src/components/Global/Modal/index.tsx (2)
21-21: Ensure user can still close the modal when hideCloseButton is true.
When hideCloseButton is set, the user loses the explicit close button. Confirm that closing the modal via overlay click, keyboard escape, or another button is still possible for a smooth user experience.
Also applies to: 39-39, 104-116
21-21: Good approach for optional close button.
Allowing the modal consumer to hide or show the close button increases flexibility. The default of false is sensible, preserving the existing behavior.
Also applies to: 39-39, 104-116
src/app/(mobile-ui)/home/page.tsx (4)
4-5: Import statements for new header components look good
The imports for WalletHeader and HomeHeader are positioned logically and do not indicate any immediate issues.
9-10: Context imports are valid
Importing useAuth and useWallet seamlessly integrates authentication and wallet functionality into the Home component.
11-14: No apparent issues with utility and library imports
The usage of classnames, framer-motion, next/navigation, and React hooks (useEffect, useRef) is standard and poses no immediate problems.
55-55: Placing WalletHeader above the carousel
Positioning the WalletHeader in this location is consistent with the PR objective to enhance wallet management visibility. Consider adding a quick test or integration check to ensure the user experience remains intuitive on mobile devices.
src/components/Global/WalletHeader/index.tsx (4)
25-26: New 'WalletHeader' component introduced
The state for showing/hiding the modal is straightforward. Good job managing local state here.
41-44: Wallet selection logic
Closing the modal after wallet selection is user-friendly and minimal in complexity. Looks good.
70-104: Modal structure
The modal’s styling, custom classes, and semantic structure look good. It’s beneficial that the background, border, and wrap classes are separated for clarity. Consider verifying that the modal is fully keyboard-navigable.
161-171: AddNewWallet
Simple, clear, and consistent. The icon usage and text provide an obvious entry point to add new wallets.
tailwind.config.js (2)
15-15: New purple color
Adding 4: '#AE7AFF' extends the color palette suitably. Keep verifying that this new color aligns with the design system and is accessible when used against certain backgrounds.
250-252: .btn-yellow utility class
This new utility class follows the existing naming conventions. The hover state to darken the background is consistent with other color variants.
src/components/Global/Icon/index.tsx (2)
2-2: Re-adding twMerge
Uncommenting or adding the twMerge import improves class name management.
205-206: New icons: arrow-down-filled and wallet
Adding these icons follows the established icons mapping. Ensure they appear correctly and are tested in the UI.
| const handleCopy = (e: React.MouseEvent<SVGElement>) => { | ||
| e.stopPropagation() | ||
| navigator.clipboard.writeText(textToCopy).then(() => { | ||
| setCopied(true) | ||
| setTimeout(() => setCopied(false), 2000) | ||
| }) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Handle potential clipboard write failures.
Currently, if navigator.clipboard.writeText fails or is not supported by the browser (e.g., older browsers, certain WebView restrictions), no fallback handling occurs. You may wish to add error handling or a fallback for improved user experience in these cases.
const handleCopy = (e: React.MouseEvent<SVGElement>) => {
e.stopPropagation()
- navigator.clipboard.writeText(textToCopy).then(() => {
- setCopied(true)
- setTimeout(() => setCopied(false), 2000)
- })
+ navigator.clipboard.writeText(textToCopy)
+ .then(() => {
+ setCopied(true)
+ setTimeout(() => setCopied(false), 2000)
+ })
+ .catch(() => {
+ // Optional: fallback to a manual copy command or show an error message
+ })
}Committable suggestion skipped: line range outside the PR's diff.
There was a problem hiding this comment.
@Hugo0, definitely! Incorporating a toast notification for success (“Copied!”) or failure (e.g., “Failed to copy”) would enhance the user experience. You can trigger a toast in the success callback and within the .catch() block if an error occurs. That way, users receive clear feedback on whether the copy action was successful or, if not, what went wrong.
| const handleCopy = (e: React.MouseEvent<SVGElement>) => { | ||
| e.stopPropagation() | ||
| navigator.clipboard.writeText(textToCopy).then(() => { | ||
| setCopied(true) | ||
| setTimeout(() => setCopied(false), 2000) | ||
| }) | ||
| } |
| <div className="flex w-[100%] flex-col gap-4 sm:w-[90%] sm:gap-2 md:w-[70%] lg:w-[50%]"> | ||
| <HomeHeader /> | ||
| <WalletHeader /> | ||
| <div |
There was a problem hiding this comment.
Thought: The carousel should prob be a component too
| classNameWrapperDiv?: string | ||
| showPrev?: boolean | ||
| onPrev?: () => void | ||
| hideCloseButton?: boolean |
There was a problem hiding this comment.
Q: Why do we need a hide close button option?
There was a problem hiding this comment.
was planning to move the modal to look like sheet for mobile screens, figured that's a little lengthy task and not high priority, for now reverted it to old structure, tracking it to and will tackle in another spirint
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (6)
src/components/Global/WalletHeader/index.tsx (6)
27-27: Remove unused isConnected variable.The
isConnectedvariable is destructured but only used in the JSX. Consider moving it directly to where it's used to improve code clarity.-const { wallets, selectedWallet, setSelectedWallet, isConnected } = useWallet() +const { wallets, selectedWallet, setSelectedWallet } = useWallet()
84-84: Fix duplicate opacity settings.The button has two opacity modifiers:
bg-yellow-4/80andbg-opacity-20. This might lead to unexpected behavior. Choose one approach for consistency.-<Button size="small" variant="yellow" className="w-fit bg-yellow-4/80 bg-opacity-20"> +<Button size="small" variant="yellow" className="w-fit bg-yellow-4/20">
78-78: Consider extracting modal styles to constants.The modal's
classWrapcontains multiple hardcoded style values. Consider extracting these to constants or a configuration object for better maintainability.const MODAL_STYLES = { wrapper: 'w-full items-center', content: 'bg-background rounded-none border-0 p-6 pt-14 w-full max-h-[65vh] md:max-h-full overflow-y-auto' }; // Usage: <Modal visible={showModal} onClose={() => setShowModal(false)} className={MODAL_STYLES.wrapper} classWrap={MODAL_STYLES.content} >
114-116: Improve wallet image fallback logic.The current fallback logic could be more robust. Consider adding error handling for cases where both
connector.iconandPeanutWalletIconfail to load.const walletImage = useMemo(() => { - return isPeanutWallet ? PeanutWalletIcon : connector?.icon || PeanutWalletIcon + if (isPeanutWallet) return PeanutWalletIcon + if (connector?.icon) return connector.icon + return PeanutWalletIcon }, [isPeanutWallet, connector])
108-157: Consider adding error boundaries.The
WalletEntryCardcomponent handles sensitive wallet information. Consider wrapping it with an error boundary to gracefully handle rendering failures.Example implementation:
class WalletEntryCardErrorBoundary extends React.Component { // ... error boundary implementation } // Usage: <WalletEntryCardErrorBoundary> <WalletEntryCard {...props} /> </WalletEntryCardErrorBoundary>
160-169: Enhance accessibility for the AddNewWallet component.Consider adding ARIA labels and roles to improve accessibility for screen readers.
const AddNewWallet = ({ onClick }: { onClick: () => void }) => ( - <Card onClick={onClick}> + <Card onClick={onClick} role="button" aria-label="Add new wallet"> <Card.Content className="flex min-h-16 w-full cursor-pointer items-center justify-center gap-3 bg-purple-4 bg-opacity-25 px-4 py-3 hover:bg-opacity-20"> <div className="flex size-7 items-center justify-center rounded-full border border-n-1"> - <Icon name="plus" fill="black" className="h-7 w-7" /> + <Icon name="plus" fill="black" className="h-7 w-7" aria-hidden="true" /> </div> <span className="font-bold">Add wallet</span> </Card.Content> </Card> )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/components/Global/Modal/index.tsx(2 hunks)src/components/Global/WalletHeader/index.tsx(1 hunks)tailwind.config.js(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/components/Global/Modal/index.tsx
🔇 Additional comments (2)
tailwind.config.js (1)
15-15: LGTM: Color palette extensions
The new color additions (purple-4: #AE7AFF and yellow-4: #FAE8A4) follow the existing color scheme pattern and naming conventions.
Also applies to: 21-21
src/components/Global/WalletHeader/index.tsx (1)
1-24: LGTM! Imports and interface definitions are well-structured.
The imports are properly organized, and the WalletEntryCardProps interface is well-defined with appropriate types.
| '.btn-yellow': { | ||
| '@apply btn bg-yellow-1 text-n-1 fill-n-1 hover:bg-yellow-1/90': {}, | ||
| }, |
There was a problem hiding this comment.
💡 Codebase verification
The btn-yellow class is defined but not used in the codebase
The verification shows that the .btn-yellow class is only defined in tailwind.config.js but is not currently used in any component files. While the implementation follows the correct pattern, it appears to be unused at the moment.
- The class is defined in
tailwind.config.jsbut no usage was found in any of the source files - Consider either:
- Adding the intended usage of this button variant in the relevant components
- Removing the unused class definition if it's not needed
🔗 Analysis chain
LGTM: Button variant implementation
The new .btn-yellow variant follows the established button styling patterns and correctly implements hover states. The implementation is consistent with other button variants in the codebase.
Let's verify the usage of this new button variant:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for usage of the new btn-yellow class
# Expected: Find components using this new button variant
rg -l "btn-yellow" --type tsx --type jsx
Length of output: 68
Script:
#!/bin/bash
# Let's try a different approach to find any usage of btn-yellow class
# Search in all files without type restriction as the file extensions might vary
rg "btn-yellow"
Length of output: 67
Script:
#!/bin/bash
# Let's also check the file extensions in the repository to understand what we should look for
fd -e tsx -e jsx -e js -e ts
Length of output: 13065
2 participants
Summary by CodeRabbit
Release Notes
New Features
WalletHeaderandHomeHeadercomponents for enhanced wallet management and navigation.CopyToClipboardcomponent for easy text copying functionality.arrow-down-filledandwalletadded for improved UI representation.hideCloseButtonproperty in modals for customizable close button visibility.Bug Fixes
Chores