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

fix: permit simulation design #26186

Merged
merged 6 commits into from
Aug 19, 2024
Merged

fix: permit simulation design #26186

merged 6 commits into from
Aug 19, 2024

Conversation

OGPoyraz
Copy link
Member

@OGPoyraz OGPoyraz commented Jul 29, 2024

Description

This PR does a small alignment and pill padding update on permit simulations.

Open in GitHub Codespaces

Related issues

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

Follow-up Cherry-picked PR: #26518

Manual testing steps

  1. Go to test-dapp, trigger permit signature to verify change

Screenshots/Recordings

Before

Screenshot 2024-07-29 at 15 32 11

After

Screenshot 2024-07-29 at 15 31 55

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.

@OGPoyraz OGPoyraz requested review from a team as code owners July 29, 2024 13:33
@metamaskbot metamaskbot added the team-confirmations Push issues to confirmations team label Jul 29, 2024
matthewwalsh0
matthewwalsh0 previously approved these changes Jul 29, 2024
digiwand
digiwand previously approved these changes Jul 29, 2024
@@ -106,7 +106,7 @@ export const ConfirmInfoRow: React.FC<ConfirmInfoRowProps> = ({
display={Display.Flex}
flexDirection={FlexDirection.Row}
justifyContent={JustifyContent.center}
alignItems={AlignItems.center}
alignItems={AlignItems.flexStart}
Copy link
Contributor

Choose a reason for hiding this comment

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

We need to ensure that it looks correct for all other cases.

Copy link
Member Author

Choose a reason for hiding this comment

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

Just to note, I will keep this convo in slack.

@metamaskbot
Copy link
Collaborator

Builds ready [9ce5b31]
Page Load Metrics (171 ± 211 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint683841126531
domContentLoaded1097292010
load492084171439211
domInteractive1097292010
Bundle size diffs
  • background: 0 Bytes (0.00%)
  • ui: 48 Bytes (0.00%)
  • common: 0 Bytes (0.00%)

@OGPoyraz OGPoyraz dismissed stale reviews from digiwand and matthewwalsh0 via 79758ea July 29, 2024 14:09
@metamaskbot
Copy link
Collaborator

Builds ready [5d21a80]
Page Load Metrics (397 ± 362 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint654011186933
domContentLoaded1097292311
load442488397754362
domInteractive1097292311
Bundle size diffs
  • background: 0 Bytes (0.00%)
  • ui: 48 Bytes (0.00%)
  • common: 0 Bytes (0.00%)

@OGPoyraz OGPoyraz force-pushed the 2845-permit-design-review branch from 4cd7e16 to ed9b488 Compare July 30, 2024 13:29
digiwand
digiwand previously approved these changes Aug 2, 2024
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.

@metamaskbot
Copy link
Collaborator

Builds ready [e96dbfe]
Page Load Metrics (85 ± 17 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint652041133416
domContentLoaded40169803617
load46180853617
domInteractive107932189
Bundle size diffs
  • background: 0 Bytes (0.00%)
  • ui: 48 Bytes (0.00%)
  • common: 0 Bytes (0.00%)

Copy link

sonarcloud bot commented Aug 19, 2024

@metamaskbot
Copy link
Collaborator

Builds ready [322919d]
Page Load Metrics (75 ± 12 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint661741033115
domContentLoaded40143732613
load47143752512
domInteractive107829178
Bundle size diffs
  • background: 0 Bytes (0.00%)
  • ui: 48 Bytes (0.00%)
  • common: 0 Bytes (0.00%)

Copy link

codecov bot commented Aug 19, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 69.98%. Comparing base (e3aef95) to head (322919d).
Report is 28 commits behind head on develop.

Additional details and impacted files
@@           Coverage Diff            @@
##           develop   #26186   +/-   ##
========================================
  Coverage    69.98%   69.98%           
========================================
  Files         1405     1405           
  Lines        48991    48992    +1     
  Branches     13697    13697           
========================================
+ Hits         34282    34283    +1     
  Misses       14709    14709           

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

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.

Updated snapshots and verified the UI on chrome 👍🏼

@jpuri jpuri merged commit e48324c into develop Aug 19, 2024
81 checks passed
@jpuri jpuri deleted the 2845-permit-design-review branch August 19, 2024 15:07
@github-actions github-actions bot locked and limited conversation to collaborators Aug 19, 2024
@metamaskbot metamaskbot added the release-12.5.0 Issue or pull request that will be included in release 12.5.0 label Aug 19, 2024
@digiwand digiwand added release-12.1.0 Issue or pull request that will be included in release 12.1.0 rc-cherry-picked and removed release-12.5.0 Issue or pull request that will be included in release 12.5.0 labels Aug 20, 2024
@digiwand
Copy link
Contributor

Follow-up cherry-pick PR into 12.1.0: #26518

Follow-up related fix to include decimals in calculation and tooltip when ellipsis is shown on fiat: #26523

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
rc-cherry-picked 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
None yet
Development

Successfully merging this pull request may close these issues.

5 participants