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

feat: pendingTransactions with links #3768

Merged
merged 17 commits into from
Apr 28, 2023
Merged

feat: pendingTransactions with links #3768

merged 17 commits into from
Apr 28, 2023

Conversation

vnmtvlv
Copy link
Contributor

@vnmtvlv vnmtvlv commented Apr 17, 2023

Issues

Adds links to pending transactions.
Store pending transactions in localStorage with some TTL and waitForTransaction on page refresh.

Fixes #3141

Changes

  1. Extended useTxStatus() composable
  2. Added txs with links to TheNavbar.vue
  3. Migrated from pendingCount to createPendingTransaction, updatePendingTransaction, removePendingTransaction

How to test

  1. Try to create delegate transactions or with safeSnap plugin

To-Do

  • Get transaction ids for safeSnap plugin

Self-review checklist

  • I have performed a full self-review of my changes
  • I have tested my changes on a preview deployment
  • I have tested my changes on a custom domain
  • I have run end-to-end tests yarn cypress:test:e2e, and they have passed

@vnmtvlv vnmtvlv requested a review from samuveth April 17, 2023 12:15
@vnmtvlv vnmtvlv self-assigned this Apr 17, 2023
@vercel
Copy link

vercel bot commented Apr 17, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
snapshot ✅ Ready (Inspect) Visit Preview 💬 Add feedback Apr 28, 2023 1:30pm

@samuveth
Copy link
Contributor

@j0hnfl0w any blockers with this PR?

@vnmtvlv vnmtvlv marked this pull request as ready for review April 24, 2023 07:59
@vnmtvlv vnmtvlv requested a review from samuveth April 24, 2023 07:59
@samuveth
Copy link
Contributor

samuveth commented Apr 25, 2023

@bonustrack do you approve this design?

Single transaction:
Screenshot 2023-04-25 at 9 21 36 AM

Multiple transaction:
image

Maybe instead of showing multiple IDs we should show a button "View transactions" that opens a modal with a list of pending and complete transactions

@vnmtvlv
Copy link
Contributor Author

vnmtvlv commented Apr 25, 2023

yep maybe we can do modal like in sx-ui
image
but i am not sure about all completed transactions

@vnmtvlv
Copy link
Contributor Author

vnmtvlv commented Apr 25, 2023

or maybe pending transactions can live here
image

@bonustrack
Copy link
Member

The text in grey with blue background doesn't work well, I'd rather make it white, or use a modal. The modal would give much more space.

@vnmtvlv
Copy link
Contributor Author

vnmtvlv commented Apr 25, 2023

@samuveth @bonustrack wdyt?
image

@bonustrack
Copy link
Member

I think we dont need the "Show more" button, we can just make the pending banner clickable and keep same height than before

@ChaituVR
Copy link
Member

Unless they hover. How can someone assume the banner is clickable? I think maybe we should add some icon that suggests the banner is clickable or show "pending transactions" as a link

@samuveth
Copy link
Contributor

samuveth commented Apr 25, 2023

Agree, there should be some indicator that transactions are viewable

Copy link
Contributor

@samuveth samuveth left a comment

Choose a reason for hiding this comment

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

tACK

@samuveth
Copy link
Contributor

Nice work @j0hnfl0w

@samuveth samuveth merged commit 9022738 into develop Apr 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[NEW FEATURE] - Show transaction link
4 participants