Add connect button loaders, improve Ledger modal #166
Conversation
|
| const contextValue = useMemo( | ||
| () => ({ | ||
| rpc, | ||
| walletDataList, | ||
| chains, | ||
| loadingWalletId, | ||
| setLoadingWalletId, |
There was a problem hiding this comment.
instead of setter you can pass wrapper function to wrap connectAsync and do on/off state setting
There was a problem hiding this comment.
I don't see how it is more convenient. Some wallets don't need loaders and will use the standart connectAsync fn to connect, some will use this non-standart wrapper function
There was a problem hiding this comment.
Because you are repeating same try {set(true) } finally {set(false)}, and bound to make mistake somewhere
There was a problem hiding this comment.
you can pass both setter and a wrapper but I don't see any non confirming wallets so far
| await connectAsync( | ||
| { connector }, | ||
| { | ||
| onSettled: () => { |
There was a problem hiding this comment.
Can you check if we need those callbacks? Perhaps you can use try,catch,finally.
There was a problem hiding this comment.
Why? These are documented callbacks, they work, why should I change them?
There was a problem hiding this comment.
Because it makes code more complicated. If we can use try/catch/finally it will much easier to understand, e.g. right now we need to refer to docs for onSettled.
Resolves SI-1229
Changes
Demo
The main part of WalletConnect and Binance code is loaded only after user clicks WalletConnect button. During the code loading and executing, a user can see the loading indicator. Other connection buttons become disabled, so the user is not able to break the connection process by randomly clicking other wallets while waiting WC/Binance to load.
WalletConnect
wc.mp4
Binance
binance.mp4
Ledger
Ledger HID connection has another logic, so I had to add a loading spinner right into the Ledger modal. Ledger connection requires even more code to load and, as in other cases, it starts to load only when a user clicks Ledger button. While the code is loading, the user can see the spinner.
ledger-fast.mp4
Before the change, users could have problems if the internet speed is relatively slow: the ledger code is not loaded yet, but we already show the invitation to connect a device and nothing happens while the code is loading in background. Also, the "retry" button wasn't working properly in that case.
Still, there is an issue if the ledger packages loading takes too long (~ >5 sec). A user will see a connection error immediately after packages were loaded. That happens because of browser restrictions: we are trying to connect to a device via browser API while last user's interaction (click) happened more than 5 seconds ago (because the user waited all this time and didn't click anywhere). See this case on a video below. At least, the user can still connect by clicking the "retry" button. This case should be fixed separately.
ledger-slow.mp4