CommunityToolkit / MVVM-Samples

Sample repo for MVVM package
Other
1.09k stars 217 forks source link

Async ViewModel initialization best pattern? #25

Open maiorfi opened 3 years ago

maiorfi commented 3 years ago

Hi.

What is the right way to initialize viewmodels (created via IoC)?

I am wondering what kind of pattern should I follow both for sync and async initialization.

Thanks.

hippieZhou commented 3 years ago

if your initialization is async , you can set it in loadedCommand

    <interactivity:Interaction.Behaviors>
        <core:EventTriggerBehavior EventName="Loaded">
            <core:InvokeCommandAction Command="{x:Bind ViewModel.LoadCommand}" />
        </core:EventTriggerBehavior>
    </interactivity:Interaction.Behaviors>
        private ICommand _loadCommand;
        public ICommand LoadCommand
        {
            get
            {
                if (_loadCommand == null)
                {
                    _loadCommand = new RelayCommand(() =>
                    {
                    });
                }
                return _loadCommand;
            }
        }
michael-hawker commented 3 years ago

@Sergio0694 @jamesmcroft this is something I see come up a lot in terms of folks needing to call some async method to load data into their UI when it's starting up. We should provide docs around this and have an example in our sample app which shows the best-practice to handle this scenario.

Sergio0694 commented 3 years ago

I generally do this with a XAML behavior wired up to the Loaded event, and an AsyncRelayCommand. That's pretty handy as it also makes it easy to display eg. a loading bar while the viewmodel is initializing, as the async command has the IsRunning property with notification support. We should definitely call this out in the docs, sure! 😄

maiorfi commented 3 years ago

Great, thanks.

mrlacey commented 3 years ago

reopening so an link to the docs that show this can be added here.

maiorfi commented 3 years ago

For sake of completeness (since it isn't so easy to find anything one needs in a single place):

(After having added Microsoft.Xaml.Behaviors.Wpf NuGet package)

View:

<UserControl x:Class="SampleShellApp.SecondSampleUserControl"
             xmlns="http://schemas.microsoft.com/winfx/2006/xaml/presentation"
             xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml"
             xmlns:mc="http://schemas.openxmlformats.org/markup-compatibility/2006" 
             xmlns:d="http://schemas.microsoft.com/expression/blend/2008"
             xmlns:Behaviors="http://schemas.microsoft.com/xaml/behaviors"
             mc:Ignorable="d" 
             d:DesignHeight="450" d:DesignWidth="800">

    <Behaviors:Interaction.Triggers>
        <Behaviors:EventTrigger EventName="Loaded">
            <Behaviors:InvokeCommandAction Command="{Binding LoadCommand}"/>
        </Behaviors:EventTrigger>
    </Behaviors:Interaction.Triggers>

<!-- UserControl content here -->

</UserControl>

Viewmodel:

public class MyViewModel : ObservableRecipient
{
        public MyViewModel ()
        {
            LoadCommand = new AsyncRelayCommand(loadCommand);
        }

        public IAsyncRelayCommand LoadCommand { get; }

        async Task loadCommand()
        {
            Trace.TraceInformation("Starting loadCommand()...");

            // Simulate long operation...
            await Task.Delay(TimeSpan.FromSeconds(3));

            // Example of GUI update...
            MySampleBoundProperty="default-property-value";

            Trace.TraceInformation("...loadCommand() done.");
        }
}
Gerardo-Sista commented 3 years ago

@maiorfi thank you so much! Code clear and straight to the point. :)

jhm-ciberman commented 2 years ago

Please, this should be added to the docs somewere. I'm still learning about xaml, wpf and mvvm and I spent almost an hour thinking about an elegant solution to this exact problem.

michael-hawker commented 2 years ago

@Sergio0694 provided pattern look good to you? Any feedback?

Happy for anyone to submit a PR to move this along in the meantime too, we can always do feedback/tweeks in a PR too.

xixixixixiao commented 1 year ago

This code introduces a new problem: Each time the user activates the UserControl, the Loaded event of the UserControl is executed repeatedly.

To solve this problem a new Initialized property can be added:

public class MyViewModel : ObservableRecipient
{
        protected bool Initialized { get; set; }

        public MyViewModel ()
        {
            LoadCommand = new AsyncRelayCommand(loadCommand);
        }

        public IAsyncRelayCommand LoadCommand { get; }

        async Task loadCommand()
        {
            if(Initialized) return;

            Trace.TraceInformation("Starting loadCommand()...");

            // Simulate long operation...
            await Task.Delay(TimeSpan.FromSeconds(3));

            // Example of GUI update...
            MySampleBoundProperty="default-property-value";

            Trace.TraceInformation("...loadCommand() done.");

            Initialized = true;
        }
}
mrlacey commented 1 year ago

This code introduces a new problem: Each time the user activates the UserControl, the Loaded event of the UserControl is executed repeatedly.

Can you define what you mean by "the user activates the UserControl"?

Also, from your example, you probably want the setter of the Initialized property to be private.

xixixixixiao commented 1 year ago

Can you define what you mean by "the user activates the UserControl"?

There is a TabControl control in the Window, and the TabControl control has two TabItem controls. One UserControl is placed in each TabItem control. The Loaded event of the each UserControl will be triggered when the each TabItem is activated.

MainWindow.xaml file:

<Window x:Class="WpfApp1.MainWindow"
        xmlns="http://schemas.microsoft.com/winfx/2006/xaml/presentation"
        xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml"
        xmlns:local="clr-namespace:WpfApp1"
        Title="MainWindow" Width="800" Height="450">
    <TabControl>
        <TabItem Header="Tab 1">
            <local:UserControl1 />
        </TabItem>
        <TabItem Header="Tab 2">
            <local:UserControl2 />
        </TabItem>
    </TabControl>
</Window>

UserControl1.xaml file:

<UserControl x:Class="WpfApp1.UserControl1"
             xmlns="http://schemas.microsoft.com/winfx/2006/xaml/presentation"
             xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml"
             Loaded="UserControl1_OnLoaded">
    <Grid>
        <TextBlock Text="Content 1" />
    </Grid>
</UserControl>

UserControl1.cs file:

public partial class UserControl1 : UserControl
{
    public UserControl1()
    {
        InitializeComponent();
    }

    private void UserControl1_OnLoaded(object sender, RoutedEventArgs e)
    {
        Trace.WriteLine("UserControl1_OnLoaded"); // This message will be printed if the `TabItem` is activated.
    }
}

Also, from your example, you probably want the setter of the Initialized property to be private.

The Initialized event is not a good choice. ViewModels are injected via ViewModelLocator generally. The ViewModel has not been injected yet when the Initialized event is triggered. The view cannot be accessed by the Initialized event.