Skip to content
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

Feat: Ether deposit validation on change and display balance #294

Merged
merged 5 commits into from
Jan 14, 2025

Conversation

brunomenezes
Copy link
Collaborator

@brunomenezes brunomenezes commented Jan 8, 2025

Summary

Code changes to improve feedback in the ether deposit form. Also, I refactored the test file for the ether-deposit form before adding the new test cases (separated commits).

Changes:

  • The validation run on input changes.
  • The ETH account balance is always visible.
  • When the Eth balance is bigger than 0, a max button is available to click.
  • When the amount has a value entered (i.e. input is considered dirty), and the user changes the account through the wallet, the validation is run again.
  • Testing mock imports refactored for eth-deposit form test file.
  • New test cases added.

@brunomenezes brunomenezes linked an issue Jan 8, 2025 that may be closed by this pull request
7 tasks
Copy link

vercel bot commented Jan 8, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
rollups-explorer-arbitrum-mainnet ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jan 12, 2025 9:55pm
rollups-explorer-arbitrum-sepolia ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jan 12, 2025 9:55pm
rollups-explorer-base-mainnet ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jan 12, 2025 9:55pm
rollups-explorer-base-sepolia ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jan 12, 2025 9:55pm
rollups-explorer-mainnet ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jan 12, 2025 9:55pm
rollups-explorer-optimism-mainnet ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jan 12, 2025 9:55pm
rollups-explorer-optimism-sepolia ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jan 12, 2025 9:55pm
rollups-explorer-sepolia ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jan 12, 2025 9:55pm
rollups-explorer-workshop ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jan 12, 2025 9:55pm

@@ -54,22 +56,27 @@ export const EtherDepositForm: FC<EtherDepositFormProps> = (props) => {
} = props;
const [advanced, { toggle: toggleAdvanced }] = useDisclosure(false);
const { chain } = useAccount();
const balance = useFormattedBalance();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems that we won't be needing useFormattedBalance hook. If this is true, please delete it and the related unit test suite.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed on c9c0c7f

<Flex c={"dark.2"} gap="3">
<Text fz="xs">Balance: {accountBalance.formatted}</Text>
{accountBalance.value > 0 && (
<UnstyledButton
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we switch to using the standard Button component instead?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey! I started with the <Button>, but I needed to unstyle it (no pun intended) to reach the same result, i.e. adding variant: transparent, height: auto and padding 0 besides the required one for the inner text. Hence, the reason to use the <UnstyledButton />.

Copy link
Contributor

@nevendyulgerov nevendyulgerov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I left a couple of comments.

@brunomenezes brunomenezes merged commit 1713cd5 into main Jan 14, 2025
24 checks passed
@brunomenezes brunomenezes deleted the feat/293-ether-deposit-improvements branch January 14, 2025 02:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Ether Deposit - add validation on change and display balance
2 participants