-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Trade Info Popup #3329
Trade Info Popup #3329
Conversation
Issue #3317
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ack, but please use the @Setter
instead of create the setter method by hand
core/src/main/java/bisq/core/payment/payload/PaymentAccountPayload.java
Outdated
Show resolved
Hide resolved
core/src/main/java/bisq/core/payment/payload/PaymentAccountPayload.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
NACK - please see my comment
core/src/main/java/bisq/core/payment/payload/PaymentAccountPayload.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
NACK - I stopped reviewing (please see my comments). Actually everything is wrong in this PR. Please have a look what you are doing in your code before pushing changes or submitting PRs.
@@ -60,8 +61,9 @@ protected void run() { | |||
failed("Payment method is banned.\n" + | |||
"Payment method=" + trade.getOffer().getPaymentMethod().getId()); | |||
} else if (filterManager.isPeersPaymentAccountDataAreBanned(paymentAccountPayload, appliedPaymentAccountFilter)) { | |||
String paymentDetails = Res.getWithCol("payment.altcoin.sender.address", trade.getOffer().getCurrencyCode()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This absolutely wrong it would change the paymentDetails displayed e.g. for MoneyBeam you would replace Res.get(paymentMethodId) + " - " + Res.getWithCol("payment.account") + " " + accountId;
with Res.getWithCol("payment.altcoin.sender.address", trade.getOffer().getCurrencyCode());
@@ -175,9 +179,9 @@ private void addContent() { | |||
nrOfDisputesAsBuyer + " / " + nrOfDisputesAsSeller); | |||
|
|||
addConfirmationLabelTextFieldWithCopyIcon(gridPane, ++rowIndex, Res.get("shared.paymentDetails", Res.get("shared.buyer")), | |||
contract.getBuyerPaymentAccountPayload().getPaymentDetails()).second.setMouseTransparent(false); | |||
buyerPaymentDetails).second.setMouseTransparent(false); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are removing the real payment details with e.g. Sender's BTC address
addConfirmationLabelTextFieldWithCopyIcon(gridPane, ++rowIndex, Res.get("shared.paymentDetails", Res.get("shared.seller")), | ||
sellerPaymentAccountPayload.getPaymentDetails()).second.setMouseTransparent(false); | ||
sellerPaymentDetails).second.setMouseTransparent(false); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same here
Issue #3317