sygmaprotocol / sygma-widget

Transfer widget for the sygmaprotocol
5 stars 2 forks source link

Reset context when disconnecting/lock wallet #113

Closed LyonSsS closed 3 months ago

LyonSsS commented 4 months ago

Severity: low | Priority: low

Note: After connecting Metamask wallet, and setting everything ready for a transfer, if the user disconnects the wallet via disconnect or lock ( metamask option), the context is not reseted.

image

MakMuftic commented 4 months ago

Hey team! Please add your planning poker estimate with Zenhub @Lykhoyda @mpetrunic @wainola

mpetrunic commented 4 months ago

@LyonSsS @MakMuftic Do we wanna reset everything? maybe we should just reset balance and amount?

LyonSsS commented 4 months ago

This is again a business decision. It would keep only source and destination ... but I would reset token type, amount and balance ... as if reconnect a new wallet & we have multiple tokens type mapping in the future for the same (source - destination mix) ... then we should let the user reselect the same token type, or another one ... and only load balance once. If I reconnect another wallet but I want to change the token type, we try to read balance for 2 token types. CC: @itsbobbyzzz168

itsbobbyzzz168 commented 4 months ago

Yep, sorry, i think @wainola already implemented this (correct me if i'm wrong @LyonSsS @wainola )? Just laying our Liviu's suggestion for clarity:

If wallet disconnected, reset;

If new wallet connected, reset:

LyonSsS commented 3 months ago

Tested on release version - It behaves fine except that we agree not to reset source and destination. We only agree to reset token type, amount and balance. Lock_wallet_orDisconnect

mpetrunic commented 3 months ago

@LyonSsS There is explanation in the PR:

The original issue, mentions NOT clear the selected and destination network. But we fetch the resource data and other data on selecting the destination network, so to avoid unclear behavior for the user to click again the destination network I clear all the data. If it's important to have the behavior to keep the state of networks, it would be better to keep at least a selected network and a clear destination network to avoid potential bugs with the data being not updated on component levels. IMO clearing the entire state is the most bulletproof way to avoid previous selected data

@itsbobbyzzz168 I would keep it like this (reset all for now) and maybe open feature req issue if we wanna handle that in the future as it introduces a lot of edge cases 🤷🏻‍♂️

Lykhoyda commented 3 months ago

@LyonSsS The current expected behavior is that after the transfer is complete, all the fields except the source network will be cleared. The changes were done in the PR https://github.com/sygmaprotocol/sygma-widget/pull/163/files

LyonSsS commented 3 months ago

Bases on the the last meeting we do agree to reset all. Closing this issue