add in-app and system notifications #540
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Closed
oshorefueled
force-pushed
the
notifications
branch
from
c0ad484
to
e44b8a9
Compare
beansgum
suggested changes
Jul 27, 2021
ui/notification/toast.go
Outdated
| return layout.Dimensions{} | ||
| } | ||
|
|
||
| t.handleToastDisplay(gtx) |
There was a problem hiding this comment.
This check should be done at the top of the function.
There was a problem hiding this comment.
moving the handle function to the top of the Layout method would lead to a nil dereference error.
ui/notification/log.go
Outdated
| @@ -0,0 +1,25 @@ | |||
| // Copyright (c) 2017, The dcrdata developers | |||
There was a problem hiding this comment.
This package does not need its own logger.
ui/notification/system.go
Outdated
| func (s *SystemNotification) Notify(message string) { | ||
| err := beeep.Notify(title, message, s.iconPath) | ||
| if err != nil { | ||
| log.Info("could not initiate desktop notification, reason:", err.Error()) |
There was a problem hiding this comment.
This error should be returned not logged.
There was a problem hiding this comment.
The log avoids handling notification errors in the ui code. All that needs to be called is the Notify method, just like toast notifications.
oshorefueled
force-pushed
the
notifications
branch
5 times, most recently
from
3b77f60
to
308d602
Compare
- add toast nofication implementation to notification page - move toast layout code to toast.go - add toast to load and common structs - remove previous toast implementation
beansgum
added a commit
to beansgum/godcr
that referenced
this pull request
Aug 17, 2021
An error that causes the app to crash occurs when multiple notification listeners are added with the same key. This occurred because a notification listener added previously in the app onResume was not removed in onStop which can happen if the OS fails to call onStop. This error is avoided by removing any notification listener added with the key before adding a new notification listener.
oshorefueled
force-pushed
the
notifications
branch
from
ba2fd5e
to
8cf655b
Compare
oshorefueled
force-pushed
the
notifications
branch
from
8cf655b
to
a106bc7
Compare
- add NotifyError method - add comments to Notify methods - call NotifyError for error messages in layout code and Notify for success messages.
oshorefueled
force-pushed
the
notifications
branch
from
9ba193e
to
8814abc
Compare
beansgum
approved these changes
Aug 18, 2021
Sirmorrison
approved these changes
Aug 19, 2021
song50119
pushed a commit
to song50119/godcr
that referenced
this pull request
Apr 24, 2022
- add toast notification implementation to notification page - move toast layout code to toast.go - add toast to load and common structs - remove previous toast implementation * implement system notification * stack toast layout on modals layout * add delay speeds to toast notification - add NotifyError method - add comments to Notify methods - call NotifyError for error messages in layout code and Notify for success messages.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR resolves #527
It implements and in-app notification (toast) which is used in displaying success or error messages to a user within the app in response to an action. It also implements a system notification which displays os specific system-wide applications. Some of the notifications displayed with the system notification could be new block notifications, receive transaction notifications, transaction confirmation notification e.t.c..
Calls to the CreateToast method used by pages in the page package are replaced with the Notify method created in this PR.