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

feat: add approval flow success and error pages #6738

Merged
merged 7 commits into from
Jul 13, 2023

Conversation

vinistevam
Copy link
Contributor

@vinistevam vinistevam commented Jul 4, 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 improves the support for approval flows adding the possibility to include a result page for the approval flow (success & error).

  • Updates @metamask/approval-controller
  • Add success and error components to be used on approval flows.

Screenshots/Recordings

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

image

Note: the recordings below are for testing the components no change was done on the add & switch approval.
approval-flow-result-error.webm
approval-flow-result-success.webm
result_page_success.webm
result_pages_error.webm

Issue

Progresses https://github.com/MetaMask/mobile-planning/issues/1027

Pipeline E2E
https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/db808fc0-22f9-4007-8b42-7e62004c24ac

Checklist

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

@github-actions
Copy link
Contributor

github-actions bot commented Jul 4, 2023

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 force-pushed the 1027-approval-flow-result-page branch 2 times, most recently from 878fcd3 to 247c54c Compare July 4, 2023 12:39
@socket-security
Copy link

Updated dependencies detected. Learn more about Socket for GitHub ↗︎

Packages Version New capabilities Transitives Size Publisher
@metamask/approval-controller 3.3.0...3.4.0 None +0/-0 107 kB metamaskbot

@vinistevam vinistevam force-pushed the 1027-approval-flow-result-page branch from eb6c11c to f79d86d Compare July 11, 2023 10:20
@vinistevam vinistevam requested a review from a team as a code owner July 13, 2023 05:15
@sonarcloud
Copy link

sonarcloud bot commented Jul 13, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug C 1 Bug
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

65.6% 65.6% Coverage
0.0% 0.0% Duplication

@vinistevam vinistevam merged commit 06a9467 into main Jul 13, 2023
11 checks passed
@vinistevam vinistevam deleted the 1027-approval-flow-result-page branch July 13, 2023 13:37
@github-actions github-actions bot locked and limited conversation to collaborators Jul 13, 2023
@metamaskbot metamaskbot added the release-7.4.0 Issue or pull request that will be included in release 7.4.0 label Jul 13, 2023
@chrisleewilcox
Copy link
Contributor

@vinistevam @matthewwalsh0 can one of you spot check this on release v7.4.0 to ensure it's is working for the release?

@vinistevam vinistevam added the No QA Needed Apply this label when your PR does not need any QA effort. label Jul 20, 2023
@vinistevam
Copy link
Contributor Author

Thanks @chrisleewilcox going forward we will ensure the labels are included properly before merging.
Just to documenting here as well. I included No QA Needed/E2E Only as this PR does not add any changes to the UI flow or new feature, but rather adds technical infrastructure for future usage as @matthewwalsh0 explained on slack.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
No QA Needed Apply this label when your PR does not need any QA effort. release-7.4.0 Issue or pull request that will be included in release 7.4.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants