Skip to content
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

add different notification messages for vote transactions #766

Merged
merged 4 commits into from May 29, 2021

Conversation

dreacot
Copy link
Contributor

@dreacot dreacot commented May 23, 2021

Resolves #731

This PR adds notification messages for the various Vote Transactions

Copy link
Contributor

@macsleven macsleven left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

move strings to other languages

@dreacot
Copy link
Contributor Author

dreacot commented May 23, 2021

move strings to other languages

I don't get what you mean?

var amount: String {
switch tx!.type {
case DcrlibwalletTxTypeVote:
return String(format: LocalizedStrings.voteReward, tx!.voteReward)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tx!.voteReward is in int64, convert it to a decimal number.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's not necessary

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's not necessary? Display the vote reward in the notification or converting the vote reward to a decimal number?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@JustinBeBoy what's not necessary?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sorry I mean "tx!.voteReward is in int64, convert it to a decimal number." not necessary

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is it not necessary to display the amount in float64 instead of int64? @JustinBeBoy

case DcrlibwalletTxTypeRevocation:
return ""
default:
return "\(LocalizedStrings.youReceived) \(tx!.dcrAmount.round(8).description) DCR"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Put this entire sentence in localizable strings.

"youReceived" = "You received %@ DCR";

var amount: String {
switch tx!.type {
case DcrlibwalletTxTypeVote:
return String(format: LocalizedStrings.voteReward, tx!.voteReward)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's not necessary

var amount: String {
switch tx!.type {
case DcrlibwalletTxTypeVote:
return String(format: LocalizedStrings.voteReward, tx!.voteReward.toDecimal)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't convert the value from atoms to dcr.

Suggested change
return String(format: LocalizedStrings.voteReward, tx!.voteReward.toDecimal)
return String(format: LocalizedStrings.voteReward, tx!.dcrVoteReward.round(8).description)

@@ -9,6 +9,12 @@
import UIKit
import Dcrlibwallet

extension Int64 {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not needed.

@beansgum beansgum dismissed stale reviews from JustinBeBoy and macsleven May 29, 2021 10:37

it's not necessary.

@beansgum beansgum merged commit 1f0da9c into planetdecred:master May 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Vote transaction notification message
4 participants