-
Notifications
You must be signed in to change notification settings - Fork 96
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
Create a hook to calculate the tx fee using given parameters #4712
Create a hook to calculate the tx fee using given parameters #4712
Conversation
7495094
to
94fdbe2
Compare
src/modules/transaction/hooks/useTransactionFee/useTransactionFee.js
Outdated
Show resolved
Hide resolved
src/modules/transaction/hooks/useTransactionFee/useTransactionFee.js
Outdated
Show resolved
Hide resolved
src/modules/transaction/hooks/useTransactionFee/useTransactionFee.test.js
Outdated
Show resolved
Hide resolved
src/modules/transaction/hooks/useTransactionFee/useTransactionFee.js
Outdated
Show resolved
Hide resolved
src/modules/transaction/hooks/useTransactionFee/useTransactionFee.test.js
Outdated
Show resolved
Hide resolved
bcb8763
to
e66ffb1
Compare
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.
one minor comment, please consider 🙏🏼
… to computeTransactionMinFee
src/modules/transaction/hooks/useTransactionFee/useTransactionFee.js
Outdated
Show resolved
Hide resolved
src/modules/transaction/hooks/useTransactionFee/useTransactionFee.test.js
Show resolved
Hide resolved
3de4199
to
1df919e
Compare
isLoading: isSchemaLoading, | ||
isFetched: isSchemaFetched, | ||
} = useCommandSchema(); | ||
const schema = getParamsSchema(transaction, moduleCommandSchemas); |
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.
Would be nice if we could use the same variable name in using functions/components. Makes easier when reading the code and following it to the function args of computeTransactionMinFee
const paramsSchema = getParamsSchema(transaction, moduleCommandSchemas);
data: auth, | ||
isLoading, | ||
isFetched, | ||
} = useAuth({ config: { params: { address: senderAddress } } }); |
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.
Maybe we should add an enabled option for this hook and useCommandSchema, which checks for isValid
?
isLoading: isSchemaLoading || /* istanbul ignore next */ isLoading, | ||
isFetched: isSchemaFetched && /* istanbul ignore next */ isFetched, | ||
total: fee, | ||
components: [{ value: fee, type: 'bytesFee' }], |
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.
How will this work?
const {components} = useTransactionFee(...);
return (<div>{components.map()}...
What i am wondering, do we really want this hook tightly coupled with components, feels like we are over abstracting it somehow 🤔
…r-all-transactions
What was the problem?
Partially resolves #4716
How was it solved?
What is not included
How was it tested?
Unit tests