-
-
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
fix: add symbol check on network add custom form #9177
Conversation
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. |
@salimtb please ignore the scan failure for now. Our team is investigating the issue 👍 |
thnx for letting me know @NicholasEllul |
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.
A few nits
app/components/Views/Settings/NetworksSettings/NetworkSettings/index.js
Outdated
Show resolved
Hide resolved
app/components/Views/Settings/NetworksSettings/NetworkSettings/index.js
Outdated
Show resolved
Hide resolved
app/components/Views/Settings/NetworksSettings/NetworkSettings/index.js
Outdated
Show resolved
Hide resolved
|
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.
Left some comments
app/components/Views/Settings/NetworksSettings/NetworkSettings/index.js
Outdated
Show resolved
Hide resolved
app/components/Views/Settings/NetworksSettings/NetworkSettings/index.js
Outdated
Show resolved
Hide resolved
3613038
to
87dacbd
Compare
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #9177 +/- ##
==========================================
+ Coverage 45.58% 45.81% +0.23%
==========================================
Files 1289 1298 +9
Lines 31973 32028 +55
Branches 3314 3328 +14
==========================================
+ Hits 14574 14674 +100
+ Misses 16528 16472 -56
- Partials 871 882 +11 ☔ View full report in Codecov by Sentry. |
3264b03
to
b84d61c
Compare
|
|
@salimtb , I was trying to see if it would be possible to confirm to add network after fill the symbol property (Because by the recordings it seems that the warning only displays after the text input being unfocused) and I notice that the confirm button doesn't appear to me at all on the simulator, am I doing something wrong? |
|
|
hey @tommasini , |
Description
Problem: When adding tokens, just like on Extension, we should add in a warning for users when the symbol doesn’t match the detected symbol.
following an initial investigation, here's what we found lacking in the mobile version:
Related issues
Fixes: #9237
Manual testing steps
Screenshots/Recordings
Before
before.mov
After
after.mov
Pre-merge author checklist
Pre-merge reviewer checklist