Skip to content
This repository has been archived by the owner on Mar 28, 2023. It is now read-only.

add reminder to connect wallet in deposit flow #326

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

brandoncurtis
Copy link
Contributor

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"

image

and changes to "Select Lot Size" once a wallet is connected

image

@@ -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>
Copy link
Contributor

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.

Copy link
Contributor Author

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 👍

Copy link
Contributor

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.

Copy link
Contributor

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.

@Shadowfiend Shadowfiend requested a review from ironng August 3, 2020 12:56
@mhluongo
Copy link
Member

@brandoncurtis I'd love to land this in the next release if you have cycles to close out the PR

@brandoncurtis
Copy link
Contributor Author

thanks @mhluongo! I'll have some time for this very soon.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants