Skip to content

Conversation

@n3ps
Copy link
Contributor

@n3ps n3ps commented Oct 18, 2025

Description

Remove unnecessary callback and dependencies

Open in GitHub Codespaces

Changelog

CHANGELOG entry: chore: remove unneccesary callback and dependencies

Related issues

Fixes:

Manual testing steps

  1. Go to this page...

Screenshots/Recordings

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.

Note

Cleans up UnifiedTransactionList by removing unused env/test logic and popover state/handlers, and directly conditionally rendering AssetListControlBar.

  • UI (Transaction List)
    • Simplify rendering: Replace renderFilterButton() with direct conditional render of AssetListControlBar when !hideNetworkFilter.
    • Remove unused logic/deps: Delete env/test network checks (getEnvironmentType, ENVIRONMENT_TYPE_*, TEST_CHAINS), isEvmNetwork, popover state/handlers, and related useMemo/useCallback wrappers.
    • Imports cleanup: Drop unused selectors and constants (e.g., getCurrentNetwork, getIsEvmMultichainNetworkSelected) aligned with removed code.

Written by Cursor Bugbot for commit 645b324. This will update automatically on new commits. Configure here.

@n3ps n3ps added the team-core-extension-ux Core Extension UX team label Oct 18, 2025
@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.

[],
);

const toggleNetworkFilterPopover = useCallback(() => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was unnecessary as a callback.

More importantly the dependencies imported here aren't used in this component

<Box className="transaction-list" {...boxProps}>
{renderFilterButton()}
{!hideNetworkFilter && (
<AssetListControlBar
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Simply inline here

@n3ps n3ps marked this pull request as ready for review October 18, 2025 02:44
@n3ps n3ps requested a review from a team as a code owner October 18, 2025 02:44
@metamaskbot
Copy link
Collaborator

metamaskbot commented Oct 18, 2025

✨ Files requiring CODEOWNER review ✨

👨‍🔧 @MetaMask/core-extension-ux (1 files, +7 -55)
  • 📁 ui/
    • 📁 components/
      • 📁 app/
        • 📁 transaction-list/
          • 📄 unified-transaction-list.component.js +7 -55

@metamaskbot
Copy link
Collaborator

📊 Page Load Benchmark Results

Current Commit: 28ba7c8 | Date: 10/18/2025

📄 Localhost MetaMask Test Dapp

Samples: 100

Summary

  • pageLoadTime-> current mean value: 1.04s (±70ms) 🟡 | historical mean value: 1.04s ⬇️ (historical data)
  • domContentLoaded-> current mean value: 732ms (±68ms) 🟢 | historical mean value: 733ms ⬇️ (historical data)
  • firstContentfulPaint-> current mean value: 77ms (±12ms) 🟢 | historical mean value: 77ms ⬆️ (historical data)
📈 Detailed Results
Metric Mean Std Dev Min Max P95 P99
pageLoadTime 1.04s 70ms 1.00s 1.30s 1.26s 1.30s
domContentLoaded 732ms 68ms 695ms 997ms 938ms 997ms
firstPaint 77ms 12ms 64ms 176ms 88ms 176ms
firstContentfulPaint 77ms 12ms 64ms 176ms 88ms 176ms
largestContentfulPaint 0ms 0ms 0ms 0ms 0ms 0ms

Results generated automatically by MetaMask CI

@metamaskbot
Copy link
Collaborator

Builds ready [28ba7c8]
UI Startup Metrics (1291 ± 71 ms)
PlatformBuildTypePageMetricMean (ms)Min (ms)Max (ms)Std Dev (ms)P 75 (ms)P 95 (ms)
ChromeBrowserifyHomeuiStartup1291116514887113441424
load110999613156711521229
domContentLoaded110299113126611411224
domInteractive19146281844
firstPaint61290123645210971207
backgroundConnect26324538314266277
firstReactRender27199192942
getState1657192031
initialActions51244614
loadScripts846745104565878965
setupStore1062531116
WebpackHomeuiStartup8397171160798521046
load63258295176639861
domContentLoaded62457794276631853
domInteractive15115881437
firstPaint21353947215204595
backgroundConnect21104162532
firstReactRender3017226253235
getState941831113
initialActions308246
loadScripts62157593273629843
setupStore1051831214
FirefoxBrowserifyHomeuiStartup13881178183311814361599
load1183102414097812421313
domContentLoaded1183102314087812421313
domInteractive1103230253112261
firstPaintNaNNaNNaNNaNNaNNaN
backgroundConnect322288103651
firstReactRender25207062436
getState937610815
initialActions3125338
loadScripts1161100813617412091290
setupStore1256312949
WebpackHomeuiStartup15481383182810015941749
load1333118515508213771499
domContentLoaded1333118515498213771499
domInteractive1083140569107326
firstPaintNaNNaNNaNNaNNaNNaN
backgroundConnect3520109143967
firstReactRender292183113063
getState947210834
initialActions41435313
loadScripts1310116915258013571464
setupStore136139171047
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 58 Bytes (0%)
  • ui: -695 Bytes (-0.01%)
  • common: 10 Bytes (0%)

@n3ps n3ps enabled auto-merge October 20, 2025 05:30
@metamaskbot
Copy link
Collaborator

❌ test-e2e-chrome-api-specs failed. View the html report here.

@metamaskbot
Copy link
Collaborator

📊 Page Load Benchmark Results

Current Commit: 645b324 | Date: 10/20/2025

📄 Localhost MetaMask Test Dapp

Samples: 100

Summary

  • pageLoadTime-> current mean value: 1.03s (±69ms) 🟡 | historical mean value: 1.04s ⬇️ (historical data)
  • domContentLoaded-> current mean value: 729ms (±67ms) 🟢 | historical mean value: 733ms ⬇️ (historical data)
  • firstContentfulPaint-> current mean value: 77ms (±10ms) 🟢 | historical mean value: 77ms ⬆️ (historical data)
📈 Detailed Results
Metric Mean Std Dev Min Max P95 P99
pageLoadTime 1.03s 69ms 993ms 1.31s 1.25s 1.31s
domContentLoaded 729ms 67ms 691ms 1.01s 931ms 1.01s
firstPaint 77ms 10ms 60ms 152ms 88ms 152ms
firstContentfulPaint 77ms 10ms 60ms 152ms 88ms 152ms
largestContentfulPaint 0ms 0ms 0ms 0ms 0ms 0ms

Results generated automatically by MetaMask CI

@metamaskbot
Copy link
Collaborator

Builds ready [645b324]
UI Startup Metrics (1245 ± 65 ms)
PlatformBuildTypePageMetricMean (ms)Min (ms)Max (ms)Std Dev (ms)P 75 (ms)P 95 (ms)
ChromeBrowserifyHomeuiStartup1245113614336512871369
load106996612446011081191
domContentLoaded106396412366011031186
domInteractive18135361731
firstPaint72186122443310941188
backgroundConnect2542382766258266
firstReactRender26194762741
getState16584102030
initialActions51526614
loadScripts81472497658852938
setupStore961821013
WebpackHomeuiStartup819702111269832975
load62257295465628792
domContentLoaded61456894564623785
domInteractive15114771435
firstPaint19153911189189591
backgroundConnect21104972736
firstReactRender26166383137
getState931531114
initialActions208245
loadScripts61256693462621775
setupStore952631113
FirefoxBrowserifyHomeuiStartup14101239187110814791587
load1200107913837612571355
domContentLoaded1200107813837612571355
domInteractive1123531356113259
firstPaintNaNNaNNaNNaNNaNNaN
backgroundConnect3419107143756
firstReactRender25207562534
getState93488829
initialActions41637311
loadScripts1178106113607312361318
setupStore13517119941
WebpackHomeuiStartup15721364216314716121932
load13441193168510213951558
domContentLoaded13441192168410213941558
domInteractive1052736567105340
firstPaintNaNNaNNaNNaNNaNNaN
backgroundConnect3920128214191
firstReactRender302276123073
getState12412818936
initialActions618512324
loadScripts1317117516139513581520
setupStore11666101030
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 58 Bytes (0%)
  • ui: -695 Bytes (-0.01%)
  • common: 10 Bytes (0%)

Copy link
Contributor

@ameliejyc ameliejyc left a comment

Choose a reason for hiding this comment

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

Approving as this all looks fine to me. I'm curious why the code to render the control bar was written like this though with so many dependencies and am wondering if there's any context we don't have. So I'd be keen for @pedronfigueiredo to take a quick look too as I see he implemented it last month.

@pedronfigueiredo
Copy link
Contributor

@ameliejyc This looks good to me, thanks for checking. This code was originally duplicated from renderFilterButton inside ui/components/app/transaction-list/transaction-list.component.js, and I didn't strip down the dependency array when copying.

@n3ps n3ps added this pull request to the merge queue Oct 20, 2025
Merged via the queue into main with commit 8659812 Oct 20, 2025
321 of 323 checks passed
@n3ps n3ps deleted the n3ps/fix-deps branch October 20, 2025 14:42
@github-actions github-actions bot locked and limited conversation to collaborators Oct 20, 2025
@metamaskbot metamaskbot added the release-13.7.0 Issue or pull request that will be included in release 13.7.0 label Oct 20, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

release-13.7.0 Issue or pull request that will be included in release 13.7.0 size-S team-core-extension-ux Core Extension UX team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants