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

Dynamic permission request #1821

Open
FrederikBolding opened this issue Oct 5, 2023 · 5 comments · May be fixed by #2088
Open

Dynamic permission request #1821

FrederikBolding opened this issue Oct 5, 2023 · 5 comments · May be fixed by #2088

Comments

@FrederikBolding
Copy link
Member

FrederikBolding commented Oct 5, 2023

Pending https://app.zenhub.com/workspaces/snaps-615b3a7c08d2b20015eb6c4e/issues/gh/metamask/snaps/1820

@eriknson
Copy link
Member

Jamming on how this could work, ideas & input welcome!

image

@kenhkan
Copy link

kenhkan commented Oct 26, 2023

From refinement:

  • To begin implementation as we are close enough in terms of design
  • We will add estimation after permission meeting on Friday

@eriknson
Copy link
Member

eriknson commented Nov 2, 2023

Design ready, we'll revisit loading / success state later if it feels laggy

@eriknson eriknson removed their assignment Nov 2, 2023
@eriknson
Copy link
Member

eriknson commented Nov 2, 2023

To summarize, it's basically just the permission request screen alongside the revoke option in the settings dropdown to begin with. Permissions can be revoked silently w/out confirmation.

@kenhkan kenhkan changed the title Dynamic permission request UI Dynamic permission request Nov 14, 2023
@david0xd david0xd linked a pull request Jan 24, 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
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants