-
Notifications
You must be signed in to change notification settings - Fork 24
fix(demo): catch unhandled wallet connection rejections and surface error in UI #118
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
Changes from all commits
53de00d
9cb7409
202fd42
e67a604
9a69104
58728f6
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 |
|---|---|---|
|
|
@@ -4,19 +4,28 @@ import { type WalletState, connectWallet, detectWallet } from "../lib/wallet"; | |
| export function useWallet() { | ||
| const [wallet, setWallet] = useState<WalletState>({ status: "disconnected" }); | ||
| const [connecting, setConnecting] = useState(false); | ||
| const [error, setError] = useState<string | null>(null); | ||
|
|
||
| useEffect(() => { | ||
|
Collaborator
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. detectWallet() is called here with no .catch(), but resolveConnectedState() (which detectWallet() calls internally) now throws instead of returning a disconnected state, per this PR's fix in lib/wallet.ts. A user with a wallet connected and allowed, but whose address retrieval fails for any reason, previously saw this resolve gracefully to |
||
| detectWallet().then(setWallet); | ||
| detectWallet() | ||
| .then(setWallet) | ||
| .catch(() => { | ||
| setWallet({ status: "disconnected" }); | ||
| }); | ||
| }, []); | ||
|
|
||
| const connect = useCallback(async () => { | ||
| setConnecting(true); | ||
| setError(null); | ||
| try { | ||
| setWallet(await connectWallet()); | ||
| const state = await connectWallet(); | ||
| setWallet(state); | ||
|
Collaborator
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. This catch never fires for the case it's meant to handle. |
||
| } catch (err) { | ||
| setError(err instanceof Error ? err.message : "Failed to connect wallet."); | ||
| } finally { | ||
| setConnecting(false); | ||
| } | ||
| }, []); | ||
|
|
||
| return { wallet, connecting, connect }; | ||
| return { wallet, connecting, error, connect }; | ||
| } | ||
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.
wallet-button-containerandwallet-error(line 34) aren't defined anywhere in App.css or index.css, so onceerroris actually set, it renders with default browser styling and undefined layout relative to the button.