Skip to content

Commit

Permalink
fix: ICommadSource Implemetation (#15496)
Browse files Browse the repository at this point in the history 
* test: CommandParameter does not change between CanExecute and Execute

* feat: CommandParameter does not change between CanExecute and Execute

* test: update
  • Loading branch information
@workgroupengineering
workgroupengineering authored Apr 30, 2024
1 parent ea28c90 commit 81d2e7d
Show file tree
Hide file tree
Showing 8 changed files with 399 additions and 168 deletions.
44 changes: 29 additions & 15 deletions src/Avalonia.Controls/Button.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,10 @@ public enum ClickMode
[PseudoClasses(pcFlyoutOpen, pcPressed)]
public class Button : ContentControl, ICommandSource, IClickableControl
{
private const string pcPressed = ":pressed";
private const string pcPressed = ":pressed";
private const string pcFlyoutOpen = ":flyout-open";
private EventHandler? _canExecuteChangeHandler = default;
private EventHandler CanExecuteChangedHandler => _canExecuteChangeHandler ??= new(CanExecuteChanged);

/// <summary>
/// Defines the <see cref="ClickMode"/> property.
Expand Down Expand Up @@ -250,10 +252,11 @@ protected override void OnAttachedToLogicalTree(LogicalTreeAttachmentEventArgs e

base.OnAttachedToLogicalTree(e);

if (Command != null)
(var command, var parameter) = (Command, CommandParameter);
if (command is not null)
{
Command.CanExecuteChanged += CanExecuteChanged;
CanExecuteChanged(this, EventArgs.Empty);
command.CanExecuteChanged += CanExecuteChangedHandler;
CanExecuteChanged(command, parameter);
}
}

Expand All @@ -269,9 +272,9 @@ protected override void OnDetachedFromLogicalTree(LogicalTreeAttachmentEventArgs

base.OnDetachedFromLogicalTree(e);

if (Command != null)
if (Command is { } command)
{
Command.CanExecuteChanged -= CanExecuteChanged;
command.CanExecuteChanged -= CanExecuteChangedHandler;
}
}

Expand Down Expand Up @@ -343,9 +346,10 @@ protected virtual void OnClick()
var e = new RoutedEventArgs(ClickEvent);
RaiseEvent(e);

if (!e.Handled && Command?.CanExecute(CommandParameter) == true)
(var command, var parameter) = (Command, CommandParameter);
if (!e.Handled && command is not null && command.CanExecute(parameter))
{
Command.Execute(CommandParameter);
command.Execute(parameter);
e.Handled = true;
}
}
Expand Down Expand Up @@ -451,25 +455,24 @@ protected override void OnPropertyChanged(AvaloniaPropertyChangedEventArgs chang

if (change.Property == CommandProperty)
{
var (oldValue, newValue) = change.GetOldAndNewValue<ICommand?>();
if (((ILogical)this).IsAttachedToLogicalTree)
{
var (oldValue, newValue) = change.GetOldAndNewValue<ICommand?>();
if (oldValue is ICommand oldCommand)
{
oldCommand.CanExecuteChanged -= CanExecuteChanged;
oldCommand.CanExecuteChanged -= CanExecuteChangedHandler;
}

if (newValue is ICommand newCommand)
{
newCommand.CanExecuteChanged += CanExecuteChanged;
newCommand.CanExecuteChanged += CanExecuteChangedHandler;
}
}

CanExecuteChanged(this, EventArgs.Empty);
CanExecuteChanged(newValue, CommandParameter);
}
else if (change.Property == CommandParameterProperty)
{
CanExecuteChanged(this, EventArgs.Empty);
CanExecuteChanged(Command, change.NewValue);
}
else if (change.Property == IsCancelProperty)
{
Expand Down Expand Up @@ -557,7 +560,18 @@ protected override void UpdateDataValidation(
/// <param name="e">The event args.</param>
private void CanExecuteChanged(object? sender, EventArgs e)
{
var canExecute = Command == null || Command.CanExecute(CommandParameter);
CanExecuteChanged(Command, CommandParameter);
}

[System.Runtime.CompilerServices.MethodImpl(System.Runtime.CompilerServices.MethodImplOptions.AggressiveInlining)]
private void CanExecuteChanged(ICommand? command, object? parameter)
{
if (!((ILogical)this).IsAttachedToLogicalTree)
{
return;
}

var canExecute = command == null || command.CanExecute(parameter);

if (canExecute != _commandCanExecute)
{
Expand Down
79 changes: 47 additions & 32 deletions src/Avalonia.Controls/MenuItem.cs
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
using System;
using System.Collections.Generic;
using System.Linq;
using Avalonia.Reactive;
using System.Windows.Input;
using Avalonia.Automation.Peers;
using Avalonia.Controls.Metadata;
Expand All @@ -13,7 +12,7 @@
using Avalonia.Input;
using Avalonia.Interactivity;
using Avalonia.LogicalTree;
using Avalonia.Layout;
using Avalonia.Reactive;

namespace Avalonia.Controls
{
Expand All @@ -24,6 +23,9 @@ namespace Avalonia.Controls
[PseudoClasses(":separator", ":radio", ":toggle", ":checked", ":icon", ":open", ":pressed", ":selected")]
public class MenuItem : HeaderedSelectingItemsControl, IMenuItem, ISelectable, ICommandSource, IClickableControl, IRadioButton
{
private EventHandler? _canExecuteChangeHandler = default;
private EventHandler CanExecuteChangedHandler => _canExecuteChangeHandler ??= new(CanExecuteChanged);

/// <summary>
/// Defines the <see cref="Command"/> property.
/// </summary>
Expand Down Expand Up @@ -83,7 +85,7 @@ public class MenuItem : HeaderedSelectingItemsControl, IMenuItem, ISelectable, I
/// </summary>
public static readonly StyledProperty<string?> GroupNameProperty =
RadioButton.GroupNameProperty.AddOwner<MenuItem>();

/// <summary>
/// Defines the <see cref="Click"/> event.
/// </summary>
Expand Down Expand Up @@ -292,7 +294,7 @@ public bool StaysOpenOnClick
get => GetValue(StaysOpenOnClickProperty);
set => SetValue(StaysOpenOnClickProperty, value);
}

/// <inheritdoc cref="IMenuItem.ToggleType" />
public MenuItemToggleType ToggleType
{
Expand All @@ -306,7 +308,7 @@ public bool IsChecked
get => GetValue(IsCheckedProperty);
set => SetValue(IsCheckedProperty, value);
}

bool IRadioButton.IsChecked
{
get => IsChecked;
Expand All @@ -319,7 +321,7 @@ public string? GroupName
get => GetValue(GroupNameProperty);
set => SetValue(GroupNameProperty, value);
}

/// <summary>
/// Gets or sets a value that indicates whether the <see cref="MenuItem"/> has a submenu.
/// </summary>
Expand Down Expand Up @@ -413,15 +415,16 @@ protected override void OnAttachedToLogicalTree(LogicalTreeAttachmentEventArgs e
{
SetCurrentValue(HotKeyProperty, _hotkey);
}

base.OnAttachedToLogicalTree(e);

if (Command != null)
(var command, var parameter) = (Command, CommandParameter);
if (command is not null)
{
Command.CanExecuteChanged += CanExecuteChanged;
command.CanExecuteChanged += CanExecuteChangedHandler;
}
TryUpdateCanExecute();

TryUpdateCanExecute(command, parameter);

var parent = Parent;

Expand All @@ -437,7 +440,7 @@ protected override void OnAttachedToLogicalTree(LogicalTreeAttachmentEventArgs e
protected override void OnAttachedToVisualTree(VisualTreeAttachmentEventArgs e)
{
base.OnAttachedToVisualTree(e);

TryUpdateCanExecute();
}

Expand All @@ -454,7 +457,7 @@ protected override void OnDetachedFromLogicalTree(LogicalTreeAttachmentEventArgs

if (Command != null)
{
Command.CanExecuteChanged -= CanExecuteChanged;
Command.CanExecuteChanged -= CanExecuteChangedHandler;
}
}

Expand All @@ -464,9 +467,10 @@ protected override void OnDetachedFromLogicalTree(LogicalTreeAttachmentEventArgs
/// <param name="e">The click event args.</param>
protected virtual void OnClick(RoutedEventArgs e)
{
if (!e.Handled && Command?.CanExecute(CommandParameter) == true)
(var command, var parameter) = (Command, CommandParameter);
if (!e.Handled && command is not null && command.CanExecute(parameter) == true)
{
Command.Execute(CommandParameter);
command.Execute(parameter);
e.Handled = true;
}
}
Expand Down Expand Up @@ -577,21 +581,25 @@ private void CloseSubmenus()
/// <param name="e">The event args.</param>
private static void CommandChanged(AvaloniaPropertyChangedEventArgs e)
{
if (e.Sender is MenuItem menuItem &&
((ILogical)menuItem).IsAttachedToLogicalTree)
var newCommand = e.NewValue as ICommand;
if (e.Sender is MenuItem menuItem)

{
if (e.OldValue is ICommand oldCommand)
if (((ILogical)menuItem).IsAttachedToLogicalTree)
{
oldCommand.CanExecuteChanged -= menuItem.CanExecuteChanged;
}
if (e.OldValue is ICommand oldCommand)
{
oldCommand.CanExecuteChanged -= menuItem.CanExecuteChangedHandler;
}

if (e.NewValue is ICommand newCommand)
{
newCommand.CanExecuteChanged += menuItem.CanExecuteChanged;
if (newCommand is not null)
{
newCommand.CanExecuteChanged += menuItem.CanExecuteChangedHandler;
}
}

menuItem.TryUpdateCanExecute();
menuItem.TryUpdateCanExecute(newCommand, menuItem.CommandParameter);
}

}

/// <summary>
Expand All @@ -602,7 +610,8 @@ private static void CommandParameterChanged(AvaloniaPropertyChangedEventArgs e)
{
if (e.Sender is MenuItem menuItem)
{
menuItem.TryUpdateCanExecute();
(var command, var parameter) = (menuItem.Command, e.NewValue);
menuItem.TryUpdateCanExecute(command, parameter);
}
}

Expand All @@ -621,21 +630,27 @@ private void CanExecuteChanged(object? sender, EventArgs e)
/// </summary>
private void TryUpdateCanExecute()
{
if (Command == null)
TryUpdateCanExecute(Command, CommandParameter);
}

[System.Runtime.CompilerServices.MethodImpl(System.Runtime.CompilerServices.MethodImplOptions.AggressiveInlining)]
private void TryUpdateCanExecute(ICommand? command, object? parameter)
{
if (command == null)
{
_commandCanExecute = !_commandBindingError;
UpdateIsEffectivelyEnabled();
return;
}

//Perf optimization - only raise CanExecute event if the menu is open
if (!((ILogical)this).IsAttachedToLogicalTree ||
Parent is MenuItem { IsSubMenuOpen: false })
{
return;
}
var canExecute = Command.CanExecute(CommandParameter);

var canExecute = command.CanExecute(parameter);
if (canExecute != _commandCanExecute)
{
_commandCanExecute = canExecute;
Expand Down Expand Up @@ -720,7 +735,7 @@ private void IsCheckedChanged(AvaloniaPropertyChangedEventArgs e)
(MenuInteractionHandler as DefaultMenuInteractionHandler)?.OnCheckedChanged(this);
}
}

/// <summary>
/// Called when the <see cref="HeaderedSelectingItemsControl.Header"/> property changes.
/// </summary>
Expand Down Expand Up @@ -834,7 +849,7 @@ private void PopupClosed(object? sender, EventArgs e)
SelectedItem = null;
}

void ICommandSource.CanExecuteChanged(object sender, EventArgs e) => this.CanExecuteChanged(sender, e);
void ICommandSource.CanExecuteChanged(object sender, EventArgs e) => CanExecuteChangedHandler(sender, e);

void IClickableControl.RaiseClick()
{
Expand Down
29 changes: 21 additions & 8 deletions src/Avalonia.Controls/SplitButton/SplitButton.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,18 @@ public KeyGesture? HotKey
/// <inheritdoc cref="ICommandSource.CanExecuteChanged"/>
private void CanExecuteChanged(object? sender, EventArgs e)
{
var canExecute = Command == null || Command.CanExecute(CommandParameter);
(var command, var parameter) = (Command, CommandParameter);
CanExecuteChanged(command, parameter);
}

private void CanExecuteChanged(ICommand? command, object? parameter)
{
if (!((ILogical)this).IsAttachedToLogicalTree)
{
return;
}

var canExecute = command is null || command.CanExecute(parameter);

if (canExecute != _commandCanExecute)
{
Expand Down Expand Up @@ -282,10 +293,11 @@ protected override void OnPropertyChanged(AvaloniaPropertyChangedEventArgs e)
{
if (e.Property == CommandProperty)
{
// Must unregister events here while a reference to the old command still exists
var (oldValue, newValue) = e.GetOldAndNewValue<ICommand?>();

if (_isAttachedToLogicalTree)
{
// Must unregister events here while a reference to the old command still exists
var (oldValue, newValue) = e.GetOldAndNewValue<ICommand?>();

if (oldValue is ICommand oldCommand)
{
Expand All @@ -298,11 +310,11 @@ protected override void OnPropertyChanged(AvaloniaPropertyChangedEventArgs e)
}
}

CanExecuteChanged(this, EventArgs.Empty);
CanExecuteChanged(newValue, CommandParameter);
}
else if (e.Property == CommandParameterProperty)
else if (e.Property == CommandParameterProperty && IsLoaded)
{
CanExecuteChanged(this, EventArgs.Empty);
CanExecuteChanged(Command, e.NewValue);
}
else if (e.Property == FlyoutProperty)
{
Expand Down Expand Up @@ -386,15 +398,16 @@ protected override void OnKeyUp(KeyEventArgs e)
/// <param name="e">The event args from the internal Click event.</param>
protected virtual void OnClickPrimary(RoutedEventArgs? e)
{
(var command, var parameter) = (Command, CommandParameter);
// Note: It is not currently required to check enabled status; however, this is a failsafe
if (IsEffectivelyEnabled)
{
var eventArgs = new RoutedEventArgs(ClickEvent);
RaiseEvent(eventArgs);

if (!eventArgs.Handled && Command?.CanExecute(CommandParameter) == true)
if (!eventArgs.Handled && command?.CanExecute(parameter) == true)
{
Command.Execute(CommandParameter);
command.Execute(parameter);
eventArgs.Handled = true;
}
}
Expand Down
Loading

0 comments on commit 81d2e7d

Please sign in to comment.