-
Notifications
You must be signed in to change notification settings - Fork 9
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
Add missing data to Consensus tx list #1462
Conversation
Deployed to Cloudflare Pages
|
fb98cf9
to
3390cbb
Compare
3390cbb
to
b857bd8
Compare
> | ||
|
||
const tableRows = transactions?.map(transaction => { | ||
const targetAddress = transaction.body.to || transaction.body.account |
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.
Transactions consensus and runtime APIs are not consistent:
runtime has props
amount?: string;
to?: Address;
where here we use body. not sure this should be unified in Nexus. Current API might work fine with new design where we will have to render different body
props depends on a tx type.
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 would say it would be nice to unify, but not high priority. Let's make it work on our side, and BE can resolve it, once they find the time.
const tableRows = transactions?.map(transaction => { | ||
const targetAddress = transaction.body.to || transaction.body.account | ||
return { | ||
key: `${transaction.hash}${transaction.index}`, |
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.
No need for ${transaction.index}
I think. Tx hash should be unique enough, unless there is a case, where it's missing or duplicated.
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.
it was needed in the past #1412
Provide data for transactions list
to
andvalue
columns.side note: these columns will be merged into
Details
column when designs are ready