Skip to content
This repository has been archived by the owner. It is now read-only.

Improve MvxCachedFragmentInfo flexibility and fix caching bugs #86

Merged
merged 26 commits into from Nov 6, 2015

Conversation

vvolkgang
Copy link
Contributor

All of the small changes where made with the final objective of providing more flexibility to inheritors. Furthermore, already includes #85.

Improvements

  • You can now create your custom MvxCachedFragmentInfo by implementing the IMvxCachedFragmentInfo and overriding MvxCachingFragmentActivity.CreateFragmentInfo(...). Sample used this for Transitions and to know if the fragment is one of the drawer root fragments
  • Newly added OnFragmentCreated(...) enables you to do one time fragment setups. This was used in the sample to inflate transitions.
  • OnFragmentChanged provides a callback so you can know when there was a change in fragments and which fragment is being presented. This is the only callback thats being called when you present a new fragment and when you go back to a previous one. Sample used this to check if the current fragment being showed is root, if so, it enables the user to interact with the menu.
  • Instead of sending the fragment tag, all callbacks now send IMvxCachedFragmentInfo
  • Added slide_left + slide_right transitions to the sample
  • Everything was copypasted to the Compat version, the one being used in the sample

Questions

  • Does OnFragmentChanged needs to be called before the FragmentTransaction.Add(...)?
  • What if in OnBackPressed there's multiple fragments being displayed? (for instance, phone vs tablet usecase) Show we be sending a list of tags or list of IMvxCachedFragmentInfo ?

Álison Fernandes added 11 commits September 27, 2015 15:10
- Renamed FragmentInfo to MvxCachedFragmentInfo
- Added interface for MvxCachedFragmentInfo
- Extrated to separate files
… enable child classes to create their own configurations
…t that is being shown to the user, when pushed and when popped from the backstack.
…tantiation so inheritors can further setup the fragment, for instance, with transitions. Furthermore, this can be used in conjunction with a custom `MvxCachedFragmentInfo` to improve performance of this type of setups, reducing unnecessary lookups
- New features: Activity will change the hamburguer menu state based on the current fragment being displayed; Activity will inflate transitions for fragments configured with them
…...)` parameter to allow this functionality to be forced
@martijn00
Copy link
Contributor

I did some testing now. There where 2 cases where i could get this to crash in the sample app:

  1. Open app
  2. Go to Settings screen
  3. Press back button
  4. Go again to settings screen: navigation fails, when clicking a bit around after that the app crashes

Crash log:

[MonoDroid] Java.Lang.IllegalStateException: Exception of type 'Java.Lang.IllegalStateException' was thrown.
[MonoDroid] at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw () [0x0000b] in /Users/builder/data/lanes/2058/58099c53/source/mono/mcs/class/corlib/System.Runtime.ExceptionServices/ExceptionDispatchInfo.cs:61
[MonoDroid] at Android.Runtime.JNIEnv.CallNonvirtualVoidMethod (intptr,intptr,intptr,Android.Runtime.JValue*) [0x00084] in /Users/builder/data/lanes/2058/58099c53/source/monodroid/src/Mono.Android/src/Runtime/JNIEnv.g.cs:1029
[MonoDroid] at Android.App.Activity.OnSaveInstanceState (Android.OS.Bundle) [0x00070] in /Users/builder/data/lanes/2058/58099c53/source/monodroid/src/Mono.Android/platforms/android-21/src/generated/Android.App.Activity.cs:4460
[MonoDroid] at Cirrious.MvvmCross.Droid.Support.AppCompat.MvxEventSourceAppCompatActivity.OnSaveInstanceState (Android.OS.Bundle) [0x0000f] in c:\Users\Gebruiker\Documents\MvvmCross-AndroidSupport\Cirrious.MvvmCross.Droid.Support.AppCompat\MvxEventSourceAppCompatActivity.cs:80
[MonoDroid] at Cirrious.MvvmCross.Droid.Support.AppCompat.MvxCachingFragmentCompatActivity.OnSaveInstanceState (Android.OS.Bundle) [0x00081] in c:\Users\Gebruiker\Documents\MvvmCross-AndroidSupport\Cirrious.MvvmCross.Droid.Support.AppCompat\MvxCachingFragmentCompatActivity.cs:218
[MonoDroid] at Android.App.Activity.n_OnSaveInstanceState_Landroid_os_Bundle_ (intptr,intptr,intptr) [0x00011] in /Users/builder/data/lanes/2058/58099c53/source/monodroid/src/Mono.Android/platforms/android-21/src/generated/Android.App.Activity.cs:4442
[MonoDroid] at (wrapper dynamic-method) object.9e64c9d4-826f-4788-aea2-a42287a4dd6f (intptr,intptr,intptr) <IL 0x00017, 0x00043>
[MonoDroid]   --- End of managed exception stack trace ---
[MonoDroid] java.lang.IllegalStateException: Recursive entry to executePendingTransactions
[MonoDroid]     at android.support.v4.app.FragmentManagerImpl.execPendingActions(FragmentManager.java:1473)
[MonoDroid]     at android.support.v4.app.FragmentManagerImpl.saveAllState(FragmentManager.java:1684)
[MonoDroid]     at android.support.v4.app.FragmentActivity.onSaveInstanceState(FragmentActivity.java:527)
[MonoDroid]     at cirrious.mvvmcross.droid.support.appcompat.MvxCachingFragmentCompatActivity.n_onSaveInstanceState(Native Method)
[MonoDroid]     at cirrious.mvvmcross.droid.support.appcompat.MvxCachingFragmentCompatActivity.onSaveInstanceState(MvxCachingFragmentCompatActivity.java:38)
[MonoDroid]     at android.app.Activity.performSaveInstanceState(Activity.java:1373)
[MonoDroid]     at android.app.Instrumentation.callActivityOnSaveInstanceState(Instrumentation.java:1286)
[MonoDroid]     at android.app.ActivityThread.callCallActivityOnSaveInstanceState(ActivityThread.java:4367)
[MonoDroid]     at android.app.ActivityThread.handleRelaunchActivity(ActivityThread.java:4326)
[MonoDroid]     at android.app.ActivityThread.access$1000(ActivityThread.java:172)
[MonoDroid]     at android.app.ActivityThread$H.handleMessage(ActivityThread.java:1428)
[MonoDroid]     at android.os.Handler.dispatchMessage(Handler.java:102)
[MonoDroid]     at android.os.Looper.loop(Looper.java:145)
[MonoDroid]     at android.app.ActivityThread.main(ActivityThread.java:5832)
[MonoDroid]     at java.lang.reflect.Method.invoke(Native Method)
[MonoDroid]     at java.lang.reflect.Method.invoke(Method.java:372)
[MonoDroid]     at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:1399)
[MonoDroid]     at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:1194)

And this:

  1. Open app
  2. Go to Settings screen
  3. Press back button
  4. Rotate screen: app crashes with same crash log

Could you look into this?

@vvolkgang
Copy link
Contributor Author

I caught that error also but wanted to talk to you first, before this change the view would bug the tabs when you did the same, my first google hit pointed out a possible bug in the way the Examples page is doing the child fragments thus, it might be related, check this:

http://stackoverflow.com/a/7481468

I'll see if this is happening in other scenarios.

@vvolkgang
Copy link
Contributor Author

The real problem is in #85, the way we're bringing caching fragments back. This wasn't a problem before because we were always creating new ones, that ft.Attach wasn't actually running in any case.

@martijn00
Copy link
Contributor

We use the latest Support libraries and use child fragment manager in the sample, so i think that is not a problem.

@vvolkgang
Copy link
Contributor Author

You're right! The problem is the way we're currently handling the detach / attach / replace of previously created fragments, it will "work again" if you unfix the #85 bug by changing this line to if(true) but, in essence, it just worked until now because were never using the cached fragments.

Álison Fernandes added 10 commits September 28, 2015 14:20
…rent + backstack frags aren't correctly tracked
…ity, all the bugs should be squashed. This will need an improvement in MvxFragmentExtensions, going in a separate commit
… cached instances. This implementation should be improved when the functionality is accepted in MvvmCross main lib.
…would stop animating transitions after some fragment changes.
…idSupport

Conflicts:
	Cirrious.MvvmCross.Droid.Support.AppCompat/MvxCachingFragmentCompatActivity.cs
	Cirrious.MvvmCross.Droid.Support.Fragging/MvxCachingFragmentActivity.cs
	Samples/Example.Droid/Activities/MainActivity.cs
	Samples/Example.Droid/Example.Droid.csproj
	Samples/Example.Droid/Resources/Resource.designer.cs
@vvolkgang vvolkgang changed the title Improve MvxCachedFragmentInfo flexibility Improve MvxCachedFragmentInfo flexibility and fix caching bugs Nov 2, 2015
…ateException if there was a previous instance of a Bundle already set.
@martijn00
Copy link
Contributor

I've tested this a bit. When i enable add to backstack, for example on the homefragment, then pressing back takes me back to a white screen, but the menu still works. What i would expect is that after the last fragment is removed from the screen, the app navigates back to the login activity.

@vvolkgang
Copy link
Contributor Author

Root fragments should not be added to the backstack or that will happen.
You should have a similar behavior If you handled the fragments directly.
If you only add to the backstack non root, i.e., inner fragments (which the
sample doesn't have) everything will work as expected.
On Thu, 5 Nov 2015 at 22:30, Martijn van Dijk notifications@github.com
wrote:

I've tested this a bit. When i enable add to backstack, for example on the
homefragment, then pressing back takes me back to a white screen, but the
menu still works. What i would expect is that after the last fragment is
removed from the screen, the app navigates back to the login activity.


Reply to this email directly or view it on GitHub
#86 (comment)
.

Regards,

Álison Fernandes

martijn00 added a commit that referenced this pull request Nov 6, 2015
Improve MvxCachedFragmentInfo flexibility and fix caching bugs
@martijn00 martijn00 merged commit ea47156 into MvvmCross:master Nov 6, 2015
@martijn00 martijn00 added this to the 4.0.0 milestone Nov 9, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants