core icon indicating copy to clipboard operation
core copied to clipboard

feat: Add `NetworkVisibilityController`

Open gambinish opened this issue 8 months ago • 3 comments

The NetworkVisibilityController is a core implementation of network ordering and enablement functionality that was previously only available in the extension. This PR moves and expands this functionality from the extension's NetworkOrder controller to core, enabling network ordering and visibility features across both extension and mobile platforms. It also renames it to NetworkVisibilityController to more accurately represent what it does. Maybe in the future this can be expanded to include other UX enhancements that don't need to be tightly coupled with the NetworkController

Key motivations for this change:

  1. Platform Parity: Network ordering was previously only implemented in the extension via NetworkOrder controller, leaving mobile without this functionality. Moving this to core will unlock the ability to have this feature built on mobile.

  2. Network Enablement Feature: This work is part of the broader Network Enablement initiative (#5737). The controller has been expanded to include enabledNetworkMap state, allowing us to:

    • Track which networks are enabled/disabled
    • Support the new network enablement feature
  3. Code Quality Improvements: The move to core has provided an opportunity to:

    • Improve test coverage of the existing network ordering logic

The controller now handles three key aspects:

  • Network ordering (preserved from extension implementation)
  • Network enablement (new functionality. recently introduced on extension)

References

  • Moves functionality from NetworkOrder controller in extension to core
  • Part of Network Enablement initiative (#5737)
  • Related to ongoing work related to Global Network Selector Removal

Checklist

  • [x] I've updated the test suite for new or updated code as appropriate
  • [x] I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate
  • [ ] I've communicated my changes to consumers by updating changelogs for packages I've changed, highlighting breaking changes as necessary
  • [ ] I've prepared draft pull requests for clients and consumer packages to resolve any breaking changes

gambinish avatar Jun 06 '25 21:06 gambinish

@gambinish Thanks for pinging me on this PR.

I know we just had a discussion where we came to the conclusion that we could integrate the network enablement and network ordering features into one controller. However, I'm not satisfied with the name "NetworkVisibilityController", and I can't come up with a good alternative. And that's telling me that perhaps these are two features that don't belong together after all. The network ordering is strictly UI-related, but the network enablement seems more than that, and so perhaps mixing them together would make things confusing.

Maybe we just want a NetworkEnablementController or EnabledNetworksController for now and we can port NetworkOrderController later?

mcmire avatar Jun 09 '25 19:06 mcmire

Maybe we just want a NetworkEnablementController or EnabledNetworksController for now and we can port NetworkOrderController later?

I think that this is a fair point. To be honest, I thought about this as well. I'll bring this back to the team, though I don't anticipate much pushback.

I'll remove the networkOrder state from this PR and just scope it to network enablement, and rename the controller accordingly.

@mcmire do you have any idea why the test files aren't able to import { Messenger } from '@metamask/base-controller';

I remember we were troubleshooting this locally, unsure why this is failing in CI when it's passing on my machine.

gambinish avatar Jun 09 '25 21:06 gambinish

@gambinish I think I figured it out. It looks like you have an extra paths field in your tsconfig.json. If you remove this then it should work locally and in CI without having to run yarn build ahead of time.

mcmire avatar Jun 10 '25 13:06 mcmire