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

refactor: trigger transaction modals using approval requests #6291

Merged
merged 9 commits into from
Jun 13, 2023

Conversation

vinistevam
Copy link
Contributor

@vinistevam vinistevam commented Apr 27, 2023

Development & PR Process

  1. Follow MetaMask Mobile Coding Standards
  2. Add release-xx label to identify the PR slated for a upcoming release (will be used in release discussion)
  3. Add needs-dev-review label when work is completed
  4. Add needs-qa label when dev review is completed
  5. Add QA Passed label when QA has signed off

Description

This PR applies a change to TransactionController that has already been merged in the core repo. Unfortunately, there are breaking changes blocking the ability to upgrade the package directly.

The changes from the core PR for visibility. It implies that confirmations for transaction requests will no longer be handled by an event, but sent directly to ApprovalController. This is part of the work to standardise and simplify how confirmations work across extension and mobile.

  1. Unifying the trigger modal approach
  2. Included in the transaction-controller (core) the capability to create an ApprovalRequest whenever the unapprovedTransaction is fired and leverage it to control the transaction modals in mobile.

Screenshots/Recordings
Transaction tests

If applicable, add screenshots and/or recordings to visualize the before and after of your change

Issue
resolves: https://github.com/MetaMask/mobile-planning/issues/770

Checklist

  • There is a related GitHub issue
  • Tests are included if applicable
  • Any added code is fully documented

@github-actions
Copy link
Contributor

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.

@vinistevam vinistevam added the DO-NOT-MERGE Pull requests that should not be merged label Apr 27, 2023
@vinistevam vinistevam force-pushed the transaction-controller-update branch from add9ad8 to d2d1d9c Compare May 24, 2023 13:10
@vinistevam vinistevam removed the DO-NOT-MERGE Pull requests that should not be merged label May 25, 2023
@vinistevam vinistevam marked this pull request as ready for review May 25, 2023 07:24
@vinistevam vinistevam requested a review from a team as a code owner May 25, 2023 07:24
@vinistevam vinistevam changed the title update transaction controller patch transaction controller to add messenger May 25, 2023
@vinistevam vinistevam changed the title patch transaction controller to add messenger Trigger transaction modals using approval requests May 26, 2023
@vinistevam vinistevam force-pushed the transaction-controller-update branch from 363b71c to cd725ad Compare May 30, 2023 07:38
@vinistevam vinistevam changed the title Trigger transaction modals using approval requests refactor: trigger transaction modals using approval requests May 30, 2023
@vinistevam vinistevam force-pushed the transaction-controller-update branch from cd725ad to 3e7b5e9 Compare May 30, 2023 12:42
@vinistevam vinistevam force-pushed the transaction-controller-update branch from 171b8bb to 94c25ed Compare June 1, 2023 11:18
@sonarcloud
Copy link

sonarcloud bot commented Jun 13, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

0.0% 0.0% Coverage
0.0% 0.0% Duplication

@vinistevam vinistevam merged commit d967533 into main Jun 13, 2023
22 checks passed
@vinistevam vinistevam deleted the transaction-controller-update branch June 13, 2023 08:34
@github-actions github-actions bot locked and limited conversation to collaborators Jun 13, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants