[VDG] UI Decoupling #24 #10730
Merged
[VDG] UI Decoupling #24 #10730
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ichthus1604
changed the title
| return new WalletSettingsModel(keyManager, true); | ||
| } | ||
|
|
||
| public IWalletModel SaveWallet(IWalletSettingsModel walletSettings) |
There was a problem hiding this comment.
The result is not used anywhere ?
| using NBitcoin; | ||
| using System.Threading.Tasks; | ||
| using WalletWasabi.Fluent.Models.Wallets; | ||
|
|
||
| namespace WalletWasabi.Fluent.Models.UI; | ||
|
|
||
| #nullable disable |
There was a problem hiding this comment.
Why not fix nullable annotations in IWalletListModel ?
There was a problem hiding this comment.
Also public IWalletModel? DefaultWallet => null; has nullable annotation which results in warning.
| .Subscribe(); | ||
|
|
||
| DefaultWallet = | ||
| wallets.FirstOrDefault(item => item.Name == Services.UiConfig.LastSelectedWallet) | ||
| ?? wallets.FirstOrDefault(); | ||
| _wallets.FirstOrDefault(item => item.Name == Services.UiConfig.LastSelectedWallet) |
There was a problem hiding this comment.
Not part of this PR but I wonder isn't second FirstOrDefault() really needed ?
|
@ichthus1604 LGTM just added some comments |
|
@ichthus1604 Fixed DS issues and CF issues (remaining are not part of this PR). |
wieslawsoltes
previously approved these changes
Jun 15, 2023
|
@molnard Seems CF did not update properly, all of the issues are already fixed. This PR can be merged but I can't because of CF. |
wieslawsoltes
approved these changes
Jun 16, 2023
wieslawsoltes
approved these changes
Jun 16, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
WalletSettingsModelintroduced in [VDG] UI Decoupling #22 #10716 because it doesn't really make sense.IWalletListModel. This abstraction has now become effectively the Wallet Repository, and thus needs to be renamed. This rename will happen in the next PR.CreateNewWalletAsync()andRecoverWalletAsync()both returnIWalletSettingsModel, to properly represent the fact that the wallet hasn't been created at that point, which happens inIWalletListModel.SaveWallet()which does properly returnIWalletModel.WalletSettingsModelto work with aKeyManagerinstead of aWalletdue to the above.KeyManager.SetAnonScore(int anonScore, bool toFile)method since it's no longer usedbool toFileparameter fromKeyManager.SetFeeRateMedianTimeFrame()method since it's no longer used.IWalletSettingsModel.IsNewWalletproperty which is internally set when creating / recovering wallets, removing the need to pass this value as a loose boolean parameter manually between ViewModels.WalletManagerin theOnNavigatedTo()event of theAddedWalletPageViewModel. This doesn't really make sense because wallet persistence should not depend on whether a particular dialog is shown or not in the UI. This logic has been moved toCoinjoinProfilesViewModel.OnNext()so that the wallet is persisted and added toWalletManagerimmediately after the user hits Next on that screen.