Skip to content

Update signature request screens#15776

Merged
amerkadicE merged 1 commit intodevelopfrom
update-all-signature-request-screens
Nov 30, 2022
Merged

Update signature request screens#15776
amerkadicE merged 1 commit intodevelopfrom
update-all-signature-request-screens

Conversation

@amerkadicE
Copy link
Copy Markdown
Contributor

@amerkadicE amerkadicE commented Sep 9, 2022

Explanation

Update of signature request screens to support the new header component and origin pill.

More Information

Fixes #15705

Screenshots/Screencaps

Before

image

After

image

Manual Testing Steps

Open the test dapp https://metamask.github.io/test-dapp/
Under the Eth Sign section, click on "Sign"
Under the Personal Sign section, click on "Sign"
Under the Sign Typed Data section, click on "Sign"
Under the Sign Typed Data V3 section, click on "Sign"
Under the Sign Typed Data V4 section, click on "Sign"

Check if design is updated for those signature request screens.

Pre-Merge Checklist

  • PR template is filled out
  • IF this PR fixes a bug, a test that would have caught the bug has been added
  • PR is linked to the appropriate GitHub issue
  • PR has been added to the appropriate release Milestone

+ If there are functional changes:

  • Manual testing complete & passed
  • "Extension QA Board" label has been applied

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Sep 9, 2022

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
Copy Markdown
Collaborator

Builds ready [66414b0]
Page Load Metrics (1838 ± 189 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint903171385828
domContentLoaded156828501836391188
load156828501838393189
domInteractive156828501836391188

highlights:

storybook

@metamaskbot
Copy link
Copy Markdown
Collaborator

Builds ready [54538be]
Page Load Metrics (1827 ± 53 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint971921222210
domContentLoaded16522070181410952
load16522070182711053
domInteractive16512070181410952

highlights:

storybook

@mirjanaKukic
Copy link
Copy Markdown
Contributor

Verified by QA

@amerkadicE amerkadicE force-pushed the update-all-signature-request-screens branch 2 times, most recently from f553169 to 316da8c Compare September 16, 2022 13:02
@amerkadicE amerkadicE marked this pull request as ready for review September 16, 2022 13:42
@amerkadicE amerkadicE requested a review from a team as a code owner September 16, 2022 13:42
@amerkadicE amerkadicE requested a review from jpuri September 16, 2022 13:42
@metamaskbot
Copy link
Copy Markdown
Collaborator

Builds ready [316da8c]
Page Load Metrics (1729 ± 49 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint91131111115
domContentLoaded1571183717098943
load15721894172910249
domInteractive1571183717098943

highlights:

storybook

@bschorchit bschorchit mentioned this pull request Sep 26, 2022
6 tasks
@amerkadicE amerkadicE force-pushed the update-all-signature-request-screens branch 3 times, most recently from e1a2731 to 0e7a763 Compare October 3, 2022 07:31
@metamaskbot
Copy link
Copy Markdown
Collaborator

Builds ready [0e7a763]
Page Load Metrics (2181 ± 62 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint872901195627
domContentLoaded19452431216111655
load19452498218112962
domInteractive19452431216111655

highlights:

storybook

Comment thread ui/components/app/signature-request/signature-request.container.js Outdated
danjm
danjm previously requested changes Oct 20, 2022
Copy link
Copy Markdown
Contributor

@danjm danjm left a comment

Choose a reason for hiding this comment

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

Only one issue, found in two files, that I've commented on

@metamaskbot
Copy link
Copy Markdown
Collaborator

Builds ready [a0b9f77]
Page Load Metrics (2221 ± 56 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint8312598105
domContentLoaded20512430220810349
load20512505222111656
domInteractive20512430220810349

highlights:

storybook

Comment thread ui/components/app/signature-request/signature-request.component.js Outdated
@amerkadicE amerkadicE force-pushed the update-all-signature-request-screens branch 2 times, most recently from 217ff6d to 3c41235 Compare November 8, 2022 08:46
@metamaskbot
Copy link
Copy Markdown
Collaborator

Builds ready [fd526a7]
Page Load Metrics (2409 ± 135 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint932961354622
domContentLoaded169127882378290139
load175927882409282135
domInteractive169127882378290139

highlights:

storybook

brad-decker
brad-decker previously approved these changes Nov 9, 2022
@amerkadicE amerkadicE requested a review from danjm November 14, 2022 06:38
@bschorchit
Copy link
Copy Markdown
Contributor

Only one issue, found in two files, that I've commented on

Could you give this a new review once you have time, @danjm ?

@amerkadicE amerkadicE force-pushed the update-all-signature-request-screens branch 2 times, most recently from 886ee31 to 7dea863 Compare November 22, 2022 10:55
@metamaskbot
Copy link
Copy Markdown
Collaborator

Builds ready [7dea863]
Page Load Metrics (2256 ± 187 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint8794115418287
domContentLoaded165932132226395190
load165932132256390187
domInteractive165932132226395190
Bundle size diffs
  • background: 0 bytes
  • ui: -1730 bytes
  • common: 0 bytes

highlights:

storybook

Fix e2e test

Update siteicon for v4 signature type

Code refactor

Code refactor

Remove origin and address in signatrue request

Update e2e tests

Use getNetworkName function

Move header component inline jsx

Update snaps
@amerkadicE amerkadicE force-pushed the update-all-signature-request-screens branch from 7dea863 to 104be10 Compare November 30, 2022 16:09
@metamaskbot
Copy link
Copy Markdown
Collaborator

Builds ready [104be10]
Page Load Metrics (2121 ± 125 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint100158122168
domContentLoaded160425872091271130
load168725872121259125
domInteractive160425872091271130
Bundle size diffs
  • background: 0 bytes
  • ui: -1763 bytes
  • common: 0 bytes

highlights:

storybook

Copy link
Copy Markdown
Contributor

@jpuri jpuri left a comment

Choose a reason for hiding this comment

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

I added a feedback about css styles. Can you plz address that and also add test coverage.

@brad-decker brad-decker requested a review from jpuri November 30, 2022 18:07
@brad-decker
Copy link
Copy Markdown
Contributor

@jpuri those styles are in a snapshot file, if that is your only feedback please upgrade to an approval <3

@brad-decker brad-decker dismissed danjm’s stale review November 30, 2022 18:09

Stale, feedback resolved

@amerkadicE amerkadicE merged commit 80e4a1f into develop Nov 30, 2022
@amerkadicE amerkadicE deleted the update-all-signature-request-screens branch November 30, 2022 18:11
@github-actions github-actions Bot locked and limited conversation to collaborators Nov 30, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Update all signature requests screens to use the new origin pill and header component

7 participants