Improve account connect/loading, check network #154

2019-09-04T16:54:24Z

raucao commented

2019-09-04 16:54:24 +00:00

(Migrated from github.com)

Sign in button to connect account later
Only use user wallet on app startup if it's already open/connected
Check required network before using user provider

closes #82

* Sign in button to connect account later * Only use user wallet on app startup if it's already open/connected * Check required network before using user provider closes #82

galfert (Migrated from github.com) reviewed 2019-09-04 16:54:24 +00:00

fsmanuel (Migrated from github.com) reviewed 2019-09-04 16:54:24 +00:00

haythemsellami (Migrated from github.com) reviewed 2019-09-04 16:54:24 +00:00

bumi (Migrated from github.com) reviewed 2019-09-05 07:04:40 +00:00

app/services/kredits.js

						
				@@ -126,3 +125,4 @@

				        } else {

				          instantiateWithoutAccount();

				        }

				      }

bumi (Migrated from github.com) commented

2019-09-05 07:04:18 +00:00

thinking out loud:
we set requireSupportedNetwork = false in environment.js:85 (if WEB3_PROVIDER_URL is set)
and if it is not false we check if the network is in this list: https://github.com/67P/kredits-contracts/blob/master/lib/kredits.js#L87

thinking out loud: we set `requireSupportedNetwork = false` in environment.js:85 (if WEB3_PROVIDER_URL is set) and if it is not false we check if the network is in this list: https://github.com/67P/kredits-contracts/blob/master/lib/kredits.js#L87

raucao (Migrated from github.com) reviewed 2019-09-05 08:00:24 +00:00

app/services/kredits.js

						
				@@ -126,3 +125,4 @@

				        } else {

				          instantiateWithoutAccount();

				        }

				      }

raucao (Migrated from github.com) commented

2019-09-05 08:00:24 +00:00

Yes, happy to review a future PR with that change! This PR is only about requiring the only currently available network, so that the current web app does the right thing.

Edit: actually, it's mostly about signing in after having loaded the app. The network requirement is the smallest part of it.

Yes, happy to review a future PR with that change! This PR is only about requiring the only currently available network, so that the current web app does the right thing. Edit: actually, it's mostly about signing in after having loaded the app. The network requirement is the smallest part of it.

gregkare commented

2019-09-10 14:32:45 +00:00

(Migrated from github.com)

Tried this locally, works as advertised, LGTM! 👍

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: kredits/kredits-web#154