-
Notifications
You must be signed in to change notification settings - Fork 79
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
fix(wallet): in app tx related notifications improvements #16732
Conversation
Jenkins BuildsClick to see older builds (35)
|
7140558
to
504236a
Compare
bf5fafc
to
1dd9733
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.
Just some minor stuff, otherwise LGTM
ui/app/mainui/AppMain.qml
Outdated
let toastType = Constants.ephemeralNotificationType.normal | ||
let toastLink = "" | ||
|
||
const sender = !!fromName? fromName : SQUtils.Utils.elideText(fromAddr, 6, 4) |
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.
Better use Utils.elideAndFormatWalletAddress()
?
ui/app/mainui/AppMain.qml
Outdated
toastType = Constants.ephemeralNotificationType.danger | ||
icon = "warning" | ||
case Constants.txStatus.failed: { | ||
toastTitle = "Send failed: %1 from %2 to %3" |
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.
qsTr()
missing
ui/app/mainui/AppMain.qml
Outdated
break | ||
} | ||
case Constants.SendType.Swap: { | ||
toastTitle = qsTr("Swapp failed: %1 to %2 in %3").arg(sentAmount).arg(receivedAmount).arg(sender) |
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.
toastTitle = qsTr("Swapp failed: %1 to %2 in %3").arg(sentAmount).arg(receivedAmount).arg(sender) | |
toastTitle = qsTr("Swap failed: %1 to %2 in %3").arg(sentAmount).arg(receivedAmount).arg(sender) |
1dd9733
to
0d43683
Compare
… instead the default one only
0d43683
to
c40aa1d
Compare
@caybro comments addressed. Thanks. |
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 👍
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.
These user stories were tested and passed. The failed status was not tested yet.
Failed status will be tested later on a master build or a specific build where we can emulate tx errors to show the fail
SP-431: notifications for sent asset (erc20 + ETH) to non-saved address
SP-432: notifications for sent asset (erc20 + ETH) to saved address or own address
SP-434: notifications for bridge
SP-435: notifications for swap
SP-436: notifications for spending cap and swap
These missing items will be handled in a different issue #16810
- notifications for WC/BC
- notifications for Community minting and airdropping
Corresponding
status-go
PR:Closes #16338