Skip to content
This repository was archived by the owner on Jan 22, 2025. It is now read-only.

explorer: Auto-update transactions until they reach max confirmation#11747

Closed
oJshua wants to merge 28 commits intosolana-labs:masterfrom
oJshua:feature/65-autorefresh-transaction-until-max
Closed

explorer: Auto-update transactions until they reach max confirmation#11747
oJshua wants to merge 28 commits intosolana-labs:masterfrom
oJshua:feature/65-autorefresh-transaction-until-max

Conversation

@oJshua
Copy link
Copy Markdown
Contributor

@oJshua oJshua commented Aug 20, 2020

Problem

When a transaction that hasn't reached max confirmations is entered, the user needs to mash the refresh button on the right until it reaches max confirmation.

Summary of Changes

Status card auto-refreshes every 2 seconds until the confirmation max is reached. Account card will display loading card while auto-refresh is in process. A loading indicator is placed in the top right. Once full confirmation is complete, the refresh button becomes available.

Fixes #11760

@oJshua oJshua requested a review from jstarry August 20, 2020 20:35
@oJshua
Copy link
Copy Markdown
Contributor Author

oJshua commented Aug 20, 2020

The ticket has a screenshot of this section in row form:

78587399-bd998500-77f1-11ea-8eac-fcbd8cf32663

I worked on the TransactionsDetails page. Is this screenshot from the tokens section?

@oJshua oJshua marked this pull request as draft August 20, 2020 20:40
Copy link
Copy Markdown
Contributor

@jstarry jstarry left a comment

Choose a reason for hiding this comment

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

It works! I think we should add a couple things to make it more user-friendly

  • loading indicator to indicate auto refreshing
  • disable / hide refresh button when auto refreshing

I also noticed that the details section (getConfirmedTransaction) gets loaded twice for some reason. Could you look into why that's happening?

Comment thread explorer/src/pages/TransactionDetailsPage.tsx Outdated
Comment thread explorer/src/pages/TransactionDetailsPage.tsx Outdated
@oJshua
Copy link
Copy Markdown
Contributor Author

oJshua commented Aug 21, 2020

This iteration is much cleaner. Let me know what you think about the loading indicator in the top right of the component.

@oJshua oJshua requested a review from jstarry August 21, 2020 19:27
@oJshua oJshua marked this pull request as ready for review August 22, 2020 02:56
Comment thread explorer/src/pages/TransactionDetailsPage.tsx Outdated
Comment thread explorer/src/pages/TransactionDetailsPage.tsx Outdated
Comment thread explorer/src/pages/TransactionDetailsPage.tsx Outdated
Comment thread explorer/src/pages/TransactionDetailsPage.tsx Outdated
Comment thread explorer/src/pages/TransactionDetailsPage.tsx Outdated
Comment thread explorer/src/pages/TransactionDetailsPage.tsx Outdated
Comment thread explorer/src/pages/TransactionDetailsPage.tsx Outdated
Comment thread explorer/src/pages/TransactionDetailsPage.tsx Outdated
Comment thread explorer/src/pages/TransactionDetailsPage.tsx Outdated
@oJshua oJshua requested a review from jstarry August 25, 2020 05:14
Comment thread explorer/src/pages/TransactionDetailsPage.tsx Outdated
Comment thread explorer/src/pages/TransactionDetailsPage.tsx Outdated
@oJshua oJshua force-pushed the feature/65-autorefresh-transaction-until-max branch from 592652b to 6f90799 Compare August 25, 2020 08:12
Comment thread explorer/src/pages/TransactionDetailsPage.tsx Outdated
Comment thread explorer/src/providers/cache.tsx Outdated
...entry,
status: action.status,
data: reconciler(entry?.data, action.data),
attempts:
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Confirmation attempts feels too specific to transactions to be added here. In the future, something like a retry counter for failed fetches would be nice here though

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Attempts in this case is just the number of successful fetches completed, not so much the number of confirmation attempts. Maybe attempts is the wrong name. Should this go elsewhere?

@oJshua oJshua force-pushed the feature/65-autorefresh-transaction-until-max branch from 685ef9c to 73b2359 Compare August 25, 2020 18:28
@oJshua
Copy link
Copy Markdown
Contributor Author

oJshua commented Aug 25, 2020

Closing because I had an unclean commit history

@oJshua oJshua closed this Aug 25, 2020
@jstarry
Copy link
Copy Markdown
Contributor

jstarry commented Aug 26, 2020

@oJshua feel free to force push next time!

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.

explorer: Auto-update transactions until they reach max confirmation

2 participants