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: Updates to the simulations component #28107

Merged
merged 1 commit into from
Nov 7, 2024
Merged

Conversation

pedronfigueiredo
Copy link
Contributor

@pedronfigueiredo pedronfigueiredo commented Oct 25, 2024

Description

Updates the copy, layout and text color for 3 scenarios: No changes, Generic Error and Fiat value not available as per design review.

Open in GitHub Codespaces

Related issues

Fixes: #27072

Manual testing steps

  1. Go to this page...

Screenshots/Recordings

Before

After

Screenshot 2024-10-25 at 14 52 54 Screenshot 2024-10-30 at 16 31 01 Screenshot 2024-10-25 at 14 14 19

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.

@pedronfigueiredo pedronfigueiredo added the team-confirmations Push issues to confirmations team label Oct 25, 2024
@pedronfigueiredo pedronfigueiredo self-assigned this Oct 25, 2024
@pedronfigueiredo pedronfigueiredo requested review from a team as code owners October 25, 2024 14:01
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.

jpuri
jpuri previously approved these changes Oct 25, 2024
@metamaskbot
Copy link
Collaborator

Builds ready [78b91e9]
Page Load Metrics (2012 ± 100 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint49323071914368177
domContentLoaded167125501977214103
load168525872012209100
domInteractive166441157
backgroundConnect107236189
firstReactRender48198993617
getState571272612
initialActions00000
loadScripts12041911145217082
setupStore1269322210
uiStartup188228922240229110
Bundle size diffs [🚀 Bundle size reduced!]
  • background: 0 Bytes (0.00%)
  • ui: 1.92 KiB (0.03%)
  • common: -187 Bytes (-0.00%)

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.

In the GH description screenshots, "No Changes" is a lighter gray. Should we update the description and/or the UI? the Figma displays the copy with a darker gray

@metamaskbot
Copy link
Collaborator

Builds ready [496075b]
Page Load Metrics (1943 ± 56 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint32522401853369177
domContentLoaded17122214189810651
load17232229194311856
domInteractive178042189
backgroundConnect10181414120
firstReactRender562021053215
getState55915157
initialActions01000
loadScripts1213159413959244
setupStore1396352713
uiStartup19322586216714369
Bundle size diffs [🚀 Bundle size reduced!]
  • background: 0 Bytes (0.00%)
  • ui: 1.92 KiB (0.03%)
  • common: -187 Bytes (-0.00%)

@pedronfigueiredo pedronfigueiredo force-pushed the pnf/27072 branch 3 times, most recently from c6e0cee to 30c9688 Compare October 30, 2024 18:05
@metamaskbot
Copy link
Collaborator

Builds ready [30c9688]
Page Load Metrics (2102 ± 112 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint29027992035457220
domContentLoaded177427842061233112
load177828002102234112
domInteractive20186623416
backgroundConnect7109393115
firstReactRender532951154723
getState491242512
initialActions01000
loadScripts129422371552214103
setupStore1170302211
uiStartup201030722365298143
Bundle size diffs [🚀 Bundle size reduced!]
  • background: 0 Bytes (0.00%)
  • ui: 1.92 KiB (0.02%)
  • common: -187 Bytes (-0.00%)

matthewwalsh0
matthewwalsh0 previously approved these changes Nov 1, 2024
alignItems={AlignItems.center}
justifyContent={JustifyContent.spaceBetween}
>
<Box
Copy link
Member

Choose a reason for hiding this comment

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

Minor, since we have three nested boxes, would the readability benefit from some local components?

<ConfirmInfoRow
label={t('simulationDetailsTitle')}
tooltip={t('simulationDetailsTitleTooltip')}
<Box
Copy link
Member

Choose a reason for hiding this comment

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

I assume the necessary formatting isn't feasible to encapsulate within the ConfirmInfoRow component?

@metamaskbot
Copy link
Collaborator

Builds ready [5e521b0]
Page Load Metrics (1875 ± 96 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint21223171699513246
domContentLoaded15342304184319192
load15422316187520096
domInteractive178442168
backgroundConnect6121313215
firstReactRender482781045024
getState4181304019
initialActions01000
loadScripts10731681133815876
setupStore1173282211
uiStartup174626612116242116
Bundle size diffs [🚀 Bundle size reduced!]
  • background: 0 Bytes (0.00%)
  • ui: 1.92 KiB (0.02%)
  • common: -187 Bytes (-0.00%)

@metamaskbot
Copy link
Collaborator

Builds ready [aa22ec9]
Page Load Metrics (1824 ± 108 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint37219961627428206
domContentLoaded154926261804226109
load155726341824226108
domInteractive177338178
backgroundConnect85323147
firstReactRender47123942211
getState45111126
initialActions01000
loadScripts11311973133318388
setupStore1093342813
uiStartup175729112047247118
Bundle size diffs [🚀 Bundle size reduced!]
  • background: 0 Bytes (0.00%)
  • ui: 1.93 KiB (0.03%)
  • common: -187 Bytes (-0.00%)

@pedronfigueiredo pedronfigueiredo added this pull request to the merge queue Nov 7, 2024
Merged via the queue into develop with commit 3401ef0 Nov 7, 2024
76 checks passed
@pedronfigueiredo pedronfigueiredo deleted the pnf/27072 branch November 7, 2024 12:45
@github-actions github-actions bot locked and limited conversation to collaborators Nov 7, 2024
@metamaskbot metamaskbot added the release-12.8.0 Issue or pull request that will be included in release 12.8.0 label Nov 7, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
release-12.8.0 Issue or pull request that will be included in release 12.8.0 team-confirmations Push issues to confirmations team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Updates to simulations component
6 participants