Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

XF view activation is fundamentally broken #1116

Open
kentcb opened this issue Jun 1, 2016 · 6 comments
Open

XF view activation is fundamentally broken #1116

kentcb opened this issue Jun 1, 2016 · 6 comments

Comments

@kentcb
Copy link
Contributor

kentcb commented Jun 1, 2016

The XF activation-for-view-fetcher attempts to support:

  • ICanActivate (as of a recent PR)
  • Pages
  • Cells
  • Views

The views bit is totally busted. It currently hooks into IsVisible on the view, because that totally worked Once Upon A Time. However, IsVisible no longer changes to false when the view's page disappears.

I figured the best way around this is to get ahold of the view's Page and hook into its Appeared and Disappeared events. If we combine that stream with another stream of IsVisible changes, we could totally support view activation.

Alas, getting ahold of a view's Page is impossible. More accurately, knowing when to get ahold of a view's Page is impossible. We can always traverse up the UI tree to find the Page, but we have no hook telling us when we should do this. There's no PageChanged event, for example. I tried everything I could think of as an alternative. The most promising options were:

  • hook into PropertyChanged where the property name is "Parent". Do this recursively all the way up the chain so that if any parent changes, we can re-determine the Page. No dice because it doesn't always fire (seemingly when deserializing XAML).
  • override OnParentChanged and forward the event. Same problem as above.

Even if we could get one of these options to work, I suspect the performance would be woeful. What we really need is to hook into whatever mechanism it is that XF itself uses to tear down bindings. Or maybe the bindings are weak and it doesn't tear them down at all - not sure.

So for my particular use case (an app I'm trying to get into production), I had to throw my hands in the air and change view activations to be manual. That is, the containing Page forwards activation calls onto the child view. It was ugly and time-consuming, but it worked.

Longer term I think we're going to need XF to support some kind of mechanism that tells us when a view is re-housed in a different Page, then update our XF activation-for-view-fetcher accordingly.

@kentcb
Copy link
Contributor Author

kentcb commented Feb 25, 2017

@rbev
Copy link

rbev commented Oct 3, 2019

I've just come across this as a giant memory leak in our application, are there any possible workarounds we can use to get the view deactivation to happen when using a ReactiveContentView?

@maratoss
Copy link
Contributor

@kentcb

What if change the activation paradigm to use ViewModel based activation?
We know what vm should be shown in our app and Views will subscribe on Activated status of a vm?

RoutedViewHost will activate VM and VM will activate View.

@rbev
Copy link

rbev commented Dec 12, 2019

We ended up fixing it generally using a platform effect to hook into the lifetime of the view renderer and routing that through to the view to be used by the IActivationForViewFetcher

We only implemented for UWP and Android, but this approach would work on iOS and WPF without an issue (and likely the rest)

In our case we subclassed ReactiveView and added the effect/ICanActivate there (to avoid unintended regressions) but ideally we would have modified the ActivationForViewFetcher

I'll try to remember to post some code samples tomorrow when I'm at work

@glennawatson glennawatson moved this from To do to On Hold - Upstream Issue in Bug Stomping Jan 27, 2020
@gsgou
Copy link

gsgou commented Jul 6, 2020

@rbev can you provide some sample code? Tks in advance.

@rbev
Copy link

rbev commented Jul 7, 2020

@gsgou

I can't find the guide i originally used that this code largely came from, but create a routing effect & singleton registration logic

   
    public class ViewLifecycleEffect : RoutingEffect
    {
        public event EventHandler<EventArgs> Loaded;
        public event EventHandler<EventArgs> Unloaded;

        private ViewLifecycleEffect() : base($"myeffectnamespace.{nameof(ViewLifecycleEffect)}") { }
        public bool IsLoaded { get; set; }

        public void RaiseLoaded(Element element) => Loaded?.Invoke(element, EventArgs.Empty);
        public void RaiseUnloaded(Element element) => Unloaded?.Invoke(element, EventArgs.Empty);

        /// <summary>
        /// Registers or returns an existing instance of the effect on the specified element.
        /// This is because the limitations of the routed effect means that we have to only have a single instance registered or we can't raise
        /// the events for all of them without causing duplicate events to be raised.
        /// </summary>
        public static ViewLifecycleEffect Register(VisualElement element)
        {
            var effect = element.Effects.OfType<ViewLifecycleEffect>().FirstOrDefault();
            if (effect == null)
            {
                effect = new ViewLifecycleEffect();
                element.Effects.Add(effect);
            }
            return effect;
        }
}

Then in the platform implementation you just need to do something simliar to this (iOS):

        protected override void OnAttached()
        {
            _viewLifecycleEffect = Element.Effects.OfType<ViewLifecycleEffect>().FirstOrDefault();
            
            UIView nativeView = Control ?? Container;
            _isLoadedObserverDisposable = nativeView?.AddObserver("superview", ObservingOptions, IsViewLoadedObserver);
        }

        protected override void OnDetached()
        {
            _viewLifecycleEffect.RaiseUnloaded(Element);
            _isLoadedObserverDisposable.Dispose();
        }

        private void IsViewLoadedObserver(NSObservedChange nsObservedChange)
        {
            if (!nsObservedChange.NewValue.Equals(NSNull.Null))
            {
                 _viewLifecycleEffect.IsLoaded = true;
                 _viewLifecycleEffect?.RaiseLoaded(Element);
            }
            else if (!nsObservedChange.OldValue.Equals(NSNull.Null))
            {
                 _viewLifecycleEffect.IsLoaded = false;
                 _viewLifecycleEffect?.RaiseUnloaded(Element);
            }
        }

and android use these events to do the same thing.....

            _nativeView = Control ?? Container;
            _nativeView.ViewAttachedToWindow += OnViewAttachedToWindow;
            _nativeView.ViewDetachedFromWindow += OnViewDetachedFromWindow;

then register this to integrate it into the XF platform!

    public class ViewLifecycleEffectActivationFetcher : IActivationForViewFetcher
    {
        public int GetAffinityForView(Type view)
        {
            //ignored items (handled acceptably in default handler)
            if (view.IsAssignableTo<ICanActivate>()) return 0;
            if (view.IsAssignableTo<Page>()) return 0;
            if (view.IsAssignableTo<Cell>()) return 0;

            //catch all the things we want to override
            if (view.IsAssignableTo<VisualElement>()) return 20;

            //ignore everything else
            return  0;
        }

        public IObservable<bool> GetActivationForView(IActivatableView view)
        {
            if (view is VisualElement element)
            {
                return Observable.Create<bool>(o =>
                {
                    var effect = ViewLifecycleEffect.Register(element);

                    void OnEffectOnLoaded(object s, EventArgs a) => o.OnNext(true);
                    void OnEffectOnUnloaded(object s, EventArgs a) => o.OnNext(false);

                    effect.Loaded += OnEffectOnLoaded;
                    effect.Unloaded += OnEffectOnUnloaded;
                    element.Effects.Add(effect);
                    return new CompositeDisposable()
                    {
                        Disposable.Create(() =>
                        {
                            effect.Loaded -= OnEffectOnLoaded;
                            effect.Unloaded -= OnEffectOnUnloaded;
                        }),
                    };
                }).DistinctUntilChanged();
            }
            return null;
        }
    }

We also use this effect to allow view activation when binding to a data template using an attached property - this is primarily used to allow list binding UI controls to handle lifecycle of data templates (and virtualisation) rather than forcing it to go through the full view locator system.
While you lose the nicites of reactiveBinding it increases performance significantly without losing viewmodel activation logic.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Bug Stomping
  
On Hold - Upstream Issue
Development

No branches or pull requests

6 participants