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

Edit offer button #5955

Merged
merged 2 commits into from Jan 12, 2022
Merged

Edit offer button #5955

merged 2 commits into from Jan 12, 2022

Conversation

ghost
Copy link

@ghost ghost commented Jan 7, 2022

Fixes #4986

Screenshots:

actions

@ghost ghost marked this pull request as ready for review January 8, 2022 11:38
ripcurlx
ripcurlx previously approved these changes Jan 11, 2022
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 - please see my comments what I changed.

@@ -138,6 +143,7 @@ private void onEditOpenOfferRemoved() {
editOfferView = null;
}

// navigation.navigateToPreviousVisitedView();
Copy link
Contributor

Choose a reason for hiding this comment

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

Please remove unused code

Comment on lines 218 to 221
if (data instanceof Offer) {
Offer offer = (Offer) data;
openOffer = openOfferManager.getOpenOfferById(offer.getId()).orElse(null);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd rather do this in the OfferBookViewModel

@bisq-github-admin-3
Copy link
Contributor

I'm still merging this PR although as Codacy has permission issues (again)

@bisq-github-admin-3 bisq-github-admin-3 merged commit 553503d into bisq-network:master Jan 12, 2022
@ghost ghost mentioned this pull request Jan 15, 2022
@pazza83
Copy link

pazza83 commented Jan 22, 2022

@xyzmaker123 thanks for working on this. Can you post a screenshot of what this looks like in the context of the whole offer book screen?

@ghost
Copy link
Author

ghost commented Jan 22, 2022

@pazza83 Sure, two examples:
edit-button-2
edit-button-1

@pazza83
Copy link

pazza83 commented Jan 22, 2022

@xyzmaker123 thanks looks good.

Not sure where the 'add duplicate icon' will be included and #5774 (comment)

Would it make sense to have 3 options:

  • Remove
  • Duplicate
  • Edit

cc: @ripcurlx

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.

Add edit offer button next to the remove button in the offer book
3 participants