Skip to content

Commit

Permalink
Merge pull request #31 from splitwise/close-unbound-fragments
Browse files Browse the repository at this point in the history
Fix crash on close when previous fragment has been destroyed
  • Loading branch information
isaac-udy authored Apr 21, 2021
2 parents 37d2bac + dbe03b1 commit 6bf43aa
Show file tree
Hide file tree
Showing 3 changed files with 90 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -105,22 +105,24 @@ object DefaultFragmentExecutor : NavigationExecutor<Any, Fragment, NavigationKey
}

val animations = animationsFor(context, NavigationInstruction.Close)
val sameFragmentManagers = previousFragment?.parentFragmentManager == context.fragment.parentFragmentManager
// Checking for non-null context seems to be the best way to make sure parentFragmentManager will
// not throw an IllegalStateException when there is no parent fragment manager
val differentFragmentManagers = previousFragment?.context != null && previousFragment.parentFragmentManager != context.fragment.parentFragmentManager

context.fragment.parentFragmentManager.commit {
setCustomAnimations(animations.enter, animations.exit)
remove(context.fragment)

if (previousFragment != null && sameFragmentManagers) {
if (previousFragment != null && !differentFragmentManagers) {
when {
previousFragment.isDetached -> attach(previousFragment)
!previousFragment.isAdded -> add(context.contextReference.getContainerId(), previousFragment)
}
}
if(sameFragmentManagers) setPrimaryNavigationFragment(previousFragment)
if(!differentFragmentManagers) setPrimaryNavigationFragment(previousFragment)
}

if(previousFragment != null && !sameFragmentManagers) {
if(previousFragment != null && differentFragmentManagers) {
previousFragment.parentFragmentManager.commit {
setPrimaryNavigationFragment(previousFragment)
}
Expand Down
81 changes: 81 additions & 0 deletions enro/src/androidTest/java/dev/enro/core/FragmentToFragmentTests.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
package dev.enro.core

import androidx.fragment.app.FragmentActivity
import androidx.fragment.app.commit
import androidx.fragment.app.commitNow
import androidx.test.core.app.ActivityScenario
import dev.enro.*
import dev.enro.expectFragment
import junit.framework.TestCase
import org.junit.Test
import java.util.*

private fun expectSingleFragmentActivity(): FragmentActivity {
return expectActivity { it::class.java.simpleName == "SingleFragmentActivity"}
}

class FragmentToFragmentTests {

@Test
fun whenFragmentOpensFragment_andFragmentIsInAHost_thenFragmentIsLaunchedIntoHost() {
val scenario = ActivityScenario.launch(ActivityWithFragments::class.java)
val handle = scenario.getNavigationHandle<ActivityWithFragmentsKey>()

val id = UUID.randomUUID().toString()
handle.forward(ActivityChildFragmentKey(id))

val parentFragment = expectFragment<ActivityChildFragment>()
val id2 = UUID.randomUUID().toString()
parentFragment.getNavigationHandle().forward(ActivityChildFragmentTwoKey(id2))

val childFragment = expectFragment<ActivityChildFragmentTwo>()
val fragmentHandle = childFragment.getNavigationHandle().asTyped<ActivityChildFragmentTwoKey>()
TestCase.assertEquals(id2, fragmentHandle.key.id)
}

@Test
fun whenFragmentOpensFragment_andFragmentIsNotInAHost_thenFragmentIsLaunchedAsSingleFragmentActivity() {
val scenario = ActivityScenario.launch(DefaultActivity::class.java)
val handle = scenario.getNavigationHandle<DefaultActivityKey>()

val id = UUID.randomUUID().toString()
handle.forward(ActivityChildFragmentKey(id))

val activity = expectSingleFragmentActivity()
val parentFragment = activity.supportFragmentManager.primaryNavigationFragment!!
val id2 = UUID.randomUUID().toString()
parentFragment.getNavigationHandle().forward(ActivityChildFragmentTwoKey(id2))

val activity2 = expectSingleFragmentActivity()
val childFragment = activity2.supportFragmentManager.primaryNavigationFragment!!
val fragmentHandle = childFragment.getNavigationHandle().asTyped<ActivityChildFragmentTwoKey>()
TestCase.assertEquals(id2, fragmentHandle.key.id)
}

@Test
fun whenFragmentOpensFragment_andFragmentIsInAHost_andIsDestroyed_thenClosingChildFragmentCreatesNewParentFragment() {
val scenario = ActivityScenario.launch(ActivityWithFragments::class.java)
val handle = scenario.getNavigationHandle<ActivityWithFragmentsKey>()

val id = "UUID.randomUUID().toString()"
handle.forward(ActivityChildFragmentKey(id))

val parentFragment = expectFragment<ActivityChildFragment>()
val id2 = UUID.randomUUID().toString()
parentFragment.getNavigationHandle().forward(ActivityChildFragmentTwoKey(id2))

val parentFragmentManager = parentFragment.parentFragmentManager

val childFragment = expectFragment<ActivityChildFragmentTwo>()
val fragmentHandle = childFragment.getNavigationHandle().asTyped<ActivityChildFragmentTwoKey>()
TestCase.assertEquals(id2, fragmentHandle.key.id)

// This will destroy the parent fragment, making it unavailable to re-use on close
parentFragmentManager.commit { remove(parentFragment) }

childFragment.getNavigationHandle().close()
val newParentFragment = expectFragment<ActivityChildFragment>()
TestCase.assertEquals(id, newParentFragment.getNavigationHandle().asTyped<ActivityChildFragmentKey>().key.id)
TestCase.assertNotSame(parentFragment, newParentFragment)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,7 @@ import dev.enro.*
import org.junit.Ignore
import org.junit.Test

@Ignore("Something isn't working with the unbound fragment test environment, tests are failing but are known to pass")
class UnboundFragmentsTest {
class UnboundFragmentsTest {

@Test
fun whenUnboundFragmentIsOpened_thenNavigationKeyIsUnbound() {
Expand All @@ -31,7 +30,8 @@ class UnboundFragmentsTest {
caught = t
}
assertTrue(caught is IllegalStateException)
assertEquals("This NavigationHandle has no NavigationKey", caught.message)
assertNotNull(caught.message)
assertTrue(caught.message!!.matches(Regex("The navigation handle for the context UnboundFragment.*has no NavigationKey")))
}

@Test
Expand Down

0 comments on commit 6bf43aa

Please sign in to comment.