-
-
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
feat: Snaps settings #8084
feat: Snaps settings #8084
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. |
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #8084 +/- ##
==========================================
+ Coverage 39.27% 39.39% +0.12%
==========================================
Files 1208 1220 +12
Lines 29738 29800 +62
Branches 2828 2831 +3
==========================================
+ Hits 11679 11740 +61
Misses 17371 17371
- Partials 688 689 +1 ☔ View full report in Codecov by Sentry. |
E2E test started on Bitrise: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/d39298f3-d5c3-4cd0-9b71-524e65d2f609 |
app/components/Views/Snaps/components/SnapDetails/SnapDetails.tsx
Outdated
Show resolved
Hide resolved
app/components/Views/Snaps/components/SnapVersionTag/SnapVersionTag.constants.ts
Show resolved
Hide resolved
app/components/Views/Snaps/components/SnapVersionTag/SnapVersionTag.styles.ts
Show resolved
Hide resolved
app/components/Views/Snaps/components/SnapVersionTag/test/SnapVersionTag.test.tsx
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.
Good job ! Few comments that need to be addressed before approval.
E2E test started on Bitrise: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/f5e6fdcf-0904-4318-8a86-ba0d7600bea3 |
d0c34a3
to
ebb939e
Compare
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!
app/components/Views/Snaps/SnapSettings/test/SnapSettings.test.tsx
Outdated
Show resolved
Hide resolved
ebb939e
to
03ac749
Compare
app/components/Views/Snaps/components/SnapElement/test/SnapElement.test.tsx
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.
LGTM. Ship the last melon 🍈
Quality Gate passedThe SonarCloud Quality Gate passed, but some issues were introduced. 1 New issue |
Description
This PR adds a settings page for each snap that allows users to read the granted permissions, remove the snap, read the description and title.
The designs for this feature can be found here
This PR does not...
2. What is the improvement/solution?
Create a stack navigator for the snaps settings and put it inside the
app/components/Nav/Main/MainNavigator.js
. This stack will then live in theSettingsFlow
stack navigatorRemoves the old placement for the snaps workflows inside the DrawerView.
Add the entry point to the SnapsSettingsList inside the settings screen under the Networks section
Renamed the old SnapsDev screen to
SnapsSettingsList.tsx
and had it render a list of SnapElements. This is the component that shows the snap name and id. When clicked it takes the user to the Settings page for a specific snap.Modified the
SnapElement
component to render a a Cell. This cell contains the snap name, snap id, a default icon for all snaps. I did not render the specific snap icon in this PR.Clicking on the SnapElement is brings you to a specific SnapSettings screen. This screen contains
SnapController.removeSnap(snap.id);
andnavigation.goBack();
The
app/components/Views/Snaps
contains all of these screens and components and matches the desired file structure for components of this nature. Each component lives within its own folder, each of which have astyles.ts
file.index.ts
file, the component itself and a test folder.New Components
SnapDescription (Low Complexity)
The designs:
My Version:
SnapElement (Low Complexity)
The designs:
My version:
SnapVersionBadge (Low Complexity)
v
prefix and has an icon beside it. As of now clicking this button does nothing but in the future it will most likely link to other versions of that snap.The designs:
My version:
SnapDetails (Medium Complexity)
The designs:
My Version:
Related issues
Progresses: https://github.com/MetaMask/accounts-planning/issues/143
Manual testing steps
yarn setup
.js.env
METAMASK_BUILD_TYPE
toexport METAMASK_BUILD_TYPE="flask"
source .js.env
and then runyarn watch:clean
Get Public key
button on the snap site, you should get a resultsettings/snaps
Before
After
Screen.Recording.2023-12-13.at.9.59.44.AM.mov
Pre-merge author checklist
Pre-merge reviewer checklist