Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
drewcook opened this issue Oct 24, 2022 · 2 comments
Labels
bug Something isn't working new issue This issue has not been triaged by the core team yet

Comments

@drewcook
Copy link
Contributor

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 drewcook added bug Something isn't working new issue This issue has not been triaged by the core team yet labels Oct 24, 2022
@drewcook drewcook changed the title Handle transaction errors in modals respective to the modal Handle transaction errors in modals respective to the pertinent modal Oct 24, 2022
@drewcook drewcook changed the title Handle transaction errors in modals respective to the pertinent modal Handle transaction errors in modals with respect to the pertinent modal Oct 24, 2022
@drewcook drewcook changed the title Handle transaction errors in modals with respect to the pertinent modal Fix and improve transaction errors in modals with respect to the pertinent modal Oct 24, 2022
@sakulstra
Copy link
Collaborator

sakulstra commented Oct 25, 2022

@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.

@drewcook
Copy link
Contributor Author

@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 drewcook reopened this Jan 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working new issue This issue has not been triaged by the core team yet
Projects
None yet
Development

No branches or pull requests

2 participants