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

[VDG][Trivial] Simplify text of Preview Transaction dialog #10932

Merged
merged 3 commits into from Jun 27, 2023

Conversation

yahiheb
Copy link
Collaborator

@yahiheb yahiheb commented Jun 24, 2023

The Transaction Preview dialog (when sending) is full of text and not user friendly imo.

This is a small PR with minimal effort to simplify the text and make the dialog less cluttered.

Master:

1

PR:

3

@yahiheb yahiheb added the UI label Jun 24, 2023
@yahiheb yahiheb changed the title [VDG] Simplify text for Transaction Preview [VDG][Trivial] Simplify text of Preview Transaction dialog Jun 25, 2023
nopara73
nopara73 previously approved these changes Jun 25, 2023
@@ -45,13 +45,12 @@

<!-- Address -->
<c:PreviewItem Icon="{StaticResource transceive_regular}"
Label="to the Bitcoin address"
Label="address"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Just a comment but not a suggestion of changes for this PR, this field and the previous one (so receiver label + address) could be merged into one, it would declutter the screen and help for global understanding.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

That makes sense to me. It can be done in another PR.

BTCparadigm
BTCparadigm previously approved these changes Jun 25, 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.

I am wondering why aren't the titles capitalized?

Co-authored-by: Turbolay <turbolay@zksnacks.com>
@yahiheb
Copy link
Collaborator Author

yahiheb commented Jun 25, 2023

I am wondering why aren't the titles capitalized?

It looked a bit better like this, but at the end it was just a choice.

@yahiheb yahiheb requested a review from soosr June 25, 2023 16:56
Copy link
Collaborator

@MarnixCroes MarnixCroes left a comment

Choose a reason for hiding this comment

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

To prevent confusion, I would make it clear that the fee gets added to the amount, not deducted/part of it

so additional fee

@yahiheb
Copy link
Collaborator Author

yahiheb commented Jun 26, 2023

To prevent confusion, I would make it clear that the fee gets added to the amount, not deducted/part of it

so additional fee

Where does this confusion come from?
If the user typed an amount to be sent, there is no reason to think it will be changed arbitrarily.

"A fee is the cost of something, or the amount of money charged."
In our context it means the cost of the transaction, and this screen shows the details of the transaction, thus saying fee should be enough.

@nopara73 nopara73 merged commit 5a27141 into zkSNACKs:master Jun 27, 2023
9 checks passed
@yahiheb yahiheb deleted the transaction-preview-less-text branch June 27, 2023 14:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants