Skip to content
This repository has been archived by the owner on Nov 6, 2020. It is now read-only.

Starting on homestead shows reload snackbar #4043

Merged
merged 2 commits into from
Jan 5, 2017
Merged

Conversation

jacogr
Copy link
Contributor

@jacogr jacogr commented Jan 4, 2017

Starting on homestead immediately showed the "reload this page" snackbar.

  • start from "(unknown)" chain
  • don't show message when previous state indicates "(unknown)" (or is the same as current)
  • create tests for the middleware & dispatch conditions

There is an additional issue - the long polling causes the new chain to only show up after some time (along with the message), needs an additional issue/PR.

@jacogr jacogr added A0-pleasereview 🤓 Pull request needs code review. M7-ui labels Jan 4, 2017
@jacogr jacogr changed the title Starting on homestead showed reload snackbar Starting on homestead shows reload snackbar Jan 4, 2017
@ngotchac
Copy link
Contributor

ngotchac commented Jan 5, 2017

One small comment regarding this PR : I think the snackbar should stick (as errors do) and only a click would close it. If you're not watching the UI, then you won't see the notification... One other solution would be to use the Browser Notification API.

Also, another comment : after switching network, you get notifications about new ETH or tokens in your account (since the balances were previously at 0). Might want to handle that in another PR/Issue

@ngotchac ngotchac added A5-grumble 🔥 Pull request has minor issues that must be addressed before merging. and removed A0-pleasereview 🤓 Pull request needs code review. labels Jan 5, 2017
@jacogr
Copy link
Contributor Author

jacogr commented Jan 5, 2017

Valid comment on the close timing, will align with errors.

The issue I mentioned and you did needs to be logged though, out of scope here.

@ngotchac ngotchac added A8-looksgood 🦄 Pull request is reviewed well. and removed A5-grumble 🔥 Pull request has minor issues that must be addressed before merging. labels Jan 5, 2017
@jacogr jacogr merged commit 1ef67f6 into master Jan 5, 2017
@jacogr jacogr deleted the jg-chain-switch-initial branch January 5, 2017 11:07
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A8-looksgood 🦄 Pull request is reviewed well.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants