Add waiting time to reconnection #2526
Conversation
henrypalacios
added
app:CowSwap
|
CLA Assistant Lite All Contributors have signed the CLA. |
|
|
Hey @henrypalacios , great ideas! However, I'd like to propose not to change 'Conneting...' state to 'Trying to connect', as it is truncated in the desktop view, + counter is not visible. Maybe we do not need to show the counter at all. just to keep 'Connecting' state up to 15 sec, then stop it and show 'Error connecting' pop-up with the last connected wallet info @alfetopito , WDYT? |
|
Yep, agree with Elena's suggestions. Keep the same text as before (Connecting) without a counter. |
|
@elena-zh, after discussing it with @alfetopito, I'll leave this PR up to here without fulfilling the last part you suggest:
would requires some data states to be uploaded of level and will make this PR grow much more. |
|
Hey @henrypalacios , great changes. Also, I have created a related low priority related issue that might be good to fix in the future #2530 |
| if (active) { | ||
| setTried(true) | ||
| } else { | ||
| timeout = setTimeout(() => { | ||
| localStorage.removeItem(STORAGE_KEY_LAST_PROVIDER) |
There was a problem hiding this comment.
I don't get it. Why do we need to add this? maybe a comment would help to understand.
There was a problem hiding this comment.
The last provider is stored before the unload component.
After a page reload when trying to reconnect a wallet from an external provider such as Fortmatic or CoinbaseWallet we have no way of knowing if the session was cut off.
The timeout would be the time we wait while the external provider reconnects the session, otherwise we assume that it has been tried, there is no lastProvider and the user can open walletModals.
There was a problem hiding this comment.
Thinking about it, maybe it makes more sense that this timeout logic is inside the reconnectProvider and this way it is only executed when there is a lastProvider.
I can do this together with #2515 but let me know if you have any suggestion on this.
There was a problem hiding this comment.
I don't fully get it TBH. In principle seems like a fragile logic. Also, i was referring to adding a comment in the code too
Maybe i would understand describing the sequence of events, and why we really need this timeout logic
| } | ||
|
|
||
| return () => timeout && clearTimeout(timeout) |
There was a problem hiding this comment.
why do we clear the timeout? I understand we don't want to do the setTried but, if you clear it you won't do the removeItem
There was a problem hiding this comment.
The clearTimeout is because if during the waiting time the provider changes its active sate, it would no longer need to remove the lastProvider.
And ultimately removeItem will be executed by handleBeforeUnload
Summary
Closes #2514
Stop the
connecting...status and delete thelastProviderfrom storage after 15sec without success