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

Hide text that is not applicable in take offer view #1775

Merged
merged 4 commits into from
Oct 17, 2018

Conversation

devinbileck
Copy link
Member

When taking an offer that does not specify an amount range, "Check if offer is available" text was being displayed when it shouldn't be.

It was explicitly hiding the offerAvailabilityBusyAnimation, but not the offerAvailabilityLabel.

Fixes #1742

When taking an offer that does not specify an amount range, hide
the "Check if offer is available" text.

Fixes bisq-network#1742
Stop the offerAvailabilityBusyAnimation sooner, prior to showing
securityDepositInfo popup, when it is hiding related fields.
Copy link
Member

@ManfredKarrer ManfredKarrer left a comment

Choose a reason for hiding this comment

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

NACK.
I added more background info in #1742. We must not hide is but change the layout so that it is better visible to the user and update the button states so that the user does not fund his wallet before the offer availibility check is successfully completed.

@devinbileck
Copy link
Member Author

@ManfredKarrer A fixed amount offer now waits until the offer has been confirmed to be available, as shown below:
image

Once confirmed, it will show the fund trade fields:
image

@ManfredKarrer
Copy link
Member

ManfredKarrer commented Oct 16, 2018

A cool! Looks good to me! @ripcurlx - What do you think? I leave it to you to review/merge.

Copy link
Member

@ManfredKarrer ManfredKarrer left a comment

Choose a reason for hiding this comment

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

utACK - I would prefer if @ripcurlx does the review/merge

Copy link
Contributor

@ripcurlx ripcurlx left a comment

Choose a reason for hiding this comment

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

ACK - tested it on Regtest with a range and a single amount offer. Everything looking good!

@ripcurlx
Copy link
Contributor

I'll merge it although Travis fails as it is related to the current Java 10 JDK not loaded issue.

@ripcurlx ripcurlx merged commit 9ce8b21 into bisq-network:master Oct 17, 2018
@devinbileck devinbileck deleted the fix-take-offer-view branch October 17, 2018 15:23
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.

3 participants