Skip to content

fix: hide totals if simulation successful#23899

Merged
matthewwalsh0 merged 9 commits intodevelopfrom
fix/simulation-hide-totals
Apr 10, 2024
Merged

fix: hide totals if simulation successful#23899
matthewwalsh0 merged 9 commits intodevelopfrom
fix/simulation-hide-totals

Conversation

@matthewwalsh0
Copy link
Copy Markdown
Member

@matthewwalsh0 matthewwalsh0 commented Apr 8, 2024 •
edited
Loading

Description

Hide the totals in the transaction confirmation if simulation is successful, in order to reduce the amount of data in the confirmation and limit the scrolling required.

Open in GitHub Codespaces

Related issues

Fixes: #2318

Manual testing steps

  1. Create transaction.
  2. Verify no totals shown.
  3. Create transaction on network not supporting simulations.
  4. Verify totals shown.
  5. Disable simulations.
  6. Create transaction.
  7. Verify totals shown.

Screenshots/Recordings

Before

After

Screenshot 2024-04-09 at 20 41 11

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.

@matthewwalsh0
Hide totals if simulation successful
7883bf3 
Use remote token list if petnames enabled.
@matthewwalsh0 matthewwalsh0 added the team-confirmations-system-deprecated DEPRECATED: please use "team-confirmations" instead label Apr 8, 2024
@codecov
Copy link
Copy Markdown

codecov bot commented Apr 8, 2024 •
edited
Loading

Codecov Report

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

Project coverage is 69.04%. Comparing base (67555ba) to head (38f9913).

❗ Current head 38f9913 differs from pull request most recent head 2eeee6d. Consider uploading reports for the commit 2eeee6d to get more accurate results

Files Patch % Lines
...saction-base/confirm-transaction-base.component.js 83.33% 1 Missing ⚠️
Additional details and impacted files 
@@             Coverage Diff             @@
##           develop   #23899      +/-   ##
===========================================
- Coverage    69.09%   69.04%   -0.04%     
===========================================
  Files         1164     1164              
  Lines        44505    44490      -15     
  Branches     11890    11891       +1     
===========================================
- Hits         30747    30717      -30     
- Misses       13758    13773      +15

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

@metamaskbot
Copy link
Copy Markdown
Collaborator

metamaskbot commented Apr 8, 2024

Builds ready [eb9d4c8]
Page Load Metrics (472 ± 378 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint672091043215
domContentLoaded95522136
load542214472786378
domInteractive95522136
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 615 Bytes (0.02%)
  • ui: 75 Bytes (0.00%)
  • common: 0 Bytes (0.00%)

@metamaskbot
Copy link
Copy Markdown
Collaborator

metamaskbot commented Apr 9, 2024

Builds ready [38f9913]
Page Load Metrics (379 ± 339 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint63176982914
domContentLoaded107130178
load512079379705339
domInteractive107130178
Bundle size diffs
  • background: 0 Bytes (0.00%)
  • ui: 47 Bytes (0.00%)
  • common: 0 Bytes (0.00%)

@matthewwalsh0 matthewwalsh0 marked this pull request as ready for review April 9, 2024 19:35
@matthewwalsh0 matthewwalsh0 requested review from a team as code owners April 9, 2024 19:35
@metamaskbot
Copy link
Copy Markdown
Collaborator

metamaskbot commented Apr 9, 2024

Builds ready [297b35b]
Page Load Metrics (1065 ± 524 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint682321264421
domContentLoaded96429147
load53251410651092524
domInteractive96429147
Bundle size diffs
  • background: 0 Bytes (0.00%)
  • ui: 47 Bytes (0.00%)
  • common: 0 Bytes (0.00%)

@sleepytanya
Copy link
Copy Markdown
Contributor

sleepytanya commented Apr 10, 2024 •
edited
Loading

Confirming that 'Total' section is not shown if simulation successful.

'Show balance changes' toggle is on, 'Total' is not shown:
Ethereum mainnet

Screenshot 2024-04-10 at 12 55 51 PM

Polygon

Screenshot 2024-04-10 at 12 58 44 PM

Optimism

Screenshot 2024-04-10 at 12 52 46 PM

Toggle is off, 'Total' is displayed:

Ethereum mainnet
Screenshot 2024-04-10 at 12 57 01 PM

Unsupported network, 'Total' is visible:

Screenshot 2024-04-10 at 12 54 31 PM

@matthewwalsh0 matthewwalsh0 merged commit 7cd2d10 into develop Apr 10, 2024
@matthewwalsh0 matthewwalsh0 deleted the fix/simulation-hide-totals branch April 10, 2024 18:51
@github-actions github-actions bot locked and limited conversation to collaborators Apr 10, 2024
@metamaskbot metamaskbot added the release-11.15.0 Issue or pull request that will be included in release 11.15.0 label Apr 10, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@cryptotavares cryptotavares cryptotavares approved these changes

@vinistevam vinistevam vinistevam approved these changes

Assignees

No one assigned

Labels

QA Passed release-11.15.0 Issue or pull request that will be included in release 11.15.0 team-confirmations-system-deprecated DEPRECATED: please use "team-confirmations" instead

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@matthewwalsh0 @metamaskbot @sleepytanya @cryptotavares @vinistevam