Skip to content

fix: addressing feedbacks in typed sign alerts#25163

Merged
jpuri merged 6 commits intodevelopfrom
typed_sign_alerts_fixes
Jun 11, 2024
Merged

fix: addressing feedbacks in typed sign alerts#25163
jpuri merged 6 commits intodevelopfrom
typed_sign_alerts_fixes

Conversation

@jpuri
Copy link
Copy Markdown
Contributor

@jpuri jpuri commented Jun 10, 2024

Description

Addressing feedbacks in alerts integrated to typed sign data pages.

Related issues

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

Manual testing steps

  1. Go to test DAPP
  2. Submit malicious typed sign request
  3. Look at the alerts displayed

Screenshots/Recordings

Screenshot 2024-06-10 at 1 31 00 PM Screenshot 2024-06-10 at 1 30 54 PM

Pre-merge author checklist

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 10, 2024
@jpuri jpuri requested review from a team as code owners June 10, 2024 08:11
@github-actions
Copy link
Copy Markdown
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.

matthewwalsh0
matthewwalsh0 previously approved these changes Jun 10, 2024
@@ -202,7 +202,7 @@ describe('ConfirmFooter', () => {
};
it('renders the review alerts button when there are unconfirmed alerts', () => {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

isn't it the case to update the description to something like: renders the confirm button when there is one unconfirmed alert?

@codecov
Copy link
Copy Markdown

codecov bot commented Jun 11, 2024

Codecov Report

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

Project coverage is 65.64%. Comparing base (ed5ec2d) to head (ebe2a8f).
Report is 15 commits behind head on develop.

Files Patch % Lines
...nents/app/alert-system/alert-modal/alert-modal.tsx 0.00% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff            @@
##           develop   #25163   +/-   ##
========================================
  Coverage    65.64%   65.64%           
========================================
  Files         1362     1362           
  Lines        54189    54189           
  Branches     14112    14112           
========================================
  Hits         35572    35572           
  Misses       18617    18617           

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

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

Builds ready [ebe2a8f]
Page Load Metrics (161 ± 221 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint7111191115
domContentLoaded9231231
load442170161461221
domInteractive9231231
Bundle size diffs [🚀 Bundle size reduced!]
  • background: 0 Bytes (0.00%)
  • ui: 153 Bytes (0.00%)
  • common: -70 Bytes (-0.00%)

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

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

Archived in project

Development

Successfully merging this pull request may close these issues.

4 participants