-
Notifications
You must be signed in to change notification settings - Fork 31
add reminder to connect wallet in deposit flow #326
base: main
Are you sure you want to change the base?
Conversation
@@ -31,7 +31,7 @@ function LoadableBase({ children, restoreDepositState, restoreRedemptionState, r | |||
if(!depositStateRestored) { | |||
return <div className="pay"> | |||
<div className="page-top"> | |||
<p>Loading...</p> | |||
<p>Connect Wallet to Continue</p> |
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.
Couldn't the page be loading with a connected wallet and still render this? Feels like this should toggle based on web3Active
or something else from useWeb3React
, unless I'm mistaken.
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.
I thought the same thing at first, but I could not find a state where this message appeared that was not awaiting wallet connection. For instance: https://dapp.test.tbtc.network/deposit/0xCd0fc3dCb7b47aCbC9bfD5230E32be69d89be7dd/redemption/congratulations
https://dapp.test.tbtc.network/deposit/0x2Df8EFf961816809cc3a0CF5b9C520D6eE49ae04/redemption/signing
but I can figure out a web3Active
toggle if you'd prefer 👍
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.
Let me pass this on to @ironng who has a more comprehensive test setup locally for now, and let's see where we end up.
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.
I gave this a test run locally. @Shadowfiend is correct, there is a state when the wallet is already connected but we're still trying to figure out the deposit state before loading the view - having the message be "Loading" instead of "Connect Wallet" would be more appropriate. Let's add a condition to check if web3 is active to decide which message.
@brandoncurtis I'd love to land this in the next release if you have cycles to close out the PR |
thanks @mhluongo! I'll have some time for this very soon. |
Issue
It's not obvious how to progress when landing on the deposit flow because the 'connect wallet' button is fairly small and the title text doesn't indicate that connecting a wallet is necessary.
Change
Title now reads "Connect Wallet to Continue"
and changes to "Select Lot Size" once a wallet is connected