-
Notifications
You must be signed in to change notification settings - Fork 96
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
Move the default network setting outside of built bundles - Closes #491 #538
Conversation
…folder from eslint
src/index.html
Outdated
@@ -257,6 +257,14 @@ | |||
|
|||
</style> | |||
<link rel="icon" type="image/png" href="./assets/images/LISK.png" /> | |||
<script type="text/javascript"> | |||
var defaultNet = "mainnet";//defaultNetwork |
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.
I would add another comment to explain that //defaultNetwork
is used npm run build:testnet
. Otherwise, seeing just this file makes it look like a useless comment that I would delete if I didn't know the context.
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.
updated
src/index.html
Outdated
var defaultNet = "mainnet";//defaultNetwork | ||
(function(targetNet) { | ||
if (window.localStorage){ | ||
window.localStorage.setItem('defaultNetwork', defaultNet); |
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.
The if
could also check if default network is already defined in local storage. And don't set it in that case. That way, we can set defaultNetwork
on localhost when developing (when needed) and it won't be overridden on each page reload.
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.
done
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.
Thank you, Pablo
What was the problem?
A flag is used at build time to set testNet or mainNet as the preferred network to show, this should be changed to be set clientside
How did I fix it?
How to test it?
(assert when passing
?showNetwork=true
) points to mainnet(assert when passing
?showNetwork=true
) points to testnetassert for both builds that wen actively setting a value on local storage of browser with:
then that value is used next time the application is reloaded.
Review checklist