This repository has been archived by the owner on Oct 7, 2024. It is now read-only.
generated from MetaMask/metamask-module-template
-
-
Notifications
You must be signed in to change notification settings - Fork 3
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
owencraston
force-pushed
the
feature/handle-redirect
branch
from
October 10, 2023 14:47
a740578
to
526d214
Compare
owencraston
added
type-enhancement
New feature or request
team-accounts
This should be handled by the Accounts Team
labels
Oct 10, 2023
danroc
approved these changes
Oct 10, 2023
14 tasks
plasmacorral
pushed a commit
to MetaMask/metamask-extension
that referenced
this pull request
Oct 12, 2023
## **Description** Progresses: MetaMask/accounts-planning#35 This PR implements the `redirectUser` callback that was added in the [eth-snap-keyring](MetaMask/eth-snap-keyring#136). This callbacks allows us to redirect the user to a given url or direct them with custom instructions to sign an async transaction. [Design](https://www.figma.com/file/K0csZegZ5XQ6TjfKk3n4Z8/Add-dialog-to-inform-about-redirection-to-snap-site-%2335?type=design&node-id=604-2586&mode=design&t=tRpxcs4qeRiRKxPl-0) ## **Manual testing steps** 1. checkout this [branch](MetaMask/snap-simple-keyring#98) in the snap simple keyring repo (this branch is wip but it will work for testing) 2. run `yarn install` and `yarn start` 3. checkout this branch and run `yarn install && yarn start --build-type flask` 4. in your browser load the dist folder into the extensions via `load unpacked` 5. setup a wallet in MM 6. 3. navigate to http://localhost:8000/ 7. click `create account` in the dapp site and approve the creation flow 8. toggle the `Use Synchronous Approval` button at the top of the page so that it is grey. This will turn on async approvals 9. navigate to https://metamask.github.io/test-dapp/ 10. connect your MM to the snap 11. scroll down and click `Personal Sign` 12. a pop up should open in MM, accept that sign 13. a new pop up should open that matches the [above designs](https://www.figma.com/file/K0csZegZ5XQ6TjfKk3n4Z8/Add-dialog-to-inform-about-redirection-to-snap-site-%2335?type=design&node-id=604-2586&mode=design&t=tRpxcs4qeRiRKxPl-0) ## **Screenshots/Recordings** Blocked URL <img width="374" alt="Screenshot 2023-10-12 at 11 16 35 AM" src="https://github.com/MetaMask/metamask-extension/assets/22918444/b2835317-5827-4edb-ad4c-b2382ed24d38"> redirect with message and URL <img width="378" alt="Screenshot 2023-10-12 at 11 14 55 AM" src="https://github.com/MetaMask/metamask-extension/assets/22918444/a08109a3-cffe-40a4-aadf-b8f70928d5d2"> without message <img width="370" alt="Screenshot 2023-10-12 at 11 18 23 AM" src="https://github.com/MetaMask/metamask-extension/assets/22918444/35b712b8-8116-42ea-b18b-439314cbe63a"> without URL <img width="392" alt="Screenshot 2023-10-12 at 11 17 33 AM" src="https://github.com/MetaMask/metamask-extension/assets/22918444/debe40ad-8ebf-49d5-94ba-18da04963a62"> ### **Before** N/A ### **After** https://github.com/MetaMask/metamask-extension/assets/22918444/05213130-ac5d-4d65-ac10-2fe32a96115a ## **Related issues** MetaMask/accounts-planning#35 ## **Pre-merge author checklist** - [ ] I’ve followed [MetaMask Coding Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md). - [ ] I've clearly explained: - [ ] What problem this PR is solving. - [ ] How this problem was solved. - [ ] How reviewers can test my changes. - [ ] I’ve indicated what issue this PR is linked to: Fixes #??? - [ ] I’ve included tests if applicable. - [ ] I’ve documented any added code. - [ ] 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)). - [ ] 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. --------- Co-authored-by: MetaMask Bot <[email protected]> Co-authored-by: Monte Lai <[email protected]> Co-authored-by: Daniel Rocha <[email protected]>
k-g-j
pushed a commit
to MetaMask/metamask-extension
that referenced
this pull request
Nov 1, 2023
## **Description** Progresses: https://github.com/MetaMask/accounts-planning/issues/35 This PR implements the `redirectUser` callback that was added in the [eth-snap-keyring](MetaMask/eth-snap-keyring#136). This callbacks allows us to redirect the user to a given url or direct them with custom instructions to sign an async transaction. [Design](https://www.figma.com/file/K0csZegZ5XQ6TjfKk3n4Z8/Add-dialog-to-inform-about-redirection-to-snap-site-%2335?type=design&node-id=604-2586&mode=design&t=tRpxcs4qeRiRKxPl-0) ## **Manual testing steps** 1. checkout this [branch](MetaMask/snap-simple-keyring#98) in the snap simple keyring repo (this branch is wip but it will work for testing) 2. run `yarn install` and `yarn start` 3. checkout this branch and run `yarn install && yarn start --build-type flask` 4. in your browser load the dist folder into the extensions via `load unpacked` 5. setup a wallet in MM 6. 3. navigate to http://localhost:8000/ 7. click `create account` in the dapp site and approve the creation flow 8. toggle the `Use Synchronous Approval` button at the top of the page so that it is grey. This will turn on async approvals 9. navigate to https://metamask.github.io/test-dapp/ 10. connect your MM to the snap 11. scroll down and click `Personal Sign` 12. a pop up should open in MM, accept that sign 13. a new pop up should open that matches the [above designs](https://www.figma.com/file/K0csZegZ5XQ6TjfKk3n4Z8/Add-dialog-to-inform-about-redirection-to-snap-site-%2335?type=design&node-id=604-2586&mode=design&t=tRpxcs4qeRiRKxPl-0) ## **Screenshots/Recordings** Blocked URL <img width="374" alt="Screenshot 2023-10-12 at 11 16 35 AM" src="https://github.com/MetaMask/metamask-extension/assets/22918444/b2835317-5827-4edb-ad4c-b2382ed24d38"> redirect with message and URL <img width="378" alt="Screenshot 2023-10-12 at 11 14 55 AM" src="https://github.com/MetaMask/metamask-extension/assets/22918444/a08109a3-cffe-40a4-aadf-b8f70928d5d2"> without message <img width="370" alt="Screenshot 2023-10-12 at 11 18 23 AM" src="https://github.com/MetaMask/metamask-extension/assets/22918444/35b712b8-8116-42ea-b18b-439314cbe63a"> without URL <img width="392" alt="Screenshot 2023-10-12 at 11 17 33 AM" src="https://github.com/MetaMask/metamask-extension/assets/22918444/debe40ad-8ebf-49d5-94ba-18da04963a62"> ### **Before** N/A ### **After** https://github.com/MetaMask/metamask-extension/assets/22918444/05213130-ac5d-4d65-ac10-2fe32a96115a ## **Related issues** https://github.com/MetaMask/accounts-planning/issues/35 ## **Pre-merge author checklist** - [ ] I’ve followed [MetaMask Coding Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md). - [ ] I've clearly explained: - [ ] What problem this PR is solving. - [ ] How this problem was solved. - [ ] How reviewers can test my changes. - [ ] I’ve indicated what issue this PR is linked to: Fixes #??? - [ ] I’ve included tests if applicable. - [ ] I’ve documented any added code. - [ ] 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)). - [ ] 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. --------- Co-authored-by: MetaMask Bot <[email protected]> Co-authored-by: Monte Lai <[email protected]> Co-authored-by: Daniel Rocha <[email protected]>
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Labels
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.
What
This PR enables this issue: https://github.com/MetaMask/accounts-planning/issues/35
This callback will be implemented in this extension PR.
For async transactions, we call this callback which will trigger a confirmation modal in the extension with the URL and the message as part of the UI. This will be used to instruct users to complete signings at the specified URL or app.
How
redirectUser
callback with the following structuresubmitRequest
, add the following block