ReactiveUI icon indicating copy to clipboard operation
ReactiveUI copied to clipboard

[Bug]: Blazor component base classes do not trigger WhenActivated()

Open rcdailey opened this issue 2 years ago • 1 comments

Describe the bug 🐞

When deriving my Page (view) from ReactiveInjectableComponentBase and providing a view model class that implements IActivatableViewModel, my callback provided to this.WhenActivated() is not invoked for me.

Step to reproduce

  1. Derive your page from the mentioned base class like so:

    @inherits ReactiveInjectableComponentBase<MonitorDeploymentsViewModel>
    
  2. Implement your view model like this:

    public class MonitorDeploymentsViewModel : ReactiveObject, IActivatableViewModel
    
  3. Provide a call to WhenActivated like so:

    this.WhenActivated(disposal =>
    {
        // Do something meaningful here
    });
    
  4. Compile and run the application. Be sure to visit the page we're talking about to trigger the issue.

Reproduction repository

This is a non-public repository; I have no MCVE.

Expected behavior

The ReactiveInjectableComponentBase class (as well as the other Blazor-specific component base classes) implement ICanActivate and already trigger the following:

  • When OnInitializedAsync() is invoked by the view, trigger Activated state.
  • When Dispose() happens (IDisposable), trigger Deactivated state.

However, by default, there are no observers for these states. This is because, in my opinion, in the setter for ReactiveInjectableComponentBase.ViewModel, code is missing to check if the supplied VM implements IActivatableViewModel and if so, attaches it to the appropriate Activated/Deactivated observables.

The workaround right now requires users to manually override OnInitializedAsync() simply to invoke ViewModel.Activator.Activate(), as well as the corresponding Dispose() override for the Deactivate(). This means that, ultimately, there's logic in the ComponentBase class that ends up not getting used at all because it's not tied in.

I think the addition necessary to the base ViewModel property is as follows:

            _viewModel = value;
            if (_viewModel is IActivatableViewModel avm)
            {
                Activated.Subscribe(_ => avm.Activator.Activate());
                Deactivated.Subscribe(_ => avm.Activator.Deactivate());
            }

Another solution I tested that seems to work:

protected override void OnInitialized()
{
  Activated.Subscribe(_ => ViewModel!.Activator.Activate());
  Deactivated.Subscribe(_ => ViewModel!.Activator.Deactivate());
  base.OnInitialized();
}

Screenshots 🖼️

No response

IDE

Rider Windows

Operating system

Windows

Version

10

Device

N/A

ReactiveUI Version

18.3.1

Additional information ℹ️

I created this issue at the request of @glennawatson, who discussed this with me in Slack.

rcdailey avatar Nov 15 '22 16:11 rcdailey

Researching, I'm happy with

protected override void OnInitialized()
{
  Activated.Subscribe(_ => ViewModel?.Activator.Activate());
  Deactivated.Subscribe(_ => ViewModel?.Activator.Deactivate());
  base.OnInitialized();
}

I would use the ? operator on the view model just in case its null at the time.

Other platforms allow you to hook into view's creation/deletion from a external events calls separate to the current view, but due to Blazors methodology this can't happen easily.

We'd likely have to make similar changes to the other Rx blazor overrides.

glennawatson avatar Dec 03 '22 00:12 glennawatson

Hey, guys. Any updates on this issue? I can see that @imundy7 have made changes in the fork.

@imundy7, any plans to merge this into main branch?

BekAllaev avatar May 10 '24 20:05 BekAllaev

This should be updated with the next Release, thank you all for your input with this.

ChrisPulman avatar May 14 '24 11:05 ChrisPulman

Thanks for the response @ChrisPulman. If you need hands for this issue I can assist. As far as I understand there is no big job to do. Just implement what was said in this comment:

Researching, I'm happy with

protected override void OnInitialized()
{
  Activated.Subscribe(_ => ViewModel?.Activator.Activate());
  Deactivated.Subscribe(_ => ViewModel?.Activator.Deactivate());
  base.OnInitialized();
}

I would use the ? operator on the view model just in case its null at the time.

Other platforms allow you to hook into view's creation/deletion from a external events calls separate to the current view, but due to Blazors methodology this can't happen easily.

We'd likely have to make similar changes to the other Rx blazor overrides.

BekAllaev avatar May 15 '24 20:05 BekAllaev

@BekAllaev I used ViewModel is IActivatableViewModel this covers a null condition and ensures its Activatable. Thank you I have a few PR'S to complete before I get the next Release out.

ChrisPulman avatar May 15 '24 22:05 ChrisPulman

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

github-actions[bot] avatar May 30 '24 00:05 github-actions[bot]