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

fix transaction IDs in legacy UI trade info views #482

Merged
merged 1 commit into from
Nov 13, 2022
Merged

fix transaction IDs in legacy UI trade info views #482

merged 1 commit into from
Nov 13, 2022

Conversation

napoly
Copy link
Contributor

@napoly napoly commented Nov 11, 2022 •
edited
Loading

Should fix #478

@napoly napoly requested a review from woodser as a code owner November 11, 2022 18:07
woodser
woodser reviewed Nov 12, 2022
View reviewed changes
core/src/main/resources/i18n/displayStrings.properties Outdated
@@ -116,6 +116,8 @@ shared.OR=OR
shared.notEnoughFunds=You don''t have enough funds in your Haveno wallet for this transaction—{0} is needed but only {1} is available.\n\nPlease add funds from an external wallet, or fund your Haveno wallet at Funds > Receive Funds.
shared.waitingForFunds=Waiting for funds...
shared.depositTransactionId=Deposit transaction ID
Copy link
Contributor

@woodser woodser Nov 12, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

I was expecting shared.yourDepositTransactionId, shared.peerDepositTransactionId, shared.makerDepositTransactionId and shared.takerDepositTransactionId with shared.depositTransactionId unused and removed based on our conversation

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed depositTransactionId completely and added taker/maker.

woodser
woodser reviewed Nov 12, 2022
View reviewed changes
desktop/src/main/java/bisq/desktop/main/overlays/windows/ContractWindow.java
@@ -265,9 +265,6 @@ private void addContent() {
addLabelTxIdTextField(gridPane, ++rowIndex, Res.get("shared.makerFeeTxId"), offer.getOfferFeePaymentTxId());
addLabelTxIdTextField(gridPane, ++rowIndex, Res.get("shared.takerFeeTxId"), "TAKER FEE TX ID NOT PART OF CONTRACT"); // TODO (woodser): should taker fee tx id be part of contract?

if (dispute.getDepositTxSerialized() != null)
Copy link
Contributor

@woodser woodser Nov 12, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

This removes the deposit tx hash from the contract window. Per our conversation, this view was to show the maker and taker deposit tx hashes using e.g. contract.getMakerDepositTxHash().

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah I misunderstood. Thought there's going to be a follow up PR. Please check again.

woodser
woodser requested changes Nov 12, 2022
View reviewed changes
Copy link
Contributor

@woodser woodser left a comment

Choose a reason for hiding this comment

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

See comments.

@napoly napoly force-pushed the fix-transaction-IDs-legacy-UI-trade-info branch from 6fc2161 to a4e13f8 Compare November 12, 2022 14:09
woodser
woodser reviewed Nov 12, 2022
View reviewed changes
desktop/src/main/java/bisq/desktop/main/overlays/windows/TradeDetailsWindow.java Outdated
if (trade.getMakerDepositTx() != null)
addLabelTxIdTextField(gridPane, ++rowIndex, Res.get("shared.depositTransactionId"), // TODO (woodser): separate UI labels for deposit tx ids
addLabelTxIdTextField(gridPane, ++rowIndex, Res.get("shared.yourDepositTransactionId"),
Copy link
Contributor

Choose a reason for hiding this comment

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

Should be shared.makerDepositTransactionId and shared.takerDepositTransactionId here.

@napoly napoly force-pushed the fix-transaction-IDs-legacy-UI-trade-info branch from a4e13f8 to bab8ca6 Compare November 12, 2022 14:36
@napoly napoly force-pushed the fix-transaction-IDs-legacy-UI-trade-info branch from bab8ca6 to 0b98cad Compare November 12, 2022 14:39
@woodser woodser merged commit 1f61e82 into haveno-dex:master Nov 13, 2022
@woodser
Copy link
Contributor

woodser commented Nov 13, 2022

Thanks :)

@napoly napoly deleted the fix-transaction-IDs-legacy-UI-trade-info branch November 13, 2022 13:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@woodser woodser woodser 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.

Fix transaction IDs in legacy UI trade info views
2 participants
@napoly @woodser