Command.CanExecute() on RadButton inside RadBusyIndicator causes unexpected (wrong) behavior on RadButton.IsEnabled property

[Zoltan]

Dear Telerik,

We have faced an issue that is caused by one of your intended bugfixes related to the order of evaluation of the Command.CanExecute and IsEnabled property. The problem is that the IsEnabled property always wins. (This is what you’ve fixed) But this fix is causing some really annoying behavior which I think is not appropriate at all.

We’ve created a small repro project the reflect the problem.

We have RadButton grouped into a RadBusyIndicator. The RadButton has a Command with a CanExecute implemented which returns false if the button should not be enabled (a list has no more items or sg like that...). Another button or interaction or whatever triggers the IsBusy on the RadBusyIndicator to simulate a background process. However as soon as the IsBusy is false again the RadButton is Enabled even if the CanExecute returns false.

The same works as expected if I use a Silverlight Button instead of RadButton.

I’ve attached a repro project.
http://dl.dropbox.com/u/80135676/RadButtonBug.rar

Repro Steps:

1. Run th app
2. Click on Toggle Enabled Button (CanExecute now returns true, Buttons are enabled)
3. Click on GetBusy Button
   1. IsBusy is Set to True (RadBusyIndicator activated)
   2. 500ms later CanExecute logic set to return false
   3. Then 500ms later IsBusy is set to False (RadBusyIndicator deactivated)
   4. At this point the buttons should be disabled. (RadButton is enabled, Silverlight Button is disabled)

The question: Do you have plans to fix this wrong behavior?

Thanks ,

Zoltan Arvai

[Zarko]

Hello Zoltan,
The main problem in SL is that when you change CanExecute it internally changes IsEnabled and this way you have two properties that can change it. The busy Indicator internally changes the IsEnabled property of its content and it has a closing animation which delays its actual closing and it sets IsEnabled to true after you set CanCommandExecute to false. The only workaround at the moment is to manually call RaiseCanExecuteChanged after closing the busy indicator.
I've changed this in your project and reattached it so could you please examine it and if you have further questions feel free to ask.z

Greetings,
Zarko
the Telerik team

[Daní]

Wow!! That's really annoying. This explains why lately I was seeing several inconsistencies in Buttons enabled states.

[Daní]

I think there's a bug with IsEnabled property and Command.CanExcecute. I'm using Q1 2012 SP1 version with Metro Theme. I have a form to perform searches on server. The form is very simple, it contains a Grid with two rows. At first row there are several input controls (TextBoxes, Comoboxes, etc..) where the user can enter search criterias and a submit button to execute the search on server. At second row there's a RadBusyIndicator that contains a RadGridView to host the search results bound to an ObservableCollection. I have defined a column in the RadGridView that contains a RadButton with a Command bound to a view model. Each time user clicks on submit button I set the IsBusy property to true, clear the results Observable Collection, and call a WebService to obtain the search results. Once the application receive the webservice result I add the results supplied to the observable collection and set the IsBusy property to false. Having read this thread, on the IsBusy setter I rereaise the CanExecute event on command. However, the first time user submits a search all the RadButtons in GridView are enable although most of them should'nt because the Command.CanExecute.

I don't know why yo changed the previous behavior (giving higher priority to IsEnabled property to the Command). I supose that several telerik users suggested it but I think this is not a good approach and is an antipattern. A button bound to a Command should be disabled if the command does not allows its execution. If someone needs to bind the button to a command and control the IsEnabled property separetly is as easy as always return true on Command's CanExecute action and handle the IsEnabled as they need. In summary, I think is a bad idea to alter the logic behavior to satisfy a more or less large handful of users that need an "special" behavior.

I solved this issue checking again if the command can be executed on the command's execute method. Not nice, as button appears as enabled, but at least I avoid inconsistencies such as null references in my command's execute methods.

[Daní]

Hi,

I'm really disapointed and annoyed. I upgraded my application (and telerik assemblies to Q1 2012 SP1) a few days ago and now I'm receiving notifications of many errors and strange behaviors. All of them related with buttons in modules that I didn't modify and worked fine from several months. My application, a large one I'm developping for more than two years, uses intensively RadBusyIndicators and RadButtons (of all types: DropDownButton, SplitButton, RibonButtons...). Now I have that many buttons that should be disabeld because the commands they are bound CANNOT BE EXECUTED are enabled because the IsEnabled property has higher priority than CanExecute method of the bound command.  The result is that my application oftens crashes due exceptions (NullException very often) and sometimes presents inconsistencies.

Now I have to review all my code, force to reraise the CanExecutedChanged event in all commands whenever a RadBusyIndicator.IsBusy changes. And this not always works. Firstly, because I don't know why but sometime a button becomes enabled even after this CanExecuteChanged reraise when an async operation that enabled the BusyIndicator is completed and, therfore, the BusyIndicator gets disabled. Secondly, because the components that handle the async operation (tipically a webservice call) and that owns the command are decoupled and there's no way reraise the event. So, I have to check again in the command execution action if the command can be executed. That solves the errors but keeps very often buttons in an inconsistent state, as they are enabled but they do nothing when pressed.

I can understand that some (or many, that's always relative) users asks to alter the default (practically standard) before of a command. Sometimes, any developper needs a component behaves differently from the "expeceted" behavior. It's more hard to me to understand why people at Telerik accept this requests. For me it's a big mistake, an antipattern, to allow that a button bound to a command that cannot be executed is enabled. Why must be enabled a button that cannot be executed?? It's really annoying this behavior and I think I'm not the only one affected. If someone needs to control give higher priority to the IsEnabled property than to the command it's as easy as create a custom command or use a DelegateCommand and always return true in CanExceute method and the handle the IsEnabled property separetly. Why alter the default behavior.

I'd like that telerik reconsider this modification anf go back to the default behavior. I've created a new entry in PITS to request it.

[Zarko]

Hello Dani,
Thank you for the feedback and we're very sorry for the caused inconvenience.
First I'd have to tell you that we can't roll back the change because other customers depend on it. We changed the "default" behavior because it wasn't standard - once you set a command the IsEnabled binding didn't work anymore (that's not the case in the Microsoft buttons), and this was causing other problems. Our goal was to make our buttons behave like the Microsoft ones, but it seems that there's a bug with the BusyIndicator and we'll fix it for the service pack (you can track the progress here).
As I've said before - the main problem is that you can change IsEnabled from two places - IsEnabled and CanExecute and in WPF this is easy because you can override the IsEnabledCore method, but in SL you can't and we had to think of some kind of workarounds (and as you've found out, they are still not working in all cases) to synchronize everything.
For now I can give you a workaround (it's not the most elegant code, but, as far as I tested, it's working) - you'll gave to set an attached property to each button you use and it'll automatically invalidate your command on IsEnabledChanged.
Could you please examine the attached project and if you have more questions please feel free to ask. 

Kind regards,
Zarko
the Telerik team

[Daní]

Hi Zarko,

I have a couple of doubts about your proposed solution.

Fisrt of all, I fear it becames a source of memory leaks due the static Dictionary that may cause that no one of the buttons using this behavior being garbaged.

Second it only works with DelegateCommand. What about with other ICommand implementations?.

Anyway, thanks for this code, I think that, at least, will be a good starting point.

[Zarko]

Hello Dani,
I'm glad that I was able to help you and we'll let you know when the fix is available so that you could test it in your solution. 

Regards,
Zarko
the Telerik team

[Daní]

Hi Zarko,

I've coded my own behavior, based on yours, to try to deal with this issue. Below, the code for this behavior:

public class RadButtonExtensions
    {
        public static readonly DependencyProperty IsEnabledButtonProperty =
            DependencyProperty.RegisterAttached("IsEnabledButton", typeof (bool), typeof (RadButtonExtensions),
                                                new PropertyMetadata(DependencyProperty.UnsetValue,
                                                                     OnButtonPropertyChanged));

        public static readonly DependencyProperty ButtonCommandProperty =
            DependencyProperty.RegisterAttached("ButtonCommand", typeof (ICommand), typeof (RadButtonExtensions),
                                                new PropertyMetadata(null, OnButtonPropertyChanged));

        public static readonly DependencyProperty ButtonCommandParameterProperty =
            DependencyProperty.RegisterAttached("ButtonCommandParameter", typeof (object), typeof (RadButtonExtensions),
                                                new PropertyMetadata(null, OnButtonPropertyChanged));

        public static bool GetIsEnabledButton(DependencyObject obj)
        {
            return (bool) obj.GetValue(IsEnabledButtonProperty);
        }

        public static void SetIsEnabledButton(DependencyObject obj, bool value)
        {
            obj.SetValue(IsEnabledButtonProperty, value);
        }


        public static ICommand GetButtonCommand(DependencyObject obj)
        {
            return (ICommand) obj.GetValue(ButtonCommandProperty);
        }

        public static void SetButtonCommand(DependencyObject obj, ICommand value)
        {
            obj.SetValue(ButtonCommandProperty, value);
        }


        public static object GetButtonCommandParameter(DependencyObject obj)
        {
            return obj.GetValue(ButtonCommandParameterProperty);
        }

        public static void SetButtonCommandParameter(DependencyObject obj, object value)
        {
            obj.SetValue(ButtonCommandParameterProperty, value);
        }


        private static void OnButtonPropertyChanged(DependencyObject d, DependencyPropertyChangedEventArgs e)
        {
            var button = d as RadButton;
            if (button != null)
            {
                if (button.Command != null)
                    button.IsEnabled = button.Command.CanExecute(button.CommandParameter);
            }
        }
    }

Just a behavior with three DependencyProperties, IsEnabledButton, ButtonCommand and ButtonCommandParameter. These three DP share the same method to handle their changes. The PropertyChanged handle method just updates the button IsEnabled property based on its Command.CanExecute method result.

In my app, I have a custom ResourceDictionary with some default styles. This resource dictionary is loaded into the Application resource by code once the application is started and the Telerik Theme for the App is set. I've just added the following lines of xaml at this ResourceDictionary:

<Style TargetType="telerik:RadRibbonButton">
        <Setter Property="Extensions:RadButtonExtensions.IsEnabledButton" Value="{Binding Path=IsEnabled, RelativeSource={RelativeSource Self}}"/>
        <Setter Property="Extensions:RadButtonExtensions.ButtonCommand" Value="{Binding Path=Command, RelativeSource={RelativeSource Self}}"/>
        <Setter Property="Extensions:RadButtonExtensions.ButtonCommandParameter" Value="{Binding Path=CommandParameter, RelativeSource={RelativeSource Self}}"/>
    </Style>
    <Style TargetType="telerik:RadButton">
        <Setter Property="Extensions:RadButtonExtensions.IsEnabledButton" Value="{Binding Path=IsEnabled, RelativeSource={RelativeSource Self}}"/>
        <Setter Property="Extensions:RadButtonExtensions.ButtonCommand" Value="{Binding Path=Command, RelativeSource={RelativeSource Self}}"/>
        <Setter Property="Extensions:RadButtonExtensions.ButtonCommandParameter" Value="{Binding Path=CommandParameter, RelativeSource={RelativeSource Self}}"/>
    </Style>

With the above xaml, I'm setting, by default, the behavior in first code block to each RadButton and RadRibbonButton (wich inherits from RadButton). So each time the IsEnabled, Command or CommandParameter properties change on a RadButton, the behaviors updates the button IsEnabled property considering if the command can be executed.

By the moment, for me is a good solution as I don't have to review all my telerik buttons and, once telerik solves this issue, I'll be able to remove this behavior quickly and safely.

Hope thi helps somebody. If any issue is detected with this solution, please let me know it.

[Zarko]

Hi Dani,
Thank you for the code snippets and I hope they'll be helpful for other customers with similar issues.
As for the bug - I'm glad to tell you that the fix should be available in our next internal (this Monday) build and it'll be great if you could download it and see if everything is working correctly.

Kind regards,
Zarko
the Telerik team

[Misha]

This bug does seem to be fixed in the 8/13 hotfix for Q2. I have not tried Q3 yet..

[Zarko]

Hi Misha,
This issue should be fixed in both Q2 SP and Q3 and if you have more questions feel free to ask. 

All the best,
Zarko
the Telerik team