-
Notifications
You must be signed in to change notification settings - Fork 4
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
feat: add email, sms, and metamask extension #148
Changes from 1 commit
f059f3b
6994d01
0ef933f
062c391
b47e977
42ec23f
59a3759
b80f9e4
cf55389
a10b8ed
eff477e
f07730e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -45,7 +45,11 @@ | |
<div className="flex flex-col w-full h-full py-6 mt-4 sm:px-8"> | ||
<div className="p-3 border rounded-full border-red-600 mx-auto aspect-1 flex-shrink-0"> | ||
<Image | ||
src={getRealLogo(logo)} | ||
src={ | ||
currentWalletName === 'Cosmos MetaMask Extension' | ||
? '/metamask.svg' | ||
: getRealLogo(logo) | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 Codebase verification 🛠️ Refactor suggestion Review needs adjustment - Image component usage is indeed inconsistent The codebase shows mixed usage of
However, suggesting
🔗 Analysis chainStandardize image component usage and improve accessibility
<Image
- src={
- currentWalletName === 'Cosmos MetaMask Extension'
- ? '/metamask.svg'
- : getRealLogo(logo)
- }
- alt="Wallet type logo"
+ src={getWalletLogo(currentWalletName, logo)}
+ alt={`${currentWalletName} wallet logo`}
className="flex-shrink-0 w-16 h-16 aspect-1"
width={16}
height={16}
/> Please add test coverage for the wallet logo rendering logic. Let me check the actual image component usage in these files to verify the inconsistency claim. Let me modify the pattern to better capture the Image component usage. 🏁 Scripts executedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Check image component usage consistency
rg -l "next/image|<img" components/react/views/
Length of output: 231 Script: #!/bin/bash
# Check actual Image/img usage patterns
echo "=== Checking Image imports ==="
rg "import.*Image.*from.*next/image" components/react/views/
echo -e "\n=== Checking Image component usage ==="
rg "<Image\s+.*?>" -U components/react/views/
echo -e "\n=== Checking img tag usage ==="
rg "<img\s+.*?>" -U components/react/views/
Length of output: 531 Script: #!/bin/bash
# Check Image component usage with more flexible pattern
echo "=== Checking Image component usage ==="
rg "<Image" -A 5 components/react/views/
echo -e "\n=== Checking img tag usage ==="
rg "<img" -A 5 components/react/views/
# Also check if there's a getWalletLogo utility
echo -e "\n=== Checking for getWalletLogo utility ==="
rg "getWalletLogo" --type ts
Length of output: 4187 🧰 Tools🪛 GitHub Check: codecov/patch[warning] 48-52: components/react/views/Error.tsx#L48-L52 |
||
alt="Wallet type logo" | ||
className="flex-shrink-0 w-16 h-16 aspect-1" | ||
width={16} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -40,7 +40,7 @@ | |
</div> | ||
<div className="flex flex-col w-full h-full py-6 mt-4 sm:px-8"> | ||
<img | ||
src={getRealLogo(logo)} | ||
src={name === 'Cosmos MetaMask Extension' ? '/metamask.svg' : getRealLogo(logo)} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 Codebase verification 🛠️ Refactor suggestion Confirmed: Wallet logo logic duplication needs refactoring The verification shows clear evidence of duplicated wallet logo logic across multiple components:
The original review suggestion to extract this logic is valid and would improve code maintainability. 🔗 Analysis chainExtract shared wallet logo logic to avoid duplication The wallet logo rendering logic is duplicated across multiple components. Consider extracting this into a shared utility function. + // utils/wallet.ts
+ export const getWalletLogo = (name: string, defaultLogo: string) => {
+ return name === WALLET_TYPES.METAMASK ? '/metamask.svg' : getRealLogo(defaultLogo);
+ };
// In component
src={getWalletLogo(name, logo)} Please add test coverage for the new logic. Let me check the actual implementation of these files to verify the duplication pattern. Let me try a different approach to verify the pattern. 🏁 Scripts executedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Find all components using similar wallet logo logic
rg -l "metamask\.svg"
# Check for existing wallet-related utilities
fd -e ts -e tsx -p "wallet.*\.(ts|tsx)$" src utils components
Length of output: 379 Script: #!/bin/bash
# Check the actual implementation of wallet logo logic in all relevant files
rg -A 2 "metamask\.svg" --type tsx
# Check if there's any existing wallet utility functions
ast-grep --pattern 'function $_(name: string, $_) {
$$$
metamask$$$
}'
Length of output: 141 Script: #!/bin/bash
# Check the actual implementation of wallet logo logic in all relevant files
rg -A 2 "metamask\.svg"
# Look for any existing utility functions related to wallet logos
rg "getRealLogo" -A 2
# Check for any constants or types related to wallet names
rg "WALLET_TYPES|WalletType" -A 2
Length of output: 5100 🧰 Tools🪛 GitHub Check: codecov/patch[warning] 43-43: components/react/views/NotExist.tsx#L43 |
||
alt={name} | ||
className="flex-shrink-0 w-16 h-16 mx-auto aspect-1" | ||
/> | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -15,15 +15,17 @@ | |
const isDarkMode = document.documentElement.classList.contains('dark'); | ||
|
||
const social = wallets.filter(wallet => | ||
['Google', 'Twitter', 'Apple', 'Discord', 'GitHub', 'Reddit'].includes( | ||
['Google', 'Twitter', 'Apple', 'Discord', 'GitHub', 'Reddit', 'Email', 'SMS'].includes( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 Codebase verification Missing test coverage for WalletList component and wallet types The verification confirms that there are no test files for the WalletList component, while other React components in the codebase do have test coverage. The new wallet types (Email, SMS) as well as the existing wallet filtering logic remain untested.
🔗 Analysis chainAdd test coverage for new wallet types The new wallet types (Email, SMS, MetaMask) lack test coverage. Would you like me to help generate test cases for the new wallet types and filtering logic? Also applies to: 24-26 🏁 Scripts executedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Check current test coverage for wallet list filtering
fd -e test.tsx -e spec.tsx . | xargs rg "describe.*WalletList|test.*(Email|SMS|MetaMask)"
Length of output: 91 Script: #!/bin/bash
# Let's first check if there are any test files related to WalletList
fd "WalletList.*\.(test|spec)\.(tsx|ts|js|jsx)$"
# Also search for any test files in components/react directory
fd "\.(test|spec)\.(tsx|ts|js|jsx)$" components/react
# Let's also check the actual WalletList component implementation
rg -A 5 "WalletList" components/react/views/WalletList.tsx
Length of output: 473 🧰 Tools🪛 GitHub Check: codecov/patch[warning] 18-18: components/react/views/WalletList.tsx#L18 |
||
wallet.walletInfo.prettyName | ||
) | ||
); | ||
|
||
const browser = wallets.filter(wallet => | ||
['Keplr', 'Cosmostation', 'Leap', 'Station'].includes(wallet.walletInfo.prettyName) | ||
['Keplr', 'Cosmostation', 'Leap', 'Station', 'Cosmos MetaMask Extension'].includes( | ||
wallet.walletInfo.prettyName | ||
) | ||
); | ||
|
||
console.log(wallets); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Remove debugging console.log statement Remove the console.log statement as it appears to be debugging code. - console.log(wallets); 🧰 Tools🪛 GitHub Check: codecov/patch[warning] 28-28: components/react/views/WalletList.tsx#L28 |
||
const mobile = wallets.filter(wallet => | ||
['Wallet Connect', 'Keplr Mobile', 'Cosmostation Mobile', 'Leap Mobile'].includes( | ||
wallet.walletInfo.prettyName | ||
|
@@ -40,7 +42,7 @@ | |
<XMarkIcon className="w-5 h-5" aria-hidden="true" /> | ||
</button> | ||
|
||
{/* Browser and Social sections - browaer hidden on mobile/tablet */} | ||
{/* Browser and Social sections - browser hidden on mobile/tablet */} | ||
<div className="hidden md:block"> | ||
<div className="space-y-2 mb-4"> | ||
{browser.map(({ walletInfo: { name, prettyName, logo } }) => ( | ||
|
@@ -50,11 +52,17 @@ | |
className="flex items-center w-full p-3 rounded-lg dark:bg-[#ffffff0c] bg-[#f0f0ff5c] dark:hover:bg-[#0000004c] hover:bg-[#a8a8a84c] transition" | ||
> | ||
<img | ||
src={getRealLogo(logo?.toString() ?? '')} | ||
src={ | ||
prettyName === 'Cosmos MetaMask Extension' | ||
? '/metamask.svg' | ||
: getRealLogo(logo?.toString() ?? '') | ||
} | ||
alt={prettyName} | ||
className="w-10 h-10 rounded-xl mr-3" | ||
/> | ||
<span className="text-md">{prettyName}</span> | ||
<span className="text-md"> | ||
{prettyName === 'Cosmos MetaMask Extension' ? 'MetaMask' : prettyName} | ||
</span> | ||
</button> | ||
))} | ||
</div> | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -33,6 +33,7 @@ | |
"@chain-registry/assets": "^1.64.83", | ||
"@cosmjs/cosmwasm-stargate": "0.32.4", | ||
"@cosmjs/stargate": "npm:@liftedinit/[email protected]", | ||
"@cosmos-kit/cosmos-extension-metamask": "^0.12.1", | ||
"@cosmos-kit/react": "2.18.0", | ||
"@cosmos-kit/web3auth": "2.11.0", | ||
"@fontsource/manrope": "^5.0.21", | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -15,6 +15,7 @@ import { | |
manifestTestnetAssets, | ||
} from '@/config'; | ||
import { SignerOptions, wallets } from 'cosmos-kit'; | ||
import { wallets as cosmosExtensionWallets } from '@cosmos-kit/cosmos-extension-metamask'; | ||
import { ChainProvider } from '@cosmos-kit/react'; | ||
import { Registry } from '@cosmjs/proto-signing'; | ||
import { TailwindModal } from '../components'; | ||
|
@@ -145,6 +146,16 @@ function ManifestApp({ Component, pageProps }: ManifestAppProps) { | |
name: 'Reddit', | ||
logo: '/reddit', | ||
}, | ||
{ | ||
provider: 'email_passwordless', | ||
name: 'Email', | ||
logo: '/email', | ||
}, | ||
{ | ||
provider: 'sms_passwordless', | ||
name: 'SMS', | ||
logo: '/sms', | ||
}, | ||
Comment on lines
+126
to
+135
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 Codebase verification Missing logo assets for email and SMS authentication methods The logo assets referenced in the authentication configuration (
🔗 Analysis chainVerify the existence of email and SMS logo assets The implementation of email and SMS login methods looks correct, but we need to verify that the referenced logo assets exist at 🏁 Scripts executedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Description: Check if the required logo assets exist in the public directory
# Look for email and SMS logo files in common image formats
echo "Searching for email logo:"
fd -t f "email\.(svg|png|jpg|jpeg)" public/
echo -e "\nSearching for SMS logo:"
fd -t f "sms\.(svg|png|jpg|jpeg)" public/
Length of output: 213 |
||
], | ||
|
||
client: { | ||
|
@@ -167,7 +178,7 @@ function ManifestApp({ Component, pageProps }: ManifestAppProps) { | |
); | ||
|
||
// combine the web3auth wallets with the other wallets | ||
const combinedWallets = [...web3AuthWallets, ...wallets]; | ||
const combinedWallets = [...web3AuthWallets, ...wallets, ...cosmosExtensionWallets]; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 Codebase verification Add MetaMask snap installation guidance in the wallet connection flow The wallet combination implementation is correct and compatible with the current cosmos-kit versions. However, while there's a basic "Install Metamask" prompt, it lacks specific guidance about the required Cosmos snap.
🔗 Analysis chainVerify cosmos-kit compatibility and add user guidance for MetaMask snap. The wallet combination looks correct, but there are important considerations:
Consider:
🏁 Scripts executedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Description: Check cosmos-kit compatibility
# Check cosmos-kit versions
echo "Checking cosmos-kit versions..."
if [ -f "package.json" ]; then
jq '.dependencies | with_entries(select(.key | contains("cosmos-kit")))' package.json
fi
# Look for Web3Auth wallet handler implementations
echo -e "\nChecking for Web3Auth wallet handlers..."
rg -g '!node_modules' -g '*.ts' -g '*.tsx' "Web3Auth.*handler"
Length of output: 500 Script: #!/bin/bash
# Let's verify the MetaMask snap integration and cosmos-kit compatibility
# Check for MetaMask snap-related code and UI guidance
echo "Checking for MetaMask snap-related code and UI guidance..."
rg -g '!node_modules' -g '*.{ts,tsx}' -B2 -A2 "metamask.*snap"
# Check for cosmos extension wallet integration
echo -e "\nChecking cosmos extension wallet integration..."
ast-grep --pattern 'const $_ = useChainWallet($$$)'
# Check for any UI components showing wallet connection guidance
echo -e "\nChecking for wallet connection UI components..."
rg -g '!node_modules' -g '*.{ts,tsx}' "Install.*MetaMask|MetaMask.*required"
Length of output: 772 |
||
|
||
// this is stop ssr errors when we render the web3auth signing modal | ||
const [isBrowser, setIsBrowser] = useState(false); | ||
|
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.
💡 Codebase verification
🛠️ Refactor suggestion
Refactor wallet name comparison for maintainability
The string 'Cosmos MetaMask Extension' is used consistently across multiple view components for wallet identification and logo rendering. To improve maintainability, create a new constant in
utils/constants.ts
:Issues found:
Please consolidate these hardcoded values into constants and update all occurrences for better maintainability.
🔗 Analysis chain
Refactor wallet name comparison for maintainability
The hardcoded string comparison makes the code fragile and harder to maintain. Consider extracting wallet identifiers into constants.
Additionally, please add test coverage for this new conditional logic.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
Length of output: 254
Script:
Length of output: 2892
Script:
Length of output: 2154
Script:
Length of output: 1770
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 46-46: components/react/views/Connecting.tsx#L46
Added line #L46 was not covered by tests