feat: adding / deleting additional RPC URLs #25452

bergeron · 2024-06-20T19:45:40Z

Description

Initial UI for adding and deleting multiple RPC URLs. The add and delete buttons don't do anything yet. Just UI until the network controller gets upgraded.

Related issues

Manual testing steps

Build with ENABLE_NETWORK_UI_REDESIGN=1 yarn start
Open networks, right click edit one
Click RPC dropdown
Add and delete RPC endpoints

Screenshots/Recordings

Before

After

Screen.Recording.2024-06-21.at.1.26.25.PM.mov

Pre-merge author checklist

I've followed MetaMask Contributor Docs and MetaMask Extension Coding Standards.
I've completed the PR template to the best of my ability
I’ve included tests if applicable
I’ve documented my code using JSDoc format if applicable
I’ve applied the right labels on the PR (see labeling guidelines). Not required for external contributors.

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.

github-actions · 2024-06-20T19:45:53Z

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.

… brian/edit-custom-network-flow4

metamaskbot · 2024-06-21T20:54:19Z

Builds ready [7ff6370]

builds: chrome, firefox
builds (beta): chrome
builds (flask): chrome, firefox
builds (MMI): chrome, firefox
builds (test): chrome, firefox
builds (test-flask): chrome, firefox
build viz: Build System
mv3: Background Module Init Stats
mv3: UI Init Stats
mv3: Module Load Stats
mv3: Bundle Size Stats
mv2: E2e Actions Stats
code coverage: Report
storybook: Storybook
typescript migration: Dashboard
all artifacts
bundle viz:
- background: 0, 1, 2, 3, 4, 5
- common: 0, 1, 2, 3, 4, 5, 6, 7, 8, 9
- content-script: 0
- offscreen: 0
- ui: 0, 1, 10, 11, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (53 ± 13 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	66	195	84	27	13
		domContentLoaded	9	76	14	14	7
		load	40	170	53	28	13
		domInteractive	9	76	14	14	7

Bundle size diffs [🚨 Warning! Bundle size has increased!]

background: 0 Bytes (0.00%)
ui: 4.96 KiB (0.07%)
common: 273 Bytes (0.00%)

codecov · 2024-06-21T20:54:25Z

Codecov Report

Attention: Patch coverage is 32.81250% with 43 lines in your changes missing coverage. Please review.

Project coverage is 69.65%. Comparing base (d403213) to head (2e4d1fb).
Report is 9 commits behind head on develop.

Files	Patch %	Lines
.../multichain/network-list-menu/network-list-menu.js	45.83%	13 Missing ⚠️
...ete-rpc-url-modal/confirm-delete-rpc-url-modal.tsx	8.33%	11 Missing ⚠️
...ings/networks-tab/networks-form/rpc-url-editor.tsx	38.46%	8 Missing ⚠️
ui/pages/routes/routes.component.js	0.00%	4 Missing ⚠️
...-list-menu/add-rpc-url-modal/add-rpc-url-modal.tsx	33.33%	2 Missing ⚠️
ui/pages/home/home.component.js	0.00%	1 Missing ⚠️
ui/pages/home/home.container.js	0.00%	1 Missing ⚠️
ui/pages/routes/routes.container.js	0.00%	1 Missing ⚠️
...ttings/networks-tab/networks-form/networks-form.js	50.00%	1 Missing ⚠️
ui/store/actions.ts	66.67%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##           develop   #25452      +/-   ##
===========================================
- Coverage    69.69%   69.65%   -0.03%     
===========================================
  Files         1350     1352       +2     
  Lines        47865    47903      +38     
  Branches     13199    13207       +8     
===========================================
+ Hits         33355    33365      +10     
- Misses       14510    14538      +28

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

darkwing · 2024-06-24T22:15:46Z

ui/components/app/modals/confirm-delete-rpc-url-modal/confirm-delete-rpc-url-modal.tsx

+    <Modal
+      isClosedOnEscapeKey={true}
+      isClosedOnOutsideClick={true}
+      isOpen={true}


I don't think ={true} is required here; I think the presence of the attribute infers true

salimtb · 2024-06-25T07:58:16Z

ui/components/app/modals/confirm-delete-rpc-url-modal/confirm-delete-rpc-url-modal.tsx

+            <ButtonPrimary
+              width={BlockSize.Full}
+              size={ButtonPrimarySize.Lg}
+              danger={true}


same here you can just use danger

salimtb · 2024-06-25T07:59:30Z

ui/components/multichain/network-list-menu/add-rpc-url-modal/add-rpc-url-modal.tsx

+        id="additional-rpc-url"
+        label={t('additionalRpcUrl')}
+        labelProps={{
+          children: undefined,


do we need to specify the value of children to undefined here? doesn't this take undefined by default ?

i got a type error without it

salimtb · 2024-06-25T08:07:58Z

hey @bergeron
I'd like to inquire about the validation process for the rpcURL. Specifically, when adding a new rpcURL by clicking the "add" button, which opens another modal, what happens if the entered rpcURL either doesn't correspond with the chainID or is a duplicate of an existing rpcURL? Will there be an alert or error message to indicate this issue? Is this scenario accounted for in the design?

salimtb · 2024-06-25T08:14:12Z

ui/components/multichain/network-list-menu/add-rpc-url-modal/add-rpc-url-modal.tsx

+      >
+        {t('addUrl')}
+      </ButtonPrimary>
+    </Box>


if we decide to include input validation here, it would be a good idea to write unit tests for this modal with the different scenarios ( rpcUrl doesn't match with chainID, rpcURL already added ...etc )

salimtb · 2024-06-25T08:15:03Z

works as expected for me, i just raised one question about input validation if you have more informations on that

bergeron · 2024-06-25T16:29:10Z

hey @bergeron I'd like to inquire about the validation process for the rpcURL.

I think it'd be best to perform that validation on the page where the url is being added. So when you click add, it validates before returning you to the main edit form. Better to get those errors earlier. I don't see explicit design for this, but I'd use the same red warnings as the other edit validations.

metamaskbot · 2024-06-27T14:33:00Z

Builds ready [2e4d1fb]

builds: chrome, firefox
builds (beta): chrome
builds (flask): chrome, firefox
builds (MMI): chrome, firefox
builds (test): chrome, firefox
builds (test-flask): chrome, firefox
build viz: Build System
mv3: Background Module Init Stats
mv3: UI Init Stats
mv3: Module Load Stats
mv3: Bundle Size Stats
mv2: E2e Actions Stats
code coverage: Report
storybook: Storybook
typescript migration: Dashboard
all artifacts
bundle viz:
- background: 0, 1, 2, 3, 4, 5
- common: 0, 1, 2, 3, 4, 5, 6, 7, 8, 9
- content-script: 0
- offscreen: 0
- ui: 0, 1, 10, 11, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (46 ± 3 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	69	91	80	6	3
		domContentLoaded	9	21	11	3	1
		load	38	62	46	6	3
		domInteractive	9	21	11	3	1

Bundle size diffs [🚨 Warning! Bundle size has increased!]

background: 0 Bytes (0.00%)
ui: 4.96 KiB (0.07%)
common: 273 Bytes (0.00%)

sahar-fehri · 2024-06-27T15:11:59Z

Looks great! Thank you! Sorry if this was already talked about 🙏 tested the following scenario, i kinda expected it to say "foobar" is not a valid url,, but it said nothing after typing nor when i clicked, is it intentional?

Screen.Recording.2024-06-27.at.17.08.52.mov

bergeron · 2024-06-27T15:52:19Z

i kinda expected it to say "foobar" is not a valid url,, but it said nothing after typing nor when i clicked, is it intentional?

Validation still needs to be added in a future PR

darkwing and others added 13 commits June 20, 2024 10:44

Network Menu Redesign: Add 3-dot menu

4ff3d6b

fix: resolve conflicts

adb9efa

Add NetworkListItemMenu

04a4c02

feat: add toast after edit

13afd6e

feat: add feature flag for menu

ab714b5

fix: fix suggested name form

d22da9f

fix: fix toast css

7537bd3

fix: fix PR comments

46d22ea

fix: fix suggested name display

3f38e5c

fix: fix linter + tests

2ef606a

fix: add enabled network section

667e58a

merge flow for adding / deleting additional RPC URLs

a2deb40

fixes

dbea8b5

bergeron changed the base branch from develop to edit-custom-network-flow June 20, 2024 19:45

bergeron mentioned this pull request Jun 20, 2024

feat: adding / deleting additional RPC URLs #25431

Closed

7 tasks

metamaskbot added the team-assets label Jun 20, 2024

remove unused message

74658e2

Base automatically changed from edit-custom-network-flow to develop June 21, 2024 14:50

Merge branch 'develop' of github.com:MetaMask/metamask-extension into…

7ff6370

… brian/edit-custom-network-flow4

bergeron marked this pull request as ready for review June 21, 2024 20:25

bergeron requested a review from a team as a code owner June 21, 2024 20:25

bergeron added the needs-assets-ux-review A shared label between the Assets and UX team to flag PRs ready for consolidated team review. label Jun 21, 2024

bergeron requested a review from salimtb June 24, 2024 18:42

darkwing reviewed Jun 24, 2024

View reviewed changes

salimtb reviewed Jun 25, 2024

View reviewed changes

Merge branch 'develop' into brian/edit-custom-network-flow4

2e4d1fb

salimtb approved these changes Jun 27, 2024

View reviewed changes

sahar-fehri approved these changes Jun 28, 2024

View reviewed changes

bergeron merged commit 1f74b08 into develop Jun 28, 2024
70 checks passed

bergeron deleted the brian/edit-custom-network-flow4 branch June 28, 2024 15:18

github-actions bot locked and limited conversation to collaborators Jun 28, 2024

metamaskbot added the release-12.1.0 Issue or pull request that will be included in release 12.1.0 label Jun 28, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: adding / deleting additional RPC URLs #25452

feat: adding / deleting additional RPC URLs #25452

bergeron commented Jun 20, 2024 •

edited

Loading

github-actions bot commented Jun 20, 2024

metamaskbot commented Jun 21, 2024

codecov bot commented Jun 21, 2024 •

edited

Loading

darkwing Jun 24, 2024

salimtb Jun 25, 2024

salimtb Jun 25, 2024

bergeron Jun 25, 2024

salimtb commented Jun 25, 2024

salimtb Jun 25, 2024

salimtb commented Jun 25, 2024

bergeron commented Jun 25, 2024

metamaskbot commented Jun 27, 2024

sahar-fehri commented Jun 27, 2024

bergeron commented Jun 27, 2024

feat: adding / deleting additional RPC URLs #25452

feat: adding / deleting additional RPC URLs #25452

Conversation

bergeron commented Jun 20, 2024 • edited Loading

Description

Related issues

Manual testing steps

Screenshots/Recordings

Before

After

Pre-merge author checklist

Pre-merge reviewer checklist

github-actions bot commented Jun 20, 2024

metamaskbot commented Jun 21, 2024

codecov bot commented Jun 21, 2024 • edited Loading

Codecov Report

darkwing Jun 24, 2024

Choose a reason for hiding this comment

salimtb Jun 25, 2024

Choose a reason for hiding this comment

salimtb Jun 25, 2024

Choose a reason for hiding this comment

bergeron Jun 25, 2024

Choose a reason for hiding this comment

salimtb commented Jun 25, 2024

salimtb Jun 25, 2024

Choose a reason for hiding this comment

salimtb commented Jun 25, 2024

bergeron commented Jun 25, 2024

metamaskbot commented Jun 27, 2024

sahar-fehri commented Jun 27, 2024

bergeron commented Jun 27, 2024

bergeron commented Jun 20, 2024 •

edited

Loading

codecov bot commented Jun 21, 2024 •

edited

Loading