"Long" delay causes my view's button not to re-enable

Viewed 126

I have a listview and a button in each column. When the user clicks a button, it triggers an asynchronous action in the viewmodel where I disable all buttons and do a big action. Once the action is completed, I re-enable them.

If the action takes too long, though, the buttons don't automatically get re-enabled, even though I'm setting the bound property to true and am notifying the view. If the user does ANY GUI action after the action is complete, the buttons will re-enable.

The other weird thing: If I do an await Task.Delay instead of doing Thread.Sleep (NB: I'm doing real work in the full application), it works correctly.

What's going on here?

I've simplified the code here by eliminating the model (all logic lives in the VM).

View-model code:

namespace WpfTestApp
{
    public class viewmodel : INotifyPropertyChanged
    {

        public event PropertyChangedEventHandler PropertyChanged;

        public void NotifyPropertyChanged([CallerMemberName] String propertyName = "")
        {
            PropertyChanged?.Invoke(this, new PropertyChangedEventArgs(propertyName));
        }

        public RelayAsyncCommand<object> RunCommand { get; private set; }

        private ObservableCollection<subVM> _subVMs;
        public ObservableCollection<subVM> SubVMs
        {
            get => _subVMs; set
            {
                _subVMs = value;
                NotifyPropertyChanged();
            }
        }
        public viewmodel()
        {
            RunCommand = new RelayAsyncCommand<object>(OnRun);
            SubVMs = new ObservableCollection<subVM>
            {
                new subVM("ItemA"),
                new subVM("ItemB"),
            };
        }

        private async void OnRun(object o)
        {
            subVM vm = o as subVM;
            if (vm != null)
            {
                ChangeRunMode(false);
                Thread.Sleep(500);
            }
            ChangeRunMode(true);
        }

        private void ChangeRunMode(bool on)
        {
            foreach (subVM vm in SubVMs)
            {
                vm.ButtonEnabled = on;
            }
        }

    }

    public class subVM : INotifyPropertyChanged
    {
        private string name = "";
        public string Name
        {
            get => name;
            set
            {
                if (value != name)
                {
                    name = value;

                }
            }
        }
        public subVM(string name)
        {
            Name = name;
        }

        private bool tsk = true;
        public bool ButtonEnabled
        {
            get => tsk;
            set
            {
                if (tsk != value)
                {
                    tsk = value;
                    NotifyPropertyChanged("ButtonEnabled");
                }
            }
        }

        public event PropertyChangedEventHandler PropertyChanged;

        public void NotifyPropertyChanged([CallerMemberName] String propertyName = "")
        {
            PropertyChanged?.Invoke(this, new PropertyChangedEventArgs(propertyName));
        }

    }
}

View XAML:

<Window x:Class="WpfTestApp.MainWindow"
        xmlns="http://schemas.microsoft.com/winfx/2006/xaml/presentation"
        xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml"
        xmlns:d="http://schemas.microsoft.com/expression/blend/2008"
        xmlns:mc="http://schemas.openxmlformats.org/markup-compatibility/2006"
        xmlns:local="clr-namespace:WpfTestApp"
        mc:Ignorable="d"
        Title="MainWindow" Height="130" Width="350">
    <Window.DataContext>
        <local:viewmodel/>
    </Window.DataContext>


        <ListView  Margin="5"
                   BorderBrush="DarkSlateGray" BorderThickness="1"
                   ScrollViewer.HorizontalScrollBarVisibility="Disabled"
                   ItemsSource="{Binding SubVMs}">
            <ListView.View>
                <GridView>
                    <GridViewColumn Header="Name" 
                                        Width="200" DisplayMemberBinding ="{Binding Name}"/>
                    <GridViewColumn>
                        <GridViewColumn.CellTemplate>
                            <DataTemplate>
                                <Button
                                           Content="Load"
                                            IsEnabled="{Binding ButtonEnabled, Mode=OneWay, UpdateSourceTrigger=PropertyChanged}"
                                           Margin="0"
                                           VerticalAlignment="Center"
                                            Command="{Binding Path=DataContext.RunCommand, IsAsync=True, RelativeSource={RelativeSource FindAncestor, AncestorType={x:Type ItemsControl}}}" 
                                            CommandParameter="{Binding}"
                                           />
                            </DataTemplate>
                        </GridViewColumn.CellTemplate>
                    </GridViewColumn>
                </GridView>
            </ListView.View>
        </ListView>
</Window>

Things I've tried: I tried await Task.Run on my actual problem, and it still happens.

RelayCommand and the Async version (I thought these were standard boilerplate, but here you go):

public class RelayAsyncCommand<T> : RelayCommand<T>
{
    private bool isExecuting = false;

    public event EventHandler Started;

    public event EventHandler Ended;

    public bool IsExecuting
    {
        get { return this.isExecuting; }
    }

    public RelayAsyncCommand(Action<T> execute, Predicate<T> canExecute)
        : base(execute, canExecute)
    {
    }

    public RelayAsyncCommand(Action<T> execute)
        : base(execute)
    {
    }

    public override Boolean CanExecute(Object parameter)
    {
        return ((base.CanExecute(parameter)) && (!this.isExecuting));
    }

    public override void Execute(object parameter)
    {
        try
        {
            this.isExecuting = true;
            if (this.Started != null)
            {
                this.Started(this, EventArgs.Empty);
            }

            Task task = Task.Factory.StartNew(() =>
            {
                this._execute((T)parameter);
            });
            task.ContinueWith(t =>
            {
                this.OnRunWorkerCompleted(EventArgs.Empty);
            }, TaskScheduler.FromCurrentSynchronizationContext());
        }
        catch (Exception ex)
        {
            this.OnRunWorkerCompleted(new RunWorkerCompletedEventArgs(null, ex, true));
        }
    }

    private void OnRunWorkerCompleted(EventArgs e)
    {
        this.isExecuting = false;
        if (this.Ended != null)
        {
            this.Ended(this, e);
        }
    }
}


public class RelayCommand<T> : ICommand
{
    #region Fields

    readonly protected Action<T> _execute;
    readonly protected Predicate<T> _canExecute;

    #endregion // Fields

    #region Constructors

    public RelayCommand(Action<T> execute)
    : this(execute, null)
    {
    }

    public RelayCommand(Action<T> execute, Predicate<T> canExecute)
    {
        if (execute == null)
            throw new ArgumentNullException("execute");

        _execute = execute;
        _canExecute = canExecute;
    }
    #endregion // Constructors

    #region ICommand Members

    [DebuggerStepThrough]
    public virtual bool CanExecute(object parameter)
    {
        return _canExecute == null ? true : _canExecute((T)parameter);
    }

    public event EventHandler CanExecuteChanged
    {
        add { CommandManager.RequerySuggested += value; }
        remove { CommandManager.RequerySuggested -= value; }
    }

    public virtual void Execute(object parameter)
    {
        _execute((T)parameter);
    }

    #endregion // ICommand Members
}
2 Answers

inside the OnRun method you are actually locking the UI thread, preventing any UI update/refresh; you should await for the long operation (and that's exactly what you noticed doing await Task.Delay):

private async void OnRun(object o)
{
    subVM vm = o as subVM;
    if (vm != null)
    {
        ChangeRunMode(false);
        await Task.Run(() =>
        {
            //put here your long operation as per your example
            for (int i = 0; i < 500; i++)
            {
                for (int k = 0; k < 100000; k++) ;
            }
        });
    }
    ChangeRunMode(true);
}

indeed if you take a closer look to your window, you'll see that currently everything is freezed during the long operation execution, not just only the buttons being disabled.

I figured out my problem:

The button deactivating isn't coming from my binding when I'm running asynchronously. It's coming from the RelayAsyncCommand's CanExecute. CanExecute here returns false while the task is running, but we don't trigger a requery ever when it's done.

Easily fixed by adding a private set to the IsExecuting property, which calls the invalidate/requery function on change (just like the standard notifypropertychanged pattern). For posterity, here's the full fixed RelayAsyncCommand:

public class RelayAsyncCommand<T> : RelayCommand<T>
{
    private bool _isExecuting = false;

    public event EventHandler Started;

    public event EventHandler Ended;

    public bool IsExecuting
    {
        get { return _isExecuting; }
        private set
        {
            if (value != _isExecuting)
            {
                _isExecuting = value;
                CommandManager.InvalidateRequerySuggested();
            }
        }
    }

    public RelayAsyncCommand(Action<T> execute, Predicate<T> canExecute)
        : base(execute, canExecute)
    {
    }

    public RelayAsyncCommand(Action<T> execute)
        : base(execute)
    {
    }

    public override bool CanExecute(object parameter)
    {
        return ((base.CanExecute(parameter)) && (!IsExecuting));
    }

    public override void Execute(object parameter)
    {
        try
        {
            IsExecuting = true;
            Started?.Invoke(this, EventArgs.Empty);

            Task task = Task.Factory.StartNew(() =>
            {
                _execute((T)parameter);
            });

            task.ContinueWith(t =>
            {
                OnRunWorkerCompleted(EventArgs.Empty);
            }, TaskScheduler.FromCurrentSynchronizationContext());
        }
        catch (Exception ex)
        {
            OnRunWorkerCompleted(new RunWorkerCompletedEventArgs(null, ex, true));
        }
    }

    private void OnRunWorkerCompleted(EventArgs e)
    {
        IsExecuting = false;
        Ended?.Invoke(this, e);
    }
}

Thanks Peter for making me actually look at (what I thought was) boilerplate code.

Related