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

feat: new components for network UI #27085

Merged
merged 11 commits into from
Sep 12, 2024
Merged

Conversation

bergeron
Copy link
Contributor

@bergeron bergeron commented Sep 11, 2024

Description

This PR splits off the new UI components from #26433. They support the new UI for editing networks directly via the network picker.

  • MultiRpcEditModal - A modal shown only to users who have multiple networks for a given chain. Allows them to customize via the network form if they don't like how we merged them.
Screenshot 2024-09-11 at 4 32 42 PM
  • DropdownEditor - A generic dropdown for selecting, adding, and deleting items. Currently shared by the editing experience for RPC endpoints and block explorers.
Screenshot 2024-09-11 at 4 34 40 PM Screenshot 2024-09-11 at 4 34 18 PM
  • AddBlockExplorerModal - The sub page within the network form where you add a block explorer
Screenshot 2024-09-11 at 4 35 49 PM
  • AddRpcUrlModal - The sub page within the network form where you add an RPC url
Screenshot 2024-09-11 at 4 35 56 PM
  • RpcListItem - A component representing a single RPC endpoint with a list. Currently shared between selecting via the SelectRpcUrlModal and editing via the RPC DropdownEditor
Screenshot 2024-09-11 at 4 50 43 PM
  • SelectRpcUrlModal - The page used to switch RPC endpoints for a chain.
Screenshot 2024-09-11 at 4 52 09 PM

Additionally some actions are modified to support new features used by these components.

Open in GitHub Codespaces

Related issues

Manual testing steps

None of these are visible in the product yet, but will be after #26433

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.

@bergeron bergeron requested review from a team as code owners September 11, 2024 23:53
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.

Comment on lines +77 to +131
const renderDropdownList = () => (
<Box>
{items?.map((item, index) => {
const row = (
<Box
alignItems={AlignItems.center}
paddingLeft={4}
paddingRight={4}
display={Display.Flex}
justifyContent={JustifyContent.spaceBetween}
key={itemKey(item)}
onClick={() => {
onItemSelected(index);
setIsDropdownOpen(false);
}}
className={classnames('dropdown-editor__item', {
'dropdown-editor__item--selected': index === selectedItemIndex,
})}
>
{index === selectedItemIndex && (
<Box
className="dropdown-editor__item-selected-pill"
borderRadius={BorderRadius.pill}
backgroundColor={BackgroundColor.primaryDefault}
/>
)}
{renderItem(item, true)}
{itemIsDeletable(item, items) && (
<ButtonIcon
marginLeft={1}
ariaLabel={t('delete')}
size={ButtonIconSize.Sm}
iconName={IconName.Trash}
color={IconColor.errorDefault}
onClick={(e: React.MouseEvent) => {
e.stopPropagation();

// Determine which item should be selected after deletion
let newSelectedIndex;
if (selectedItemIndex === undefined || items.length <= 1) {
newSelectedIndex = undefined;
} else if (index === selectedItemIndex) {
newSelectedIndex = 0;
} else if (index > selectedItemIndex) {
newSelectedIndex = selectedItemIndex;
} else if (index < selectedItemIndex) {
newSelectedIndex = selectedItemIndex - 1;
}

onItemDeleted(index, newSelectedIndex);
}}
/>
)}
</Box>
);
Copy link
Contributor

@gambinish gambinish Sep 12, 2024

Choose a reason for hiding this comment

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

Not a dealbreaker, but for testing/reusability purposes I would typically prefer to have this live as it's own component, rather than in a callback within dropdown-editor.tsx. Maybe something like multichain-dropdown-list.tsx, and then pass it whatever props it needs.

Doesn't need to block though.

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree! Let's consider this as a follow up!

@metamaskbot
Copy link
Collaborator

Builds ready [435b3d0]
Page Load Metrics (1726 ± 129 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint144826071731269129
domContentLoaded144023711691227109
load144826311726269129
domInteractive156632147
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 0 Bytes (0.00%)
  • ui: 1.82 KiB (0.03%)
  • common: 308 Bytes (0.00%)

@metamaskbot
Copy link
Collaborator

Builds ready [26a58c4]
Page Load Metrics (1761 ± 137 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint154225541766282135
domContentLoaded153125061743273131
load153925581761284137
domInteractive227335157
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 0 Bytes (0.00%)
  • ui: 1.82 KiB (0.03%)
  • common: 705 Bytes (0.01%)

@metamaskbot
Copy link
Collaborator

Builds ready [6dcaa56]
Page Load Metrics (1806 ± 82 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint15862245181317182
domContentLoaded15782189178315977
load15862242180617182
domInteractive12542984
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 0 Bytes (0.00%)
  • ui: 1.82 KiB (0.03%)
  • common: 705 Bytes (0.01%)

Copy link

@metamaskbot
Copy link
Collaborator

Builds ready [7a74433]
Page Load Metrics (1802 ± 140 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint23324411662555267
domContentLoaded146522871777278133
load147324261802292140
domInteractive176332126
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 0 Bytes (0.00%)
  • ui: 1.82 KiB (0.03%)
  • common: 705 Bytes (0.01%)

Copy link

codecov bot commented Sep 12, 2024

Codecov Report

Attention: Patch coverage is 42.16867% with 96 lines in your changes missing coverage. Please review.

Project coverage is 70.03%. Comparing base (5053c3e) to head (7a74433).

Files with missing lines Patch % Lines
...nts/multichain/dropdown-editor/dropdown-editor.tsx 0.00% 57 Missing ⚠️
...-block-explorer-modal/add-block-explorer-modal.tsx 0.00% 17 Missing ⚠️
.../app/multi-rpc-edit-modal/multi-rpc-edit-modal.tsx 53.85% 6 Missing ⚠️
...-list-menu/add-rpc-url-modal/add-rpc-url-modal.tsx 62.50% 6 Missing ⚠️
...edit-modal/network-list-item/network-list-item.tsx 75.00% 5 Missing ⚠️
...nts/multichain/network-list-menu/rpc-list-item.tsx 87.50% 3 Missing ⚠️
ui/store/actions.ts 33.33% 2 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop   #27085      +/-   ##
===========================================
- Coverage    70.12%   70.03%   -0.09%     
===========================================
  Files         1426     1432       +6     
  Lines        49705    49868     +163     
  Branches     13905    13966      +61     
===========================================
+ Hits         34853    34921      +68     
- Misses       14852    14947      +95     

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

@bergeron bergeron merged commit daacc1a into develop Sep 12, 2024
78 checks passed
@bergeron bergeron deleted the brian/new-network-components3 branch September 12, 2024 15:20
@github-actions github-actions bot locked and limited conversation to collaborators Sep 12, 2024
@metamaskbot metamaskbot added the release-12.6.0 Issue or pull request that will be included in release 12.6.0 label Sep 12, 2024
@metamaskbot metamaskbot added the release-12.5.0 Issue or pull request that will be included in release 12.5.0 label Sep 29, 2024
@metamaskbot metamaskbot removed the release-12.6.0 Issue or pull request that will be included in release 12.6.0 label Sep 29, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
release-12.5.0 Issue or pull request that will be included in release 12.5.0 team-assets
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants