-
Notifications
You must be signed in to change notification settings - Fork 3
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
Testnets #136
Testnets #136
Conversation
fix: advice at registration to check PoHID and wallet connected
WalkthroughThe changes involve modifications to two components, Changes
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 (
|
✅ Deploy Preview for proof-of-humanity-v2 ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
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
🧹 Outside diff range and nitpick comments (6)
src/app/[pohid]/claim/Review.tsx (2)
162-177
: Consider improving the warning message presentation.While the warning message is comprehensive and informative, its length might make it harder for users to quickly grasp the key points.
Consider restructuring the warning message for better readability:
- {pohId.toLowerCase() !== address?.toLowerCase() ? ( - <span className="text-orange mt-2"> - <span className="font-semibold underline">Beware</span>: Your PoH - ID differs from the wallet address connected to your account. If - you're registering for the first time, this discrepancy is a - consequence of changing the connected wallet during the - registration process. In such cases, to make both addresses match, - you may need to change the connected wallet to the original wallet - used at the beginning of the registration process, or else - reinitiate the registration process. If you are not a newcomer and - wish to reclaim your ID from a different wallet (e.g., if you have - lost the private key to your original wallet), please confirm that - the PoH ID you are using is the one from your initial - registration. - </span> - ) : null} + {pohId.toLowerCase() !== address?.toLowerCase() ? ( + <div className="text-orange mt-2 space-y-2"> + <div className="font-semibold">⚠️ Address Mismatch Warning</div> + <div>Your PoH ID differs from your connected wallet address.</div> + <div className="space-y-1"> + <p><strong>If you're a new user:</strong></p> + <ul className="list-disc pl-4"> + <li>This might be due to changing wallets during registration</li> + <li>Connect the wallet used at the start of registration, or</li> + <li>Start a new registration process</li> + </ul> + </div> + <div className="space-y-1"> + <p><strong>If you're reclaiming an existing ID:</strong></p> + <ul className="list-disc pl-4"> + <li>Ensure this PoH ID matches your initial registration</li> + </ul> + </div> + </div> + ) : null}
162-177
: Consider enhancing deposit input validation.The deposit input could benefit from explicit validation feedback to improve user experience.
Consider adding validation feedback:
<Field type="number" className="no-spinner text-right" step="any" min={0} max={formatEther(totalCost)} value={selfFunded} + onInvalid={(e) => { + const input = e.target as HTMLInputElement; + if (input.validity.rangeOverflow) { + input.setCustomValidity(`Maximum deposit is ${formatEther(totalCost)} ${nativeCurrency.symbol}`); + } else if (input.validity.rangeUnderflow) { + input.setCustomValidity('Deposit cannot be negative'); + } + }} + onInput={(e) => (e.target as HTMLInputElement).setCustomValidity('')} onChange={(event) => selfFunded$.set(+event.target.value)} />src/app/[pohid]/claim/Form.tsx (4)
Line range hint
144-150
: Remove commented-out code.The commented-out
steps
logic should either be removed or implemented if needed. Commented code can lead to confusion and maintenance issues.- // const steps = useMemo( - // () => - // renewal - // ? ["Photo", "Video", "Review"] - // : ["Info", "Photo", "Video", "Review"], - // [renewal] - // );
Line range hint
82-108
: Add error handling for IPFS uploads.The
submit
function performs IPFS uploads without proper error handling. Network issues or IPFS service failures could lead to a poor user experience.Consider wrapping the IPFS operations in try-catch blocks:
const submit = async () => { if (!media.photo || !media.video) return; + try { loading.start("Uploading media"); let data = new FormData(); data.append("###", "file.json"); data.append("name", state.name); data.append("photo", media.photo.content); data.append("video", media.video.content); const fileURI = await uploadToIPFS(data); loading.start("Uploading evidence files"); data = new FormData(); data.append("###", "registration.json"); data.append("name", "Registration"); data.append("fileURI", fileURI); state$.uri.set(await uploadToIPFS(data)); loading.stop(); + } catch (error) { + loading.stop(); + toast.error("Failed to upload files. Please try again."); + console.error("IPFS upload failed:", error); + } };
Line range hint
151-161
: Consider extracting connection validation logic.The connection and renewal validation logic could be extracted into a separate function for better maintainability and reusability.
Consider refactoring to:
+const isValidConnection = ( + isConnected: boolean, + renewal: FormProps['renewal'], + address: string, + chainId: number +) => { + if (!isConnected) return false; + if (renewal && ( + renewal.claimer.id !== address.toLowerCase() || + renewal.chain.id !== chainId + )) return false; + return true; +}; + -if ( - !isConnected || - (renewal && - (renewal.claimer.id !== address!.toLowerCase() || - renewal.chain.id !== chainId)) -) +if (!isValidConnection(isConnected, renewal, address!, chainId))
Line range hint
110-127
: Consider using a more robust state management pattern.The current state change handler combines multiple responsibilities and could benefit from a more structured approach.
Consider:
- Separating the funding calculation logic
- Using a dedicated submission handler
- Adding proper type guards for the renewal flow
This would improve maintainability and make the code more testable.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
src/app/[pohid]/claim/Form.tsx
(1 hunks)src/app/[pohid]/claim/Review.tsx
(2 hunks)
🔇 Additional comments (3)
src/app/[pohid]/claim/Review.tsx (2)
1-14
: LGTM! Clean import organization.
The imports are well-organized and all appear to be used within the component.
Line range hint 1-177
: LGTM! Well-structured component implementation.
The component demonstrates good practices:
- Proper TypeScript usage
- Clear separation of concerns
- Appropriate loading state handling
- Good user feedback mechanisms
src/app/[pohid]/claim/Form.tsx (1)
3-25
: LGTM! Import statements are well organized.
The imports have been logically grouped and ordered, improving code readability while maintaining all necessary dependencies.
Summary by CodeRabbit
New Features
Review
component with a warning message to inform users if their PoH ID does not match the connected wallet address, improving user guidance during registration.Refactor
Form
component for improved readability, while maintaining existing functionality.