canton7 / Stylet

A very lightweight but powerful ViewModel-First MVVM framework for WPF for .NET Framework and .NET Core, inspired by Caliburn.Micro.
MIT License
988 stars 143 forks source link

IConductor.ItemActivated event #140

Closed Yaevh closed 4 years ago

Yaevh commented 4 years ago

Right now the there's no way to intercept item activation other than listening for PropertyChanged event and filtering out ActiveItem property, and even that doesn't work with Conductor<T>.Collection.AllActive. It would be helpful to introduce ItemActivated, ItemDeactivated and ItemClosed events to IConductor<T> interface.

Will probably submit a PR for this change.

canton7 commented 4 years ago

Note that listening to INPC is made easier by Bind:

this.Bind(s => s.ActiveItem, (o, e) => DoSomethingWith(e.NewValue));

What exactly do you mean by "intercept item activation"? Events don't give you a way of intercepting anything. What exactly are you trying to achieve? If you're just looking for a way to hook into whenever an item is activated (including because the parent is activated), override ActivateItem.

Yaevh commented 4 years ago

By "intercepting item activation" I mean being notified when some Conductor activates its child, thus the event :)

The detailed scenario is as follows: I have a Conductor<T>.StackNavigation that guides the user through a series of steps, with each step represented by a screen (let's call them A, B and C). When each step is completed, current screen forces the Conductor to navigate to next step. However, some of these steps may be already completed, and in this case I want to skip them, and I want the screen itself to define when it should be skipped. For the sake of example let's assume that I've just completed step A and screen A has called Parent.ActivateItem(new B()). I want screen B to effectively intercept this ActiveItem change and force the Conductor to "skip" B and load screen C instead.

So my first thought was to override B.OnInitialActivate() and activate the next step from there:

public class B : Screen
//...
protected override void OnInitialActivate()
{
    if (CheckStepAlreadyCompleted())
    {
        Parent.ActivateItem(new C());
        Parent.CloseItem(this);
    }
}

This however doesn't work - ActiveItem is set to B after calling B.OnActivate(); this prevents this kind of "nested activation" because even if I activate C from within B.OnActivate(), ActiveItem would get overwritten and set to B eventually.

If we could change ConductorBaseWithActiveItem.ChangeActiveItem() to call this.__activeItem = newItem before calling ScreenExtensions.TryActivate(newItem) (and still defer calling this.NotifyOfPropertyChange("ActiveItem") to the very end of the process), this kind of "nested activation" would become possible. This however would probably introduce a breaking change (or would it? it would be sweet if it wouldn't), so I thought instead of raising an event at the very end of ActivateItem(). B could then consume that event and safely activate C after the Conductor has done its job switching ActiveItem to B.

canton7 commented 4 years ago

Thanks for the detailed explanation, that helps.

From your example, it looks like you wouldn't want B to be in the StackNavigation's history stack?

In that case, I'd try and avoid B being added to the stack at all, and do something like:

public override void ActivateItem(T item)
{
    if (!item.ProgressIfStepCompleted())
    {
        base.ActivateItem(item);
    }
}

where each child VM implements the method:

public bool ProgressIfStepCompleted()
{
    if (CheckStepAlreadyCompleted())
    {
        Parent.ActivateItem(new C());
        return true;
    }
    return false;
}

(or perhaps that could return a new C() or null for the parent to activate)

Otherwise, I think setting the parent's ActiveItem before activating that child is sensible, and I can't think how that would break anything.

canton7 commented 4 years ago

Fixed in v1.3.4