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

[VDG] UI Decoupling #56 #11817

Merged
merged 8 commits into from
Oct 31, 2023
Merged

[VDG] UI Decoupling #56 #11817

merged 8 commits into from
Oct 31, 2023

Conversation

ichthus1604
Copy link
Collaborator

  • Decouples CoinJoinDetailsViewModel
  • Decouples CoinJoinsDetailsViewModel

@ichthus1604 ichthus1604 mentioned this pull request Oct 27, 2023
Open
@MarnixCroes MarnixCroes mentioned this pull request Oct 27, 2023
Closed
soosr
soosr requested changes Oct 30, 2023
View reviewed changes
Copy link
Collaborator

@soosr soosr left a comment

Choose a reason for hiding this comment

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

Update doesn't work.

  1. Open a cj group/cj details.
  2. Wait until a new filter is received/txn confirms
  3. Dialog didn't update the values.

@SuperJMN
Copy link
Collaborator

SuperJMN commented Oct 30, 2023 •
edited

Update doesn't work.

  1. Open a cj group/cj details.
  2. Wait until a new filter is received/txn confirms
  3. Dialog didn't update the values.

This is caused because TransactionModel isn't getting its properties refreshed. I think I have a solution for this.

soosr
soosr reviewed Oct 31, 2023
View reviewed changes
WalletWasabi.Fluent/ViewModels/Wallets/Home/History/Details/CoinJoinDetailsViewModel.cs Outdated
IsConfirmed = Confirmations > 0;

TransactionId = _coinJoin.Transaction.Id;
if (_wallet.Transactions.TryGetById(TransactionId, out var transaction))
Copy link
Collaborator

Choose a reason for hiding this comment

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

This method is wrong, only finds parents, and not children.

image

Copy link
Collaborator

Choose a reason for hiding this comment

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

8a4e9ca fixes it

soosr
soosr approved these changes Oct 31, 2023
View reviewed changes
Copy link
Collaborator

@soosr soosr left a comment

Choose a reason for hiding this comment

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

tACK

@soosr soosr merged commit a4b49c1 into WalletWasabi:master Oct 31, 2023
2 of 6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@soosr soosr soosr approved these changes

Assignees
No one assigned
Labels
size/M
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@ichthus1604 @SuperJMN @soosr