-
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
chalabi/mobile on desktop #150
Changes from 3 commits
f059f3b
7ff68a0
2a0dfa4
a8cb41d
d5ee8ec
639ff84
2bc2040
e67eeb7
87cd32d
bf3b8e3
a0a7c71
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 |
---|---|---|
|
@@ -57,7 +57,7 @@ | |
<Image | ||
height={0} | ||
width={0} | ||
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. 🛠️ Refactor suggestion Add test coverage for MetaMask logo conditional. The new conditional logic for MetaMask logo handling is currently not covered by tests. Consider adding test cases to verify:
describe('Connected component', () => {
it('should render MetaMask logo for Cosmos MetaMask Extension', () => {
render(<Connected name="Cosmos MetaMask Extension" logo="some-logo.svg" {...defaultProps} />);
expect(screen.getByAltText('Cosmos MetaMask Extension')).toHaveAttribute('src', '/metamask.svg');
});
it('should render wallet logo for other wallets', () => {
const logo = 'wallet-logo.svg';
render(<Connected name="Other Wallet" logo={logo} {...defaultProps} />);
expect(screen.getByAltText('Other Wallet')).toHaveAttribute('src', getRealLogo(logo));
});
}); 🧰 Tools🪛 GitHub Check: codecov/patch[warning] 60-60: components/react/views/Connected.tsx#L60 |
||
alt={name} | ||
className="w-8 h-8 rounded-full mr-2" | ||
/> | ||
|
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. 🛠️ Refactor suggestion Extract shared wallet logo logic The logo rendering logic is duplicated across multiple components (Connecting, NotExist, Error). Consider creating a shared utility: + // Add to utils/wallet.ts
+ export const getWalletLogo = (name: string, logo: string): string => {
+ return name === WALLET_TYPES.METAMASK ? WALLET_LOGOS.METAMASK : getRealLogo(logo);
+ };
- src={name === 'Cosmos MetaMask Extension' ? '/metamask.svg' : getRealLogo(logo)}
+ src={getWalletLogo(name, logo)} This would improve maintainability and ensure consistent behavior across components.
🧰 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 | ||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -16,21 +16,28 @@ | |||||||||||||||||||||||||||||||||
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( | ||||||||||||||||||||||||||||||||||
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 | ||||||||||||||||||||||||||||||||||
) | ||||||||||||||||||||||||||||||||||
); | ||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||
const mobile = wallets.filter(wallet => | ||||||||||||||||||||||||||||||||||
['Wallet Connect', 'Keplr Mobile', 'Cosmostation Mobile', 'Leap Mobile'].includes( | ||||||||||||||||||||||||||||||||||
wallet.walletInfo.prettyName | ||||||||||||||||||||||||||||||||||
) | ||||||||||||||||||||||||||||||||||
['Keplr Mobile', 'Cosmostation Mobile', 'Leap Mobile'].includes(wallet.walletInfo.prettyName) | ||||||||||||||||||||||||||||||||||
); | ||||||||||||||||||||||||||||||||||
const { isMobile } = useDeviceDetect(); | ||||||||||||||||||||||||||||||||||
const hasMobileVersion = (prettyName: string) => { | ||||||||||||||||||||||||||||||||||
return mobile.some(w => w.walletInfo.prettyName.startsWith(prettyName)); | ||||||||||||||||||||||||||||||||||
}; | ||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||
const getMobileWalletName = (browserName: string) => { | ||||||||||||||||||||||||||||||||||
return mobile.find(w => w.walletInfo.prettyName.startsWith(browserName))?.walletInfo.name; | ||||||||||||||||||||||||||||||||||
}; | ||||||||||||||||||||||||||||||||||
Comment on lines
+34
to
+40
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. 🛠️ Refactor suggestion Add input validation to utility functions. The new utility functions Consider this implementation: const hasMobileVersion = (prettyName: string) => {
+ if (!prettyName) return false;
return mobile.some(w => w.walletInfo.prettyName.startsWith(prettyName));
};
const getMobileWalletName = (browserName: string) => {
+ if (!browserName) return undefined;
return mobile.find(w => w.walletInfo.prettyName.startsWith(browserName))?.walletInfo.name;
}; 📝 Committable suggestion
Suggested change
🧰 Tools🪛 GitHub Check: codecov/patch[warning] 35-41: components/react/views/WalletList.tsx#L35-L41 |
||||||||||||||||||||||||||||||||||
return ( | ||||||||||||||||||||||||||||||||||
<div className="p-1 relative max-w-sm mx-auto"> | ||||||||||||||||||||||||||||||||||
<h1 className="text-sm font-semibold text-center mb-6">Connect Wallet</h1> | ||||||||||||||||||||||||||||||||||
|
@@ -42,22 +49,41 @@ | |||||||||||||||||||||||||||||||||
<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={`${isMobile ? 'hidden' : 'block'}`}> | ||||||||||||||||||||||||||||||||||
<div className="space-y-2 mb-4"> | ||||||||||||||||||||||||||||||||||
{browser.map(({ walletInfo: { name, prettyName, logo } }) => ( | ||||||||||||||||||||||||||||||||||
<button | ||||||||||||||||||||||||||||||||||
key={name} | ||||||||||||||||||||||||||||||||||
onClick={() => onWalletClicked(name)} | ||||||||||||||||||||||||||||||||||
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() ?? '')} | ||||||||||||||||||||||||||||||||||
alt={prettyName} | ||||||||||||||||||||||||||||||||||
className="w-10 h-10 rounded-xl mr-3" | ||||||||||||||||||||||||||||||||||
/> | ||||||||||||||||||||||||||||||||||
<span className="text-md">{prettyName}</span> | ||||||||||||||||||||||||||||||||||
</button> | ||||||||||||||||||||||||||||||||||
<div key={name} className="w-full"> | ||||||||||||||||||||||||||||||||||
<button | ||||||||||||||||||||||||||||||||||
onClick={() => onWalletClicked(name)} | ||||||||||||||||||||||||||||||||||
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={ | ||||||||||||||||||||||||||||||||||
prettyName === 'Cosmos MetaMask Extension' | ||||||||||||||||||||||||||||||||||
? '/metamask.svg' | ||||||||||||||||||||||||||||||||||
: getRealLogo(logo?.toString() ?? '') | ||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||
alt={prettyName} | ||||||||||||||||||||||||||||||||||
className="w-10 h-10 rounded-xl mr-3" | ||||||||||||||||||||||||||||||||||
/> | ||||||||||||||||||||||||||||||||||
<span className="text-md flex-1 text-left"> | ||||||||||||||||||||||||||||||||||
{prettyName === 'Cosmos MetaMask Extension' ? 'MetaMask' : prettyName} | ||||||||||||||||||||||||||||||||||
</span> | ||||||||||||||||||||||||||||||||||
{hasMobileVersion(prettyName) && ( | ||||||||||||||||||||||||||||||||||
<div | ||||||||||||||||||||||||||||||||||
onClick={e => { | ||||||||||||||||||||||||||||||||||
e.stopPropagation(); | ||||||||||||||||||||||||||||||||||
onWalletClicked(getMobileWalletName(prettyName) || ''); | ||||||||||||||||||||||||||||||||||
}} | ||||||||||||||||||||||||||||||||||
className="p-1.5 rounded-lg dark:hover:bg-[#ffffff1a] hover:bg-[#0000000d] dark:bg-[#ffffff37] bg-[#d5d5e4] transition cursor-pointer" | ||||||||||||||||||||||||||||||||||
title={`Connect with ${prettyName} Mobile`} | ||||||||||||||||||||||||||||||||||
> | ||||||||||||||||||||||||||||||||||
<img src={getRealLogo('/sms')} alt="mobile" className="w-5 h-5" /> | ||||||||||||||||||||||||||||||||||
</div> | ||||||||||||||||||||||||||||||||||
)} | ||||||||||||||||||||||||||||||||||
</button> | ||||||||||||||||||||||||||||||||||
</div> | ||||||||||||||||||||||||||||||||||
))} | ||||||||||||||||||||||||||||||||||
</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 |
---|---|---|
|
@@ -12,6 +12,7 @@ import { useEffect, useMemo, useState } from 'react'; | |
import SignModal from '@/components/react/authSignerModal'; | ||
import { manifestAssets, manifestChain } 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'; | ||
|
@@ -123,6 +124,16 @@ function ManifestApp({ Component, pageProps }: ManifestAppProps) { | |
name: 'Reddit', | ||
logo: '/reddit', | ||
}, | ||
{ | ||
provider: 'email_passwordless', | ||
name: 'Email', | ||
logo: '/email', | ||
}, | ||
{ | ||
provider: 'sms_passwordless', | ||
name: 'SMS', | ||
logo: '/sms', | ||
}, | ||
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 Web3Auth configuration requires additional setup The verification reveals missing critical configuration:
To enable email and SMS authentication:
🔗 Analysis chainVerify email and SMS authentication configuration. The addition of email and SMS authentication methods enhances accessibility, but requires proper configuration to function correctly. Let me gather more information about the Web3Auth configuration and usage in the codebase. Let me check the Web3Auth configuration and initialization code to understand how email and SMS providers are set up. 🏁 Scripts executedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Description: Verify Web3Auth configuration for email and SMS providers
# Check for required environment variables and configuration files
# Check if Web3Auth client ID is configured
if ! grep -q "NEXT_PUBLIC_WEB3_CLIENT_ID" .env*; then
echo "Warning: Web3Auth client ID environment variable not found"
fi
# Check Web3Auth configuration in package.json
if [ -f "package.json" ]; then
jq '.dependencies["@web3auth/base"]' package.json
jq '.dependencies["@web3auth/modal"]' package.json
fi
Length of output: 351 Script: #!/bin/bash
# Search for Web3Auth related imports and configurations
echo "=== Searching for Web3Auth related imports ==="
rg "@web3auth" -A 3
echo -e "\n=== Searching for Web3Auth configuration patterns ==="
ast-grep --pattern 'new Web3Auth($$$)'
echo -e "\n=== Checking for environment variable usage ==="
rg "NEXT_PUBLIC_WEB3" -A 2
echo -e "\n=== Looking for configuration files ==="
fd -e config.js -e config.ts
Length of output: 882 Script: #!/bin/bash
# Check Web3Auth configuration and provider setup
echo "=== Checking Web3Auth Modal configuration ==="
rg -A 10 "new Web3Modal"
echo -e "\n=== Looking for provider configurations ==="
rg "provider.*['\"]email_passwordless['\"]|provider.*['\"]sms_passwordless['\"]" -A 5
echo -e "\n=== Checking authentication related functions ==="
ast-grep --pattern 'async function $_($$_) {
$$$
login($$$)
$$$
}'
Length of output: 923 |
||
], | ||
|
||
client: { | ||
|
@@ -145,7 +156,7 @@ function ManifestApp({ Component, pageProps }: ManifestAppProps) { | |
); | ||
|
||
// combine the web3auth wallets with the other wallets | ||
const combinedWallets = [...web3AuthWallets, ...wallets]; | ||
const combinedWallets = [...web3AuthWallets, ...wallets, ...cosmosExtensionWallets]; | ||
|
||
// 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.
Remove unnecessary non-null assertions
The code uses unnecessary non-null assertions (!). Consider using optional chaining instead.
Also applies to: 151-151
🧰 Tools
🪛 Biome (1.9.4)
[error] 147-147: Forbidden extra non-null assertion.
Safe fix: Remove extra non-null assertion.
(lint/suspicious/noExtraNonNullAssertion)
🪛 GitHub Check: codecov/patch
[warning] 147-148: components/react/modal.tsx#L147-L148
Added lines #L147 - L148 were not covered by tests