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] Limit amount to 8 decimal digits #992

Merged
merged 1 commit into from
Mar 20, 2024

Conversation

pythcoiner
Copy link
Collaborator

@pythcoiner pythcoiner commented Mar 3, 2024

This PR replace #977 and fixes #798
I've followed jp1ac4 advice and create a Form::new_amount_btc() method that act like Form::new_trimmed() w/ few more (filtering) features:
- allow only input of "0123456789,." characters
- "," is replaced by "."
- only one "." separator
- maximum 8 digit after separator

these features works for keyboard input or paste

i've tryied to add a "0" in case the String start with a "." but this make the cursor have a bad location and i do not find a way to control the cursor location w/ iced, so i revert this feature

@pythcoiner
Copy link
Collaborator Author

pythcoiner commented Mar 3, 2024

not related to this PR: should we add gui/Cargo.lock to .gitignore?

@jp1ac4
Copy link
Collaborator

jp1ac4 commented Mar 5, 2024

Instead of using regex, another option might be to use bitcoin::Amount::from_str_in to determine if the new string can be parsed as a BTC amount and then use this to decide whether to pass this updated value to the on_change function or keep the previous value. I see we already have bitcoin as a dependency in the ui project so wouldn't need to add anything.

@darosior
Copy link
Member

darosior commented Mar 9, 2024

I agree with @jp1ac4, let's not reinvent the wheel down here if they already have parsing upstream.

@pythcoiner
Copy link
Collaborator Author

i've done adviced changes, not yet tested because my crash issue

Copy link
Collaborator

@jp1ac4 jp1ac4 left a comment

Choose a reason for hiding this comment

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

Changes are working well. Just had a couple of comments.

@pythcoiner
Copy link
Collaborator Author

adressed comments & squash

@edouardparis
Copy link
Member

Please remove the unused regex dep. Thanks

@pythcoiner pythcoiner force-pushed the new_amount_btc branch 4 times, most recently from b2ad9be to ed87fb6 Compare March 14, 2024 11:06
@pythcoiner
Copy link
Collaborator Author

Please remove the unused regex dep. Thanks

oops, done & rebased on master

Copy link
Collaborator

@jp1ac4 jp1ac4 left a comment

Choose a reason for hiding this comment

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

ACK 9415dc9.

@darosior darosior requested a review from edouardparis March 19, 2024 10:30
Copy link
Member

@edouardparis edouardparis left a comment

Choose a reason for hiding this comment

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

ACK 9415dc9

@edouardparis edouardparis merged commit d7b8f53 into wizardsardine:master Mar 20, 2024
21 checks passed
@pythcoiner pythcoiner deleted the new_amount_btc branch March 26, 2024 02:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Limit the send amout value field to 8 decimals
4 participants