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] Send View - BTC and Fiat currency inputs #7690
Conversation
ichthus1604
commented
Apr 4, 2022
- Enables typing on both the BTC and the fiat inputs in the send screen
- User preference is persisted in UI config
This looks very promising. |
Yes, this is great, it addresses #5165 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tested 12e26bd
sometimes, clicking the switch button does not switch it, not sure exactly why, but I can sometimes reproduce.
also, when it switches, it highlights/selects the left amount, probably better to just put the courser there.
Dan mentioned to make it look like a single textbox, but I think I would prefer how it currently is with two distinct text boxes.
Also add the wave icon "is roughly" always before the fiat option.
Latest commit includes visual improvements, and the control's readonly state template, which wasn't properly designed before @MaxHillebrand can't reproduce the button issue. clicking swaps every time for me.
This is the default behavior of the currency entry, also I think it's good ux, because if you're swapping that means you probably want to enter the exact right side amount specifically
Should be there already? |
@MaxHillebrand latest commit adds ~ symbol to the fiat watermark, maybe that's what you were seeing? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
# Conflicts: # WalletWasabi.Fluent/Controls/CurrencyEntryBox.axaml.cs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tACK
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tACK
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tACK eed9091
Looks pretty and it's a wonderful solution that combines both possible UX approaches.
Very well done!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tACK.
I took special attention to the code. I've not seen any noticeable problem in it. It's quite clear and readable.
Good work @ichthus1604!