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 wrong display of amount in transaction list (if sender==receiver) #487

Merged
merged 1 commit into from Dec 29, 2017

Conversation

Projects
None yet
5 participants
@Nasicus

Nasicus commented Dec 25, 2017

A minor thing I saw when testing:

If you send yourself something (which obviously apart from the darknet nobody does) the transaction explorer showed it like this:

image

However this is wrong, since you did not magically get +3 ark and only lost the fee cost.

Correct would either be to show only the fee as a negative value or - and that's what I've done - show it like this:

image

I wonder if the background-color shouldn't be "negative" (default-red-200)?

@luciorubeens

This comment has been minimized.

Show comment
Hide comment
@luciorubeens

luciorubeens Dec 27, 2017

Member

makes sense, but seems confusing. what do you think? @boldninja

Member

luciorubeens commented Dec 27, 2017

makes sense, but seems confusing. what do you think? @boldninja

@boldninja

This comment has been minimized.

Show comment
Hide comment
@boldninja

boldninja Dec 27, 2017

Member

Makes sense, but its confusing in any case :)

Member

boldninja commented Dec 27, 2017

Makes sense, but its confusing in any case :)

@Nasicus

This comment has been minimized.

Show comment
Hide comment
@Nasicus

Nasicus Dec 27, 2017

Well if you guys prefer we could just show it as:

-0.1 (which would be just the fee)

But in my opinion this is even more confusing and less complete/correct than the other approach.

Nasicus commented Dec 27, 2017

Well if you guys prefer we could just show it as:

-0.1 (which would be just the fee)

But in my opinion this is even more confusing and less complete/correct than the other approach.

@alexbarnsley

This comment has been minimized.

Show comment
Hide comment
@alexbarnsley

alexbarnsley Dec 28, 2017

Member

I actually think it should show -3.1 / +3 showing what's gone out first, then what's come in. I'm not sure there's a right or wrong answer though. I think it at least needs a tooltip detailing what happened. E.g. You sent 3 ARK to the same address, excluding the transaction fee or something like that. That's just my thoughts though

Member

alexbarnsley commented Dec 28, 2017

I actually think it should show -3.1 / +3 showing what's gone out first, then what's come in. I'm not sure there's a right or wrong answer though. I think it at least needs a tooltip detailing what happened. E.g. You sent 3 ARK to the same address, excluding the transaction fee or something like that. That's just my thoughts though

@Nasicus

This comment has been minimized.

Show comment
Hide comment
@Nasicus

Nasicus Dec 29, 2017

Well the question is also if it's worth "introducing" a new translation / toast for this, even though this probably almost never occurs :D
Concering "-" or "+" first, I personally don't care :)

Nasicus commented Dec 29, 2017

Well the question is also if it's worth "introducing" a new translation / toast for this, even though this probably almost never occurs :D
Concering "-" or "+" first, I personally don't care :)

@j-a-m-l j-a-m-l self-requested a review Dec 29, 2017

@j-a-m-l

Just change it to -3.1 / +3.

About the tooltip, I think that it is unnecessary.

@Nasicus

This comment has been minimized.

Show comment
Hide comment
@Nasicus

Nasicus commented Dec 29, 2017

done

@j-a-m-l j-a-m-l merged commit a69c55f into ArkEcosystem:master Dec 29, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@j-a-m-l

This comment has been minimized.

Show comment
Hide comment
@j-a-m-l

j-a-m-l Dec 29, 2017

Member

+5

Member

j-a-m-l commented Dec 29, 2017

+5

@Nasicus Nasicus deleted the Nasicus:bug/wrong-amount-on-self-send branch Dec 30, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment