-
Notifications
You must be signed in to change notification settings - Fork 13
Fix fetching tx fee #379
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
Fix fetching tx fee #379
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughWalkthroughThe changes involve updating the Changes
Possibly related PRs
Recent review detailsConfiguration used: CodeRabbit UI Files ignored due to path filters (1)
Files selected for processing (2)
Files skipped from review as they are similar to previous changes (1)
Additional comments not posted (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
src/components/Request/Pay/utils.ts
Outdated
export enum RequestScreens { | ||
LOADING = 'LOADING', | ||
CLAIM = 'CLAIM', | ||
NOT_FOUND = 'NOT_FOUND', | ||
} |
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.
- Is this the same enum we have in claim screens?
- Are these all the possible states?
- Is RequestScreens the correct name for an enum of strings?
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's a bit different, this enum, checks the status of the fetching route
- Yes, all states are possible
- I changed name of this enum
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.
Actionable comments posted: 1
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- src/components/Request/Pay/Views/Initial.view.tsx (5 hunks)
- src/context/tokenSelector.context.tsx (2 hunks)
Additional comments not posted (12)
src/context/tokenSelector.context.tsx (1)
118-118
: LGTM!Adding the
setSelectedTokenDecimals
function to the context value is necessary to make it accessible to the child components that consume thetokenSelectorContext
.src/components/Request/Pay/Views/Initial.view.tsx (11)
15-16
: LGTM!The new imports are relevant and necessary for the component.
32-32
: LGTM!The extracted values from the
tokenSelectorContext
are relevant for the component's functionality.
38-39
: LGTM!The new state variables,
isFeeEstimationError
andlinkState
, are relevant for improving the error handling mechanism and managing the request process states. The initial values are appropriate.
58-84
: LGTM!The refactored
estimateTxFee
function improves the robustness of the fee estimation process. It handles different scenarios appropriately, such as matching request data, successful fee estimation, and error cases. The state updates and error messages are set based on the outcome of the fee estimation process.
87-87
: LGTM!The addition of
selectedTokenAddress
andselectedChainID
to theuseEffect
hook's dependency array ensures that theestimateTxFee
function is re-run whenever these values change. This is necessary to estimate the transaction fee based on the selected token and chain.
273-284
: LGTM!The
TokenSelectorXChain
component is used appropriately to allow the user to select a token and chain for fulfilling the request. TheonReset
callback function provides a way to reset the selected values to the initial request data, which is useful for resetting the state when needed. The prop values are appropriate for the component's functionality.
286-286
: LGTM!The conditional rendering based on
!isFeeEstimationError
ensures that the network cost section is displayed only when there is no fee estimation error. This improves the user experience by hiding the network cost information when an error occurs.
319-319
: LGTM!Disabling the button based on the
linkState
andisLoading
state ensures that the button is not clickable when the request is in loading state, not found state, or when the component is in a loading state. This improves the user experience by preventing unwanted interactions during these states.
325-331
: LGTM!The conditional rendering based on
linkState === RequestStatus.LOADING
ensures that the animated logo is displayed when the request is in the loading state. The animated logo provides visual feedback to the user during the loading process, enhancing the user experience.
332-333
: LGTM!Displaying "Create or Connect Wallet" when the user is not connected provides clear guidance to the user about the action they need to take. This improves the user experience by explicitly indicating the requirement to create or connect a wallet.
333-333
: LGTM!Displaying "Pay" when the user is connected and the component is not in a loading state provides a clear call-to-action for the user to proceed with the payment. This improves the user experience by explicitly indicating the action the user can take.
selectedTokenAddress: '', | ||
selectedChainID: '', | ||
selectedTokenDecimals: 0 as number | undefined, | ||
setSelectedTokenDecimals: (decimals: number | undefined) => {}, |
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.
Fix the syntax error and implement the function body.
The setSelectedTokenDecimals
function has an empty body, which is a syntax error in TypeScript. Implement the function body to update the selectedTokenDecimals
state using the setSelectedTokenDecimals
state updater function.
Apply this diff to fix the issue:
- setSelectedTokenDecimals: (decimals: number | undefined) => {},
+ setSelectedTokenDecimals: (decimals: number | undefined) => {
+ setSelectedTokenDecimals(decimals);
+ },
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
setSelectedTokenDecimals: (decimals: number | undefined) => {}, | |
setSelectedTokenDecimals: (decimals: number | undefined) => { | |
setSelectedTokenDecimals(decimals); | |
}, |
Adapting UI to changes in SDK regarding network fee calculation, improving UX related to route fetching.
Summary by CodeRabbit
Summary by CodeRabbit
New Features
LOADING
,CLAIM
, andNOT_FOUND
for better user interface management.Bug Fixes
Dependency Updates
@squirrel-labs/peanut-sdk
version from^0.5.0
to^0.5.2
, potentially introducing new features and improvements.