Toggle option to enable/disable balance and Token rate checking for using third-party API #16772
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
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. |
|
I think that with these modifications to this PR, we can greatly simplify the changes needed to controllers: https://github.com/MetaMask/metamask-extension/compare/optional-currency-conversion...optional-currency-conversion-alt?expand=1 I believe that with those changes, we will only need these changes to the controllers: https://github.com/MetaMask/controllers/compare/allow-disabled-config-tokenratescontroller?expand=1 |
NiranjanaBinoy
force-pushed
the
optional-currency-conversion
branch
from
5caf4c1 to
4ab6822
Compare
|
Hi @NiranjanaBinoy I was also working on this issue as part of my PR that fixes this broader Security and Privacy settings page #16727 and this design Can you confirm your PR is following this design as well @danjm it seems there's some duplication of tasks here. For instance a PR already added this (merged to develop too recently). Which is part of the issue I'm working on as well. Here's my PR that implemented the design. As at the time I'm writing this, I have implemented everything according to that design except the last two. I hope to finish the designs today/tomorrow and then work on making sure the unit tests passed. |
|
Hi @segun, Looks like we do have some overlapping. My PR will add the new toggle and the changes related to what the toggle will control. I think the work was broken down to add the new toggles as separate issues, and the purpose of this ticket was to reorganize the toggles based on the new design. @danjm How should we proceed? For this PR, once we can merge and do a release on the |
NiranjanaBinoy
force-pushed
the
optional-currency-conversion
branch
from
f6edf66 to
1d926f2
Compare
NiranjanaBinoy
force-pushed
the
optional-currency-conversion
branch
2 times, most recently
from
31f748c to
2d32f79
Compare
4 tasks
jpuri
reviewed
Dec 12, 2022
jpuri
reviewed
Dec 12, 2022
jpuri
reviewed
Dec 12, 2022
jpuri
reviewed
Dec 12, 2022
jpuri
reviewed
Dec 12, 2022
NiranjanaBinoy
force-pushed
the
optional-currency-conversion
branch
from
a837517 to
f27e156
Compare
…ing thirdparty api
…ict the display of -zsh
…urrncyRateCheck flag
|
@seaona Nice catch; I have updated the changes for swaps now; kindly verify it. |
Builds ready [f037623]
Page Load Metrics (1303 ± 206 ms)
Bundle size diffs [🚀 Bundle size reduced!]
highlights:storybook
|
digiwand
reviewed
Jan 16, 2023
Builds ready [5127185]
Page Load Metrics (1495 ± 168 ms)
Bundle size diffs [🚀 Bundle size reduced!]
highlights:storybook
|
Builds ready [8b96364]
Page Load Metrics (2715 ± 416 ms)
Bundle size diffs [🚀 Bundle size reduced!]
highlights:storybook
|
Codecov Report
@@ Coverage Diff @@
## develop #16772 +/- ##
===========================================
+ Coverage 59.67% 59.69% +0.02%
===========================================
Files 937 937
Lines 36013 36082 +69
Branches 9232 9254 +22
===========================================
+ Hits 21489 21537 +48
- Misses 14524 14545 +21
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
pedronfigueiredo
approved these changes
Jan 17, 2023
digiwand
approved these changes
Jan 17, 2023
danjm
approved these changes
Jan 17, 2023
seaona
approved these changes
Jan 17, 2023
gauthierpetetin
added
the
team-extension-ux
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #16724
Explanation
We have added a new toggle to the
Privacy and Securitytab to control the use of third-party APIs like coinGecko and CryptoCompare for balance and Token rate checking.Screenshots/Screencaps
Before
After
Manual Testing Steps
Onboard v1:
Show balance and token price checkerin the security tab and toggle it ON.Onboarding V2:
Show balance and token price checkerfrom the Privacy settings.Show balance and token price checker.Pre-merge author checklist
Pre-merge reviewer checklist
If further QA is required (e.g. new feature, complex testing steps, large refactor), add the
Extension QA Boardlabel.In this case, a QA Engineer approval will be be required.