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

Add origin to signature request confirmation page #10300

Merged
merged 2 commits into from
Jan 28, 2021

Conversation

Gudahtt
Copy link
Member

@Gudahtt Gudahtt commented Jan 27, 2021

Fixes #6071

The origin of the dapp that suggested signing has been added to the signature request confirmation page. This only applies to eth_sign, personal_sign, eth_signTypedData, and eth_signTypedData_v1. The confirmation page for eth_signTypedData_v3 and eth_signTypedData_v4 already featured the origin.

Screenshots

eth_sign:

sig-req-origin

eth_signTypedData:

sig-req-typed

Fixes #6071

The origin of the dapp that suggested signing has been added to the
signature request confirmation page. This only applies to `eth_sign`,
`personal_sign`, `eth_signTypedData`, and `eth_signTypedData_v1`. The
confirmation page for `eth_signTypedData_v3` and `eth_signTypedData_v4`
already featured the origin.
@metamaskbot
Copy link
Collaborator

Builds ready [fe7861d]
Page Load Metrics (561 ± 26 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint407353105
domContentLoaded3536325595426
load3546325615326
domInteractive3536315595426

@Gudahtt Gudahtt marked this pull request as ready for review January 27, 2021 23:21
@Gudahtt Gudahtt requested a review from a team as a code owner January 27, 2021 23:21
@Gudahtt Gudahtt requested a review from darkwing January 27, 2021 23:21
@Gudahtt Gudahtt requested a review from rachelcope January 28, 2021 00:16
rachelcope
rachelcope previously approved these changes Jan 28, 2021
Copy link

@rachelcope rachelcope left a comment

Choose a reason for hiding this comment

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

Looks good for now as a quick fix

@rachelcope
Copy link

I'll be pushing for design refresh for this page soon 😸

<div className="request-signature__headline">
{this.context.t('yourSigRequested')}
<div className="request-signature__origin-row">
<div className="request-signature__origin-label">Origin:</div>
Copy link
Contributor

Choose a reason for hiding this comment

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

Should "Origin:" be translated?

Copy link
Member Author

Choose a reason for hiding this comment

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

Of course 🤦 Yes it should, good catch!

Copy link
Member Author

Choose a reason for hiding this comment

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

Resolved in a774097

Copy link
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.

Left one comment, otherwise code looks good

Copy link
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.

e2e fail seems to be unrelated. LGTM. Approved.

@metamaskbot
Copy link
Collaborator

Builds ready [a774097]
Page Load Metrics (513 ± 41 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint40735594
domContentLoaded2965925118641
load2975935138641
domInteractive2955925118641

@Gudahtt Gudahtt merged commit d899388 into develop Jan 28, 2021
@Gudahtt Gudahtt deleted the add-origin-to-signature-request branch January 28, 2021 17:59
@github-actions github-actions bot locked and limited conversation to collaborators Jan 28, 2021
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.

Signature request screen must show requesting domain
5 participants