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

Notify user on transaction received #3782

Merged
merged 4 commits into from
Dec 10, 2016
Merged

Notify user on transaction received #3782

merged 4 commits into from
Dec 10, 2016

Conversation

ngotchac
Copy link
Contributor

@ngotchac ngotchac commented Dec 9, 2016

Closes #2556 until we have a pub/sub mechanism.

Browser notifications will be created whenever we detect an increase in an account value (in ETH or any other registered token). On click, the user is taken to the account page.
This might not work if the account receive n eth while sending n eth in the same block.
A pub/sub mechanism would be better, but isn't available atm.

@ngotchac ngotchac added A0-pleasereview 🤓 Pull request needs code review. M7-ui labels Dec 9, 2016
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.5%) to 85.592% when pulling 129056a on ng-transaction-notify into 341777d on master.

@jacogr
Copy link
Contributor

jacogr commented Dec 10, 2016

Merge in master for gitlab builds.

@jacogr
Copy link
Contributor

jacogr commented Dec 10, 2016

Not 100% convinced of this UX -

monosnap 2016-12-10 20-13-44

Seems to work however. Closes way too quickly to even try to interact with it and click.

@ngotchac
Copy link
Contributor Author

This is the default Chrome behavior with notifications. We cannot get away without it.

The timeout is set to 5sec. Could increase it. 10 ? 20 ?

@coveralls
Copy link

Coverage Status

Coverage increased (+0.003%) to 85.604% when pulling f65695c on ng-transaction-notify into 08a47ea on master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 85.601% when pulling f65695c on ng-transaction-notify into 08a47ea on master.

@jacogr
Copy link
Contributor

jacogr commented Dec 10, 2016

20s is probably a bit long, but ok as a start, easy enough to adjust downwards.

@jacogr jacogr added A8-looksgood 🦄 Pull request is reviewed well. and removed A0-pleasereview 🤓 Pull request needs code review. labels Dec 10, 2016
@jacogr jacogr merged commit b0f1665 into master Dec 10, 2016
@jacogr jacogr deleted the ng-transaction-notify branch December 10, 2016 22:55
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

3 participants