[1319.1] Change Network toggler - BASE PR (WIP) #1344

W3stside · 2021-08-26T20:23:33Z

BASE PR - several will follow

Summary

Part of #1319 - will be broken down into a couple more PRs likely

Implements the core network switching logic and only a simple component, no real styling, no edge case testing, no real sexy UX

To Test

click the networks on the top and see how it changes in MM
test other wallets (haven't tested this)

Always show all network connect buttons. Right now we hide the button for the current active network. I'd also show the current active network button with a ✅ for visual confirmation. I think right now it's more evident when you're on mainnet given we don't show you the network card there. I think we essentially have to 'glue' together the current NetworkCard component and your network select/switch menu.
For the very simple proof of concept, you could opt using a select menu (<select>).
When the user currently is not connected to any wallet, on click of a network button we could catch this and ask the user to connect their wallet (in combination with the right network!). Or we don't show the network selector when you're not connected, and when the user decides to connect their wallet, the default network is the one we detect from the client? 🤔
There's a scenario where for some reason, the 'change network' modal from the client wallet (in this case MetaMask) is hidden/currently not visible to the user. When the user then repeatedly presses the network button in our UI, nothing happens. There is an error message being returned which can specifically catch/target this. My thinking is, we could alert the user about this (Network change request already pending...):

W3stside · 2021-08-27T10:27:50Z

Edge case possibly:

Opened the app

Metamask was locked. Unlocked it. It was connected to xDai

This pop up keep showing up

Switch on MM to mainnet, pop up went away.

Clicked on the switcher to xDai, confirmed change notification on wallet

Pop up is back asking me to import wxdai

isn't from this PR, this is the behaviour all the time unfortunately. but it goes away quickly on other branches when switching. i think it's accentuated with this pr tho. will investigate

W3stside · 2021-08-27T10:31:27Z

@W3stside Great you started this. Here a couple of suggestions:

Always show all network connect buttons. Right now we hide the button for the current active network. I'd also show the current active network button with a ✅ for visual confirmation. I think right now it's more evident when you're on mainnet given we don't show you the network card there. I think we essentially have to 'glue' together the current NetworkCard component and your network select/switch menu.

Yes that's the idea, for now this is very much as u mentioned a proof of concept and leaving it in here was weird.

For the very simple proof of concept, you could opt using a select menu (<select>).

sure, but wont change anything here, i think it's best in a follow up PR to just make the pretty comp with all the choices and nothing hooked up.

When the user currently is not connected to any wallet, on click of a network button we could catch this and ask the user to connect their wallet (in combination with the right network!). Or we don't show the network selector when you're not connected, and when the user decides to connect their wallet, the default network is the one we detect from the client? 🤔

good idea, imho we shouldn't show it at all until they connect. best not to show too many buttons/things to do before they're even connected. the main CTA when disconnected is the big ass bright orange CONNECT WALLET button and we should keep that the main focus

There's a scenario where for some reason, the 'change network' modal from the client wallet (in this case MetaMask) is hidden/currently not visible to the user. When the user then repeatedly presses the network button in our UI, nothing happens. There is an error message being returned which can specifically catch/target this. My thinking is, we could alert the user about this (Network change request already pending...):

good catch, definitely sth to add as a banner maybe since it's close to where this is already happening (header) at least in web. i think even with the "header" in the footer in mobile we can leave it as a top level banner warning

elena-zh · 2021-08-27T10:52:43Z

isn't from this PR, this is the behaviour all the time unfortunately. but it goes away quickly on other branches when switching. i think it's accentuated with this pr tho. will investigate

This issue was mentioned in #1061 (case 1)

elena-zh · 2021-08-27T11:15:45Z

I think I will start adding comments one by one here.
My first issue is when connect to a MM account using WC:

networks are not changed when swicth them in the connected MM wallet (nether in Android nor in iOS).
the same is when I try to switch networks in the app: no changes in the connected MM in a mobile device.

The same is with imToken wallet

elena-zh · 2021-08-27T11:20:44Z

The next case is that a lot of wallets do not support several networks.
As an example, TrustWallet, TokenPocket. So it looks a bit odd when I connect such wallet and see an option to switch to xDAI or Rinkeby.

And, as expected, nothing happens when I press on networks buttons in the app.

elena-zh · 2021-08-27T11:27:45Z

There is no network switcher in a mobile view

It would be nice to add it to test changes in wallets' integrated browsers
UPD: actually, it is a known issue #1168 is not related exactly to this pr, but nevertheless

W3stside · 2021-08-27T14:13:47Z

thanks @elena-zh

Feel free to add these as a checklist of things to do in the PR base description or to the issue #1319

Keep in mind this PR is not meant to be a finished anything and is simply a POC (kinda)

MareenG · 2021-08-31T07:59:27Z

Super nice. I like it.
I will add also some thoughts to #1319

* Move single hop toggle GA event Move GA event from setSingleHopOnly hook to toggle function * Fix code style issues with ESLint * refactor ternary operator out Co-authored-by: Lint Action <[email protected]>

W3stside · 2021-11-16T10:29:05Z

closed in favour of the latest uni stuff

W3stside added 5 commits August 26, 2021 18:59

enum to list methods

edaebb1

added chain json files

07b8d24

create network switching file and hook

264a4a0

NetworkPicker component

be6d925

added in header

84f0c3c

W3stside requested review from a team August 26, 2021 20:23

W3stside added 3 commits August 26, 2021 21:26

json edit

33e9723

logic cleanup

e44ccb5

import path

440bcfe

xdai info

7f2e006

alfetopito reviewed Aug 26, 2021

View reviewed changes

src/custom/constants/chains/json/homestead.json Outdated Show resolved Hide resolved

src/custom/components/NetworkPicker/index.tsx Show resolved Hide resolved

mainnet

8792533

W3stside changed the title ~~[1319.1] Change Network toggler - main logic and simple component~~ [1319.1] Change Network toggler - BASE PR (WIP) Aug 27, 2021

elena-zh mentioned this pull request Aug 27, 2021

Easily change networks #1319

Closed

W3stside mentioned this pull request Sep 9, 2021

[🦄 🦄 Uniswap Merge] 4.13.0 #1413

Merged

1 task

anxolin force-pushed the develop branch from f993477 to 8cb5c71 Compare October 14, 2021 15:58

W3stside closed this Nov 16, 2021

alfetopito deleted the 1319/network-toggler branch November 16, 2021 17:19

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[1319.1] Change Network toggler - BASE PR (WIP) #1344

[1319.1] Change Network toggler - BASE PR (WIP) #1344

W3stside commented Aug 26, 2021 •

edited

Loading

github-actions bot commented Aug 26, 2021

alfetopito left a comment

fairlighteth commented Aug 27, 2021 •

edited

Loading

W3stside commented Aug 27, 2021

W3stside commented Aug 27, 2021

elena-zh commented Aug 27, 2021

elena-zh commented Aug 27, 2021 •

edited

Loading

elena-zh commented Aug 27, 2021 •

edited

Loading

elena-zh commented Aug 27, 2021 •

edited

Loading

W3stside commented Aug 27, 2021

MareenG commented Aug 31, 2021

W3stside commented Nov 16, 2021

[1319.1] Change Network toggler - BASE PR (WIP) #1344

[1319.1] Change Network toggler - BASE PR (WIP) #1344

Conversation

W3stside commented Aug 26, 2021 • edited Loading

BASE PR - several will follow

Summary

To Test

Next:

github-actions bot commented Aug 26, 2021

alfetopito left a comment

Choose a reason for hiding this comment

fairlighteth commented Aug 27, 2021 • edited Loading

W3stside commented Aug 27, 2021

W3stside commented Aug 27, 2021

elena-zh commented Aug 27, 2021

elena-zh commented Aug 27, 2021 • edited Loading

elena-zh commented Aug 27, 2021 • edited Loading

elena-zh commented Aug 27, 2021 • edited Loading

W3stside commented Aug 27, 2021

MareenG commented Aug 31, 2021

W3stside commented Nov 16, 2021

W3stside commented Aug 26, 2021 •

edited

Loading

fairlighteth commented Aug 27, 2021 •

edited

Loading

elena-zh commented Aug 27, 2021 •

edited

Loading

elena-zh commented Aug 27, 2021 •

edited

Loading

elena-zh commented Aug 27, 2021 •

edited

Loading