[*] Drop MenuItem.IsEnabledPropertyName (#4659)
authorStephane Delcroix <stephane@delcroix.org>
Fri, 21 Dec 2018 08:06:52 +0000 (09:06 +0100)
committerGitHub <noreply@github.com>
Fri, 21 Dec 2018 08:06:52 +0000 (09:06 +0100)
* [*] Drop MenuItem.IsEnabledPropertyName

While doing one of the big refactoring of 2006, we missed an
opportunity to use a propertyKey for IsEnabled, ending up in some
less-than-optimal  design choices, like IMenuController.IsEnabledPropertyName.

Properly using a public r-o BP removes the need for IsEnabledPropertyName.

* other plats

* use the key

16 files changed:
Xamarin.Forms.Core/IMenuItemController.cs
Xamarin.Forms.Core/MenuItem.cs
Xamarin.Forms.Core/Shell/MenuItemCollection.cs [moved from Xamarin.Forms.Core/MenuItemCollection.cs with 100% similarity]
Xamarin.Forms.Core/Shell/Shell.cs
Xamarin.Forms.Platform.Android/Renderers/ShellToolbarTracker.cs
Xamarin.Forms.Platform.Android/Renderers/ToolbarButton.cs
Xamarin.Forms.Platform.Android/Renderers/ToolbarImageButton.cs
Xamarin.Forms.Platform.GTK/Cells/CellBase.cs
Xamarin.Forms.Platform.GTK/GtkToolbarTracker.cs
Xamarin.Forms.Platform.MacOS/Extensions/NSMenuExtensions.cs
Xamarin.Forms.Platform.MacOS/NativeToolbarTracker.cs
Xamarin.Forms.Platform.Tizen/Native/ToolbarItemButton.cs
Xamarin.Forms.Platform.UAP/MenuItemCommand.cs
Xamarin.Forms.Platform.WPF/Renderers/VisualPageRenderer.cs
Xamarin.Forms.Platform.iOS/ContextActionCell.cs
Xamarin.Forms.Platform.iOS/Extensions/ToolbarItemExtensions.cs

index 33bcfc2..3311f59 100644 (file)
@@ -1,10 +1,11 @@
+using System.ComponentModel;
+
 namespace Xamarin.Forms
 {
+       [EditorBrowsable(EditorBrowsableState.Never)]
        public interface IMenuItemController
        {
                bool IsEnabled { get; set; }
-               string IsEnabledPropertyName { get; }
-
                void Activate();
        }
 }
index 6e9daf4..425d889 100644 (file)
@@ -1,98 +1,83 @@
 using System;
-using System.Collections.Generic;
 using System.ComponentModel;
 using System.Windows.Input;
 
 namespace Xamarin.Forms
 {
-
        public class MenuItem : BaseMenuItem, IMenuItemController
        {
                public static readonly BindableProperty AcceleratorProperty = BindableProperty.CreateAttached(nameof(Accelerator), typeof(Accelerator), typeof(MenuItem), null);
 
-               public static Accelerator GetAccelerator(BindableObject bindable) => (Accelerator)bindable.GetValue(AcceleratorProperty);
-
-               public static void SetAccelerator(BindableObject bindable, Accelerator value) => bindable.SetValue(AcceleratorProperty, value);
+               public static readonly BindableProperty CommandProperty = BindableProperty.Create(nameof(Command), typeof(ICommand), typeof(MenuItem), null,
+                       propertyChanging: (bo, o, n) => ((MenuItem)bo).OnCommandChanging(),
+                       propertyChanged: (bo, o, n) => ((MenuItem)bo).OnCommandChanged());
 
-               public static readonly BindableProperty TextProperty = BindableProperty.Create("Text", typeof(string), typeof(MenuItem), null);
+               public static readonly BindableProperty CommandParameterProperty = BindableProperty.Create(nameof(CommandParameter), typeof(object), typeof(MenuItem), null,
+                       propertyChanged: (bo, o, n) => ((MenuItem)bo).OnCommandParameterChanged());
 
-               public static readonly BindableProperty CommandProperty = BindableProperty.Create("Command", typeof(ICommand), typeof(MenuItem), null,
-                       propertyChanging: (bo, o, n) => ((MenuItem)bo).OnCommandChanging(), propertyChanged: (bo, o, n) => ((MenuItem)bo).OnCommandChanged());
+               public static readonly BindableProperty IsDestructiveProperty = BindableProperty.Create(nameof(IsDestructive), typeof(bool), typeof(MenuItem), false);
 
-               public static readonly BindableProperty CommandParameterProperty = BindableProperty.Create("CommandParameter", typeof(object), typeof(MenuItem), null,
-                       propertyChanged: (bo, o, n) => ((MenuItem)bo).OnCommandParameterChanged());
+               public static readonly BindableProperty IconProperty = BindableProperty.Create(nameof(Icon), typeof(FileImageSource), typeof(MenuItem), default(FileImageSource));
 
-               public static readonly BindableProperty IsDestructiveProperty = BindableProperty.Create("IsDestructive", typeof(bool), typeof(MenuItem), false);
+               static readonly BindablePropertyKey IsEnabledPropertyKey = BindableProperty.CreateReadOnly(nameof(IsEnabled), typeof(bool), typeof(ToolbarItem), true);
+               public static readonly BindableProperty IsEnabledProperty = IsEnabledPropertyKey.BindableProperty;
 
-               public static readonly BindableProperty IconProperty = BindableProperty.Create("Icon", typeof(FileImageSource), typeof(MenuItem), default(FileImageSource));
+               public static readonly BindableProperty TextProperty = BindableProperty.Create(nameof(Text), typeof(string), typeof(MenuItem), null);
 
-               [EditorBrowsable(EditorBrowsableState.Never)]
-               public static readonly BindableProperty IsEnabledProperty = BindableProperty.Create("IsEnabled", typeof(bool), typeof(ToolbarItem), true);
+               public static Accelerator GetAccelerator(BindableObject bindable) => (Accelerator)bindable.GetValue(AcceleratorProperty);
 
-               [EditorBrowsable(EditorBrowsableState.Never)]
-               public string IsEnabledPropertyName
-               {
-                       get
-                       {
-                               return IsEnabledProperty.PropertyName;
-                       }
-               }
+               public static void SetAccelerator(BindableObject bindable, Accelerator value) => bindable.SetValue(AcceleratorProperty, value);
 
                public ICommand Command
                {
-                       get { return (ICommand)GetValue(CommandProperty); }
-                       set { SetValue(CommandProperty, value); }
+                       get => (ICommand)GetValue(CommandProperty);
+                       set => SetValue(CommandProperty, value);
                }
 
                public object CommandParameter
                {
-                       get { return GetValue(CommandParameterProperty); }
-                       set { SetValue(CommandParameterProperty, value); }
+                       get => GetValue(CommandParameterProperty);
+                       set => SetValue(CommandParameterProperty, value);
                }
 
                public FileImageSource Icon
                {
-                       get { return (FileImageSource)GetValue(IconProperty); }
-                       set { SetValue(IconProperty, value); }
+                       get => (FileImageSource)GetValue(IconProperty);
+                       set => SetValue(IconProperty, value);
                }
 
                public bool IsDestructive
                {
-                       get { return (bool)GetValue(IsDestructiveProperty); }
-                       set { SetValue(IsDestructiveProperty, value); }
+                       get => (bool)GetValue(IsDestructiveProperty);
+                       set => SetValue(IsDestructiveProperty, value);
                }
 
                public string Text
                {
-                       get { return (string)GetValue(TextProperty); }
-                       set { SetValue(TextProperty, value); }
+                       get => (string)GetValue(TextProperty);
+                       set => SetValue(TextProperty, value);
                }
 
-               [EditorBrowsable(EditorBrowsableState.Never)]
                public bool IsEnabled
                {
-                       get { return (bool)GetValue(IsEnabledProperty); }
-                       set { SetValue(IsEnabledProperty, value); }
+                       get => (bool)GetValue(IsEnabledProperty);
+                       [EditorBrowsable(EditorBrowsableState.Never)] set => SetValue(IsEnabledPropertyKey, value);
                }
 
                bool IsEnabledCore
                {
-                       set { SetValueCore(IsEnabledProperty, value); }
+                       set => SetValueCore(IsEnabledPropertyKey, value);
                }
 
                public event EventHandler Clicked;
 
-               protected virtual void OnClicked()
-                       => Clicked?.Invoke(this, EventArgs.Empty);
+               protected virtual void OnClicked() => Clicked?.Invoke(this, EventArgs.Empty);
 
                [EditorBrowsable(EditorBrowsableState.Never)]
-               public void Activate()
+               void IMenuItemController.Activate()
                {
-                       if (Command != null)
-                       {
-                               if (IsEnabled)
-                                       Command.Execute(CommandParameter);
-                       }
+                       if (IsEnabled)
+                               Command?.Execute(CommandParameter);
 
                        OnClicked();
                }
@@ -104,13 +89,10 @@ namespace Xamarin.Forms
 
                void OnCommandChanged()
                {
+                       IsEnabledCore = Command?.CanExecute(CommandParameter) ?? true;
+
                        if (Command == null)
-                       {
-                               IsEnabledCore = true;
                                return;
-                       }
-
-                       IsEnabledCore = Command.CanExecute(CommandParameter);
 
                        Command.CanExecuteChanged += OnCommandCanExecuteChanged;
                }
index e2d8b48..3a58b76 100644 (file)
@@ -253,7 +253,7 @@ namespace Xamarin.Forms
 
                        switch (element) {
                        case MenuShellItem menuShellItem:
-                               menuShellItem.MenuItem.Activate();
+                               ((IMenuItemController)menuShellItem.MenuItem).Activate();
                                break;
                        case ShellItem i:
                                shellItem = i;
@@ -268,7 +268,7 @@ namespace Xamarin.Forms
                                shellContent = c;
                                break;
                        case MenuItem m:
-                               m.Activate();
+                               ((IMenuItemController)m).Activate();
                                break;
                        }
 
index 9778f72..24e025f 100644 (file)
@@ -352,7 +352,7 @@ namespace Xamarin.Forms.Platform.Android
                                        UpdateMenuItemIcon(_shellContext.AndroidContext, menuitem, item);
                                        menuitem.SetEnabled(item.IsEnabled);
                                        menuitem.SetShowAsAction(ShowAsAction.Always);
-                                       menuitem.SetOnMenuItemClickListener(new GenericMenuClickListener(item.Activate));
+                                       menuitem.SetOnMenuItemClickListener(new GenericMenuClickListener(((IMenuItemController)item).Activate));
                                        menuitem.Dispose();
                                }
                        }
index afa8710..52a539e 100644 (file)
@@ -6,16 +6,15 @@ namespace Xamarin.Forms.Platform.Android
 {
        internal sealed class ToolbarButton : global::Android.Widget.Button, IToolbarButton
        {
-               IMenuItemController Controller => Item;
+               MenuItem MenuItem => Item;
+
                public ToolbarButton(Context context, ToolbarItem item) : base(context)
                {
-                       if (item == null)
-                               throw new ArgumentNullException("item", "you should specify a ToolbarItem");
-                       Item = item;
-                       Enabled = Controller.IsEnabled;
+                       Item = item ?? throw new ArgumentNullException(nameof(item), "you should specify a ToolbarItem");
+                       Enabled = MenuItem.IsEnabled;
                        Text = Item.Text;
                        SetBackgroundColor(new Color(0, 0, 0, 0).ToAndroid());
-                       Click += (sender, e) => Controller.Activate();
+                       Click += (sender, e) => ((IMenuItemController)MenuItem).Activate();
                        Item.PropertyChanged += HandlePropertyChanged;
                }
 
@@ -30,8 +29,8 @@ namespace Xamarin.Forms.Platform.Android
 
                void HandlePropertyChanged(object sender, PropertyChangedEventArgs e)
                {
-                       if (e.PropertyName == Controller.IsEnabledPropertyName)
-                               Enabled = Controller.IsEnabled;
+                       if (e.PropertyName == MenuItem.IsEnabledProperty.PropertyName)
+                               Enabled = MenuItem.IsEnabled;
                }
        }
 }
\ No newline at end of file
index 3b333b4..38ac11e 100644 (file)
@@ -8,18 +8,17 @@ namespace Xamarin.Forms.Platform.Android
 {
        internal sealed class ToolbarImageButton : AImageButton, IToolbarButton
        {
-               IMenuItemController Controller => Item;
+               MenuItem MenuItem => Item;
+
                public ToolbarImageButton(Context context, ToolbarItem item) : base(context)
                {
-                       if (item == null)
-                               throw new ArgumentNullException("item", "you should specify a ToolbarItem");
-                       Item = item;
-                       Enabled = Controller.IsEnabled;
+                       Item = item ?? throw new ArgumentNullException(nameof(item), "you should specify a ToolbarItem");
+                       Enabled = MenuItem.IsEnabled;
                        Bitmap bitmap;
                        bitmap = Context.Resources.GetBitmap(Item.Icon);
                        SetImageBitmap(bitmap);
                        SetBackgroundColor(new Color(0, 0, 0, 0).ToAndroid());
-                       Click += (sender, e) => Controller.Activate();
+                       Click += (sender, e) => ((IMenuItemController)MenuItem).Activate();
                        bitmap.Dispose();
                        Item.PropertyChanged += HandlePropertyChanged;
                }
@@ -35,8 +34,8 @@ namespace Xamarin.Forms.Platform.Android
 
                void HandlePropertyChanged(object sender, PropertyChangedEventArgs e)
                {
-                       if (e.PropertyName == Controller.IsEnabledPropertyName)
-                               Enabled = Controller.IsEnabled;
+                       if (e.PropertyName == MenuItem.IsEnabledProperty.PropertyName)
+                               Enabled = MenuItem.IsEnabled;
                }
        }
 }
\ No newline at end of file
index c3fd602..bfcef2e 100644 (file)
@@ -140,7 +140,7 @@ namespace Xamarin.Forms.Platform.GTK.Cells
 
                                menuItem.ButtonPressEvent += (sender, args) =>
                                {
-                                       item.Activate();
+                                       ((IMenuItemController)item).Activate();
                                };
 
                                menu.Add(menuItem);
index 96f434c..cea2d5f 100644 (file)
@@ -217,7 +217,7 @@ namespace Xamarin.Forms.Platform.GTK
                        {
                                ToolButton newToolButton = ToolButtonHelper.CreateToolButton(toolBarItem);
                                _toolbarSection.PackStart(newToolButton, false, false, GtkToolbarConstants.ToolbarItemSpacing);
-                               newToolButton.Clicked += (sender, args) => { toolBarItem.Activate(); };
+                               newToolButton.Clicked += (sender, args) => { ((IMenuItemController)toolBarItem).Activate(); };
 
                                toolBarItem.PropertyChanged -= OnToolbarItemPropertyChanged;
                                toolBarItem.PropertyChanged += OnToolbarItemPropertyChanged;
@@ -241,7 +241,7 @@ namespace Xamarin.Forms.Platform.GTK
 
                                        menuItem.ButtonPressEvent += (sender, args) =>
                                        {
-                                               secondaryToolBarItem.Activate();
+                                               ((IMenuItemController)secondaryToolBarItem).Activate();
                                        };
 
                                        secondaryToolBarItem.PropertyChanged -= OnToolbarItemPropertyChanged;
index fc1887b..a67efba 100644 (file)
@@ -35,7 +35,7 @@ namespace Xamarin.Forms.Platform.macOS.Extensions
                        if (i != -1)
                                nsMenuItem.Tag = i;
                        nsMenuItem.Enabled = menuItem.IsEnabled;
-                       nsMenuItem.Activated += (sender, e) => menuItem.Activate();
+                       nsMenuItem.Activated += (sender, e) => ((IMenuItemController)menuItem).Activate();
                        if (!string.IsNullOrEmpty(menuItem.Icon))
                                nsMenuItem.Image = new NSImage(menuItem.Icon);
 
index bfdef9c..ea8000b 100644 (file)
@@ -387,7 +387,7 @@ namespace Xamarin.Forms.Platform.MacOS
                                        var element = toolbarItems[i];
 
                                        var item = new NSToolbarItem(element.Text ?? "");
-                                       item.Activated += (sender, e) => element.Activate();
+                                       item.Activated += (sender, e) => ((IMenuItemController)element).Activate();
 
                                        var button = new NSButton();
                                        button.Title = element.Text ?? "";
@@ -401,7 +401,7 @@ namespace Xamarin.Forms.Platform.MacOS
                                        button.Frame = new CGRect(currentX + i * itemSpacing, 0, buttonWidth, ToolbarItemHeight);
                                        currentX += buttonWidth;
                                        totalWidth += button.Frame.Width;
-                                       button.Activated += (sender, e) => element.Activate();
+                                       button.Activated += (sender, e) => ((IMenuItemController)element).Activate();
 
                                        button.BezelStyle = NSBezelStyle.TexturedRounded;
                                        if (!string.IsNullOrEmpty(element.Icon))
index bd0f4a1..2101816 100644 (file)
@@ -36,7 +36,7 @@ namespace Xamarin.Forms.Platform.Tizen.Native
 
                void OnClicked(object sender, EventArgs e)
                {
-                       _item.Activate();
+                       ((IMenuItemController)_item).Activate();
                }
 
                void OnToolbarItemPropertyChanged(object sender, PropertyChangedEventArgs e)
index 02438ed..6dba366 100644 (file)
@@ -23,7 +23,7 @@ namespace Xamarin.Forms.Platform.UWP
 
                public void Execute(object parameter)
                {
-                       _menuItem.Activate();
+                       ((IMenuItemController)_menuItem).Activate();
                }
 
                void OnCanExecuteChanged()
index 50f40c3..b147697 100644 (file)
@@ -112,7 +112,7 @@ namespace Xamarin.Forms.Platform.WPF
                                {
                                        if (appBar.DataContext is ToolbarItem toolbarItem)
                                        {
-                                               toolbarItem.Activate();
+                                               ((IMenuItemController)toolbarItem).Activate();
                                        }
                                };
 
index 6760cc5..2686de1 100644 (file)
@@ -343,9 +343,8 @@ namespace Xamarin.Forms.Platform.iOS
                                        var action = UIAlertAction.Create(item.Text, UIAlertActionStyle.Default, a =>
                                        {
                                                _scroller.SetContentOffset(new PointF(0, 0), true);
-                                               MenuItem mi;
-                                               if (weakItem.TryGetTarget(out mi))
-                                                       mi.Activate();
+                                               if (weakItem.TryGetTarget(out MenuItem mi))
+                                                       ((IMenuItemController)mi).Activate();
                                        });
                                        actionSheet.AddAction(action);
                                }
@@ -463,7 +462,7 @@ namespace Xamarin.Forms.Platform.iOS
                        else
                        {
                                _scroller.SetContentOffset(new PointF(0, 0), true);
-                               _cell.ContextActions[(int)button.Tag].Activate();
+                               ((IMenuItemController)_cell.ContextActions[(int)button.Tag]).Activate();
                        }
                }
 
@@ -719,7 +718,7 @@ namespace Xamarin.Forms.Platform.iOS
 
                                // do not activate a -1 index when dismissing by clicking outside the popover
                                if (buttonIndex >= 0)
-                                       Items[(int)buttonIndex].Activate();
+                                       ((IMenuItemController)Items[(int)buttonIndex]).Activate();
                        }
                }
        }
index 760bcc1..2532e17 100644 (file)
@@ -32,7 +32,7 @@ namespace Xamarin.Forms.Platform.iOS
                                        UpdateTextAndStyle();
                                UpdateIsEnabled();
 
-                               Clicked += (sender, e) => _item.Activate();
+                               Clicked += (sender, e) => ((IMenuItemController)_item).Activate();
                                item.PropertyChanged += OnPropertyChanged;
 
                                if (item != null && !string.IsNullOrEmpty(item.AutomationId))
@@ -51,7 +51,7 @@ namespace Xamarin.Forms.Platform.iOS
 
                        void OnPropertyChanged(object sender, PropertyChangedEventArgs e)
                        {
-                               if (e.PropertyName == _item.IsEnabledPropertyName)
+                               if (e.PropertyName == MenuItem.IsEnabledProperty.PropertyName)
                                        UpdateIsEnabled();
                                else if (e.PropertyName == MenuItem.TextProperty.PropertyName)
                                {
@@ -106,7 +106,7 @@ namespace Xamarin.Forms.Platform.iOS
                                UpdateIcon();
                                UpdateIsEnabled();
 
-                               ((SecondaryToolbarItemContent)CustomView).TouchUpInside += (sender, e) => _item.Activate();
+                               ((SecondaryToolbarItemContent)CustomView).TouchUpInside += (sender, e) => ((IMenuItemController)_item).Activate();
                                item.PropertyChanged += OnPropertyChanged;
 
                                if (item != null && !string.IsNullOrEmpty(item.AutomationId))
@@ -129,7 +129,7 @@ namespace Xamarin.Forms.Platform.iOS
                                        UpdateText();
                                else if (e.PropertyName == MenuItem.IconProperty.PropertyName)
                                        UpdateIcon();
-                               else if (e.PropertyName == _item.IsEnabledPropertyName)
+                               else if (e.PropertyName == MenuItem.IsEnabledProperty.PropertyName)
                                        UpdateIsEnabled();
                                else if (e.PropertyName == AutomationProperties.HelpTextProperty.PropertyName)
                                        this.SetAccessibilityHint(_item);