Fix: add chain check in Delete Tx modal #4656
Conversation
Branch preview✅ Deploy successful! Website: Storybook: |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| {isLoading ? <CircularProgress size={20} /> : 'Yes, delete'} | ||
| </Button> | ||
| )} | ||
| </CheckWallet> | ||
| </DialogActions> | ||
| </Dialog> | ||
| ) |
There was a problem hiding this comment.
- Avoid inline styles like
sx={{ minWidth: '122px', minHeight: '36px' }}. Use styled-components or a CSS class for maintainability. - Consider extracting the
<CheckWallet>rendering logic from the component to adhere to the Single Responsibility Principle by separating business logic from UI presentation.
| @@ -79,7 +79,7 @@ const DeleteTxButton = ({ | |||
| <Tooltip | |||
| arrow | |||
| placement="top" | |||
| title={isDeletable ? '' : 'You can only delete the last transaction in the queue or duplicates'} | |||
| title={isDeletable ? '' : 'You can only delete the last transaction in the queue, or a duplicate transaction.'} | |||
| > | |||
| <span style={{ width: '100%' }}> | |||
| <Track {...REJECT_TX_EVENTS.DELETE_OFFCHAIN_BUTTON} as="div"> | |||
There was a problem hiding this comment.
Consider extracting the inline style for the <span> to a CSS class to enhance readability and maintainability. This aligns with best practices to avoid inline styles and keeps styles centralized.
📦 Next.js Bundle Analysis for safe-wallet-webThis analysis was generated by the Next.js Bundle Analysis action. 🤖 🎉 Global Bundle Size Decreased
DetailsThe global bundle is the javascript bundle that loads alongside every page. It is in its own category because its impact is much higher - an increase to its size means that every page on your website loads slower, and a decrease means every page loads faster. Any third party scripts you have added directly to your app using the If you want further insight into what is behind the changes, give @next/bundle-analyzer a try! |
Coverage report
Show files with reduced coverage 🔻
Test suite run success1712 tests passing in 232 suites. Report generated by 🧪jest coverage report action from 6bc704a |
| color="primary" | ||
| onClick={onConfirm} | ||
| disabled={!isOk || isLoading} | ||
| sx={{ minWidth: '122px', minHeight: '36px' }} |
There was a problem hiding this comment.
can we extend this component to avoid "inline" styles?
|
Would be nice if we can cover it with unit tests by the way |
|
Gotta say never thought about this. Well spotted. Looks good to me. |
What it solves
This modal was lacking a check for the current chain causing cryptic error messages.