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

Frontend send success #3118

Open
wants to merge 11 commits into
base: master
Choose a base branch
from

Conversation

thisconnect
Copy link
Collaborator

No description provided.

The backend either returns success true or false plus aborted, or
false with errorMessage and optional errorCode.
The view component already has a success icon, this adds a
Only render sendResult if available and do not cover.
Adds done and continue buttons to send result view instead of
showing the success/error for 5 seconds.
So it can be called after send result
So it can be reused in send or sendresult.
@thisconnect thisconnect force-pushed the frontend-send-success branch from ba5ec85 to d1e7d07 Compare January 9, 2025 14:23
- show errors in send result view instead of popups
- add edit transaction if there was an error
- add new transaction on success
- add buy ETH if there was gasfeetoolow error
@thisconnect thisconnect force-pushed the frontend-send-success branch from d1e7d07 to 8fb23cc Compare January 9, 2025 14:55
@@ -494,7 +500,6 @@ class Send extends Component<Props, State> {
coinCode={account.coinCode}
transactionDetails={waitDialogTransactionDetails}
/>
<SendResult result={sendResult} />
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I am not sure if we should do this commit, it changes the behavior as it prevents the other components from being rendered, especially FeeTargets or CoinContorl UTXOs component.

This could lead to problems when there was an error and the user goes back, the selected UTXO's in the subcomponent might not be selected anymore

I think it is cleaner this way, but it is not requierd.. need to do more testing.

@thisconnect thisconnect requested review from shonsirsha and benma and removed request for benma January 14, 2025 15:59
</span>
</p>
{(proposedAmount && proposedAmount.conversions && proposedAmount.conversions[activeCurrency]) ? (
<FiatValue baseCurrencyUnit={activeCurrency} amount={proposedAmount.conversions[activeCurrency] || ''} />
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

this should probably have an estimation sign unless it is a coin/token

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.

1 participant