-
Notifications
You must be signed in to change notification settings - Fork 121
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: listen for the transfer success event #1490
Conversation
}; | ||
}, deps); | ||
}, [event]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I lost many hours debugging until realizing this dependencies was the issue 😢
Maybe we should enable the "react-hooks/exhaustive-deps": "warn"
and, if you don't want to use, at least you need to add the lint ignore comment
What do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Daaaamn
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we concatenate this [event]
with the former deps
array, if it's not empty?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we don't need the deps
anymore, because the function is saved on the ref
, so you are free to pass anything, even a new function everytime
hash: string | ||
): Promise<WrapperTransaction> => { | ||
// indexer only accepts the hash as lowercase | ||
return (await api.apiV1ChainWrapperTxIdGet(hash.toLowerCase())).data; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've created a small utility function that might be useful here :) utils/address.ts@sanitizeAddress
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cool thanks!
}; | ||
}, deps); | ||
}, [event]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Daaaamn
}; | ||
}, deps); | ||
}, [event]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we concatenate this [event]
with the former deps
array, if it's not empty?
clearPendingNotifications(id); | ||
const storedTx = searchAllStoredTxByHash(tx.hash); | ||
const storedTx = searchAllStoredTxByHash(e.detail.tx[0].hash); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please check if what you want is not the tx[0].innerTxHashes[0]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Listen for the transfer
Success
event and apply it to the notifications and transaction pageAlso, enable the watcher for cases where the transaction delays and the user closes the browser
Closes #1428
Closes #1485
Screen.Recording.2025-01-09.at.15.27.14.mov