-
Notifications
You must be signed in to change notification settings - Fork 570
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
Iss 279 - When selecting a new Node under Settings -> Access, don't hard reload the browser #380
Conversation
app/routerTransition.js
Outdated
@@ -45,7 +45,7 @@ const filterAndSortURLs = (count, latencies) => { | |||
return urls; | |||
}; | |||
|
|||
const willTransitionTo = (nextState, replaceState, callback) => { | |||
const willTransitionTo = (nextState, replaceState, callback, appInit=true) => { //appInit is true when node is manually selected in access settings |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't this say that appInit is false when selecting nodes in the settings?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, because appInit (which includes fallbacks) should only happen when it's not user choice. I'm using this to differentiate between user picking a server and server being chosen when the app is loaded.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Exactly, so when a user picks a server appInit is false, but on first load of the app it is true, which is the opposite of what the comment says. You are calling it with false on line 36
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lolz. Yea, just need to fix the comment.
app/routerTransition.js
Outdated
console.log("db init error:", err); | ||
} | ||
return Promise.all([db, SettingsStore.init()]).then(() => { | ||
return callback(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This reset callback needs to include the promises loading the WalletDb and PrivateKeyActions data, (PrivateKeyActions.loadDbData() and WalletDb.loadDbData()), since the wallet that is loaded depends on the chain id of the websocket API you're connected to. If you switch from the mainnet to the testnet you need to load the correct wallet and private keys. I'm not sure how well those methods play with on the fly switching
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No problem, can refactor this so the logic is included for both.
@svk31 There is now a shared process that should take care of the private key actions and wallet stuff...please double check as I'm not sure exactly how to totally verify this behavior. Otherwise, all seems to be well. Happy to do more digging if you can tell me what I should be testing / cautious about. |
Addresses issue # 279.
This PR causes the init which sets up API server to be called again when user selects a new node so that page does not hard refresh.