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

Enable the Token Allowance flow by default for all users #16740

Merged
merged 6 commits into from
Jan 23, 2023

Conversation

VSaric
Copy link
Contributor

@VSaric VSaric commented Nov 30, 2022

Explanation

  • Enable the Token Allowance flow by default for all users.
  • Removes code related to enabling/disabling the Token Allowance feature.
  • Persists the feature for all users.
  • Removes What's New notification which prompted users to turn on the setting.
  • Removes the Experimental toggle for Token Allowance feature.

Screenshots/Screencaps

Before

After

Test-Dapp

Screen.Recording.2022-11-30.at.12.24.05.mov

Etherscan

Screen.Recording.2022-11-30.at.12.27.26.mov

Manual Testing Steps

Test-dapp

  1. Open the test dapp https://metamask.github.io/test-dapp/
  2. Under the send tokens section, click on Create Token
  3. Wait for transaction to be confirmed and click on Approve Tokens
    or
  4. Wait for transaction to be confirmed and click on Approve Tokens Without Gas

Etherscan

  1. Go to https://etherscan.io/token/0x6b175474e89094c44da98b954eedeac495271d0f#writeContract
  2. Connect your wallet
  3. Under approve input any address and number
  4. Click on write

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

@VSaric VSaric self-assigned this Nov 30, 2022
@metamaskbot
Copy link
Collaborator

Builds ready [222ab3d]
Page Load Metrics (2241 ± 100 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint1022591323416
domContentLoaded176926432227209100
load176926432241207100
domInteractive176926432227209100
Bundle size diffs [🚀 Bundle size reduced!]
  • background: -203 bytes
  • ui: -4970 bytes
  • common: -502 bytes

highlights:

storybook

@@ -1,362 +1,506 @@
/* eslint-disable mocha/no-skipped-tests */
Copy link
Contributor

Choose a reason for hiding this comment

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

@mirjanaKukic can we remove this line now? I don't believe its needed anymore

Copy link
Contributor

Choose a reason for hiding this comment

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

Done.

@metamaskbot
Copy link
Collaborator

Builds ready [f751878]
Page Load Metrics (1976 ± 99 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint95129109105
domContentLoaded15192206194619192
load15192237197620699
domInteractive15192206194619192
Bundle size diffs [🚀 Bundle size reduced!]
  • background: -203 bytes
  • ui: -4970 bytes
  • common: -502 bytes

highlights:

storybook

@mirjanaKukic mirjanaKukic force-pushed the token-allowance-flow-by-default branch from 658db12 to 88ec6a0 Compare December 2, 2022 10:43
@VSaric VSaric requested review from PeterYinusa and seaona December 2, 2022 11:06
@metamaskbot
Copy link
Collaborator

Builds ready [88ec6a0]
Page Load Metrics (1991 ± 78 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint91140113126
domContentLoaded15412207197815976
load15412207199116278
domInteractive15412207197815976
Bundle size diffs [🚀 Bundle size reduced!]
  • background: -203 bytes
  • ui: -4970 bytes
  • common: -502 bytes

highlights:

storybook

@VSaric VSaric marked this pull request as ready for review December 2, 2022 12:07
@VSaric VSaric requested a review from a team as a code owner December 2, 2022 12:07
@mirjanaKukic
Copy link
Contributor

Verified by QA

@metamaskbot
Copy link
Collaborator

Builds ready [f4b50ef]
Page Load Metrics (2235 ± 143 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint1062861463818
domContentLoaded165227562213300144
load165227562235297143
domInteractive165127562213300144
Bundle size diffs [🚀 Bundle size reduced!]
  • background: -203 bytes
  • ui: -4970 bytes
  • common: -502 bytes

highlights:

storybook

@VSaric VSaric force-pushed the token-allowance-flow-by-default branch from f4b50ef to ad29194 Compare December 5, 2022 08:44
@metamaskbot
Copy link
Collaborator

Builds ready [ad29194]
Page Load Metrics (2302 ± 79 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint1031991312110
domContentLoaded19852583227016177
load20582583230216479
domInteractive19852583227016177
Bundle size diffs [🚀 Bundle size reduced!]
  • background: -203 bytes
  • ui: -4970 bytes
  • common: -502 bytes

highlights:

storybook

@VSaric VSaric force-pushed the token-allowance-flow-by-default branch from ad29194 to aa8537f Compare December 6, 2022 09:07
@metamaskbot
Copy link
Collaborator

Builds ready [aa8537f]
Page Load Metrics (2463 ± 239 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint1031307200256123
domContentLoaded188439822446509244
load190239832463498239
domInteractive188439822446509244
Bundle size diffs [🚀 Bundle size reduced!]
  • background: -203 bytes
  • ui: -6719 bytes
  • common: -502 bytes

highlights:

storybook

@danjm danjm added the DO-NOT-MERGE Pull requests that should not be merged label Dec 6, 2022
@VSaric VSaric force-pushed the token-allowance-flow-by-default branch from aa8537f to 79cb332 Compare December 7, 2022 11:48
@metamaskbot
Copy link
Collaborator

Builds ready [79cb332]
Page Load Metrics (2177 ± 94 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint961741232311
domContentLoaded15992434214319694
load16062538217719694
domInteractive15992434214319694
Bundle size diffs [🚀 Bundle size reduced!]
  • background: -203 bytes
  • ui: -6712 bytes
  • common: -502 bytes

highlights:

storybook

@metamaskbot
Copy link
Collaborator

Builds ready [745ecee]
Page Load Metrics (2238 ± 128 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint962701363718
domContentLoaded172628802228266127
load172628802238267128
domInteractive172628802228266127
Bundle size diffs [🚀 Bundle size reduced!]
  • background: -203 bytes
  • ui: -6712 bytes
  • common: -502 bytes

highlights:

storybook

digiwand
digiwand previously approved these changes Dec 12, 2022
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.

LGTM 🙌

@metamaskbot
Copy link
Collaborator

Builds ready [9c0bd95]
Page Load Metrics (1179 ± 105 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint843031174522
domContentLoaded93819041161220106
load93819041179219105
domInteractive93819041161220106
Bundle size diffs [🚀 Bundle size reduced!]
  • background: -203 bytes
  • ui: -6987 bytes
  • common: -228 bytes

highlights:

storybook

@VSaric VSaric dismissed stale reviews from pedronfigueiredo and jpuri via 1c50a86 January 19, 2023 08:48
@VSaric VSaric force-pushed the token-allowance-flow-by-default branch from 9c0bd95 to 1c50a86 Compare January 19, 2023 08:48
@metamaskbot
Copy link
Collaborator

Builds ready [1c50a86]
Page Load Metrics (1449 ± 149 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint1051921332311
domContentLoaded98620141397299143
load108821451449311149
domInteractive98620141397299143
Bundle size diffs [🚀 Bundle size reduced!]
  • background: -203 bytes
  • ui: -7057 bytes
  • common: -228 bytes

highlights:

storybook

@metamaskbot
Copy link
Collaborator

Builds ready [660f03d]
Page Load Metrics (1107 ± 93 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint901129863
domContentLoaded8691544109618689
load8691544110719593
domInteractive8691544109618689

highlights:

storybook

jpuri
jpuri previously approved these changes Jan 20, 2023
Copy link
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.

Good work 👍

Fixing lint verify locales

changed add approve token tests

skipped approve token tests from metamask-ui file

added test for max spending cap

failOnConsoleError

removed tests from metamask-ui

fix

add delay

removed failOnConsoleError
@VSaric VSaric dismissed stale reviews from jpuri and pedronfigueiredo via b686323 January 23, 2023 08:27
@VSaric VSaric force-pushed the token-allowance-flow-by-default branch from 660f03d to b686323 Compare January 23, 2023 08:27
@metamaskbot
Copy link
Collaborator

Builds ready [b686323]
Page Load Metrics (1459 ± 123 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint103163129189
domContentLoaded107318801429246118
load107319231459256123
domInteractive107318801429246118
Bundle size diffs [🚀 Bundle size reduced!]
  • background: -203 bytes
  • ui: -7057 bytes
  • common: -228 bytes

</Button>
{setshowContractDetails && (
<ContractDetailsModal
onClose={() => this.setState({ setshowContractDetails: false })}
Copy link
Contributor

Choose a reason for hiding this comment

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

In a follow up PR we should change setshowContractDetails to setShowContractDetails but I won't block on that.

Copy link
Contributor

@darkwing darkwing left a comment

Choose a reason for hiding this comment

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

Excellent work! This is great!

@danjm danjm removed the DO-NOT-MERGE Pull requests that should not be merged label Jan 23, 2023
new Date(UI_NOTIFICATIONS[16].date),
)
: '',
},
17: {
Copy link
Contributor

Choose a reason for hiding this comment

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

Should 17 be made 16 now ?

@jpuri jpuri merged commit f988dc1 into develop Jan 23, 2023
@jpuri jpuri deleted the token-allowance-flow-by-default branch January 23, 2023 16:19
@github-actions github-actions bot locked and limited conversation to collaborators Jan 23, 2023
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.

✨ Make the new Token Allowance flow default to all users
10 participants