-
Notifications
You must be signed in to change notification settings - Fork 76
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(@desktop/activity): Handle token received notification #13447
Conversation
Jenkins BuildsClick to see older builds (31)
|
df736be
to
b5fd412
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.
Code LGTM, I just have 1 question and suggestion.
I didn't test it bc I don't have currently the necessary setup. If you want me to explicitly test, please ping me!
chainId: id.contractID.chainID, | ||
txHash: txHash, | ||
name: collectibleName, | ||
amount: 1, |
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.
Will this always be 1?? I can mint 100 collectibles of the same "type" in my community and then airdrop 3 to the same wallet address. Will they always be treated as individual collectibles? This question could be indeed linked with this pending task: #12940
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.
I went and double checked what exactly happens. It's a little complicated :)
When I airdrop 3 community collectibles, itself it is 3 unique collectibles. Each one of them has separate TokenID.
Each collectible airdrop has same tx hash, but is displayed separately in wallet activity feed:
Current implementation will also emit 5 notifications.
We could start discussion here whether consider 3 collectibles airdrop as single notification, because each one of them will open same tx details anyway.
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.
Yeah, I think it makes sense to just consider the 3 collectibles are the "same" although they have different tokenID
. As said before, the following task #12940 and design considers them as a unique item (like a collectible container) with an specific amount. I'm thinking that whenever the #12940 task is done, it will directly emit only 1 notification, so it will come together with this resolution, isn't it?
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.
ui/app/mainui/activitycenter/views/ActivityNotificationCommunityTokenReceived.qml
Outdated
Show resolved
Hide resolved
b5fd412
to
dea2ed3
Compare
73bb3ea
to
7f46bc2
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
7f46bc2
to
5c0c053
Compare
5c0c053
to
41edc7b
Compare
Tasks
BACKEND
: Save notification additional data #13123communityTokenReceived
signal data to providetokenType
,isFirst
andwalletAccountName
information #13250Related status-go change: status-im/status-go#4682
What does the PR do
tokenData
propertyAffected areas
Screenshot of functionality (including design for comparison)
Toast Messages
Collectibles
Assets
Activity Center
Collectibles
Assets