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

TX History Refinement fix: Icon + Tooltip of normal transactions #11382

Merged
merged 23 commits into from
Sep 5, 2023

Conversation

TChukwuleta
Copy link
Contributor

@TChukwuleta TChukwuleta commented Aug 27, 2023

Fixes #11172

Changed the Tool tip to cater to individual transactions (incoming, outgoing, self-transfer).
Changed the icon for incoming and outgoing transactions.

Left the default icon to cater for self-transfer. From my UX point, it seemed to speak to that regard.

Open to review. Thanks

11172

@yahiheb
Copy link
Collaborator

yahiheb commented Aug 27, 2023

I tested this a bit, LGTM

@pull-request-size pull-request-size bot added size/S and removed size/M labels Aug 30, 2023
Copy link
Collaborator

@yahiheb yahiheb left a comment

Choose a reason for hiding this comment

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

CI is failing because you have accidently removed some bitcoind binaries.

@TChukwuleta
Copy link
Contributor Author

CI is failing because you have accidently removed some bitcoind binaries.

Yea.. Thank you.

Added it back

Copy link
Collaborator

@yahiheb yahiheb left a comment

Choose a reason for hiding this comment

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

Now that there are different icons should the method TransactionsSearchSource.GetIcon be updated?

@pull-request-size pull-request-size bot added size/M and removed size/S labels Sep 2, 2023
@pull-request-size pull-request-size bot added size/S and removed size/M labels Sep 3, 2023
Comment on lines 96 to 97
<StreamGeometry x:Key="outgoing_arrow">M54.9667 51.3071C54.8519 51.3052 54.7384 51.2806 54.6329 51.2347C54.5275 51.1889 54.432 51.1227 54.3522 51.04C54.2723 50.9572 54.2096 50.8595 54.1676 50.7525C54.1254 50.6455 54.1049 50.5312 54.107 50.4163L54.4649 30.2585L28.2828 55.5271C28.1158 55.6883 27.8916 55.7765 27.6596 55.7724C27.4276 55.7683 27.2067 55.6721 27.0455 55.5052C26.8844 55.3382 26.7961 55.1141 26.8002 54.882C26.8044 54.6499 26.9005 54.4291 27.0675 54.2679L53.2495 28.9993L33.0918 28.6414C32.9768 28.6394 32.8635 28.6147 32.758 28.5688C32.6526 28.5229 32.5572 28.4568 32.4774 28.3741C32.3975 28.2914 32.3348 28.1936 32.2926 28.0867C32.2505 27.9797 32.23 27.8655 32.232 27.7505C32.234 27.6356 32.2587 27.5222 32.3045 27.4168C32.3504 27.3113 32.4166 27.2159 32.4993 27.1361C32.5821 27.0562 32.6797 26.9935 32.7867 26.9514C32.8937 26.9093 33.0079 26.8887 33.1229 26.8907L55.3932 27.2861C55.5082 27.288 55.6217 27.3127 55.7272 27.3585C55.8326 27.4044 55.9279 27.4706 56.0077 27.5534C56.0877 27.6361 56.1504 27.7337 56.1925 27.8408C56.2346 27.9477 56.2552 28.062 56.253 28.177L55.8576 50.4474C55.8557 50.5623 55.8311 50.6758 55.7852 50.7813C55.7393 50.8867 55.6732 50.982 55.5904 51.062C55.5077 51.1418 55.41 51.2046 55.3031 51.2466C55.196 51.2887 55.0817 51.3093 54.9667 51.3071Z</StreamGeometry>
<StreamGeometry x:Key="incoming_arrow">M26.8613 30.9335C26.9762 30.9334 27.0901 30.956 27.1964 31C27.3026 31.0439 27.3992 31.1084 27.4805 31.1897C27.5618 31.271 27.6263 31.3676 27.6702 31.4738C27.7142 31.5801 27.7368 31.694 27.7367 31.8089L27.7367 51.9699L53.4661 26.2405C53.6302 26.0764 53.8528 25.9842 54.0849 25.9842C54.3169 25.9842 54.5395 26.0764 54.7036 26.2405C54.8677 26.4046 54.9599 26.6271 54.9599 26.8592C54.9599 27.0913 54.8677 27.3138 54.7036 27.4779L28.9742 53.2073L49.1351 53.2073C49.2501 53.2073 49.3639 53.23 49.4702 53.274C49.5764 53.318 49.6729 53.3824 49.7542 53.4637C49.8355 53.545 49.9 53.6416 49.944 53.7478C49.988 53.854 50.0106 53.9678 50.0106 54.0828C50.0106 54.1978 49.988 54.3116 49.944 54.4178C49.9 54.5241 49.8355 54.6206 49.7542 54.7019C49.6729 54.7832 49.5764 54.8477 49.4702 54.8917C49.3639 54.9357 49.2501 54.9583 49.1351 54.9583L26.8613 54.9583C26.7463 54.9584 26.6324 54.9358 26.5261 54.8918C26.4199 54.8478 26.3234 54.7833 26.2421 54.702C26.1607 54.6207 26.0963 54.5242 26.0523 54.4179C26.0083 54.3117 25.9857 54.1978 25.9858 54.0828L25.9858 31.8089C25.9857 31.694 26.0083 31.5801 26.0523 31.4738C26.0963 31.3676 26.1607 31.2711 26.2421 31.1897C26.3234 31.1084 26.4199 31.0439 26.5261 31C26.6324 30.956 26.7463 30.9334 26.8613 30.9335Z</StreamGeometry>
Copy link
Collaborator

Choose a reason for hiding this comment

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

image

These icons don't fit the fluent style. In general, we use icons from the fluent icon pack:

In the future try to only use icons from those. If we don't find a good icon, then we usually create one based on the fluent icon design rules.

I picked icons from the pack mentioned above, so they are like this now:

image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh.. Noted.

Thank you

@pull-request-size pull-request-size bot added size/M and removed size/S labels Sep 5, 2023
Copy link
Collaborator

@soosr soosr left a comment

Choose a reason for hiding this comment

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

tACK

@soosr soosr requested a review from yahiheb September 5, 2023 08:28
@MarnixCroes
Copy link
Collaborator

Would be good to squash commits.

Copy link
Collaborator

@yahiheb yahiheb left a comment

Choose a reason for hiding this comment

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

tACK

@soosr soosr merged commit c6748d1 into WalletWasabi:master Sep 5, 2023
6 of 7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Minor TX History Refinement: Icon + Tooltip of normal transactions
4 participants