From 31192250e127b7c71dae41b08d6d571123206c3d Mon Sep 17 00:00:00 2001 From: Krzysztof Magiera Date: Tue, 26 Nov 2019 15:32:41 +0100 Subject: [PATCH] Fix stack nesting on Android. (#234) This change fixes two issues related to stack nesting on Android. First one being the fact that root and nested fragments were relying on the same fragment manager as opposed to using nested fragment manager structure (via getChildFragmentManager). This resulted in an unprodictable behavior when displaying the top screen. This commit changes this behavior by correctly returning child fragment manager or root fragment manager depending on the containers hierarchy. Second issue was related to back stack handling and resulted in always the root fragment manager interacting. Instead we want the bottommost active stack to handle back presses. This has been addressed by adding `setPrimaryNavigationFragment` when creating back stack entries. --- .../swmansion/rnscreens/ScreenContainer.java | 21 ++++++++++---- .../com/swmansion/rnscreens/ScreenStack.java | 29 +++++++++++++++---- 2 files changed, 39 insertions(+), 11 deletions(-) diff --git a/android/src/main/java/com/swmansion/rnscreens/ScreenContainer.java b/android/src/main/java/com/swmansion/rnscreens/ScreenContainer.java index 925c575d..e64922ec 100644 --- a/android/src/main/java/com/swmansion/rnscreens/ScreenContainer.java +++ b/android/src/main/java/com/swmansion/rnscreens/ScreenContainer.java @@ -23,6 +23,7 @@ public class ScreenContainer extends ViewGroup { protected final ArrayList mScreenFragments = new ArrayList<>(); private final Set mActiveScreenFragments = new HashSet<>(); + private @Nullable FragmentManager mFragmentManager; private @Nullable FragmentTransaction mCurrentTransaction; private boolean mNeedUpdate; private boolean mIsAttached; @@ -108,11 +109,18 @@ public class ScreenContainer extends ViewGroup { return mScreenFragments.get(index).getScreen(); } - protected final FragmentActivity findRootFragmentActivity() { + private FragmentManager findFragmentManager() { ViewParent parent = this; - while (!(parent instanceof ReactRootView) && parent.getParent() != null) { + // We traverse view hierarchy up until we find screen parent or a root view + while (!(parent instanceof ReactRootView || parent instanceof Screen) && parent.getParent() != null) { parent = parent.getParent(); } + // If parent is of type Screen it means we are inside a nested fragment structure. + // Otherwise we expect to connect directly with root view and get root fragment manager + if (parent instanceof Screen) { + return ((Screen) parent).getFragment().getChildFragmentManager(); + } + // we expect top level view to be of type ReactRootView, this isn't really necessary but in order // to find root view we test if parent is null. This could potentially happen also when the view // is detached from the hierarchy and that test would not correctly indicate the root view. So @@ -131,11 +139,14 @@ public class ScreenContainer extends ViewGroup { throw new IllegalStateException( "In order to use RNScreens components your app's activity need to extend ReactFragmentActivity or ReactCompatActivity"); } - return (FragmentActivity) context; + return ((FragmentActivity) context).getSupportFragmentManager(); } - protected FragmentManager getFragmentManager() { - return findRootFragmentActivity().getSupportFragmentManager(); + protected final FragmentManager getFragmentManager() { + if (mFragmentManager == null) { + mFragmentManager = findFragmentManager(); + } + return mFragmentManager; } protected FragmentTransaction getOrCreateTransaction() { diff --git a/android/src/main/java/com/swmansion/rnscreens/ScreenStack.java b/android/src/main/java/com/swmansion/rnscreens/ScreenStack.java index b2001fa0..6e7221ab 100644 --- a/android/src/main/java/com/swmansion/rnscreens/ScreenStack.java +++ b/android/src/main/java/com/swmansion/rnscreens/ScreenStack.java @@ -61,8 +61,16 @@ public class ScreenStack extends ScreenContainer { @Override protected void onDetachedFromWindow() { super.onDetachedFromWindow(); - getFragmentManager().removeOnBackStackChangedListener(mBackStackListener); - getFragmentManager().popBackStack(BACK_STACK_TAG, FragmentManager.POP_BACK_STACK_INCLUSIVE); + FragmentManager fm = getFragmentManager(); + if (!fm.isStateSaved()) { + // state save means that the container where fragment manager was installed has been unmounted. + // This could happen as a result of dismissing nested stack. In such a case we don't need to + // reset back stack as it'd result in a crash caused by the fact the fragment manager is no + // longer attached. + fm.removeOnBackStackChangedListener(mBackStackListener); + fm.popBackStack(BACK_STACK_TAG, FragmentManager.POP_BACK_STACK_INCLUSIVE); + } + } @Override @@ -88,7 +96,11 @@ public class ScreenStack extends ScreenContainer { getOrCreateTransaction().remove(screen); } } - ScreenStackFragment newTop = null; + + // When going back from a nested stack with a single screen on it, we may hit an edge case + // when all screens are dismissed and no screen is to be displayed on top. We need to gracefully + // handle the case of newTop being NULL, which happens in several places below + ScreenStackFragment newTop = null; // newTop is nullable, see the above comment ^ ScreenStackFragment belowTop = null; // this is only set if newTop has TRANSPARENT_MODAL presentation mode for (int i = mScreenFragments.size() - 1; i >= 0; i--) { @@ -106,7 +118,6 @@ public class ScreenStack extends ScreenContainer { } } - for (ScreenStackFragment screen : mScreenFragments) { // add all new views that weren't on stack before if (!mStack.contains(screen) && !mDismissed.contains(screen)) { @@ -127,7 +138,10 @@ public class ScreenStack extends ScreenContainer { } }); } - getOrCreateTransaction().show(newTop); + + if (newTop != null) { + getOrCreateTransaction().show(newTop); + } if (!mStack.contains(newTop)) { // if new top screen wasn't on stack we do "open animation" so long it is not the very first screen on stack @@ -165,7 +179,9 @@ public class ScreenStack extends ScreenContainer { tryCommitTransaction(); - setupBackHandlerIfNeeded(mTopScreen); + if (mTopScreen != null) { + setupBackHandlerIfNeeded(mTopScreen); + } for (ScreenStackFragment screen : mStack) { screen.onStackUpdate(); @@ -208,6 +224,7 @@ public class ScreenStack extends ScreenContainer { .hide(topScreen) .show(topScreen) .addToBackStack(BACK_STACK_TAG) + .setPrimaryNavigationFragment(topScreen) .commitAllowingStateLoss(); getFragmentManager().addOnBackStackChangedListener(mBackStackListener); }