-
Notifications
You must be signed in to change notification settings - Fork 245
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
feat_: dapp connected notification #5616
base: develop
Are you sure you want to change the base?
Conversation
Jenkins BuildsClick to see older builds (30)
|
9bc67a0
to
c9d0277
Compare
f794844
to
f406cb5
Compare
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.
LGTM, left minor comment
f406cb5
to
177688f
Compare
Will request manual QA on the mobile side after 2.30 release |
177688f
to
d608df7
Compare
} | ||
|
||
_, err := m.persistence.SaveActivityCenterNotification(notification, true) | ||
|
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.
To notify client here, you'd do something like this:
if m.config.messengerSignalsHandler != nil {
response := &MessengerResponse{}
response.AddActivityCenterNotification(notification)
m.config.messengerSignalsHandler.MessengerResponse(response)
}
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.
Though combine it with my next comment.
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.
thanks, i added and now works fine.
Signed-off-by: yqrashawn <[email protected]>
d608df7
to
708ddec
Compare
708ddec
to
24f56c7
Compare
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.
Sorry for blocking.
We're discussing now with the @status-im/status-go-guild how to properly pass the Mesenger to Wallet in such cases
Hey @igor-sirotin, please let me know about final decision once ready and if i should apply any changes here, thanks |
hey @mohsen-ghafouri ! Logic:
cc @status-im/status-go-guild |
@mohsen-ghafouri what's the state on this one? Please let me know if you need any help with the subscriber pattern! |
Hey @igor-sirotin, sorry i was working on other priority task, will be in touch once i started working on this. thanks |
create a new activity center notification when new wallet connect session added
mobile PR status-im/status-mobile#20905
mobile ISSUE status-im/status-mobile#20467
Important changes:
InjectedMessenger
in wallet service to avoid import cycleAddWalletConnectSession
activity_center_notifications
table