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

[414] remove NetworkSelector on sc wallet session #507

Merged
merged 12 commits into from
May 5, 2022

Conversation

W3stside
Copy link
Contributor

@W3stside W3stside commented May 3, 2022

Closes #414

image

Test

  1. connect SC wallet
  2. no net selector

@W3stside W3stside requested review from a team May 3, 2022 14:22
@github-actions
Copy link
Contributor

github-actions bot commented May 3, 2022

CLA Assistant Lite:
Thank you for your submission, we really appreciate it. Like many open-source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution. You can sign the CLA by just posting a Pull Request Comment same as the below format.


I have read the CLA Document and I hereby sign the CLA


You can retrigger the CLA Action by commenting recheckcla in this Pull Request

@github-actions
Copy link
Contributor

github-actions bot commented May 3, 2022

  • 🔭 GP Swap: CoW Protocol v2 Swap UI

@elena-zh
Copy link

elena-zh commented May 3, 2022

Gnosis Safe in Rinkeby:
image

Gnosis safe in GC:
image

Copy link

@elena-zh elena-zh left a comment

Choose a reason for hiding this comment

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

See the notes above

@elena-zh
Copy link

elena-zh commented May 3, 2022

Also, there is no network selector to a not connected user:
image

@elena-zh
Copy link

elena-zh commented May 3, 2022

Network selector presents when connect to Gnosis Safe mobile wallet (all networks)
image

@W3stside
Copy link
Contributor Author

W3stside commented May 3, 2022 via email

@elena-zh
Copy link

elena-zh commented May 3, 2022

@elena-zh can you check this is the case outside of this PR?

On Tue, 3 May 2022, 15:43 Elena, @.> wrote: Network selector presents when connect to Gnosis Safe mobile wallet (all networks) [image: image] https://user-images.githubusercontent.com/70885163/166475861-51ab04f4-938b-4ec0-a8dd-5c8bddacf5ca.png — Reply to this email directly, view it on GitHub <#507 (comment)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/AFCY4CYCTP54OZWSU4WGHXLVIE3STANCNFSM5U7B5MOA . You are receiving this because you authored the thread.Message ID: @.>

DEV environment: the issue is reproducible there

Prod environment: the network selector is disabled for this case.

Let me know please if I need to retest it somewhere else.
Thanks!

@W3stside W3stside requested a review from elena-zh May 3, 2022 20:48
@elena-zh
Copy link

elena-zh commented May 4, 2022

Hey @W3stside , now the network switcher is displayed to a not connected user. And the network switcher is removed when connected to Gnosis Safe in Mainnet, and to a mobile Gnosis Safe wallet in Mainnet.

But still, the selector is displayed when connected to Argent wallet
argent
Pillar wallet
image

Gnosis Safe in Gnosis Chain
GC

Gnosis Safe in Rinkeby
rink

@W3stside
Copy link
Contributor Author

W3stside commented May 4, 2022

@elena-zh yeah didnt have the right changes in there, could u recheck after the build is remade?

@alfetopito
Copy link
Collaborator

@W3stside
Screen Shot 2022-05-04 at 11 41 11

@elena-zh
Copy link

elena-zh commented May 4, 2022

Hey @W3stside , works nicely now with Gnosis Safe wallet for all networks.

However, I still see the network selector when connected to Argent or Pillar wallets. Could we hide it?

Thanks

@W3stside
Copy link
Contributor Author

W3stside commented May 4, 2022

@elena-zh pillar i wont do rn since i have no experience with it but argent should be done now

@elena-zh
Copy link

elena-zh commented May 4, 2022

@W3stside , seems that there is an issue with the latest build

@W3stside
Copy link
Contributor Author

W3stside commented May 4, 2022

@W3stside , seems that there is an issue with the latest build

there wasn't, what do you see wrong?

image

@elena-zh
Copy link

elena-zh commented May 4, 2022

I have not seen changes in the PR.
Let me retest please.

@elena-zh
Copy link

elena-zh commented May 4, 2022

Argent still shows the network switcher
image

@W3stside
Copy link
Contributor Author

W3stside commented May 5, 2022

Argent still shows the network switcher image

hmm it doesn't do that for me
image

if it comes up again let's hotfix

@W3stside W3stside merged commit 3223084 into release/1.14 May 5, 2022
@github-actions github-actions bot locked and limited conversation to collaborators May 5, 2022
@elena-zh
Copy link

elena-zh commented May 5, 2022

@W3stside , I opened a separate low-prio issue #526 for this case, as the issue is still reproducible to me.

@alfetopito alfetopito deleted the 414/sc-wallet-disable-selector branch May 5, 2022 12:45
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants