Merge pull request #21 from silentnuke/kz/restore_state_properly

Fixed bug with incorrect UI state after activity recreate
cookpad · Oct 5, 2020 · 2de21e0 · 2de21e0
2 parents fd65560 + 7a5cd11
commit 2de21e0
Show file tree

Hide file tree

Showing 10 changed files with 100 additions and 9 deletions.
diff --git a/app/build.gradle b/app/build.gradle
@@ -42,9 +42,8 @@ dependencies {
     implementation 'androidx.appcompat:appcompat:1.1.0'
     implementation 'androidx.core:core-ktx:1.3.0'
     implementation 'com.google.android.material:material:1.1.0'
-    implementation 'androidx.navigation:navigation-runtime:2.3.0'
-    implementation 'androidx.navigation:navigation-fragment:2.3.0'
-    implementation 'androidx.navigation:navigation-ui-ktx:2.3.0'
+    implementation "androidx.navigation:navigation-fragment-ktx:2.3.0"
+    implementation "androidx.navigation:navigation-ui-ktx:2.3.0"
     implementation 'androidx.constraintlayout:constraintlayout:1.1.3'
 
     androidTestImplementation 'androidx.test.ext:junit:1.1.1'

diff --git a/app/src/androidTest/java/cookpad/com/bottomnavwatson/HomeTest.kt b/app/src/androidTest/java/cookpad/com/bottomnavwatson/HomeTest.kt
@@ -21,6 +21,7 @@ class HomeScreen : Screen<HomeScreen>() {
     val textViewSecondTab = KTextView { withId(R.id.textViewSecondTab) }
     val textViewThirdTab = KTextView { withId(R.id.textViewThirdTab) }
     val tvTestDeepLinks = KTextView { withId(R.id.tvTestDeepLinks) }
+    val tvWithoutBottomMenu = KTextView { withId(R.id.tvWithoutBottomMenu) }
 }
 
 object SystemScreen : UiScreen<SystemScreen>() {
@@ -193,4 +194,28 @@ class HomeTest : TestCase() {
             }
         }
     }
+
+    @Test
+    fun verifyBottomMenuNotVisibleAfterScreenRotate() {
+        run {
+            step("Select second tab and navigate to detail without bottom menu") {
+                onScreen<HomeScreen> {
+                    bottomNavigationView { setSelectedItem(R.id.secondTabFragment) }
+                    textViewSecondTab { click() }
+                    tvTestDeepLinks { isDisplayed() }
+                    tvWithoutBottomMenu { click() }
+                    bottomNavigationView { isNotDisplayed() }
+                }
+            }
+            step("Rotate the device to detonate a config change") {
+                activityRule.activity.requestedOrientation = ActivityInfo.SCREEN_ORIENTATION_LANDSCAPE
+            }
+
+            step("Check that bottom menu is not displayed") {
+                onScreen<HomeScreen> {
+                    bottomNavigationView { isNotDisplayed() }
+                }
+            }
+        }
+    }
 }
diff --git a/app/src/main/java/cookpad/com/bottomnavwatson/DetailFragment.kt b/app/src/main/java/cookpad/com/bottomnavwatson/DetailFragment.kt
@@ -7,17 +7,22 @@ import android.os.Bundle
 import android.view.View
 import androidx.core.app.NotificationCompat
 import androidx.core.app.NotificationManagerCompat
+import androidx.core.view.isVisible
 import androidx.fragment.app.Fragment
 import androidx.navigation.NavDeepLinkBuilder
 import androidx.navigation.fragment.NavHostFragment.findNavController
+import androidx.navigation.fragment.navArgs
 import androidx.navigation.ui.AppBarConfiguration
 import androidx.navigation.ui.setupWithNavController
 import kotlinx.android.synthetic.main.detail_fragment.toolbar
 import kotlinx.android.synthetic.main.detail_fragment.tvTestDeepLinks
+import kotlinx.android.synthetic.main.detail_fragment.tvWithoutBottomMenu
 import java.util.Random
 
 class DetailFragment : Fragment(R.layout.detail_fragment) {
 
+    private val args: DetailFragmentArgs by navArgs()
+
     override fun onViewCreated(view: View, savedInstanceState: Bundle?) {
         super.onViewCreated(view, savedInstanceState)
 
@@ -39,6 +44,7 @@ class DetailFragment : Fragment(R.layout.detail_fragment) {
             val explicitDeepLink = NavDeepLinkBuilder(requireContext())
                 .setGraph(R.navigation.nav_graph)
                 .setDestination(R.id.detailFragment)
+                .setArguments(args.toBundle())
                 .setComponentName(HomeActivity::class.java)
                 .createPendingIntent()
 
@@ -54,6 +60,17 @@ class DetailFragment : Fragment(R.layout.detail_fragment) {
                     }
                 }
         }
+
+        toolbar.setTitle(
+            if (args.hideNavBar) R.string.detail_without_bottom_menu_fragment else R.string.detail_fragment
+        )
+        tvWithoutBottomMenu.isVisible = !args.hideNavBar
+        if (!args.hideNavBar) {
+            tvWithoutBottomMenu.setOnClickListener {
+                findNavController(this)
+                    .navigate(NavGraphDirections.actionDetailFragment().setHideNavBar(true))
+            }
+        }
     }
 
     companion object {

diff --git a/app/src/main/java/cookpad/com/bottomnavwatson/HomeActivity.kt b/app/src/main/java/cookpad/com/bottomnavwatson/HomeActivity.kt
@@ -2,6 +2,10 @@ package cookpad.com.bottomnavwatson
 
 import android.os.Bundle
 import androidx.appcompat.app.AppCompatActivity
+import androidx.core.view.isVisible
+import androidx.navigation.NavController
+import androidx.navigation.NavController.OnDestinationChangedListener
+import androidx.navigation.NavDestination
 import com.cookpad.watson.setupWithNavController
 import kotlinx.android.synthetic.main.home_activity.bottomNavigation
 
@@ -14,7 +18,17 @@ class HomeActivity : AppCompatActivity(R.layout.home_activity) {
             activity = this,
             initialSelectedTabId = R.id.firstTabFragment,
             enabledTabs = listOf(R.id.firstTabFragment, R.id.secondTabFragment, R.id.thirdTabFragment),
-            containerId = R.id.navigationHostFragment
+            containerId = R.id.navigationHostFragment,
+            destinationChangedListener = destinationChangedListener()
         )
     }
+
+    private fun destinationChangedListener(): OnDestinationChangedListener {
+        return OnDestinationChangedListener { controller: NavController, destination: NavDestination, args: Bundle? ->
+            bottomNavigation.isVisible = when (destination.id) {
+                R.id.detailFragment -> if (args != null) !DetailFragmentArgs.fromBundle(args).hideNavBar else true
+                else -> true
+            }
+        }
+    }
 }
diff --git a/app/src/main/res/color/navigation_text_selector.xml b/app/src/main/res/color/navigation_text_selector.xml
@@ -0,0 +1,5 @@
+<?xml version="1.0" encoding="utf-8"?>
+<selector xmlns:android="http://schemas.android.com/apk/res/android">
+    <item android:color="@android:color/black" android:state_selected="true" />
+    <item android:color="@android:color/white" />
+</selector>
diff --git a/app/src/main/res/layout/detail_fragment.xml b/app/src/main/res/layout/detail_fragment.xml
@@ -25,8 +25,22 @@
         android:padding="10dp"
         android:text="@string/test_deep_links"
         android:textAppearance="@style/TextAppearance.AppCompat.Large"
-        app:layout_constraintBottom_toBottomOf="parent"
+        app:layout_constraintBottom_toTopOf="@id/tvWithoutBottomMenu"
         app:layout_constraintEnd_toEndOf="parent"
         app:layout_constraintStart_toStartOf="parent"
         app:layout_constraintTop_toTopOf="parent" />
+
+    <TextView
+        android:id="@+id/tvWithoutBottomMenu"
+        android:layout_width="wrap_content"
+        android:layout_height="wrap_content"
+        android:gravity="center"
+        android:padding="10dp"
+        android:text="@string/open_without_bottom_menu"
+        android:textAppearance="@style/TextAppearance.AppCompat.Large"
+        app:layout_constraintBottom_toBottomOf="parent"
+        app:layout_constraintEnd_toEndOf="parent"
+        app:layout_constraintStart_toStartOf="parent"
+        app:layout_constraintTop_toBottomOf="@id/tvTestDeepLinks" />
+
 </androidx.constraintlayout.widget.ConstraintLayout>
diff --git a/app/src/main/res/layout/home_activity.xml b/app/src/main/res/layout/home_activity.xml
@@ -26,7 +26,7 @@
             android:layout_gravity="bottom"
             android:background="@color/colorPrimary"
             android:theme="@style/Theme.MaterialComponents.Light"
-            app:itemTextColor="@color/colorWhite"
+            app:itemTextColor="@color/navigation_text_selector"
             app:labelVisibilityMode="labeled"
             app:layout_constraintBottom_toBottomOf="parent"
             app:layout_insetEdge="bottom"

diff --git a/app/src/main/res/navigation/nav_graph.xml b/app/src/main/res/navigation/nav_graph.xml
@@ -27,6 +27,11 @@
 
     <fragment
         android:id="@+id/detailFragment"
-        android:name="cookpad.com.bottomnavwatson.DetailFragment" />
+        android:name="cookpad.com.bottomnavwatson.DetailFragment">
+        <argument
+            android:name="hideNavBar"
+            android:defaultValue="false"
+            app:argType="boolean" />
+    </fragment>
 
 </navigation>
diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml
@@ -7,7 +7,9 @@
     <string name="third_tab">Third tab</string>
     <string name="third_tab_action">Click to navigate from third tab to a detail</string>
     <string name="detail_fragment">Detail fragment</string>
+    <string name="detail_without_bottom_menu_fragment">Detail without bottom menu</string>
     <string name="test_deep_links">Test deep links by clicking here and creating a notification</string>
     <string name="watson_deep_links">Watson deep links</string>
     <string name="watson_deep_links_desc">Click the notification to the open the sample app</string>
+    <string name="open_without_bottom_menu">Click to navigate to a detail without bottom menu</string>
 </resources>
diff --git a/bottom-nav-watson/src/main/kotlin/com/cookpad/watson/MultipleBackStacks.kt b/bottom-nav-watson/src/main/kotlin/com/cookpad/watson/MultipleBackStacks.kt
@@ -32,7 +32,8 @@ internal class MultipleBackStacks(
     private val selectedNavController = MutableLiveData<NavController>()
     private val initialSelectedTabIndex = enabledTabs.indexOf(initialSelectedTabId)
     private val initialFragmentTag = getFragmentTag(initialSelectedTabIndex)
-    private var isOnInitialFragment = true
+    private val isOnInitialFragment: Boolean
+        get() = fragmentTagsViewModel.selectedFragmentTag?.name == initialFragmentTag
 
     fun onBottomNavigationView(
         bottomNavigationView: BottomNavigationView,
@@ -47,6 +48,9 @@ internal class MultipleBackStacks(
                 tabId = initialSelectedTabId,
                 destinationChangedListener = destinationChangedListener
             )
+            fragmentTagsViewModel.selectedFragmentTag = fragmentTagsViewModel.tabIdToFragmentTag[initialSelectedTabId]
+        } else {
+            restoreNavController(destinationChangedListener)
         }
 
         bottomNavigationView.apply {
@@ -61,6 +65,13 @@ internal class MultipleBackStacks(
         return selectedNavController
     }
 
+    private fun restoreNavController(destinationChangedListener: NavController.OnDestinationChangedListener?) {
+        val selectedFragment = fragmentManager.primaryNavigationFragment as NavHostFragment
+        selectedNavController.value = selectedFragment.navController.apply {
+            destinationChangedListener?.let { addOnDestinationChangedListener(it) }
+        }
+    }
+
     private fun BottomNavigationView.setupOnNavigationItemSelectedListener(
         destinationChangedListener: NavController.OnDestinationChangedListener?
     ) {
@@ -90,7 +101,6 @@ internal class MultipleBackStacks(
                         selectedNavController.value?.removeOnDestinationChangedListener(destinationChangedListener)
                     }
                     fragmentTagsViewModel.selectedFragmentTag = newlySelectedFragmentTag
-                    isOnInitialFragment = newlySelectedFragmentTag.name == initialFragmentTag
                     selectedNavController.value = selectedFragment.navController.apply {
                         destinationChangedListener?.let {
                             addOnDestinationChangedListener(destinationChangedListener)