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

Adds some functionality and fixes to wallet #661

Merged
merged 18 commits into from
May 24, 2022
Merged

Conversation

IlyasRidhuan
Copy link
Contributor

  1. Syncing indicator for Nightfall to show when the wallet has completely synced the circuits and chain state

  2. Chain detection, wallet will check to see the user is currently connected to Ethereum main net and disallow deposits otherwise.

  3. User inputs in deposit and withdraw - raises a warning if the amounts are greater than the current restrictions.

  4. Updated the txinfo modal that pops up when clicking on a transaction in the 'transactionPage'

@IlyasRidhuan IlyasRidhuan requested review from imagobea and daveroga May 13, 2022 08:15
Copy link
Contributor

@imagobea imagobea left a comment

Choose a reason for hiding this comment

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

Just leaving comments for future iterations. These are certainly nice additions, thank you.

  1. Syncing indicator - We could improve this by taking timber into account, which is what takes longer (the indicator was green, but I was still receiving blocks)
  2. Chain detection - Works fine but Metamask is always difficult (dialogs were not 'popping up' and that was conflicting with the sync indicator...)
  3. L2 Bridge page validations - I would disable the Transfer button altogether (looks like you can carry on with the tx). Also, we should expand validations for balance and 0 value.

@Westlad Westlad merged commit 757389a into master May 24, 2022
@Westlad Westlad deleted the ilyas/browser-fixes-2 branch May 24, 2022 11:17
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.

4 participants