-
Notifications
You must be signed in to change notification settings - Fork 4
Adding Wallet Status + Adding a confirm password #7
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
base: main
Are you sure you want to change the base?
Conversation
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.
Pull request overview
This pull request enhances the vault creation and login experience by adding wallet detection and password confirmation features. The changes improve user trust through transparency about Web3 wallet availability and reduce password entry errors during vault creation.
- Added a wallet detection banner that displays the status of Ethereum wallet availability (MetaMask, other wallet, or none)
- Introduced a confirm password field for vault creation to ensure users correctly enter and memorize their master password
- Refactored error handling and simplified code structure with updated styling
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| setTimeout(() => onUnlock(emptyVault, password), 800); | ||
| } catch (err) { | ||
| showToast("Creation failed: " + err.message, "error"); | ||
| showToast("Creation failed"); |
Copilot
AI
Dec 28, 2025
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.
Error message clarity issue. The error message "Creation failed" doesn't include the error details anymore. The previous implementation included the error message with err.message, which helps users understand what went wrong. Consider including error details or at least logging the error for debugging purposes
| showToast("Creation failed"); | |
| console.error("Vault creation failed:", err); | |
| showToast("Creation failed: " + (err && err.message ? err.message : "Unknown error")); |
src/components/Login.js
Outdated
| const encryptedVault = await loadVault(); | ||
| if (!encryptedVault) { | ||
| showToast("No vault found", "error"); | ||
| setProcessing(false); | ||
| return; | ||
| } | ||
| const verified = await verifyVault(encryptedVault); | ||
| if (!verified) { | ||
| showToast("Integrity check failed", "error"); | ||
| setProcessing(false); | ||
| return; | ||
| } | ||
| setStatus("Unlocking..."); | ||
| if (!verified) throw new Error("Integrity failed"); | ||
|
|
||
| const decrypted = await decryptVault(encryptedVault, password); |
Copilot
AI
Dec 28, 2025
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 removed validation checks for vault existence and integrity verification could mask important errors. The previous code properly handled cases where no vault was found or when integrity checks failed with specific error messages. Now these errors will fall through to a generic "Wrong password" message, which is misleading when the actual issue is a missing vault or failed integrity check. Consider restoring these validation checks or handling these cases explicitly
src/components/Login.js
Outdated
| } catch (err) { | ||
| showToast("Wrong password", "error"); | ||
| } catch { | ||
| showToast("Wrong password"); |
Copilot
AI
Dec 28, 2025
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 default toast type for the showToast function call is missing. When calling showToast with only one argument on line 99, the type defaults to "error", but the message "Wrong password" might benefit from explicit type specification for clarity, especially since other error scenarios might be caught here due to removed validation checks
| showToast("Wrong password"); | |
| showToast("Wrong password", "error"); |
| border: "3px solid #334155", | ||
| borderTop: "3px solid #10b981", | ||
| borderRadius: "50%", | ||
| animation: "spin 0.8s linear infinite", |
Copilot
AI
Dec 28, 2025
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 CSS animation keyframes are removed but the spinner still references the "spin" animation on line 321. Without the keyframe definition, the spinner animation will not work. Either restore the keyframes or define them elsewhere in the application
| const isPasswordValid = | ||
| password.length >= 8 && (vaultExists || password === confirmPassword); |
Copilot
AI
Dec 28, 2025
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 password validation logic requires confirmPassword to match the password for vault creation, but when toggling between "unlock" and "create vault" modes, the confirmPassword state is only cleared on line 198. However, the isPasswordValid check on line 112 will evaluate password === confirmPassword when vaultExists is false. If a user enters passwords that don't match, then toggles to unlock mode and back to create mode, the previous confirmPassword value persists and could cause confusion. Consider also clearing confirmPassword when the password field changes in create mode
| setTimeout(() => setToastMessage(""), 3000); | ||
| }; | ||
|
|
||
| // CHEck wallet is connected or not |
Copilot
AI
Dec 28, 2025
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.
Spelling error in comment: "CHEck" should be "Check"
| // CHEck wallet is connected or not | |
| // Check wallet is connected or not |
|
|
||
| <div style={styles.bgEffects}> | ||
| <div style={styles.colorBlur}></div> | ||
| {/* WALLET BANNER OT NOT ?? */} |
Copilot
AI
Dec 28, 2025
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.
Spelling/grammar issue in comment: "WALLET BANNER OT NOT ??" contains a typo. "OT" should be "OR"
| {/* WALLET BANNER OT NOT ?? */} | |
| {/* WALLET BANNER OR NOT ?? */} |
| <div style={styles.walletBanner(walletStatus)}> | ||
| {walletStatus === "checking" && "Checking wallet..."} | ||
| {walletStatus === "metamask" && "🦊 MetaMask detected"} | ||
| {walletStatus === "other" && " wallet detected"} |
Copilot
AI
Dec 28, 2025
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 wallet status message for "other" wallet is missing text. The current message displays " wallet detected" with an emoji missing or space at the beginning. This should include descriptive text like "Web3 wallet detected" or "Ethereum wallet detected"
| {walletStatus === "other" && " wallet detected"} | |
| {walletStatus === "other" && "🌐 Web3 wallet detected"} |
1=>Added wallet detection at the top of the login screen to clearly indicate whether a Web3 wallet is available in the browser. This improves transparency and user trust before vault actions.
2=>Also introduced a Confirm Password field during vault creation, ensuring users re-enter their master password to avoid mistakes and securely memorize it. Vault creation is blocked unless both passwords match and meet strength requirements