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.

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

[VDG] Privacy warnings Improved UX #11463

Merged
merged 20 commits into from
Mar 1, 2024
Merged
Show file tree
Hide file tree
Changes from 14 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
54 changes: 54 additions & 0 deletions WalletWasabi.Fluent/Behaviors/PrivacyWarningFadeOutBehavior.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
using Avalonia;
using Avalonia.Animation;
using Avalonia.Controls;
using Avalonia.Controls.Primitives;
using ReactiveUI;
using System.Collections.Generic;
using System.Collections.ObjectModel;
using System.Linq;
using System.Reactive.Disposables;
using System.Reactive.Linq;
using WalletWasabi.Fluent.Extensions;
using WalletWasabi.Fluent.Models.Transactions;

namespace WalletWasabi.Fluent.Behaviors;

public class PrivacyWarningFadeOutBehavior : AttachedToVisualTreeBehavior<Control>
{
private const string FadeOutClassName = "fadeout";

public static readonly StyledProperty<IEnumerable<PrivacyWarning>> PreviewWarningsProperty =
AvaloniaProperty.Register<PrivacyWarningFadeOutBehavior, IEnumerable<PrivacyWarning>>(nameof(PreviewWarnings));

public IEnumerable<PrivacyWarning> PreviewWarnings
{
get => GetValue(PreviewWarningsProperty);
set => SetValue(PreviewWarningsProperty, value);
}

protected override void OnAttachedToVisualTree(CompositeDisposable disposable)
{
if (AssociatedObject?.DataContext is not PrivacyWarning current)
{
return;
}

this.WhenAnyValue(x => x.PreviewWarnings)
.WhereNotNull()
.Throttle(TimeSpan.FromMilliseconds(250), RxApp.MainThreadScheduler)
ichthus1604 marked this conversation as resolved.
Show resolved Hide resolved
.Do(_ =>
{
var fadeOut = !PreviewWarnings.Any(p => p == current);
if (fadeOut && !AssociatedObject.Classes.Contains(FadeOutClassName))
{
AssociatedObject.Classes.Add(FadeOutClassName);
}
else if (!fadeOut && AssociatedObject.Classes.Contains(FadeOutClassName))
{
AssociatedObject.Classes.Remove(FadeOutClassName);
}
})
.Subscribe()
.DisposeWith(disposable);
}
}
8 changes: 5 additions & 3 deletions WalletWasabi.Fluent/Models/Transactions/PrivacySuggestion.cs
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,13 @@

public record LabelManagementSuggestion(BuildTransactionResult? Transaction = null, LabelsArray? NewLabels = null) : PrivacySuggestion(Transaction);

public record FullPrivacySuggestion(BuildTransactionResult Transaction, decimal Difference, string DifferenceFiatText, IEnumerable<SmartCoin> Coins, bool IsChangeless) : PrivacySuggestion(Transaction);
public record FullPrivacySuggestion(BuildTransactionResult Transaction, decimal Difference, string DifferenceText, string DifferenceAmountText, IEnumerable<SmartCoin> Coins, bool IsChangeless) : PrivacySuggestion(Transaction);

public record BetterPrivacySuggestion(BuildTransactionResult Transaction, string DifferenceFiatText, IEnumerable<SmartCoin> Coins, bool IsChangeless) : PrivacySuggestion(Transaction);
public record BetterPrivacySuggestion(BuildTransactionResult Transaction, string DifferenceText, string DifferenceAmountText, IEnumerable<SmartCoin> Coins, bool IsChangeless) : PrivacySuggestion(Transaction);

public record ChangeAvoidanceSuggestion(BuildTransactionResult Transaction, string DifferenceFiatText, bool IsMore, bool IsLess) : PrivacySuggestion(Transaction)
public record ChangeAvoidanceSuggestion(BuildTransactionResult Transaction, string DifferenceText, string DifferenceAmountText, bool IsMore, bool IsLess) : PrivacySuggestion(Transaction)
{
public Money GetAmount(BitcoinAddress destination) => Transaction!.CalculateDestinationAmount(destination);

public bool IsSameAmount => !IsMore && !IsLess;

Check warning on line 25 in WalletWasabi.Fluent/Models/Transactions/PrivacySuggestion.cs

View check run for this annotation

CodeScene Delta Analysis / CodeScene Cloud Delta Analysis (master)

❌ New issue: Excess Number of Function Arguments

ChangeAvoidanceSuggestion has 5 arguments, threshold = 4. This function has too many arguments, indicating a lack of encapsulation. Avoid adding more arguments.
}
38 changes: 24 additions & 14 deletions WalletWasabi.Fluent/Models/Transactions/PrivacySuggestionsModel.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
using DynamicData;

Check notice on line 1 in WalletWasabi.Fluent/Models/Transactions/PrivacySuggestionsModel.cs

View check run for this annotation

CodeScene Delta Analysis / CodeScene Cloud Delta Analysis (master)

✅ Getting better: Overall Code Complexity

The mean cyclomatic complexity decreases from 4.77 to 4.50, threshold = 4. This file has many conditional statements (e.g. if, for, while) across its implementation, leading to lower code health. Avoid adding more conditionals.
using NBitcoin;
using Nito.AsyncEx;
using System.Collections.Generic;
Expand Down Expand Up @@ -173,9 +173,10 @@

if (amountDifferencePercentage <= MaximumDifferenceTolerance && (canModifyTransactionAmount || amountDifference == 0m))
{
var differenceFiat = GetDifferenceFiat(transactionInfo, newTransaction, usdExchangeRate);
var differenceFiatText = GetDifferenceFiatText(differenceFiat);
fullPrivacySuggestion = new FullPrivacySuggestion(newTransaction, amountDifference, differenceFiatText, allPrivateCoin, isChangeless);
var (differenceBtc, differenceFiat) = GetDifference(transactionInfo, newTransaction, usdExchangeRate);
var differenceText = GetDifferenceText(differenceBtc);
var differenceAmountText = GetDifferenceAmountText(differenceBtc, differenceFiat);
fullPrivacySuggestion = new FullPrivacySuggestion(newTransaction, amountDifference, differenceText, differenceAmountText, allPrivateCoin, isChangeless);
yield return fullPrivacySuggestion;
}
}
Expand All @@ -196,9 +197,10 @@

if (amountDifferencePercentage <= MaximumDifferenceTolerance && (canModifyTransactionAmount || amountDifference == 0m))
{
var differenceFiat = GetDifferenceFiat(transactionInfo, newTransaction, usdExchangeRate);
var differenceFiatText = GetDifferenceFiatText(differenceFiat);
yield return new BetterPrivacySuggestion(newTransaction, differenceFiatText, coins, isChangeless);
var (btcDifference, fiatDifference) = GetDifference(transactionInfo, newTransaction, usdExchangeRate);
var differenceText = GetDifferenceText(btcDifference);
var differenceAmountText = GetDifferenceAmountText(btcDifference, fiatDifference);
yield return new BetterPrivacySuggestion(newTransaction, differenceText, differenceAmountText, coins, isChangeless);
}
}
}
Expand Down Expand Up @@ -331,8 +333,10 @@

if (transaction is not null)
{
var differenceFiat = GetDifferenceFiat(transactionInfo, transaction, usdExchangeRate);
yield return new ChangeAvoidanceSuggestion(transaction, GetDifferenceFiatText(differenceFiat), IsMore: differenceFiat > 0, IsLess: differenceFiat < 0);
var (btcDifference, fiatDifference) = GetDifference(transactionInfo, transaction, usdExchangeRate);
var differenceText = GetDifferenceText(btcDifference);
var differenceAmountText = GetDifferenceAmountText(btcDifference, fiatDifference);
yield return new ChangeAvoidanceSuggestion(transaction, differenceText, differenceAmountText, IsMore: fiatDifference > 0, IsLess: fiatDifference < 0);
}
}
}
Expand Down Expand Up @@ -380,25 +384,31 @@
return true;
}

private decimal GetDifferenceFiat(TransactionInfo transactionInfo, BuildTransactionResult transaction, decimal usdExchangeRate)
private (decimal BtcDifference, decimal FiatDifference) GetDifference(TransactionInfo transactionInfo, BuildTransactionResult transaction, decimal usdExchangeRate)
{
var originalAmount = transactionInfo.Amount.ToDecimal(MoneyUnit.BTC);
var totalAmount = transaction.CalculateDestinationAmount(transactionInfo.Destination);
var total = totalAmount.ToDecimal(MoneyUnit.BTC);
var btcDifference = total - originalAmount;
var fiatTotal = total * usdExchangeRate;
var fiatOriginal = originalAmount * usdExchangeRate;
var fiatDifference = fiatTotal - fiatOriginal;

return fiatDifference;
return (btcDifference, fiatDifference);
}

private string GetDifferenceFiatText(decimal fiatDifference)
private string GetDifferenceText(decimal btcDifference)
soosr marked this conversation as resolved.
Show resolved Hide resolved
{
return fiatDifference switch
return btcDifference switch
{
> 0 => $"{fiatDifference.ToUsd()} more",
< 0 => $"{Math.Abs(fiatDifference).ToUsd()} less",
> 0 => "more",
< 0 => "less",
_ => "the same amount"
};
}

private string GetDifferenceAmountText(decimal btcDifference, decimal fiatDifference)
{
return $"{Math.Abs(btcDifference).FormattedBtc()} BTC {Math.Abs(fiatDifference).ToUsdAproxBetweenParens()}";
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
using DynamicData;
using ReactiveUI;
using System.Collections;
using System.Collections.Generic;
using System.Collections.ObjectModel;
using System.Linq;
using System.Reactive.Subjects;
using System.Threading;
using System.Threading.Tasks;
using WalletWasabi.Blockchain.TransactionBuilding;
Expand All @@ -11,6 +16,7 @@ namespace WalletWasabi.Fluent.ViewModels.Wallets.Send;
public partial class PrivacySuggestionsFlyoutViewModel : ViewModelBase
{
private readonly PrivacySuggestionsModel _privacySuggestionsModel;
private readonly Subject<IEnumerable<PrivacyWarning>> _previewWarnings = new();

[AutoNotify] private PrivacySuggestion? _previewSuggestion;
[AutoNotify] private PrivacySuggestion? _selectedSuggestion;
Expand All @@ -28,6 +34,27 @@ public PrivacySuggestionsFlyoutViewModel(Wallet wallet)

public ObservableCollection<PrivacyWarning> Warnings { get; } = new();
public ObservableCollection<PrivacySuggestion> Suggestions { get; } = new();
public IObservable<IEnumerable<PrivacyWarning>> PreviewWarnings => _previewWarnings;

public async Task UpdatePreviewWarningsAsync(TransactionInfo info, BuildTransactionResult transaction, CancellationToken cancellationToken)
{
var previewWarningList = new List<PrivacyWarning>();

await foreach (var item in _privacySuggestionsModel.BuildPrivacySuggestionsAsync(info, transaction, cancellationToken))
{
if (item is PrivacyWarning warning)
{
previewWarningList.Add(warning);
}
}
ichthus1604 marked this conversation as resolved.
Show resolved Hide resolved

_previewWarnings.OnNext(previewWarningList);
}

public void ClearPreviewWarnings()
{
_previewWarnings.OnNext(Warnings);
}

/// <remarks>Method supports being called multiple times. In that case the last call cancels the previous one.</remarks>
public async Task BuildPrivacySuggestionsAsync(TransactionInfo info, BuildTransactionResult transaction, CancellationToken cancellationToken)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,17 +65,20 @@
DisplayedTransactionSummary = CurrentTransactionSummary;

PrivacySuggestions.WhenAnyValue(x => x.PreviewSuggestion)
.Subscribe(x =>
.DoAsync(async x =>
{
if (x?.Transaction is { } transaction)
{
UpdateTransaction(PreviewTransactionSummary, transaction);
await PrivacySuggestions.UpdatePreviewWarningsAsync(_info, transaction, _cancellationTokenSource.Token);
}
else
{
DisplayedTransactionSummary = CurrentTransactionSummary;
PrivacySuggestions.ClearPreviewWarnings();
}
});
})
.Subscribe();

Check warning on line 81 in WalletWasabi.Fluent/ViewModels/Wallets/Send/TransactionPreviewViewModel.cs

View check run for this annotation

CodeScene Delta Analysis / CodeScene Cloud Delta Analysis (master)

❌ Getting worse: Large Method

TransactionPreviewViewModel increases from 78 to 81 lines of code, threshold = 70. Large functions with many lines of code are generally harder to understand and lower the code health. Avoid adding more lines to this function.

PrivacySuggestions.WhenAnyValue(x => x.SelectedSuggestion)
.SubscribeAsync(async suggestion =>
Expand Down
32 changes: 26 additions & 6 deletions WalletWasabi.Fluent/Views/Wallets/Send/PrivacyWarningsView.axaml
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,23 @@
<Setter Property="Background" Value="{DynamicResource NavBarItemPointerOverBackgroundBrush}" />
<Setter Property="Padding" Value="5" />
<Setter Property="Margin" Value="5 0 5 5" />
<Setter Property="(Interaction.Behaviors)">
<BehaviorCollectionTemplate>
<BehaviorCollection>
<behaviors:PrivacyWarningFadeOutBehavior PreviewWarnings="{Binding PreviewWarnings^}" />
</BehaviorCollection>
</BehaviorCollectionTemplate>
</Setter>
<Setter Property="Transitions">
<Transitions>
<DoubleTransition Property="Opacity" Duration="0:0:0.275" Easing="{StaticResource FluentEasing}" />
</Transitions>
</Setter>
</Style>

<Style Selector="Border.warning.fadeout">
<Setter Property="Opacity" Value=".4" />
<Setter Property="IsEnabled" Value="False" />
</Style>

<Style Selector="Border.warning PathIcon">
Expand Down Expand Up @@ -235,8 +252,9 @@
<TextBlock Text="Full Privacy" Classes="title" />
<StackPanel Orientation="Horizontal" Classes="description">
<TextBlock Text="Send" />
<TextBlock Text="{Binding DifferenceFiatText}" FontWeight="Bold" Foreground="{DynamicResource SystemAccentColor}" />
<TextBlock Text="to use only private coins." />
<TextBlock Text="{Binding DifferenceText}"
Foreground="{DynamicResource SystemAccentColor}" />
<TextBlock Text="{Binding DifferenceAmountText, StringFormat='by {0}'}" />
</StackPanel>
</StackPanel>
</DockPanel>
Expand All @@ -252,8 +270,9 @@
<TextBlock Text="Better Privacy" Classes="title" />
<StackPanel Orientation="Horizontal" Classes="description">
<TextBlock Text="Send" />
<TextBlock Text="{Binding DifferenceFiatText}" FontWeight="Bold" Foreground="{DynamicResource SystemAccentColor}" />
<TextBlock Text="to avoid non-private coins." />
<TextBlock Text="{Binding DifferenceText}"
Foreground="{DynamicResource SystemAccentColor}" />
<TextBlock Text="{Binding DifferenceAmountText, StringFormat='by {0}'}" />
</StackPanel>
</StackPanel>
</DockPanel>
Expand Down Expand Up @@ -281,8 +300,9 @@
<TextBlock Text="Change Avoidance" Classes="title" />
<StackPanel Orientation="Horizontal" Classes="description">
<TextBlock Text="Send" />
<TextBlock Text="{Binding DifferenceFiatText}" FontWeight="Bold" Foreground="{DynamicResource SystemAccentColor}" />
<TextBlock Text="to remove change." />
<TextBlock Text="{Binding DifferenceText}"
Foreground="{DynamicResource SystemAccentColor}" />
<TextBlock Text="{Binding DifferenceAmountText, StringFormat='by {0}'}" IsVisible="{Binding !IsSameAmount}" />
</StackPanel>
</StackPanel>
</DockPanel>
Expand Down