interface icon indicating copy to clipboard operation
interface copied to clipboard

Fix and improve transaction errors in modals with respect to the pertinent modal

Open drewcook opened this issue 2 years ago • 2 comments

Describe the bug

Currently, errors for transactions will show up on any given modal that is open, rather than the modal for the action that initiated the transaction. Errors should be scoped to show in only the modal that is open for that given action, or show as a snackbar-style message/alert globally, so that the error still is alerted to the user even after closing the modal.

Example is the following steps, when seeing an error for a repay action within a withdraw modal. This is due in part from the current implementation of TxModalDetails, which persists the error state across all modals. This will be much easier to address and fix after the Zustand work is implemented, as we'll have a global store to start managing state.

To Reproduce Steps to reproduce the behavior:

  1. Initiate a repay (close modal)
  2. Don't confirm in MM
  3. Initiate a withdraw (new modal opens)
  4. Don't confirm in MM
  5. Reject the initial repay pending tx in MM
  6. See error for the repay when the withdraw on the withdraw modal
  7. Still pending withdraw tx in MM that can be confirmed...

Expected behavior A clear and concise description of what you expected to happen.

Screenshots

Error for repay action showing in the withdraw modal:

image

Additional context

Zustand PR is #964

drewcook avatar Oct 24 '22 22:10 drewcook

@drewcook I think it would actually make sense to have transaction manager (keeping track of pending transactions - not only a single txn at a time) as it makes handling this kind of problem a lot easier.

We were thinking about doing this in #964 as we did sth similar in our projects (see https://github.com/bgd-labs/fe-shared), but eventually ended up not including it as aave-interface is stuck on an outdated web3-react making out code somewhat incompatible. For anyone looking into this, it might be a good reference though.

sakulstra avatar Oct 25 '22 09:10 sakulstra

@sakulstra I think that's a good idea. It would be great to build a more elegant system like that that would manage the transactions like a FIFO queue. And possibly, no required but supplementary, even build some UI around it as well so the user is aware of the order of operations that is happening (of course, wallets do this already). But primarily we need a more robust and mature system than what currently exists in the app. It would be very easy to reference the class or HOC that manages this from any other component in the app, and we could integrate our state management system with it using a new store slice in Zustand.

drewcook avatar Oct 25 '22 15:10 drewcook