fix: setup copy changes #745
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
Caution Review failedThe pull request is closed. WalkthroughThis pull request standardizes address rendering across multiple components by replacing plain text addresses with the new Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant PC as Payment Component
participant AL as AddressLink Component
U->>PC: Initiate Payment Process
PC->>AL: Render recipient address with provided data
AL->>AL: Validate address (using isAddress) and format display (using printableAddress/ENS)
AL-->>PC: Return clickable address element
PC-->>U: Display processed address
sequenceDiagram
participant U as User
participant IP as InstallPWA Component
participant DD as Device Detector
participant QR as QRCodeWrapper
U->>IP: Open Install PWA Page
IP->>DD: Check device type via user agent regex
DD-->>IP: Return device type (desktop/mobile)
alt Desktop
IP->>IP: Set deviceType to "desktop"
IP->>QR: Generate QR code linking to current origin
IP-->>U: Display QR code & alternative install button
else Mobile
IP->>IP: Set deviceType as mobile (iOS/Android)
IP-->>U: Display mobile installation instructions
end
Possibly related PRs
Suggested reviewers
Poem
Tip ⚡🧪 Multi-step agentic review comment chat (experimental)
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🪧 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 (
|
1 similar comment
| import { type GetUserLinksResponse, fetchWithSentry } from '@/utils' | ||
| import { hitUserMetric } from '@/utils/metrics.utils' | ||
| import { ToastId, useToast } from '@chakra-ui/react' | ||
| import { ToastId, useToast } from '@chakra-ui/react' // TODO: use normmal toasts we use throughout the app, not chakra toasts! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/components/Payment/Views/Payment.status.view.tsx (1)
21-21: Remove unused import.The
isAddressfunction is imported from 'viem' but doesn't appear to be used anywhere in this component. Consider removing this unused import to keep the code clean.-import { isAddress } from 'viem'
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
src/components/Global/ImageGeneration/LinkPreview.tsx(2 hunks)src/components/Global/ListItemView/index.tsx(1 hunks)src/components/Global/WalletHeader/index.tsx(0 hunks)src/components/Home/WalletCard.tsx(3 hunks)src/components/Payment/History/index.tsx(2 hunks)src/components/Payment/Views/Payment.status.view.tsx(2 hunks)
💤 Files with no reviewable changes (1)
- src/components/Global/WalletHeader/index.tsx
🚧 Files skipped from review as they are similar to previous changes (3)
- src/components/Payment/History/index.tsx
- src/components/Global/ListItemView/index.tsx
- src/components/Home/WalletCard.tsx
🧰 Additional context used
🧠 Learnings (1)
src/components/Global/ImageGeneration/LinkPreview.tsx (1)
Learnt from: jjramirezn
PR: peanutprotocol/peanut-ui#460
File: src/components/Global/ImageGeneration/LinkPreview.tsx:125-128
Timestamp: 2025-03-12T19:04:37.402Z
Learning: 'tokenPrice' is not used currently and will be removed in future PRs.
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Deploy-Preview
🔇 Additional comments (4)
src/components/Global/ImageGeneration/LinkPreview.tsx (2)
2-2: Good addition of the AddressLink component import.This import aligns with the PR's objective to ensure consistency in address display throughout the application.
71-71: Clean implementation of the AddressLink component.Replacing the plain text address with the AddressLink component improves consistency across the UI while maintaining readability with the "no-underline" class. This change successfully implements part of the PR objective to standardize address display throughout the application.
src/components/Payment/Views/Payment.status.view.tsx (2)
143-143: Great documentation practice.Including a reference to the Discord conversation that influenced this resolution logic is excellent. This helps future developers understand the reasoning behind the implementation decisions.
145-147: Well-structured address resolution hierarchy.The simplification of the recipient address resolution logic improves code clarity while maintaining the proper resolution hierarchy. Using the
AddressLinkcomponent consistently helps ensure a uniform user experience across the application.Also applies to: 149-150
|
|
||
| const recipientLink = useMemo(() => { | ||
| if (!requestDetails) return null | ||
| // context on resolution logic: https://discord.com/channels/972435984954302464/1351613681867427932/1351632018177392650 |
There was a problem hiding this comment.
comment(blocking): We should check if the recipientAccount type is PEANUT_WALLET and use user.username for that case, recipientAddress for everything else
There was a problem hiding this comment.
reason i didnt do that is because addresslink alrdy resolves to username.peanut.me
But thats wrong reasoning, we only need to see username. Good comment thx!
There was a problem hiding this comment.
praise: long needed change
| const { isKernelClientReady, isRegistering, isLoggingIn, isSendingUserOp, address } = useZerodevStore() | ||
| const { setWebAuthnKey, getClientForChain } = useKernelClient() | ||
|
|
||
| // Future note: could be `${handle}.${process.env.NEXT_PUBLIC_JUSTANAME_ENS_DOMAIM || 'peanut.me'}` (have to change BE too) |
|
|
||
| const recipientLink = useMemo(() => { | ||
| if (!requestDetails) return null | ||
| // context on resolution logic: https://discord.com/channels/972435984954302464/1351613681867427932/1351632018177392650 |
There was a problem hiding this comment.
reason i didnt do that is because addresslink alrdy resolves to username.peanut.me
But thats wrong reasoning, we only need to see username. Good comment thx!
| recipientAddressFormatter: (address: string) => { | ||
| const sanitizedAddressOrName = isAddress(address) ? printableAddress(address) : address | ||
| return `To ${sanitizedAddressOrName}` | ||
| return ( |
There was a problem hiding this comment.
blocking: @Hugo0 we can remove the recipientAddressFormatter from here, its' not being used anywhere, check the updated ListItemView and how it is being used in history page.
| {createType === 'direct' && | ||
| `You will send the tokens directly to ${recipient.name ?? recipient.address}. Ensure the recipient address is correct, else the funds might be lost.`} | ||
| `You will send the tokens directly to ${ | ||
| recipient.name ?? <AddressLink address={recipient.address ?? ''} /> |
There was a problem hiding this comment.
qn: not linking recipient.name to users' profile here?
There was a problem hiding this comment.
You're correct- adding todo comment, since isnt used anywhere
| subText: new Date(entry.timestamp).toLocaleDateString(), | ||
| }} | ||
| metadata={{ | ||
| recipientAddress: entry.senderAccount |
There was a problem hiding this comment.
blocking: same recipientAddressFormatter not being used in the ListItemView
| title: `Your Passkey is already setup now`, | ||
| description: `Use your face or fingerprint to verify it's you. There's no need for a password.`, | ||
| title: `Your Passkey is set up!`, | ||
| description: `From now on, Peanut will use your face or fingerprint to verify it's you. Never lose your seed phrase again.`, |
There was a problem hiding this comment.
Never lose your seed phrase again
not sure about this part in copy, there is no seed phares involved, it makes sense in the passkey-permission view since we are letting users know, but in success view i'm not sure if it makes sense
There was a problem hiding this comment.
adjusted copy slightly
| description: 'Have another wallet? Add it here to keep everything in one place.', | ||
| title: 'Add External Wallet', | ||
| description: | ||
| 'Want to manage multiple wallets from peanut? Add it now to keep everything in one place - or do it later', |
| } else if (isMobileDevice) { | ||
| setDeviceType('android') | ||
| } else { | ||
| // Log user agent and detection results |
| To add Peanut wallet to your Home screen, tap the (<ShareIcon />) icons and then {`"Add home Screen"`}{' '} | ||
| in your Safari browser. | ||
| To add Peanut to your Home screen, tap the (<ShareIcon />) icons and then {`"Add to home screen"`} in | ||
| your Safari browser. |
There was a problem hiding this comment.
thought: user can be on ios but on chrome browser, we saying Safari explicitly, should be more generic imo?
| return <AndroidInstructions /> | ||
| default: | ||
| return <DesktopInstructions /> | ||
| console.log('Showing instructions for:', deviceType) |
There was a problem hiding this comment.
I think these are fine!
jjramirezn
force-pushed
the
peanut-wallet-dev
branch
from
de1b195 to
a9652c6
Compare
jjramirezn
force-pushed
the
fix/copy-issues-mar-10
branch
from
5535c39 to
d2b2aec
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
src/components/Global/AddressLink/index.tsx (4)
1-1: Remove unused import.The
RecipientTypeimport is no longer used in the component after the refactoring.-import { RecipientType } from '@/lib/url-parser/types/payment'
7-7: Consider consolidating imports from the same module.You're importing from
@/utilsin two separate import statements (line 2 and line 7).-import * as utils from '@/utils' -import { printableAddress } from '@/utils' +import { printableAddress, ...other needed utils... } from '@/utils'
9-13: The interface looks good, but children prop is unused.You've defined a
childrenprop in the interface, but it's not destructured or used in the component.Either remove the unused prop from the interface or update the component to use it:
interface AddressLinkProps { address: string className?: string - children?: React.ReactNode }Or alternatively, if you intend to use it:
-const AddressLink = ({ address, className = '' }: AddressLinkProps) => { +const AddressLink = ({ address, className = '', children }: AddressLinkProps) => {And then in the return statement:
<Link className={`cursor-pointer underline ${className}`} href={url} target="_blank"> - {displayAddress} + {children || displayAddress} </Link>
39-43: Consider adding a title attribute for accessibility.For better accessibility, consider adding a title attribute to the link that explains where it leads.
<Link className={`cursor-pointer underline ${className}`} href={url} target="_blank" + title={`View details for ${displayAddress}`} > {displayAddress} </Link>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
src/app/(mobile-ui)/history/page.tsx(1 hunks)src/app/[...recipient]/layout.tsx(2 hunks)src/components/Create/Link/Confirm.view.tsx(3 hunks)src/components/Global/AddressLink/index.tsx(1 hunks)src/components/Global/ListItemView/index.tsx(1 hunks)src/components/Global/WalletHeader/index.tsx(0 hunks)src/components/Home/WalletCard.tsx(3 hunks)src/components/Payment/History/index.tsx(2 hunks)src/components/Payment/PaymentForm/index.tsx(2 hunks)src/components/Payment/PaymentInfoRow.tsx(1 hunks)src/components/Payment/Views/Confirm.payment.view.tsx(3 hunks)src/components/Payment/Views/Payment.status.view.tsx(2 hunks)src/components/Setup/Setup.consts.tsx(6 hunks)src/components/Setup/Views/InstallPWA.tsx(7 hunks)src/context/authContext.tsx(1 hunks)src/hooks/useZeroDev.ts(1 hunks)
💤 Files with no reviewable changes (1)
- src/components/Global/WalletHeader/index.tsx
🚧 Files skipped from review as they are similar to previous changes (14)
- src/app/(mobile-ui)/history/page.tsx
- src/context/authContext.tsx
- src/components/Payment/PaymentInfoRow.tsx
- src/hooks/useZeroDev.ts
- src/app/[...recipient]/layout.tsx
- src/components/Payment/Views/Payment.status.view.tsx
- src/components/Setup/Views/InstallPWA.tsx
- src/components/Payment/PaymentForm/index.tsx
- src/components/Payment/History/index.tsx
- src/components/Payment/Views/Confirm.payment.view.tsx
- src/components/Global/ListItemView/index.tsx
- src/components/Home/WalletCard.tsx
- src/components/Create/Link/Confirm.view.tsx
- src/components/Setup/Setup.consts.tsx
🧰 Additional context used
🧬 Code Definitions (1)
src/components/Global/AddressLink/index.tsx (1)
src/utils/general.utils.ts (1) (1)
printableAddress(63-66)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Deploy-Preview
🔇 Additional comments (3)
src/components/Global/AddressLink/index.tsx (3)
16-18: Good initial state setup.The initialization of
displayAddresswith conditional logic is a good practice, ensuring a sensible default value.
27-34: Improved address display logic.The updated effect properly prioritizes ENS names when available, with sensible fallbacks. The code logic is cleaner and more maintainable.
36-37:✅ Verification successful
Simpler URL construction.
The URL construction has been simplified, which is good. However, make sure this simplified approach works for all use cases where this component is used.
🏁 Script executed:
#!/bin/bash # Check if there are any instances where AddressLink might expect different URL formats rg -A 2 -B 2 "AddressLink" --type ts --type tsxLength of output: 75
It looks like the previous search command didn’t work because the file type
tsxwasn’t recognized. Let’s try verifying the usage ofAddressLinkacross both.tsand.tsxfiles using a file glob pattern instead. Please run the following script:After you check the output, we can decide if any additional changes or manual verifications are needed.
🏁 Script executed:
#!/bin/bash # Search for usage of AddressLink in both .ts and .tsx files using a glob pattern. rg -A 2 -B 2 "AddressLink" -g "*.{ts,tsx}" || echo "No instances found"Length of output: 18334
Simplified URL Construction Verified
Our search across
.tsand.tsxfiles shows that all instances ofAddressLinkpass a plain address—there’s no evidence that any usage requires a more complex URL format. The simplified URL construction (const url =/${address}``) appears to be working consistently across the codebase. Please continue to monitor usage should new requirements arise.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/components/Setup/Views/InstallPWA.tsx (1)
58-58: Potentially unused state variableThe
isDesktopstate is assigned but never referenced in the rendered logic. Consider removing it if not needed or clarify future usage to avoid confusion.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/components/Create/Link/Confirm.view.tsx(2 hunks)src/components/Payment/History/index.tsx(1 hunks)src/components/Setup/Setup.consts.tsx(6 hunks)src/components/Setup/Views/InstallPWA.tsx(7 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- src/components/Payment/History/index.tsx
- src/components/Create/Link/Confirm.view.tsx
- src/components/Setup/Setup.consts.tsx
🔇 Additional comments (6)
src/components/Setup/Views/InstallPWA.tsx (6)
4-4: The QRCodeWrapper import is aligned with the PR's objectiveThis import cleanly integrates a dedicated QR code component without introducing any conflicts.
80-99: Refine device detection logic and address logging duplication
Including
"Mac|Macintosh"inisIOSDevicemight be redundant asisMobileDeviceis false for typical macOS user agents. Review if this pattern inadvertently flags non-iOS devices asiosin edge cases (e.g., certain iPads with user agents containing"Mac").The
console.logusage replicates an existing “nit: log” comment from a previous review. Since you've indicated comfort with logs, continuing them is fine, but keep in mind React state updates are asynchronous, so the output may show a stale value fordeviceType.
118-121: Updated iOS copy looks goodRenaming to "Install Peanut" and clarifying the share icon instructions is consistent with the new text guidelines.
130-135: Rebranded Android actions align wellThe updated button label and step title (“Install Peanut”) provide clarity and maintain textual consistency.
174-182: Instruction selection behaves as expectedThe updated control flow for desktop, iOS, and Android instructions aligns with the overall device detection logic.
207-215: Conditional image rendering is consistentDisplaying the Peanut pointing image for mobile users only is clear and intuitive.
| // Desktop instructions tell the user to install on their phone | ||
| const DesktopInstructions = () => ( | ||
| <div> | ||
| {canInstall ? ( | ||
| <Button onClick={handleInstall} className="w-full"> | ||
| <InstallPWADesktopIcon /> | ||
| Install Peanut Wallet | ||
| </Button> | ||
| ) : ( | ||
| <> | ||
| <div className="space-y-4"> | ||
| <StepTitle text="Install on Desktop" /> | ||
| <p> | ||
| Look for the install icon (<InstallPWADesktopIcon />) in your browser's address bar and | ||
| click it. | ||
| </p> | ||
| <div className="space-y-4"> | ||
| <StepTitle text="Install on your Phone" /> | ||
| <p className="mb-4"> | ||
| For the best experience, we recommend installing Peanut on your phone. Scan this QR code with your | ||
| phone's camera: | ||
| </p> | ||
| <div className="mx-auto rounded-lg bg-background p-4"> | ||
| <QRCodeWrapper url={process.env.NEXT_PUBLIC_BASE_URL + '/setup' || window.location.origin} /> | ||
| </div> | ||
| {/* TODO: we need to have setup instructions after login! This currently wont fully work */} | ||
| <p className="text-center text-sm text-gray-600"> | ||
| After scanning, log in with your passkey and follow the installation instructions. | ||
| </p> | ||
| {canInstall && ( | ||
| <div className="mt-4 border-t pt-4"> | ||
| <p className="mb-2 text-sm text-gray-600">Alternatively, you can install on desktop:</p> | ||
| <Button onClick={handleInstall} className="w-full"> | ||
| <InstallPWADesktopIcon /> | ||
| Install Peanut | ||
| </Button> | ||
| </div> | ||
| </> | ||
| )} | ||
| )} | ||
| </div> |
There was a problem hiding this comment.
Fix the fallback for the QR code URL
Using the expression process.env.NEXT_PUBLIC_BASE_URL + '/setup' || window.location.origin can yield an unintended 'undefined/setup' if the environment variable is undefined. This breaks the QR code link. Use a nullish or conditional check instead.
Apply this diff to ensure a proper fallback:
-<QRCodeWrapper url={process.env.NEXT_PUBLIC_BASE_URL + '/setup' || window.location.origin} />
+<QRCodeWrapper
+ url={
+ process.env.NEXT_PUBLIC_BASE_URL
+ ? process.env.NEXT_PUBLIC_BASE_URL + '/setup'
+ : window.location.origin
+ }
/>📝 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.
| // Desktop instructions tell the user to install on their phone | |
| const DesktopInstructions = () => ( | |
| <div> | |
| {canInstall ? ( | |
| <Button onClick={handleInstall} className="w-full"> | |
| <InstallPWADesktopIcon /> | |
| Install Peanut Wallet | |
| </Button> | |
| ) : ( | |
| <> | |
| <div className="space-y-4"> | |
| <StepTitle text="Install on Desktop" /> | |
| <p> | |
| Look for the install icon (<InstallPWADesktopIcon />) in your browser's address bar and | |
| click it. | |
| </p> | |
| <div className="space-y-4"> | |
| <StepTitle text="Install on your Phone" /> | |
| <p className="mb-4"> | |
| For the best experience, we recommend installing Peanut on your phone. Scan this QR code with your | |
| phone's camera: | |
| </p> | |
| <div className="mx-auto rounded-lg bg-background p-4"> | |
| <QRCodeWrapper url={process.env.NEXT_PUBLIC_BASE_URL + '/setup' || window.location.origin} /> | |
| </div> | |
| {/* TODO: we need to have setup instructions after login! This currently wont fully work */} | |
| <p className="text-center text-sm text-gray-600"> | |
| After scanning, log in with your passkey and follow the installation instructions. | |
| </p> | |
| {canInstall && ( | |
| <div className="mt-4 border-t pt-4"> | |
| <p className="mb-2 text-sm text-gray-600">Alternatively, you can install on desktop:</p> | |
| <Button onClick={handleInstall} className="w-full"> | |
| <InstallPWADesktopIcon /> | |
| Install Peanut | |
| </Button> | |
| </div> | |
| </> | |
| )} | |
| )} | |
| </div> | |
| // Desktop instructions tell the user to install on their phone | |
| const DesktopInstructions = () => ( | |
| <div> | |
| <div className="space-y-4"> | |
| <StepTitle text="Install on your Phone" /> | |
| <p className="mb-4"> | |
| For the best experience, we recommend installing Peanut on your phone. Scan this QR code with your | |
| phone's camera: | |
| </p> | |
| <div className="mx-auto rounded-lg bg-background p-4"> | |
| <QRCodeWrapper | |
| url={ | |
| process.env.NEXT_PUBLIC_BASE_URL | |
| ? process.env.NEXT_PUBLIC_BASE_URL + '/setup' | |
| : window.location.origin | |
| } | |
| /> | |
| </div> | |
| {/* TODO: we need to have setup instructions after login! This currently wont fully work */} | |
| <p className="text-center text-sm text-gray-600"> | |
| After scanning, log in with your passkey and follow the installation instructions. | |
| </p> | |
| {canInstall && ( | |
| <div className="mt-4 border-t pt-4"> | |
| <p className="mb-2 text-sm text-gray-600">Alternatively, you can install on desktop:</p> | |
| <Button onClick={handleInstall} className="w-full"> | |
| <InstallPWADesktopIcon /> | |
| Install Peanut | |
| </Button> | |
| </div> | |
| )} | |
| </div> | |
| </div> | |
| ); |
flagging: Badly named branch
fixes:
Summary by CodeRabbit
New Features
Refactor
Style
Bug Fixes