Skip to content

feat: permit signature copy changes#24975

Merged
jpuri merged 1 commit intodevelopfrom
permit_signature_copy_changes
Jun 12, 2024
Merged

feat: permit signature copy changes#24975
jpuri merged 1 commit intodevelopfrom
permit_signature_copy_changes

Conversation

@jpuri
Copy link
Copy Markdown
Contributor

@jpuri jpuri commented Jun 1, 2024 •
edited
Loading

Description

Copy changes in permit signature.

Related issues

Fixes: https://github.com/MetaMask/MetaMask-planning/issues/2578

Manual testing steps

  1. go to test dapp
  2. trigger permit signature
  3. check the text displayed

Screenshots/Recordings

Screenshot 2024-06-01 at 9 59 17 PM

Pre-merge author checklist

  • I’ve followed MetaMask Coding Standards.
  • I've completed the PR template to the best of my ability
  • 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.

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 needs-dev-review PR needs reviews from other engineers (in order to receive required approvals) confirmation-redesign team-confirmations Push issues to confirmations team labels Jun 1, 2024
@jpuri jpuri requested review from a team as code owners June 1, 2024 16:32
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Jun 1, 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.

matthewwalsh0
matthewwalsh0 reviewed Jun 3, 2024
View reviewed changes
@codecov
Copy link
Copy Markdown

codecov bot commented Jun 5, 2024 •
edited
Loading

Codecov Report

Attention: Patch coverage is 97.67442% with 1 line in your changes missing coverage. Please review.

Project coverage is 65.67%. Comparing base (57b9c82) to head (228f73b).
Report is 1 commits behind head on develop.

Files Patch % Lines
ui/pages/confirmations/utils/confirm.ts 90.00% 1 Missing ⚠️
Additional details and impacted files 
@@             Coverage Diff             @@
##           develop   #24975      +/-   ##
===========================================
+ Coverage    65.65%   65.67%   +0.02%     
===========================================
  Files         1368     1368              
  Lines        54269    54302      +33     
  Branches     14192    14207      +15     
===========================================
+ Hits         35627    35659      +32     
- Misses       18642    18643       +1

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

@metamaskbot
Copy link
Copy Markdown
Collaborator

metamaskbot commented Jun 5, 2024

Builds ready [28b2591]
Page Load Metrics (56 ± 6 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint7312792147
domContentLoaded9141111
load4510356136
domInteractive9141111

@jpuri jpuri requested a review from matthewwalsh0 June 5, 2024 10:22
digiwand
digiwand reviewed Jun 5, 2024
View reviewed changes
Copy link
Copy Markdown
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.

Nice! one thing related to an updated translation that I think needs to be addressed before approving

I think the Fixes: ticket in the description needs to be updated

matthewwalsh0
matthewwalsh0 previously approved these changes Jun 11, 2024
View reviewed changes
@jpuri jpuri force-pushed the permit_signature_copy_changes branch from 91ee72e to 010a1e0 Compare June 12, 2024 10:09
Base automatically changed from permit_signature_simulation_info to develop June 12, 2024 12:07
@jpuri jpuri dismissed matthewwalsh0’s stale review June 12, 2024 12:07

The base branch was changed.

@jpuri jpuri force-pushed the permit_signature_copy_changes branch from 010a1e0 to feb8367 Compare June 12, 2024 12:11
@jpuri jpuri force-pushed the permit_signature_copy_changes branch from feb8367 to 228f73b Compare June 12, 2024 12:15
@metamaskbot
Copy link
Copy Markdown
Collaborator

metamaskbot commented Jun 12, 2024

Builds ready [228f73b]
Page Load Metrics (53 ± 4 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint741038594
domContentLoaded9141121
load41655384
domInteractive9141121
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 0 Bytes (0.00%)
  • ui: -473 Bytes (-0.01%)
  • common: 1.29 KiB (0.02%)

@jpuri jpuri merged commit 85cf430 into develop Jun 12, 2024
@jpuri jpuri deleted the permit_signature_copy_changes branch June 12, 2024 13:27
@github-actions github-actions bot removed the needs-dev-review PR needs reviews from other engineers (in order to receive required approvals) label Jun 12, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Jun 12, 2024
@metamaskbot metamaskbot added the release-12.1.0 Issue or pull request that will be included in release 12.1.0 label Jun 12, 2024
@SayaGT
Copy link
Copy Markdown

SayaGT commented Jun 13, 2024

@jpuri @bschorchit here is the design QA

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

Reviewers

@OGPoyraz OGPoyraz OGPoyraz approved these changes

@matthewwalsh0 matthewwalsh0 matthewwalsh0 left review comments

+1 more reviewer

@digiwand digiwand digiwand approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

confirmation-redesign release-12.1.0 Issue or pull request that will be included in release 12.1.0 team-confirmations Push issues to confirmations team

Projects

PR review queue
Archived in project

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@jpuri @metamaskbot @SayaGT @OGPoyraz @matthewwalsh0 @digiwand