Skip to content
This repository has been archived by the owner on Oct 1, 2024. It is now read-only.

feat: update notification display for transaction processing #63

Merged

Conversation

jshiohaha
Copy link
Contributor

Description

Right now, the UI only gives feedback after (1) a transaction is confirmed, in which a signature is displayed via a notification, or (2) we are unable to confirm a transaction, and a stringified message is displayed via a notification

In either case, there is a non-zero amount of time between when a user does something related to a transaction and when they see visual feedback. The changes proposed here seek to give immediate visual feedback. However, there is still a lot left on the table to better handle transaction processing, which will come in the FaaS work. This change is only meant to improve the current working thing.

Demo

Current functionality

Users must wait until a transaction is confirmed before seeing a mantine/core notification with the transaction signature(s). If the transaction fails to confirm, the user must wait ~30s before any visual feedback tells them that we were unable to verify the transaction status.

Functionality with proposed updates

Immediately after signed transactions are sent to the cluster, we will display a notification showing the current confirmation status. The user can also click through to view the transaction in an explorer immediately.

metadao-tx-popup.mp4

Note: to reproduce and test the mocked functionality shown above, you can reference this branch on my forked version of the repo here (https://github.com/jshiohaha/meta-dao-frontend/tree/feature/improve-notification-tx-display-with-mock). Once running the branch's code, visit http://localhost:3000/mocked-notifications

Here are example screenshots from minting conditional tokens on a proposal:

Transaction submitted and status is pending
transaction pending
Transaction submitted but unconfirmed within 30s
transaction unconfirmed
Transaction submitted and successfully confirmed
transaction confirmed

Run this branch

There is no special logic to get this branch up and running. Simply follow these steps:

  • pull branch
  • run npm i
  • run npm run dev
  • visit http://localhost:3000 in your browser of choice

Testing

Copy link
Collaborator

@LukasDeco LukasDeco left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a couple really minor comments on the overall interface. In general though, really fantastic work!!

hooks/useTransactionSender.tsx Outdated Show resolved Hide resolved
hooks/useTransactionSender.tsx Outdated Show resolved Hide resolved
hooks/useTransactionSender.tsx Outdated Show resolved Hide resolved
hooks/useTransactionSender.tsx Show resolved Hide resolved
@R-K-H
Copy link
Member

R-K-H commented Apr 12, 2024

@jshiohaha I've made some changes to support v0.2 and v0.3 which have modified the way some things work, if you could resolve from master, and you're ready I'm happy to get this into the current UI.

@jshiohaha
Copy link
Contributor Author

jshiohaha commented Apr 12, 2024

Awesome, will update after resolving differences from master. I also have an updated version of send + confirm transaction logic almost ready to take a look at, which I think can build on top of what's here.

@jshiohaha jshiohaha force-pushed the feature/improve-notification-tx-display branch from f7d559b to 43847b1 Compare April 12, 2024 20:31
@jshiohaha jshiohaha force-pushed the feature/improve-notification-tx-display branch from 43847b1 to 0f0ed93 Compare April 12, 2024 20:33
@R-K-H R-K-H dismissed LukasDeco’s stale review April 13, 2024 07:43

These have been resolved, and now are good for merge.

@R-K-H R-K-H self-requested a review April 13, 2024 07:43
Copy link
Member

@R-K-H R-K-H left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Let's let er rip!

@R-K-H R-K-H merged commit 37cda36 into metaDAOproject:master Apr 13, 2024
1 check passed
@jshiohaha jshiohaha deleted the feature/improve-notification-tx-display branch April 13, 2024 14:49
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants