feat(suite): added blockbook link to approval tx #16434
Conversation
| @@ -139,9 +145,13 @@ export const CoinmarketOfferExchangeSendApproval = () => { | |||
| > | |||
| {dexTx.to} | |||
| </InfoItem> | |||
| {selectedQuote.approvalSendTxHash && ( | |||
| {selectedQuote.approvalSendTxHash && blockchain && ( | |||
There was a problem hiding this comment.
💅
| {selectedQuote.approvalSendTxHash && blockchain && ( | |
| {selectedQuote.approvalSendTxHash && blockchain != null && ( |
There was a problem hiding this comment.
Actually, I think selectedQuote.approvalSendTxHash && blockchain && is more accurate. In this version, the property blockchain can be undefined because the symbol can be undefined. If the blockchain is undefined, the app will crash.
Additionally, instead of != please use strict type check equality !==.
There was a problem hiding this comment.
I think that the != null catches both null and undefined, but I have no problem changing it back to just blockchain
There was a problem hiding this comment.
Oh, ok. Sorry then. But && blockchain is more readable IMHO.
There was a problem hiding this comment.
it is also used in the majority of cases throughout the codebase, so I'll change it back to just blockchain
There was a problem hiding this comment.
@adderpositive We discussed using full conditionals instead of implicit boolean check because I think it is more readable and cleaner (we know right away that blockchain isn't just a boolean). I also prefer using != instead of !== because of the reasons that @TomasBoda wrote above. I found no code-style rules about these cases and if it is common in the codebase, I have no problem with it.
There was a problem hiding this comment.
@izmy Thank you for the explanation. In that case, it is fine by me as well. Anyway, both cases are correct, and if the full conditionals are required, I am ok with this!
My apologies for the confusion.
| @@ -51,6 +52,8 @@ export const CoinmarketOfferExchangeSendApproval = () => { | |||
| selectedQuote?.status === 'CONFIRM' ? 'APPROVED' : 'MINIMAL', | |||
| ); | |||
|
|
|||
| const blockchainNetworks = useSelector(state => state.wallet.blockchain); | |||
There was a problem hiding this comment.
Nitpick:
const blockchain = useSelector(state => state.wallet.blockchain); or const { blockchain } = useSelector(state => state.wallet);.
and then use blockchainForSend or something like that.
There was a problem hiding this comment.
In the codebase is used const blockchain = useSelector(state => state.wallet.blockchain);, that is the reason. 🤭
Description
After approving a token to swap, while the blockchain is confirming the approval transaction, allow the user to copy the approval transaction ID as well as redirect to blockbook transaction detail page.
Related Issue
Resolve #14840
Screenshots:
On moving the mouse over the transaction ID, options to copy the transaction ID or go to the blockbook transaction detail page are displayed.
Upon clicking the redirect button, the user is redirected to the blockbook transaction detail page.