Skip to content

Add origin to transaction confirmation#10296

Merged
Gudahtt merged 1 commit intodevelopfrom
add-origin-to-transaction-confirmation
Jan 28, 2021
Merged

Add origin to transaction confirmation#10296
Gudahtt merged 1 commit intodevelopfrom
add-origin-to-transaction-confirmation

Conversation

@Gudahtt
Copy link
Copy Markdown
Member

@Gudahtt Gudahtt commented Jan 27, 2021

Fixes #5611

The origin that suggests a transaction is now shown on the transaction confirmation page. If the transaction was initiated from within MetaMask (e.g. via the 'Send' flow or swaps), no origin is shown.

This was based upon designs that were linked in the PR #9377. This is a temporary measure until our newer transaction confirmation designs can be implemented.

Manual testing steps:

  • Initiate a transaction from any dapp (ETH send, token transfer, or any other contract interaction)

  • See that the origin is shown

  • Initiate a transaction from the Send flow

  • See that the origin is not shown

Screenshot:

transaction-confirmation-origin

@Gudahtt Gudahtt force-pushed the add-origin-to-transaction-confirmation branch from f59edc1 to 0e0aae0 Compare January 27, 2021 16:12
@Gudahtt
Copy link
Copy Markdown
Member Author

Gudahtt commented Jan 27, 2021

This depends upon #10295

@Gudahtt Gudahtt force-pushed the remove-unused-transaction-base-props branch from 106e514 to eef7dd8 Compare January 27, 2021 16:37
@Gudahtt Gudahtt force-pushed the add-origin-to-transaction-confirmation branch from 0e0aae0 to a39693b Compare January 27, 2021 16:38
@metamaskbot
Copy link
Copy Markdown
Collaborator

Builds ready [a39693b]
Page Load Metrics (785 ± 71 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint56907094
domContentLoaded39195178314771
load39395378514771
domInteractive39195178314771

rachelcope
rachelcope previously approved these changes Jan 28, 2021
Copy link
Copy Markdown

@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.

LGTM!

Base automatically changed from remove-unused-transaction-base-props to develop January 28, 2021 17:17
@Gudahtt Gudahtt dismissed rachelcope’s stale review January 28, 2021 17:17

The base branch was changed.

Fixes #5611

The origin that suggests a transaction is now shown on the transaction
confirmation page. If the transaction was initiated from within
MetaMask (e.g. via the 'Send' flow or swaps), no origin is shown.

This was based upon designs that were linked in the PR #9377. This is a
temporary measure until our newer transaction confirmation designs can
be implemented.
@Gudahtt Gudahtt force-pushed the add-origin-to-transaction-confirmation branch from a39693b to 7baa574 Compare January 28, 2021 17:17
@Gudahtt Gudahtt marked this pull request as ready for review January 28, 2021 17:18
@Gudahtt Gudahtt requested a review from a team as a code owner January 28, 2021 17:18
@Gudahtt Gudahtt requested a review from brad-decker January 28, 2021 17:18
Copy link
Copy Markdown
Contributor

@brad-decker brad-decker left a comment

Choose a reason for hiding this comment

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

LGTM

@metamaskbot
Copy link
Copy Markdown
Collaborator

Builds ready [7baa574]
Page Load Metrics (579 ± 12 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint458156105
domContentLoaded5376485772612
load5386505792612
domInteractive5366485772612

@Gudahtt Gudahtt merged commit c053294 into develop Jan 28, 2021
@Gudahtt Gudahtt deleted the add-origin-to-transaction-confirmation 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.

Show origin domain on Confirm screen and tx detail view

4 participants