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.

Sign up for GitHub

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

Jump to bottom

Add amount validation to CreateOfferAmount screen #1150

Merged
merged 7 commits into from
Sep 5, 2023
Merged

Add amount validation to CreateOfferAmount screen #1150

merged 7 commits into from
Sep 5, 2023

Conversation

namloan
Copy link
Contributor

@namloan namloan commented Sep 2, 2023

@HenrikJannsen
Copy link
Contributor

HenrikJannsen commented Sep 3, 2023 •
edited
Loading

I tested it and got an exception at AmountParser.parse at AmountInput line 159. I could not reproduce the error but it happened while quickly entering at both fields invalid values.
This is not a bug from your changes but was not handled before but would be good to catch the exception. But can be done in a new PR as well...

Also saw that the classed in bisq.desktop.components.controls.validator lack the Bisq license header. You can add the header to IDE so its included at each new class by default. Would be good to add that to the .idea files and commit it as well.

HenrikJannsen
HenrikJannsen approved these changes Sep 3, 2023
View reviewed changes
Copy link
Contributor

@HenrikJannsen HenrikJannsen left a comment

Choose a reason for hiding this comment

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

ACK

@namloan
Copy link
Contributor Author

namloan commented Sep 3, 2023

Sure, I'll tackle those issues in next PRs. Thanks for reviewing!

alvasw
alvasw approved these changes Sep 5, 2023
View reviewed changes
Copy link
Contributor

@alvasw alvasw left a comment

Choose a reason for hiding this comment

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

utACK

@alvasw alvasw merged commit 8c37a11 into bisq-network:main Sep 5, 2023
3 checks passed
@namloan namloan deleted the input_validators branch September 5, 2023 17:51
@namloan namloan mentioned this pull request Sep 18, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alvasw alvasw alvasw approved these changes

@HenrikJannsen HenrikJannsen HenrikJannsen approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@namloan @HenrikJannsen @alvasw