-
Notifications
You must be signed in to change notification settings - Fork 7
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
In TX details, display ETH hash (if available) #239
Conversation
Currently the ETH hash is displayed in the TX details. Where else would it be useful, @lukaw3d ? |
@@ -145,6 +145,15 @@ export const TransactionDetailView: FC<{ | |||
<TransactionLink hash={transaction.hash} layer={transaction.layer} /> | |||
<CopyToClipboard value={transaction.hash} /> | |||
</dd> | |||
{transaction.eth_hash && ( | |||
<> | |||
<dt>{t('common.ethHash')}</dt> |
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.
-
If tx has eth hash, that should be the only one displayed (at least until advanced user settings).
-
Lists of tx should show it, and link by it.
-
It should be displayed and copied with "0x" prefix.
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.
Done. One difficulty: when loading events from the API ( via the /paratime/events
), we only get a single tx_hash
, which is an Oasis hash not an ETH hash, so we need to use it for linking.
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.
Pain
fb03558
to
a5993ba
Compare
There is one more thing we should consider. When displaying TX details, we can say "Hash" or "ETH hash" based on what data we find. But how to handle tables? In the header, we currently say "Hash", even if for some (or all) of the rows will have and ETH hash instead. Is this acceptable? Can we simply call both Oasis and ETH hashes "hash"? If yes, then maybe we shouldn't even try to differentiate in TX details, either.. |
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 think yes
(Also simplify code a bit, and handle some copy-paste errors from earlier)
1ce07a8
to
86a614e
Compare
Fixes #169