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

Create an example Snap for Dynamic Permissions #2198

Open
david0xd opened this issue Feb 19, 2024 · 0 comments · May be fixed by #2219
Open

Create an example Snap for Dynamic Permissions #2198

david0xd opened this issue Feb 19, 2024 · 0 comments · May be fixed by #2219
Assignees

Comments

@david0xd
Copy link
Contributor

Create an example Snap which demonstrates how Dynamic Permissions work.

@david0xd david0xd self-assigned this Feb 19, 2024
@david0xd david0xd linked a pull request Feb 26, 2024 that will close this issue
david0xd added a commit to MetaMask/metamask-extension that referenced this issue Apr 24, 2024
## **Description**
This PR introduces some major code refactoring and UI updates to
**_Connect_** and **_Permission Approval_** screens.

#### Motivation
There are several blockers encountered while trying to introduce
(integrate) new features provided by Snaps Core Platform such as Dynamic
Permissions.

The major reason for a blocker is the state of the current
implementation of the _Permission Approval_ component. More
specifically, the large difference and inconsistencies between the UI
components and overall UI structure used for the native and Snaps flows
related to the permission approval features are making it close to
impossible to maintain and extend with the new functionalities. Building
even more fragmented conditional UI in order to introduce new features,
as part of the same flows and features, would dramatically increase code
complexity and maintenance demands, while resulting in poor UX/UI in the
final product.

Hence, after multiple discussions inside the teams and between the
different teams, it is determined that the best approach would be
complete UI refactoring and consolidation of the user interface between
native and Snaps flows. This PR is the first major step made in order to
achieve the proposed results and unblock the new features waiting for
integration.

#### What is done in this PR
- Updated UI for Account Connection and Permission Approval screens.
- Refactoring of the Connection and Permission Approval components.
- Refactoring of the other components related to Connect and Permission
pages.
- Replaced old way of UI styling with the new one aligned with Design
System approach. This includes replacement of all deprecated Design
System components with the current.
- Removed redundant SCSS code. UI styling is now mostly handled by
Design System components.
- Updated some parts of the Storybook for the related components.

[![Open in GitHub
Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/23557?quickstart=1)

## **Related issues**
Fixes: #23316

Design:
https://www.figma.com/file/jr7XsFvcPCnf4HOMvNWFfn/Permission-System?node-id=1%3A2&mode=dev
(see facelift section).

Blocks: MetaMask/snaps#1821
Blocks: MetaMask/snaps#2088
Blocks: MetaMask/snaps#2198

## **Manual testing steps**
1. Go to [E2E Test Dapp](https://metamask.github.io/test-dapp/) or [Test
Snaps](https://metamask.github.io/snaps/test-snaps/latest/) website.
2. Try connecting the accounts to the Test Dapp. Try approving
permission requested. Check the UI.
3. Try installing a Snap and check all the related UI starting from a
connection to the website.
4. Try installing Ethereum Provider Snap and verify the UI for the
complete flow including triggering option for getting Ethereum accounts.
5. Try using Multi Install Snap feature provided by the Test Snaps web
application.
6. Check any other website connection or permission approval related
flows and features and make sure that there are no regressions.
7. Verify that there are no regressions in all the UX/UI related to the
all features mentioned above. Also, verify the functionality of the
related features.

## **Screenshots/Recordings**

### **Before**
![Screenshot 2024-03-28 at 16 05
48](https://github.com/MetaMask/metamask-extension/assets/13301024/568fc097-f2f2-4516-a921-a6596d33afca)
<img width="1018" alt="Screenshot 2024-03-29 at 15 35 51"
src="https://github.com/MetaMask/metamask-extension/assets/13301024/b06fa630-3912-445a-80e8-00f85a0ce301">
<img width="1462" alt="Screenshot 2024-03-29 at 15 37 26"
src="https://github.com/MetaMask/metamask-extension/assets/13301024/20b4cea6-9258-482a-9eb4-38de2234f15a">
<img width="927" alt="Screenshot 2024-03-29 at 15 39 52"
src="https://github.com/MetaMask/metamask-extension/assets/13301024/c7643215-78cc-43b8-8c89-5ae504e05bb2">


### **After**
![Screenshot 2024-04-22 at 18 40
35](https://github.com/MetaMask/metamask-extension/assets/13301024/b2c903ca-0a7a-47df-ac56-f9d9b6587245)
![Screenshot 2024-04-22 at 18 41
08](https://github.com/MetaMask/metamask-extension/assets/13301024/4ad1211d-7de4-43b8-82c1-e4e5c3e23aa9)
![Screenshot 2024-04-22 at 18 41
50](https://github.com/MetaMask/metamask-extension/assets/13301024/b48b04c3-36ec-4a15-a56b-1e3dd7e466ef)
![Screenshot 2024-04-22 at 18 42
35](https://github.com/MetaMask/metamask-extension/assets/13301024/a6fd4e7f-6dd8-4482-bc72-b73f33333b4b)
![Screenshot 2024-03-29 at 15 48
17](https://github.com/MetaMask/metamask-extension/assets/13301024/ac0d3d0c-9ade-484c-b2e2-35857d47c588)
![Screenshot 2024-04-17 at 16 28
03](https://github.com/MetaMask/metamask-extension/assets/13301024/2a2bd11d-238b-488c-994e-d531a0bc281a)


## **Pre-merge author checklist**
- [x] I’ve followed [MetaMask Coding
Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md).
- [x] I've clearly explained what problem this PR is solving and how it
is solved.
- [x] I've linked related issues
- [x] I've included manual testing steps
- [x] I've included screenshots/recordings if applicable
- [x] I’ve included tests if applicable
- [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format
if applicable
- [x] I’ve applied the right labels on the PR (see [labeling
guidelines](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/LABELING_GUIDELINES.md)).
Not required for external contributors.
- [x] I’ve properly set the pull request status:
  - [ ] In case it's not yet "ready for review", I've set it to "draft".
- [ ] In case it's "ready for review", I've changed it from "draft" to
"non-draft".

## **Pre-merge reviewer checklist**
- [ ] I've manually tested the PR (e.g. pull and build branch, run the
app, test code being changed).
- [ ] I confirm that this PR addresses all acceptance criteria described
in the ticket it closes and includes the necessary testing evidence such
as recordings and or screenshots.
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 a pull request may close this issue.

1 participant