Skip to content

feat: add info alert linked to account type #31840

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

Merged
merged 23 commits into from
Apr 24, 2025

Conversation

vinistevam
Copy link
Contributor

@vinistevam vinistevam commented Apr 10, 2025

Description

This PR aims to:

  • add an info alert linked to the Account type row.
  • Change Type to Account Type for batch transactions.
  • Hide Account row for batch transactions.
  • Hide Interacting with from TransactionDetails and show in TransactionAccountDetails for batch transactions.

Open in GitHub Codespaces

Related issues

Fixes: https://github.com/MetaMask/MetaMask-planning/issues/4660

Manual testing steps

  1. Go to test dapp
  2. Under EIP 5792, click on Send Calls
  3. See pill with Includes x transactions under the confirmation title

Screenshots/Recordings

image

image

Before

After

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.

@vinistevam vinistevam added the team-confirmations Push issues to confirmations team label Apr 10, 2025
@vinistevam vinistevam changed the base branch from main to feat/nested-transaction-tag April 10, 2025 13:45
@vinistevam vinistevam force-pushed the feat/add-info-account-type-upgrade-alert branch from fece23e to bb47510 Compare April 10, 2025 13:46
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.

@vinistevam vinistevam force-pushed the feat/add-info-account-type-upgrade-alert branch from bb47510 to 28102bc Compare April 11, 2025 13:20
@vinistevam vinistevam marked this pull request as ready for review April 11, 2025 13:21
@vinistevam vinistevam requested a review from a team as a code owner April 11, 2025 13:21
@metamaskbot
Copy link
Collaborator

metamaskbot commented Apr 11, 2025

✨ Files requiring CODEOWNER review ✨

✅ @MetaMask/confirmations

  • ui/pages/confirmations/components/confirm/info/batch/transaction-account-details/transaction-account-details.test.tsx
  • ui/pages/confirmations/components/confirm/info/batch/transaction-account-details/transaction-account-details.tsx
  • ui/pages/confirmations/components/confirm/info/batch/transaction-account-details/transasction-account-details.test.tsx
  • ui/pages/confirmations/components/confirm/info/hooks/useNestedTransactionLabels.ts
  • ui/pages/confirmations/components/confirm/info/shared/transaction-details/transaction-details.test.tsx
  • ui/pages/confirmations/components/confirm/info/shared/transaction-details/transaction-details.tsx
  • ui/pages/confirmations/components/transactions/nested-transaction-tag/nested-transaction-tag.tsx
  • ui/pages/confirmations/hooks/alerts/transactions/AccountTypeMessage.tsx
  • ui/pages/confirmations/hooks/alerts/transactions/useAccountTypeUpgrade.test.ts
  • ui/pages/confirmations/hooks/alerts/transactions/useAccountTypeUpgrade.ts
  • ui/pages/confirmations/hooks/useConfirmationAlerts.ts

@vinistevam vinistevam force-pushed the feat/add-info-account-type-upgrade-alert branch from 8b153cd to 0c3ccb5 Compare April 11, 2025 15:28
@metamaskbot
Copy link
Collaborator

Builds ready [0c3ccb5]
UI Startup Metrics (1213 ± 67 ms)
PlatformBuildTypePageMetricMean (ms)Min (ms)Max (ms)Std Dev (ms)P 75 (ms)P 95 (ms)
ChromeBrowserifyHomeuiStartup1213109114306712541332
load10619431280661189990
domContentLoaded10559361275661275990
domInteractive17136061729
firstPaint6801051157416246980
backgroundConnect7449678
firstReactRender20154562034
getState155591068
initialActions001001
loadScripts816690101865849926
setupStore8518279
WebpackHomeuiStartup21811736268118622982452
load17151278214015517892018
domContentLoaded17071272213615217812009
domInteractive181286141462
firstPaint172663906326385
backgroundConnect2810182223061
firstReactRender188543681085788
getState1643103079
initialActions317134
loadScripts17011270210614917771979
setupStore23638546309
FirefoxBrowserifyHomeuiStartup13361157177810713801578
load11981031163510712521436
domContentLoaded11981031163410712511435
domInteractive9838198289098
firstPaintNaNNaNNaNNaNNaNNaN
backgroundConnect211386112044
firstReactRender22195552328
getState7425279
initialActions001001
loadScripts11791018161610712331398
setupStore7470867
WebpackHomeuiStartup15171324182211815941763
load13051133156710813891517
domContentLoaded13041133156710813891516
domInteractive9238176198897
firstPaintNaNNaNNaNNaNNaNNaN
backgroundConnect20157472132
firstReactRender34286253644
getState8436679
initialActions102111
loadScripts12861117155010813721501
setupStore9555879

Base automatically changed from feat/nested-transaction-tag to main April 16, 2025 10:33
@metamaskbot
Copy link
Collaborator

Builds ready [b6f4539]
UI Startup Metrics (1223 ± 62 ms)
PlatformBuildTypePageMetricMean (ms)Min (ms)Max (ms)Std Dev (ms)P 75 (ms)P 95 (ms)
ChromeBrowserifyHomeuiStartup1223109914466212571329
load105895012676010871176
domContentLoaded105294512575910851165
domInteractive181364101731
firstPaint698133127342610641161
backgroundConnect74122810
firstReactRender21164152133
getState1354581928
initialActions002001
loadScripts81571999056847920
setupStore85213812
WebpackHomeuiStartup20781693251018322232333
load15971295204613617091783
domContentLoaded15911292204213617051776
domInteractive151159101345
firstPaint196641543150231324
backgroundConnect24966132857
firstReactRender20054413119326363
getState1145871222
initialActions317135
loadScripts15861290202013617021775
setupStore196286282033
FirefoxBrowserifyHomeuiStartup13601163172411313971605
load12151030158411812621481
domContentLoaded12151030158411812621481
domInteractive1143925340128199
firstPaintNaNNaNNaNNaNNaNNaN
backgroundConnect2013105151939
firstReactRender22195042228
getState7428379
initialActions002001
loadScripts11961014157011712471465
setupStore842092067
WebpackHomeuiStartup14641308175810515291662
load1251111915699413171446
domContentLoaded1251111915689313161446
domInteractive82353273389135
firstPaintNaNNaNNaNNaNNaNNaN
backgroundConnect2214102112244
firstReactRender34286553644
getState94426925
initialActions002111
loadScripts1231110115479212941424
setupStore95436825
Benchmark value 114 exceeds gate value 110 for firefox browserify home mean domInteractive
Benchmark value 199 exceeds gate value 195 for firefox browserify home p95 domInteractive
Sum of mean exceeds: 4ms | Sum of p95 exceeds: 4ms
Sum of all benchmark exceeds: 8ms

Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 0 Bytes (0%)
  • ui: 3.58 KiB (0.05%)
  • common: 295 Bytes (0%)

@vinistevam vinistevam force-pushed the feat/add-info-account-type-upgrade-alert branch from b6f4539 to f3f6cfd Compare April 21, 2025 13:37
@metamaskbot
Copy link
Collaborator

Builds ready [f3f6cfd]
UI Startup Metrics (1192 ± 64 ms)
PlatformBuildTypePageMetricMean (ms)Min (ms)Max (ms)Std Dev (ms)P 75 (ms)P 95 (ms)
ChromeBrowserifyHomeuiStartup1192109014106412261307
load103394112335710581144
domContentLoaded102793512295710531135
domInteractive17136981627
firstPaint721131118539710471139
backgroundConnect74223812
firstReactRender20154452132
getState1254381529
initialActions001001
loadScripts79370797454819897
setupStore85243811
WebpackHomeuiStartup20881706246416421792337
load16091335200512117131772
domContentLoaded16021331199312017051765
domInteractive161157101347
firstPaint1906460287219338
backgroundConnect23976132553
firstReactRender21554365116322353
getState1032851319
initialActions315135
loadScripts15971330197111917011762
setupStore217293371931
FirefoxBrowserifyHomeuiStartup13381154172810613811538
load11941022159111312561398
domContentLoaded11941022159111312551398
domInteractive1103435043115198
firstPaintNaNNaNNaNNaNNaNNaN
backgroundConnect2213185182040
firstReactRender22195042327
getState7437389
initialActions001001
loadScripts1174977157411412361381
setupStore6421268
WebpackHomeuiStartup14871343196311515541732
load12691133176410313301452
domContentLoaded12691132176410313301452
domInteractive76421471981119
firstPaintNaNNaNNaNNaNNaNNaN
backgroundConnect2314112122239
firstReactRender35286163747
getState94325923
initialActions002111
loadScripts12481119171310112921429
setupStore11530430820
Benchmark value 339 exceeds gate value 334 for chrome webpack home p95 firstPaint
Benchmark value 111 exceeds gate value 110 for firefox browserify home mean domInteractive
Benchmark value 198 exceeds gate value 195 for firefox browserify home p95 domInteractive
Sum of mean exceeds: 1ms | Sum of p95 exceeds: 8ms
Sum of all benchmark exceeds: 9ms

Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 0 Bytes (0%)
  • ui: 3.58 KiB (0.05%)
  • common: 295 Bytes (0%)

jpuri
jpuri previously approved these changes Apr 22, 2025
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.

The changes look good, but it conflict with my PR: #32034

nestedTransactions: NestedTransactionMetadata[] | undefined,
): boolean {
return Boolean(nestedTransactions?.length);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it possible to use the check: transaction.type === TransactionType.Batch

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That was my first approach, but in a previous PR, Matthew said that we may have other batch transaction types in future, so safer to do it using the length of the nestedTransactions.


const isBatch =
Boolean(nestedTransactions?.length) &&
to?.toLowerCase() === from.toLowerCase();
Copy link
Contributor

Choose a reason for hiding this comment

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

Utility fn can be used here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Pushed the utility function here

Copy link
Contributor

Choose a reason for hiding this comment

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

link led to merge commit. isBatchTransaction added in the commit before 7078eb1

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!

@vinistevam vinistevam enabled auto-merge April 23, 2025 08:46
@vinistevam vinistevam disabled auto-merge April 23, 2025 12:21
@vinistevam vinistevam enabled auto-merge April 23, 2025 12:42
@metamaskbot
Copy link
Collaborator

Builds ready [a5433db]
UI Startup Metrics (1202 ± 68 ms)
PlatformBuildTypePageMetricMean (ms)Min (ms)Max (ms)Std Dev (ms)P 75 (ms)P 95 (ms)
ChromeBrowserifyHomeuiStartup1202107515456812351306
load104293613586410731142
domContentLoaded103792513396210661134
domInteractive17136361629
firstPaint67580134941910521106
backgroundConnect74192710
firstReactRender23165982542
getState1355191832
initialActions001001
loadScripts801701109360831902
setupStore74192813
WebpackHomeuiStartup22271819265318723602573
load17341392223015318311988
domContentLoaded17271387222515218221981
domInteractive161268111448
firstPaint1987149777250329
backgroundConnect3411348433573
firstReactRender21859377122335368
getState15499131833
initialActions317146
loadScripts17221386222315018191957
setupStore217192252537
FirefoxBrowserifyHomeuiStartup13621185171711714021625
load12141059159811912601496
domContentLoaded12141059159811912601495
domInteractive1124238345126185
firstPaintNaNNaNNaNNaNNaNNaN
backgroundConnect2414178212058
firstReactRender23195862336
getState74525811
initialActions001001
loadScripts11931027158412012341480
setupStore7448669
WebpackHomeuiStartup14551272181910415201626
load1245110415549413071425
domContentLoaded1244110415549413071425
domInteractive78311512284131
firstPaintNaNNaNNaNNaNNaNNaN
backgroundConnect21135672135
firstReactRender34287563543
getState94366930
initialActions001011
loadScripts1226109115299312891406
setupStore85465819
cc: @HowardBraham
Benchmark value 2227 exceeds gate value 2192 for chrome webpack home mean uiStartup
Benchmark value 1735 exceeds gate value 1711 for chrome webpack home mean load
Benchmark value 1728 exceeds gate value 1704 for chrome webpack home mean domContentLoaded
Benchmark value 1722 exceeds gate value 1699 for chrome webpack home mean loadScripts
Benchmark value 2573 exceeds gate value 2454 for chrome webpack home p95 uiStartup
Benchmark value 112 exceeds gate value 110 for firefox browserify home mean domInteractive
Benchmark value 1496 exceeds gate value 1495 for firefox browserify home p95 load
Benchmark value 1480 exceeds gate value 1475 for firefox browserify home p95 loadScripts
Sum of mean exceeds: 108ms | Sum of p95 exceeds: 125ms
Sum of all benchmark exceeds: 233ms

Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 0 Bytes (0%)
  • ui: 3.93 KiB (0.06%)
  • common: 295 Bytes (0%)

@vinistevam vinistevam added this pull request to the merge queue Apr 23, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Apr 23, 2025
@vinistevam vinistevam enabled auto-merge April 24, 2025 04:37
@vinistevam vinistevam added this pull request to the merge queue Apr 24, 2025
@metamaskbot
Copy link
Collaborator

Builds ready [6e39fb1]
UI Startup Metrics (1181 ± 58 ms)
PlatformBuildTypePageMetricMean (ms)Min (ms)Max (ms)Std Dev (ms)P 75 (ms)P 95 (ms)
ChromeBrowserifyHomeuiStartup1181108713995812111304
load102693611865410531139
domContentLoaded102093211785310481133
domInteractive17136191634
firstPaint739130118639710441121
backgroundConnect74354712
firstReactRender20164152131
getState1252871727
initialActions001001
loadScripts78469993352813892
setupStore75142711
WebpackHomeuiStartup21011690246817422302365
load16321315200314617361914
domContentLoaded16261311199814517311902
domInteractive161155101346
firstPaint1856666278222306
backgroundConnect241163122555
firstReactRender22254392122339358
getState134216221423
initialActions316135
loadScripts16211309197514317291871
setupStore1564061831
FirefoxBrowserifyHomeuiStartup12591110171811613011522
load1122982154411011731330
domContentLoaded1121981154411011731330
domInteractive964222229106164
firstPaintNaNNaNNaNNaNNaNNaN
backgroundConnect1912108102031
firstReactRender21185752229
getState74233710
initialActions001001
loadScripts1104964152811111581311
setupStore8317417622
WebpackHomeuiStartup14681313198611415231723
load12531115172710413031510
domContentLoaded12531115172610413031510
domInteractive83472853186134
firstPaintNaNNaNNaNNaNNaNNaN
backgroundConnect20135672132
firstReactRender35296553645
getState84446817
initialActions102111
loadScripts12351100170310312841493
setupStore85365818
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 0 Bytes (0%)
  • ui: 3.93 KiB (0.06%)
  • common: 295 Bytes (0%)

Merged via the queue into main with commit 913ffa3 Apr 24, 2025
168 of 169 checks passed
@vinistevam vinistevam deleted the feat/add-info-account-type-upgrade-alert branch April 24, 2025 08:18
@github-actions github-actions bot locked and limited conversation to collaborators Apr 24, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
team-confirmations Push issues to confirmations team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants