OnActivate should not be called #342

Closed
lploumen opened this Issue Jun 19, 2016 · 17 comments

Projects

None yet

3 participants

@lploumen

Hi,
I noticed something strange when running the Features sample from the Samples branch. I am executing the Features.Forms.WinPhone sample. I updated to the latest XF nuget version.
I go to the Conductors view, at then end of the list and play a bit with tabs, no problem.
When going back this view, OnDeactivate from ConductorViewModel is called, but also OnActivate which, I guess should not be called.
I did not notice this problem with the XF version included in the sample.

My original problem is that I included a WebView inside a ContentView which is included in a TabbedPage (using cm:View.Model="{Binding}" in the TabbedPage.ItemTemplate, like in the sample). The first time I display the TabbedView with the WebView, it is correctly displayed, but the second time, the WebView is flickering. If I don't use the CM mechanism ( = directly add my ContentView in <TabbedPage.Children>), it works.
That's why I suspect some events are wired twice to it although I'm not sure.

@nigel-sampson nigel-sampson added the bug label Jun 20, 2016
@lploumen
lploumen commented Jul 6, 2016

Did some more tests today, putting breakpoints in the OnAppearing event of a page. When leaving a Page, its OnAppearing event is being fired. That's why the OnActivate method is also called.
I guess it's a XF bug rather than CM (Although I cannot find anything in their Bugzilla DB regarding this).

@lploumen
lploumen commented Jul 6, 2016 edited

Well I'm not so sure anymore. I took the Xamarin Forms sample and ran the NavigationSample. The OnAppearing method is correctly called. The CM samples can also reproduce the bug.
Can someone investigate this problem ?
Regards,

@nigel-sampson
Contributor

Finally found some time to look into this, it's most likely caused by these lines which should remove the event handler after it's fired.

@nigel-sampson nigel-sampson added this to the v3.0.2 milestone Jul 7, 2016
@lploumen
lploumen commented Jul 8, 2016 edited

I made the change and it works.
The OnAppearing from the page left is still called twice but with the change, the OnActivate is fired only once. I can also try do a pull request (I've never done this...).
Thank you

@lploumen
lploumen commented Aug 11, 2016 edited

There's a side effect with the fix.
When going from page A to B, everything is ok and OnActivate is called when needed.
However, when going back from page B to A, OnActivate is not called on page A.
This is really annoying since the my app highly rely on VM lifecycles.
Hope someone can fix it soon!

@Yakyyaky
Yakyyaky commented Sep 6, 2016

Hi Iploumen,
While I am unclear as to whether you're also doing this on Android and iOS, please careful when dealing with OnActivate and OnDeactivate. If you're relying on OnActivate to perform tasks, ie refreshes. You might get caught with Lifetime issues. This is because CM is relying on OnAppearing and OnDisappearing to perform Activate/Deactivate on ViewModels and there are inconsistency across platform with them.
Due to:
https://bugzilla.xamarin.com/show_bug.cgi?id=41322
https://bugzilla.xamarin.com/show_bug.cgi?id=43815

@lploumen
lploumen commented Sep 7, 2016 edited

Hi Yakyyaky,
Yes it looks like a similar problem.
I use OnActivate to to perform task when a page is shown. I should also notice that it I'm testing on a Windows Phone silverlight project.
How to deal with it ? Is there a workaround ? I guess almost all apps need to use viewmodels lifecycles. Should we avoid using OnAppearing an instead use platform specific activation logic?
If I remember correctly, mvvmcross has a Start() method on viewmodel which is called on OnAppearing. I should maybe check if they have problems too.
This is quite an annoying bug :(

@Yakyyaky
Yakyyaky commented Sep 7, 2016

I worked around the issue in a specific way that works for my requirement. This meant that I basically abandoned XF's NavigationPage. And used Navigation with Application.MainPage instead. There are benefits and limitations to this.
Things that I lose:

  • Titlebar due to lose of NavigationPage. My app is full screen, so I also created by own title bar and back button concepts.
  • Loss of Back button. Software and Hardware, Full screen, so I didn't have to fully deal with hardware back button, but it's entirely possible to deal with it.
  • Screen transition animation.

Things I Did for my own NavigationService:
As CM's Navigation is based on working with Navigation Page. I wrote a NavigationServiceEx that implements an almost exact interface to CM's INavigationService but without the View Navigation (I prefer the ViewModel first Navigation approach).

  • With MainPage only ever contain 1 page. This provided me the ability to control Activate/Deactivation individually and trust OnAppearing/OnDiappearing events as they get assigned and unassigned to MainPage variable.
  • As I control each page's life cycle, I am able to implement IDeactivate's void Deactivate(bool close) properly. CM's XF implementation's close is hard coded to false. In my implementation, I know when my view model is going out of scope. I believe this can be improved on CM's by hooking to events on NavigationPage.
  • Implement Expose XF's App OnSleep() and OnResume() for when the application goes to the background/foreground to Activate/Deactivate the application.
  • Navigation with MainPage also allowed me to switch navigation concepts as required. XF's NavigationPage does not allow that. You cannot switch from Single content Page to a Master/Detail. With my implementation I can switch to different navigation concept anytime.
@lploumen
lploumen commented Sep 8, 2016

Hi,
There seems to be two problems:

  • The inconsistency between platform, it's a bug we can only fix it in a specific way.
  • The main problem I have when I submitted this thread is that OnActivate is incorrectly called when leaving a page. When I test with the Caliburn Feature sample (https://github.com/Caliburn-Micro/Caliburn.Micro/tree/samples) on Windows phone (On this platform, the Appearing/Disappearing seems consistent), when leaving the BindingView, the OnActivate is called on it's ViewModel. I don't understand how it is possible since the OnActivate is linked to the Appearing event. I also test it on a XF project without using Caliburn, tracing the Appearing/Disappearing events on Windows Phone and they are called correctly. I spent several hours yersterday without finding anything.

@nigel-sampson Your proposed fix would not work because if you remove the event handler after it is fired, when you go back to page A, it would not call the OnActivate method.

@lploumen

I had these problems when using Xamarin Forms with a Windows Phone Silverlight application.
It seems the lifecycles events on this platform doesn't work as expected (and also as @Yakyyaky said).
I tested the same app on Windows Phone 8.1 (RT, not Silverlight) and it seems to work better regarding the OnActivate and so on...
On XF website, they say WP Silverlight apps are supported but may become obsolete in future version. I guess I'm the only one targeting Silverlight and will move to more recent and supported project type :)

@nigel-sampson
Contributor

I'd love to move away from OnAppearing and OnDisappearing as the points for activation / deactivation.

Right now however it's our only option. While NavigatationPage exposes Pushed and Popped these events fire after then event happened meaning we don't have access to the previous page in order to deactivate it's view model in the same way we can in other platforms.

The inconsistency of the events firing on different platforms in different ways is also proving problematic.

I'm going to have a play around tonight with the NavigationPageAdapter holding a reference to the "current page" which would allow us to property deactivate old ones.

Would mean however that activation would have happen immediately and not when the page appears so would need to be something guarding against that.

@nigel-sampson
Contributor

@lploumen there's an update on the samples branch I'd love you check out, it's looking good for me but want to see how it goes in your app.

Thanks

@lploumen

I tested using the samples branch and my app. From what I see, everything looks good, although I need to test it a bit more.
I'll post here in case of problems.
Thank you

@nigel-sampson
Contributor

Cheers, I'll cherry pick the change over to master in a few days barring any issues from you.

@nigel-sampson
Contributor

@lploumen assume no issues here?

@lploumen
lploumen commented Nov 1, 2016

Looks good to me

@nigel-sampson
Contributor

Added the changes to master, will be in the next release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment