Skip to content

Porting existing MMI section to new page #22807

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 4 commits into from
Mar 12, 2024
Merged

Porting existing MMI section to new page #22807

merged 4 commits into from
Mar 12, 2024

Conversation

@jpuri
Copy link
Contributor

@jpuri jpuri commented Feb 5, 2024

Description

Porting existing MMI section to new page. It does not include adding test coverage or storybook - as this code is owned by MMI team - PR is not adding new code but only moving copying exiting code over to new page.

Related issues

Fixes: #22745

Manual testing steps

NA

Screenshots/Recordings

Before

Screenshot 2024-02-05 at 11 58 36 AM

After

Screenshot 2024-02-05 at 11 55 14 AM

Pre-merge author checklist

  • I’ve followed MetaMask Coding Standards.
  • I've clearly explained what problem this PR is solving and how it is solved.
  • I've linked related issues
  • I've included manual testing steps
  • I've included screenshots/recordings if applicable
  • I’ve included tests if applicable
  • I’ve documented my code using JSDoc format if applicable
  • I’ve applied the right labels on the PR (see labeling guidelines). Not required for external contributors.
  • 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.

@jpuri jpuri added team-confirmations-secure-ux-PR PRs from the confirmations team needs-dev-review PR needs reviews from other engineers (in order to receive required approvals) confirmation-redesign labels Feb 5, 2024
@jpuri jpuri requested a review from a team as a code owner February 5, 2024 06:33
@github-actions
Copy link
Contributor

github-actions bot commented Feb 5, 2024

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.

@codecov
Copy link

codecov bot commented Feb 5, 2024 •
edited
Loading

Codecov Report

Attention: Patch coverage is 44.73684% with 21 lines in your changes are missing coverage. Please review.

Project coverage is 68.69%. Comparing base (75a91b0) to head (db6613d).
Report is 19 commits behind head on develop.

❗ Current head db6613d differs from pull request most recent head 255a429. Consider uploading reports for the commit 255a429 to get more accurate results

Files Patch % Lines
...rm/mmi-signature-section/mmi-signature-section.tsx 0.00% 19 Missing ⚠️
...-mismatch-banner/mmi-signature-mismatch-banner.tsx 89.47% 2 Missing ⚠️
Additional details and impacted files 
@@             Coverage Diff             @@
##           develop   #22807      +/-   ##
===========================================
- Coverage    68.71%   68.69%   -0.02%     
===========================================
  Files         1105     1107       +2     
  Lines        43248    43286      +38     
  Branches     11561    11577      +16     
===========================================
+ Hits         29717    29734      +17     
- Misses       13531    13552      +21

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@metamaskbot
Copy link
Collaborator

metamaskbot commented Feb 5, 2024

Builds ready [ca16bab]
Page Load Metrics (751 ± 13 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint9512810684
domContentLoaded10511784
load7098137512813
domInteractive10511784
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 0 Bytes (0.00%)
  • ui: 1.21 KiB (0.02%)
  • common: 314 Bytes (0.01%)

@jpuri jpuri marked this pull request as draft February 9, 2024 12:21
@metamaskbot
Copy link
Collaborator

metamaskbot commented Feb 13, 2024

Builds ready [881835c]
Page Load Metrics (1243 ± 77 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint1323152185727
domContentLoaded10110452813
load9671572124316077
domInteractive10110452813
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 0 Bytes (0.00%)
  • ui: 1.21 KiB (0.02%)
  • common: 314 Bytes (0.01%)

@jpuri jpuri marked this pull request as ready for review February 13, 2024 11:35
@metamaskbot
Copy link
Collaborator

metamaskbot commented Feb 14, 2024

Builds ready [5898b3f]
Page Load Metrics (1104 ± 79 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint1212401883617
domContentLoaded9101402512
load9211541110416579
domInteractive9101402512
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 0 Bytes (0.00%)
  • ui: 1.21 KiB (0.02%)
  • common: 314 Bytes (0.01%)

segun
segun previously approved these changes Feb 16, 2024
View reviewed changes
@metamaskbot
Copy link
Collaborator

metamaskbot commented Feb 22, 2024

Builds ready [0b981b4]
Page Load Metrics (1805 ± 88 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint1062761534220
domContentLoaded96226168
load15772224180518388
domInteractive96226168
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 0 Bytes (0.00%)
  • ui: 1.21 KiB (0.02%)
  • common: 314 Bytes (0.01%)

digiwand
digiwand suggested changes Feb 26, 2024
View reviewed changes
Copy link
Contributor

@digiwand digiwand left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

interesting this component wasn't added to Sign-in With Ethereum (SIWE). This might have been missed when SIWE was implemented.

requesting changes for file path name, classname and action item to port over CSS

@jpuri jpuri requested a review from digiwand March 4, 2024 07:34
Base automatically changed from personal_sign_message to develop March 6, 2024 15:04
@jpuri jpuri dismissed segun’s stale review March 6, 2024 15:04

The base branch was changed.

@jpuri jpuri force-pushed the personal_sign_mmi_section branch 5 times, most recently from 8867dae to db6613d Compare March 6, 2024 16:55
@metamaskbot
Copy link
Collaborator

metamaskbot commented Mar 6, 2024

Builds ready [db6613d]
Page Load Metrics (1543 ± 300 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint721971243215
domContentLoaded107030178
load5920621543626300
domInteractive107029178
Bundle size diffs
  • background: 0 Bytes (0.00%)
  • ui: 0 Bytes (0.00%)
  • common: 0 Bytes (0.00%)

blackdevelopa
blackdevelopa previously approved these changes Mar 9, 2024
View reviewed changes
@jpuri jpuri removed the request for review from digiwand March 12, 2024 05:09
@jpuri jpuri force-pushed the personal_sign_mmi_section branch from 4218d29 to 807c16c Compare March 12, 2024 08:47
@jpuri jpuri force-pushed the personal_sign_mmi_section branch from 807c16c to dbf544d Compare March 12, 2024 08:49
@metamaskbot
Copy link
Collaborator

metamaskbot commented Mar 12, 2024

Builds ready [dbf544d]
Page Load Metrics (1187 ± 402 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint702271234421
domContentLoaded1088322110
load5621861187836402
domInteractive1088322110
Bundle size diffs
  • background: 0 Bytes (0.00%)
  • ui: 0 Bytes (0.00%)
  • common: 0 Bytes (0.00%)

@jpuri jpuri force-pushed the personal_sign_mmi_section branch from 32fcfb4 to dbf544d Compare March 12, 2024 10:05
@metamaskbot
Copy link
Collaborator

metamaskbot commented Mar 12, 2024

Builds ready [5c54b36]
Page Load Metrics (1391 ± 315 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint712591395024
domContentLoaded990372713
load5719271391655315
domInteractive990372713
Bundle size diffs
  • background: 0 Bytes (0.00%)
  • ui: 0 Bytes (0.00%)
  • common: 0 Bytes (0.00%)

@digiwand digiwand self-requested a review March 12, 2024 17:24
digiwand
digiwand approved these changes Mar 12, 2024
View reviewed changes
Copy link
Contributor

@digiwand digiwand left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lgtm!

documentation note:
The code logic in this PR was extracted from ui/pages/confirmations/components/signature-request-original/signature-request-original.component.js

@jpuri jpuri merged commit 6e397d0 into develop Mar 12, 2024
@jpuri jpuri deleted the personal_sign_mmi_section branch March 12, 2024 17:38
@github-actions github-actions bot locked and limited conversation to collaborators Mar 12, 2024
@github-actions github-actions bot removed the needs-dev-review PR needs reviews from other engineers (in order to receive required approvals) label Mar 12, 2024
@metamaskbot metamaskbot added the release-11.14.0 Issue or pull request that will be included in release 11.14.0 label Mar 12, 2024
@metamaskbot
Copy link
Collaborator

metamaskbot commented Mar 12, 2024

Builds ready [5fd7761]
Page Load Metrics (1225 ± 408 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint642441364924
domContentLoaded117432209
load5823121225850408
domInteractive117432209
Bundle size diffs
  • background: 0 Bytes (0.00%)
  • ui: 0 Bytes (0.00%)
  • common: 0 Bytes (0.00%)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@cryptotavares cryptotavares cryptotavares left review comments

@pedronfigueiredo pedronfigueiredo pedronfigueiredo approved these changes

+3 more reviewers

@digiwand digiwand digiwand approved these changes

@blackdevelopa blackdevelopa blackdevelopa approved these changes

@segun segun segun left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

confirmation-redesign release-11.14.0 Issue or pull request that will be included in release 11.14.0 team-confirmations-secure-ux-PR PRs from the confirmations team

Projects

PR review queue
Archived in project

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Confirmation redesign - port MMI warning to new designs

8 participants

@jpuri @metamaskbot @segun @pedronfigueiredo @cryptotavares @digiwand @blackdevelopa