-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Remove condition to disable transaction confirm button if user has no balance #7335
Conversation
…-mobile into confirmation_error_fix
CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes. |
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.
Lgtm! can merge after ci / scripts (audit:ci)
test works again. more details can be found here
e508bcd
to
63607cf
Compare
Codecov ReportAll modified lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #7335 +/- ##
==========================================
- Coverage 34.61% 34.61% -0.01%
==========================================
Files 1019 1019
Lines 27192 27191 -1
Branches 2217 2218 +1
==========================================
- Hits 9412 9411 -1
Misses 17289 17289
Partials 491 491
☔ View full report in Codecov by Sentry. |
The base branch was changed.
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.
just curious what was the thought around removing senderBalanceIsZero
entirely? Should we preserve this for networks we know that use gas?
Kudos, SonarCloud Quality Gate passed! |
Description
Currently we have these 2 conditions checked to disable submit button on confirmation pages:
0
The second condition disables a
0
value transaction on a gasless network - something we support in MetaMask extension.The PR removes the second condition and the first one alone is sufficient.
Manual testing steps
0
Related issues
_Fixes #6567
Pre-merge author checklist
Pre-merge reviewer checklist