Skip to content
This repository has been archived by the owner on Dec 11, 2019. It is now read-only.

add network connection notifications #4038

Merged
merged 1 commit into from
Sep 15, 2016
Merged

Conversation

mrose17
Copy link
Member

@mrose17 mrose17 commented Sep 15, 2016

supercedes #3973 (reduce to 1 commit)

Test Plan:

disconnect the network. reconnect the network. 1 second later the ledger client code will run as will the code that gets the balance.

auditor: @bridiver

notifications on network up/down to the ledger

supercedes #3973
@luixxiul
Copy link
Contributor

Would someone please specify STR to confirm the issue has been fixed?

@bridiver
Copy link
Collaborator

there is a test plan above, but I think it needs a better method to verify in the release builds. @mrose17 is there something they can check in the interface? Log output is not readily available in the release builds to confirm that the code is running

@mrose17
Copy link
Member Author

mrose17 commented Sep 22, 2016

@bridiver - you can always get log information, it's just a matter of how you start the browser, e.g., LEDGER_LOGGING=true LEDGER_VERBOSE=true /Applications/Brave.app/Contents/MacOS/Brave

although i agree with the point that it ought to be easier to test it.

@ayumi - what do you think about a follow-on issue in which, on appConstants.APP_NETWORK_CONNECTED, we update ledgerInfo.error with a string saying "Internet inaccessible, so updates unavailable." ?

@ayumi
Copy link
Contributor

ayumi commented Sep 22, 2016

@mrose17 Right now there's a message saying "error, can't retrieve data" which covers being offline I think.
Are we going to show general notification of offline status – I think maybe @bsclifton suggested it here #3973 (comment)?
If so the user would know they're offline.

@mrose17
Copy link
Member Author

mrose17 commented Sep 22, 2016

@ayumi - perhaps this is a non-issue, then.

certainly a general notification along the lines of what @bsclifton suggested in a post-1.0 timeframe would suffice.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants