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

Take minimum_makers config var value into account on Send page #116

Merged
5 commits merged into from
Feb 22, 2022

Conversation

theborakompanioni
Copy link
Collaborator

@theborakompanioni theborakompanioni commented Feb 21, 2022

Closes #101

This PR fetches the minimum_makers config variable and uses it as minimum amount of collaborators on the Send page.
This means a taker operation cannot be started with a lower value than what is specified in the config file.

One little caveat is that the initial value for the amount collaborators is randomized between [5, 7] (min +1, min +3), which can lead to the initial value not being shown as active button in the CollaboratorsSelector component.

Is this acceptable for now? What do you think?

Screenshot for minimum_makers := 2:


Also, the approach of parsing an error message from text/html is removed as this has been fixed in joinmarket-clientserver#1170

Edit: this also does not take into account if the value is greater than 99 - which is the default maximum amount. Guess that is an unreasonable value and can therefore be ignored.

@theborakompanioni theborakompanioni self-assigned this Feb 21, 2022
@theborakompanioni theborakompanioni added the bug Something isn't working label Feb 21, 2022
@theborakompanioni
Copy link
Collaborator Author

This is what the CollaboratorsSelector component looks like with the default value 4 for minimum_makers:
image

@theborakompanioni theborakompanioni marked this pull request as draft February 21, 2022 13:03
@theborakompanioni theborakompanioni marked this pull request as ready for review February 21, 2022 13:36
Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

Nice change! Yeah I'd say we can live with the 99 being hardcoded for now. 👍

src/libs/JmWalletApi.js Outdated Show resolved Hide resolved
src/libs/JmWalletApi.js Show resolved Hide resolved
src/components/Send.jsx Outdated Show resolved Hide resolved
@dergigi dergigi added this to the v0.0.4 - Bugfixes milestone Feb 22, 2022
Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

Cool! ✅

@ghost ghost merged commit d2c36bf into master Feb 22, 2022
@ghost ghost deleted the minimum-makers-on-send-page branch February 22, 2022 12:20
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Take minimum_makers config var value into account on Send page
2 participants