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

[GUI] Add differentiation for Shielded Memos #2865

Merged
merged 1 commit into from
Jul 19, 2023

Conversation

Liquid369
Copy link
Member

This introduces a new TransactionRecord RecvWithShieldedAddressMemo and icon/image for the transaction.

With the conversation in Issue #2786 , there is something we can do and that changes the icon and checking for memo to not be empty.

@js

Color for blue is not in this PR, but to make it easier to see is in the picture.

I have also spoken with a few, where they feel more may still be warranted to make this stand out so open for options/feedback for us to update this more.

Closes #2786

Copy link
Collaborator

@Fuzzbawls Fuzzbawls left a comment

Choose a reason for hiding this comment

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

First look at code only; looks overall good. One issue with css assignment though, and I do think that the changes here are narrow-focused enough to where the commits can be squashed down into a single commit.

I'm also not completely sold on the particular icon being used to identify memo-containing transactions (my first instinct when I see the "down arrow" is that there is a download). That, however, can be left for further discussion and refinement in a later PR, if need be.

src/qt/pivx/txrow.cpp Show resolved Hide resolved
Add new icon for memo and add to qrc/makefile

Remove CSS from RecvWithShieldedAddressMemo

Add css

Adjust CSS
Copy link
Collaborator

@Fuzzbawls Fuzzbawls left a comment

Choose a reason for hiding this comment

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

ACK 2543f6b

Copy link

@panleone panleone left a comment

Choose a reason for hiding this comment

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

tACK 2543f6b
Work as expected, I don't like the icon (it does not give the idea that there is a memo in the tx) but eventually it can be changed in another PR

@Fuzzbawls Fuzzbawls merged commit 559e4f1 into PIVX-Project:master Jul 19, 2023
@Fuzzbawls Fuzzbawls modified the milestones: 6.0.0, 5.6.0 Feb 6, 2024
@Fuzzbawls Fuzzbawls changed the title [Qt] Add differentiation for Shielded Memos [GUI] Add differentiation for Shielded Memos Feb 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

[Feature Request] Home screen should color code Shield Transactions containing an encrypted message.
3 participants