-
-
Notifications
You must be signed in to change notification settings - Fork 1
feat(txn-table): transaction table #37
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
Conversation
@debuggingfuture oh i found the option, btw if we good with the entries I will carry on with integrating a blockscout wrapper around it? |
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.
good stuff we just need to ensure extract common utils between TokenInfo
import { formatEther } from "viem"; | ||
|
||
export const txnTypeMap: Record<string, string> = { | ||
contract_call: "Contract Call", |
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.
duplicated with findDisplayedTxType
in asTransactionMeta
apps/storybook/src/lib/blockscout/api.ts
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.
hmm doesnt seem so, findDisplayedTxType
is re-aliasing of the naming, while txnTypeMap
kinda converts the snake_case
name to a plain language.
export const findDisplayedTxType = (transaction_types: any[]): string => {
if (transaction_types.includes("contract_call")) {
return "contract_call";
}
if (transaction_types.includes("coin_transfer")) {
return "coin_transfer";
}
return "native_transfer";
};
export const txnTypeMap: Record<string, string> = {
contract_call: "Contract Call",
native_transfer: "Native Token Transfer",
token_transfer: "Token Transfer",
coin_transfer: "Token Transfer",
};
Or would you prefer to render the snake_case in the table?
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.
oh my bad you're right, I quoted the wrong code
haven't even merge that
Let's use yours first I can follow up
export default meta; | ||
type Story = StoryObj<typeof meta>; | ||
|
||
export const TransactionTableWithoutFetching: Story = { |
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.
Lets 1. separate into 2 components, 2 stories
TransactionTableWithDetails
for presentation only
TransactionTable
for fetching
similar to TokenInfo
2.rename stories in this fashion?
TransactionTableWithDetailsSome
TransactionTableWithDetailsMany
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.
IMO TransactionTable
for presentation and TransactionTableWithDetails
for fetching, the details will come from the existing block explorers
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 check, I renamed some names
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 know where you coming from, while I wonder if we make it easy to be "lazy"
lazy=>use higher level component, "smart" under the hood
lazy=>shorter name
explicit=>more control=>longer name
no conclusion yet we could bring discussion to this
https://huly.app/workbench/geist/document/implementation-guide-67403d3146d1633c4ef22c52
Fixes #28
Milestones To-do: