[READY] Disable submit button when the quote is outdated#613
Conversation
✅ Deploy Preview for pendulum-pay ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Sharqiewicz
changed the title
There was a problem hiding this comment.
Pull Request Overview
This pull request disables the submit button when the quote is outdated and adds a loading animation to indicate that the quote is being fetched. Key changes include:
- Adding quote and quoteLoading hooks into RampSubmitButtons and ramp components.
- Incorporating isQuoteOutdated logic to update button disabled and pending states.
- Enabling a loading indicator in AssetNumericInput when the quote is being fetched.
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| frontend/src/components/RampSubmitButtons/index.tsx | Introduces quote comparison logic using Big number equality to determine when the quote is outdated. |
| frontend/src/components/Ramp/Onramp/index.tsx | Adds the quoteLoading state to the props and updates dependencies in memoized selections. |
| frontend/src/components/Ramp/Offramp/index.tsx | Adds the quoteLoading state and updates the dependency array to support the new loading behavior. |
| frontend/src/components/AssetNumericInput/index.tsx | Adds a new loading prop to display a loading indicator instead of the NumericInput when needed. |
| @@ -55,8 +61,9 @@ export const RampSubmitButtons: FC<RampSubmitButtonsProps> = ({ toAmount }) => { | |||
| return t('components.swapSubmitButton.confirm'); | |||
| }; | |||
|
|
|||
There was a problem hiding this comment.
Consider adding a comment to explain the rationale behind using Big number equality for comparing quote input amounts with the current input, which would improve code clarity for future maintainers.
| // Use Big.js for equality comparison to ensure precise comparison of arbitrary-precision decimal numbers. |
| /> | ||
| ), | ||
| [toToken, form, toAmount, openTokenSelectModal], | ||
| [toToken.networkAssetIcon, toToken.assetSymbol, form, quoteLoading, toAmount, openTokenSelectModal], |
There was a problem hiding this comment.
Verify that breaking the 'toToken' object into specific properties for the dependency array aligns with the intended re-render behavior; if 'toToken' might update in ways that affect this memoization, consider including those properties or the entire object as needed.
| [toToken.networkAssetIcon, toToken.assetSymbol, form, quoteLoading, toAmount, openTokenSelectModal], | |
| [toToken, form, quoteLoading, toAmount, openTokenSelectModal], |
| /> | ||
| ), | ||
| [toToken.fiat.assetIcon, toToken.fiat.symbol, form, toAmount, openTokenSelectModal], | ||
| [toToken.fiat.assetIcon, toToken.fiat.symbol, quoteLoading, form, toAmount, openTokenSelectModal], |
There was a problem hiding this comment.
Double-check that the dependency array correctly reflects all necessary changes; explicitly listing properties like fiat.assetIcon and fiat.symbol should be intentional to avoid unwanted re-renders if related properties of 'toToken.fiat' change.
| [toToken.fiat.assetIcon, toToken.fiat.symbol, quoteLoading, form, toAmount, openTokenSelectModal], | |
| [toToken.fiat, quoteLoading, form, toAmount, openTokenSelectModal], |
Uh oh!
There was an error while loading. Please reload this page.