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

Extra connect status #814

Open
wants to merge 7 commits into
base: develop
Choose a base branch
from

Conversation

hpmaxi
Copy link
Contributor

@hpmaxi hpmaxi commented Jan 22, 2021

Closes #614

Finally, this is working and the solution has also improved. This uses an intermediate state and some logic was moved outside the connects functions to avoid re-running effects that generated a loop in a previous solution.

To get this working there were two extra changes.

  • First, we connect to Infura and then we try to connect to a cachedProvider (this is effect logic)
  • To avoid showing the message immediately between switching accounts I've created a hook to check if a variable was a given amount of milliseconds in the same state.

@hpmaxi hpmaxi self-assigned this Jan 22, 2021
@ghost
Copy link

ghost commented Jan 22, 2021

Travis automatic deployment:
https://pr814--conditionaltokensexplorer.review.gnosisdev.com

@hpmaxi hpmaxi marked this pull request as draft January 22, 2021 20:38
@gabitoesmiapodo
Copy link
Contributor

@hpmaxi can't you just show the same a non connected user would see?

If you can't, a message like "You need to unlock or connect your wallet" could be more appropriate.

…er into bug/empty-screen-metamask-not-unblocked
@hpmaxi hpmaxi marked this pull request as ready for review January 26, 2021 21:07
@hpmaxi
Copy link
Contributor Author

hpmaxi commented Jan 26, 2021

@gabitoesmiapodo To show what a non connected user would see we need to connect to infura while the code is inside the await for connecting to the web3-provider. So I've just changed the message.

I did another version but is stashed because it doesn't behave very well.

In this version, the modal appears when you switch between networks. It far from ideal.

@ghost
Copy link

ghost commented Jan 26, 2021

Travis automatic deployment:
https://pr814--conditionaltokensexplorer.review.gnosisdev.com

@hpmaxi hpmaxi marked this pull request as draft January 26, 2021 21:23
@ghost
Copy link

ghost commented Feb 3, 2021

Travis automatic deployment:
https://pr814--conditionaltokensexplorer.review.gnosisdev.com

@ghost
Copy link

ghost commented Feb 3, 2021

Travis automatic deployment:
https://pr814--conditionaltokensexplorer.review.gnosisdev.com

@hpmaxi hpmaxi marked this pull request as ready for review February 3, 2021 18:53
@elena-zh
Copy link
Contributor

elena-zh commented Feb 17, 2021

@hpmaxi , I have checked the fix.
Now, user gets a warning message to unlock a metamask account to proceed working with the app.
However, I have found several side issues:

  1. User gets a connection error when opens Walletconnect modal and then closes it (reproducible in this pr only)
    https://drive.google.com/file/d/1arMCE-kpO4vSiDpdCkkyXjr4_wwu4eEA/view
  2. The app is not automatically reloaded when switch to an unsupported network, then switch back to a supported one (reproducible in this pr only)
    after network change.jpg

Could you please take a look into these issues? Or let me know, whether I need to create additional tasks for them.
Thanks)

@mariano-aguero mariano-aguero removed their request for review February 4, 2022 02:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Empty screen is displayed when open the app in new browser's window
3 participants