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
[UI] Make CJ settings more discoverable via MusicBox #12694
[UI] Make CJ settings more discoverable via MusicBox #12694
Conversation
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.
- agree with [UI] Make CJ settings more discoverable via MusicBox #12694 (comment)
- ci is failing
- shouldn't it navigate to the coinjoin tab at Wallet Settings dialog?
Co-authored-by: Max Hillebrand <30683012+MaxHillebrand@users.noreply.github.com>
Done :) |
|
||
CoinJoinStateViewModel = new CoinJoinStateViewModel(uiContext, WalletModel); | ||
CoinJoinStateViewModel = new CoinJoinStateViewModel(uiContext, WalletModel, coinJoinSettingsCommand); |
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.
I'm not particularly proud of this line, but I felt I needed to reuse the original command to avoid having a dependency on WalletSettingsViewModel
just to set the correct SelectedTab
number.
@ichthus1604 can you think of a better way?
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.
@SuperJMN Looks like you can remove the command completely from WalletViewModel
, and keep it in CoinJoinStateViewModel
.
It's not being used in XAML other than the MusicBox, and the only place where WalletViewModel
references it is line 240, which could be replaced by CoinJoinStateViewModel.CoinJoinSettingsCommand
.
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.
OK, I had to add a dependency on WalletSettingsViewModel to do that. Please check :)
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.
tACK
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.
it's not working correcly for watch-only wallets: Wallet Settings are greyed out and not clickable/accessable
Thanks for the catch! It should be fixed now. |
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.
tACK.
Makes CJ settings easier to discover by a "..." menu in the Music Box.
Fixes #9432