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

fix: ICommadSource Implemetation #15496

Merged
merged 3 commits into from
Apr 30, 2024
Merged

fix: ICommadSource Implemetation #15496

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1d3c5b5
test: CommandParameter does not change between CanExecute and Execute
workgroupengineering Apr 26, 2024
d81ecd6
feat: CommandParameter does not change between CanExecute and Execute
workgroupengineering Apr 26, 2024
fe7c841
test: update
workgroupengineering Apr 26, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 29 additions & 15 deletions src/Avalonia.Controls/Button.cs
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Loading